Repository navigation
fix: pin the collation so the program text depends on the rules, not the locale - #897
Merged
Merged
Conversation
swapnilpaliwal-sd
force-pushed
the
fix/895-locale-stable-program-text
branch
from
September 18, 2026 05:22
524c28e to
e6af32e
Compare
…the locale The #include lines are assembled from shell globs, and bash orders a glob by LC_COLLATE rather than by byte value. A UTF-8 collation ignores punctuation, so call-site.dl and callee-resolution.dl swap places against their byte order. The include order is part of the program text and the program text is hashed, so the cache key follows the user's locale rather than the rules. Measured on one installed package: java and python ids differ between macOS and MSYS2, and between LC_ALL=C and en_US.UTF-8 on glibc. typescript and javascript agree only because no pair of their filenames collides. Today the cost is a spurious recompile when the locale changes, and two agents on one machine with different locales never sharing a cached binary. Once engines are matched by id it decides whether a published binary is accepted at all, and the refusal names the rules rather than the locale. A UTF-8 locale is the default on most Linux desktops, in macOS terminals and in Git Bash, while CI runs under a C-ish locale, so the mismatch would be the common case. graph/test/tools/engine-id-locale-test.sh asserts it. Two of its three halves cannot run on every platform, so the third asserts the guard itself everywhere and the test fails if the whole run asserted nothing. Verified it fails with the pin removed. Fixes #895
swapnilpaliwal-sd
force-pushed
the
fix/895-locale-stable-program-text
branch
from
September 18, 2026 05:25
e6af32e to
221ba5a
Compare
swapnilpaliwal-sd
added a commit
that referenced
this pull request
Sep 18, 2026
…478) (#903) Reverts f6ba1c2. The engine binaries work is not ready to ship, so it goes back to a pull request and #454 reopens with it. Not a plain revert, because two changes landed on top of it and both have to keep working: - #897 pinned the collation inside write_program, which #478 introduced. Reverting removes that function and would take the guard with it, and the restored executor has the same defect: its #include lines come from unguarded globs, so the cache key follows the user's locale. The guard is re-applied to the restored program generation, and engine-id-locale-test.sh now accepts either shape of the executor so it keeps measuring rather than failing for the wrong reason. Its --emit-program checks skip here, since that flag belongs to #478, and they report the skip rather than passing silently. - #887 added bin, files and dependencies, which are independent of the engine work and stay. optionalDependencies goes with the feature: nothing resolves a packaged engine any more, so those entries would name packages the executor never looks for. What comes back: the executor compiles with a local souffle and caches the binary, exactly as before #478. engine.conf, packaging/ and the two engine tests go with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #895. P0: this decides whether a published engine is usable, and the common configuration is the broken one.
The defect
The
#includelines are assembled from shell globs. Bash orders a glob byLC_COLLATE, not by byte value, and a UTF-8 collation ignores punctuation when comparing:The include order is part of the program text, the program text is hashed, so the key follows the user's locale rather than the rules. The
sortcalls beside these globs were already forced toLC_ALL=Cfor exactly this reason; the globs were not.Measured
Same installed package, ids computed on two platforms:
16a17ec0d79f318ce8df2d7ce8df2d7c934aa0181302327180c135763fc135763fTypeScript and JavaScript agree only because no pair of their rule filenames collides. Java and Python each contain one that does. Reproduced independently on glibc by switching
LC_ALLalone.Impact
On main today the cost is a spurious full recompile when the locale changes, and two agents on one machine with different locales never sharing a cached binary.
It becomes correctness-affecting once engines are matched by id: an engine built in CI under a C-ish locale is refused on a user's machine under a UTF-8 locale, with an error naming the rules rather than the locale. A UTF-8 locale is the default on most Linux desktops, in macOS terminals and in Git Bash, so that is the common case. Verified end to end on Windows with a correctly built engine installed: java and python both fail with
not using it, while typescript and javascript succeed.Fix
Pin the collation where the program text is assembled, so the ordering is a property of the rules and not of the environment.
Test
graph/test/tools/engine-id-locale-test.sh, wired into the java preflight.Two of its three halves cannot run everywhere: the fixture-ordering check needs a libc that distinguishes the two collations, and the program-text comparison needs
--emit-program, which lands with #478. Rather than let it report PASS having asserted nothing, which is how this defect survived, the third half asserts on every platform that the executor pins the collation and does so before the first rule glob, and the run fails outright if no check executed. Verified failing with the pin removed.Note for reviewers
This is invisible on macOS, whose collation matches C under both locales. The first attempt to reproduce it there found no difference and read as a clean bill of health. Assert it on glibc or MSYS2.