MagicBeanDev/Longshot
MagicBeanDev/Longshot
Watch
24
31
Fork
You've already forked Longshot
 43
Code Issues 62 Pull requests 127 Projects Releases 8 Packages Wiki Activity Actions 312

fix(scripts): check_third_party reads git check-ignore in bytes, so Windows skips ignored files #80

Open
d4nkn3ss wants to merge 2 commits from d4nkn3ss/Longshot:fix/check-third-party-windows-newlines into main
pull from: d4nkn3ss/Longshot:fix/check-third-party-windows-newlines
merge into: MagicBeanDev:main
Conversation 2 Commits 2 Files changed 2 +10 -4
d4nkn3ss commented 2026-11-13 02:16:47 +00:00
Contributor

Why

On Windows, scripts/check_third_party.py fails on any checkout set up the way CONTRIBUTING.md says: the Builder's binaries copied into third_party/ (a64emit.dll, lld-link.exe, SDL2.dll, zbic.dll and others) are counted although .gitignore leaves them out, so a64-kit, sdl2 and zbic "differ from their notes".

What changed

  • scripts: ignored() in scripts/check_third_party.py asks git check-ignore --no-index -z --stdin, NUL-separated and as UTF-8 bytes both ways, instead of newline-separated text. Two bugs went with the old form:
    • In text mode on Windows, Python writes each as `

, so git read every path with a trailing
`, matched no rule, and nothing was skipped.

  • Without -z, git quotes a path with a non-ASCII byte in its answer ("third_party/x/é.dll"), which matched none of ours, so an ignored é.dll was hashed with its part, on every OS (from HumanBean's review).
  • tests: test_a_clean_tree_passes in tests/test_check_third_party.py adds an ignored é.dll beside foo.dll; it fails without -z on Linux too, where CI runs.

Flags and settings

None

Measurements

Not a performance change, so no fps or timings: the check's result on both sides.

main: 1be180bb (2026-11-13)
this PR: fix/check-third-party-windows-newlines @ 6c00bdcf
Hardware: Windows 11, Python 3 from python.org, git 2.56.1 (core.autocrlf=true)
Scene: n/a (game-free)
Method: each command run once on each side, on the same checkout, set up as CONTRIBUTING.md says with the Builder's third_party binaries copied in
How to reproduce: on Windows, copy the Builder's tools\third_party binaries in as CONTRIBUTING.md says, then run the two commands below

main @ 1be180bb this PR @ 6c00bdcf
python -I scripts/check_third_party.py 3 errors: a64-kit, sdl2, zbic differ from their notes third_party: 11 parts as their notes say
python -I -B tests/test_check_third_party.py FAILED (failures=2) OK (9 tests, 1 skipped)
the same with the new é.dll case, against this PR's first commit (bytes, no -z) FAILED (failures=1) OK

Testing

Tested: the two commands above on Windows 11, on main and on this PR, and:

  • git check-ignore --no-index --stdin given third_party/sdl2/SDL2.dll\n through Python with text=True prints nothing (exit 1); given the same bytes it prints the path (exit 0).
  • The tracked files of those three parts are byte-identical to HEAD on disk, so the difference was only the ignored binaries.
  • python -I -B tests/run_all.py on this branch: 456 passed, 4 failed. test_setup_runtime.py and test_release.py fail on main @ 77f605a2 too. test_fpx_sticky.py and test_spawn_emit.py pass when run alone, on main and here. In the full run, test_fpx_sticky.py failed because LONGSHOT_MINGW wasn't set in that shell; why test_spawn_emit.py failed only in the full run I don't know.

Not tested: Linux and macOS: the bytes and text paths are the same there, and CI's third-party job runs on Linux.

Needs other testers: None

Screenshots

Nothing on screen

Risk and rollback

Only the check script changes; reverting the commit is enough. With -z git neither quotes nor escapes the paths, so what comes back is byte for byte what was sent.

Reviewer notes

CI runs this check on Linux only, which is why it didn't catch this.

Issues

None

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing. (No game code changed: a script fix.)
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (n/a: not a performance change; both sides' results are in the table.)
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.

🤖 Generated with Claude Code

## Why On Windows, `scripts/check_third_party.py` fails on any checkout set up the way CONTRIBUTING.md says: the Builder's binaries copied into `third_party/` (`a64emit.dll`, `lld-link.exe`, `SDL2.dll`, `zbic.dll` and others) are counted although `.gitignore` leaves them out, so a64-kit, sdl2 and zbic "differ from their notes". ## What changed - scripts: `ignored()` in `scripts/check_third_party.py` asks `git check-ignore --no-index -z --stdin`, NUL-separated and as UTF-8 bytes both ways, instead of newline-separated text. Two bugs went with the old form: - In text mode on Windows, Python writes each ` ` as ` `, so git read every path with a trailing ` `, matched no rule, and nothing was skipped. - Without `-z`, git quotes a path with a non-ASCII byte in its answer (`"third_party/x/é.dll"`), which matched none of ours, so an ignored `é.dll` was hashed with its part, on every OS (from HumanBean's review). - tests: `test_a_clean_tree_passes` in `tests/test_check_third_party.py` adds an ignored `é.dll` beside `foo.dll`; it fails without `-z` on Linux too, where CI runs. ## Flags and settings None ## Measurements Not a performance change, so no fps or timings: the check's result on both sides. main: 1be180bb (2026-11-13) this PR: fix/check-third-party-windows-newlines @ 6c00bdcf Hardware: Windows 11, Python 3 from python.org, git 2.56.1 (`core.autocrlf=true`) Scene: n/a (game-free) Method: each command run once on each side, on the same checkout, set up as CONTRIBUTING.md says with the Builder's third_party binaries copied in How to reproduce: on Windows, copy the Builder's `tools\third_party` binaries in as CONTRIBUTING.md says, then run the two commands below | | main @ 1be180bb | this PR @ 6c00bdcf | |---|---|---| | `python -I scripts/check_third_party.py` | 3 errors: a64-kit, sdl2, zbic differ from their notes | `third_party: 11 parts as their notes say` | | `python -I -B tests/test_check_third_party.py` | FAILED (failures=2) | OK (9 tests, 1 skipped) | | the same with the new `é.dll` case, against this PR's first commit (bytes, no `-z`) | FAILED (failures=1) | OK | ## Testing **Tested:** the two commands above on Windows 11, on main and on this PR, and: - `git check-ignore --no-index --stdin` given `third_party/sdl2/SDL2.dll\n` through Python with `text=True` prints nothing (exit 1); given the same bytes it prints the path (exit 0). - The tracked files of those three parts are byte-identical to `HEAD` on disk, so the difference was only the ignored binaries. - `python -I -B tests/run_all.py` on this branch: 456 passed, 4 failed. `test_setup_runtime.py` and `test_release.py` fail on main @ 77f605a2 too. `test_fpx_sticky.py` and `test_spawn_emit.py` pass when run alone, on main and here. In the full run, `test_fpx_sticky.py` failed because `LONGSHOT_MINGW` wasn't set in that shell; why `test_spawn_emit.py` failed only in the full run I don't know. **Not tested:** Linux and macOS: the bytes and text paths are the same there, and CI's third-party job runs on Linux. **Needs other testers:** None ## Screenshots Nothing on screen ## Risk and rollback Only the check script changes; reverting the commit is enough. With `-z` git neither quotes nor escapes the paths, so what comes back is byte for byte what was sent. ## Reviewer notes CI runs this check on Linux only, which is why it didn't catch this. ## Issues None ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. (No game code changed: a script fix.) - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (n/a: not a performance change; both sides' results are in the table.) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
HumanBean reviewed 2026-11-13 05:49:50 +00:00
HumanBean left a comment

I read the diff against ignored() and its callers in scripts/check_third_party.py, and tests/test_check_third_party.py. The diagnosis holds: I tried it with git 2.56 here, and git check-ignore --no-index --stdin matches nothing for a.dll\r\n, so the \r that text mode writes on Windows really does stop every rule from matching. Sending bytes fixes that, and Linux behaves the same as before.

Two things before it goes in. Neither blocks it, but both are cheap:

  • Without -z, git quotes any path that has a non-ASCII byte in it (core.quotePath). An ignored third_party/x/é.dll comes back as "third_party/x/\303\251.dll", doesn't match, and gets hashed with no error. So the Risk section's "the decode would fail loudly" isn't what happens. -z on both sides fixes the \r and the quoting together, and the encode/decode then really are byte for byte.
  • Nothing in the PR fails without the fix on Linux, where CI runs. A test with an ignored non-ASCII file (é.dll beside foo.dll in test_a_clean_tree_passes) would fail on Linux today and pass with -z. It would also guard the Windows case, because it pins the same parsing.

The title is a fine Conventional Commit, and no check under tests/ cuts this function by its text.

I read the diff against `ignored()` and its callers in `scripts/check_third_party.py`, and `tests/test_check_third_party.py`. The diagnosis holds: I tried it with git 2.56 here, and `git check-ignore --no-index --stdin` matches nothing for `a.dll\r\n`, so the `\r` that text mode writes on Windows really does stop every rule from matching. Sending bytes fixes that, and Linux behaves the same as before. Two things before it goes in. Neither blocks it, but both are cheap: - Without `-z`, git quotes any path that has a non-ASCII byte in it (`core.quotePath`). An ignored `third_party/x/é.dll` comes back as `"third_party/x/\303\251.dll"`, doesn't match, and gets hashed with no error. So the Risk section's "the decode would fail loudly" isn't what happens. `-z` on both sides fixes the `\r` and the quoting together, and the `encode`/`decode` then really are byte for byte. - Nothing in the PR fails without the fix on Linux, where CI runs. A test with an ignored non-ASCII file (`é.dll` beside `foo.dll` in `test_a_clean_tree_passes`) would fail on Linux today and pass with `-z`. It would also guard the Windows case, because it pins the same parsing. The title is a fine Conventional Commit, and no check under tests/ cuts this function by its text.
scripts/check_third_party.py Outdated
@ -59,2 +58,2 @@
capture_output=True, text=True, encoding="utf-8")
return set(out.stdout.split("\n")) - {""}
# (bytes, not text: in text mode Windows sends each path with a \r, which matches no rule, so ignored files counted)
out = subprocess.run(["git", "check-ignore", "--no-index", "--stdin"], cwd=root,
HumanBean commented 2026-11-13 05:49:50 +00:00
Owner

Suggest -z here: ["git", "check-ignore", "-z", "--no-index", "--stdin"] with input="\0".join(paths).encode("utf-8"). Without it git C-quotes non-ASCII paths in its output (core.quotePath, on by default), so an ignored é.dll comes back as "third_party/x/\303\251.dll", never matches, and is hashed silently. I checked with git 2.56: printf 'é.dll\n' | git check-ignore --no-index --stdin prints the quoted form, and with -z it prints the raw UTF-8 bytes. With -z, input lines are NUL-terminated, so the \r problem goes away too.

Suggest `-z` here: `["git", "check-ignore", "-z", "--no-index", "--stdin"]` with `input="\0".join(paths).encode("utf-8")`. Without it git C-quotes non-ASCII paths in its output (`core.quotePath`, on by default), so an ignored `é.dll` comes back as `"third_party/x/\303\251.dll"`, never matches, and is hashed silently. I checked with git 2.56: `printf 'é.dll\n' | git check-ignore --no-index --stdin` prints the quoted form, and with `-z` it prints the raw UTF-8 bytes. With `-z`, input lines are NUL-terminated, so the `\r` problem goes away too.
scripts/check_third_party.py Outdated
@ -61,0 +58,4 @@
# (bytes, not text: in text mode Windows sends each path with a \r, which matches no rule, so ignored files counted)
out = subprocess.run(["git", "check-ignore", "--no-index", "--stdin"], cwd=root,
input=("\n".join(paths) + "\n").encode("utf-8"), capture_output=True)
return set(out.stdout.decode("utf-8").splitlines()) - {""}
HumanBean commented 2026-11-13 05:49:50 +00:00
Owner

With -z this becomes out.stdout.decode("utf-8").split("\0"). Small thing: as it stands, splitlines() also splits on \x0b, \x0c, \x1c-\x1e, \x85 and \u2028/\u2029. That's harmless for real paths, but split("\n") (or "\0") says exactly what git sends.

With `-z` this becomes `out.stdout.decode("utf-8").split("\0")`. Small thing: as it stands, `splitlines()` also splits on `\x0b`, `\x0c`, `\x1c`-`\x1e`, `\x85` and `\u2028`/`\u2029`. That's harmless for real paths, but `split("\n")` (or `"\0"`) says exactly what git sends.
d4nkn3ss force-pushed fix/check-third-party-windows-newlines from 388d24f8f2
Some checks failed
Linux AppImage / appimage (pull_request) Blocked by required conditions
Details
Windows builder / Longshot Builder zip (pull_request) Blocked by required conditions
Details
Windows builder / Windows Vulkan back end (llvm-mingw) (pull_request) Blocked by required conditions
Details
Windows builder / dav1d for Windows (llvm-mingw) (pull_request) Blocked by required conditions
Details
PR description / Description follows the template (pull_request_target) Successful in 2s
Details
CI / Conventional Commit title (pull_request) Has been cancelled
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
Details
CI / Python source checks (windows-2022, part 1/2) (pull_request) Has been cancelled
Details
CI / Python source checks (windows-2022, part 2/2) (pull_request) Has been cancelled
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
Details
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Details
Third-party parts / As their notes say (pull_request) Has been cancelled
Details
Third-party parts / The same as upstream (pull_request) Has been cancelled
Details
to 6c00bdcff9
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
Details
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Details
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Details
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Details
PR description / Description follows the template (pull_request_target) Successful in 3s
Details
2026-11-13 07:47:34 +00:00 Compare
d4nkn3ss commented 2026-11-13 07:47:56 +00:00
Author
Contributor

Thanks, both points taken in 6c00bdcf (rebased onto main 1be180bb):

  • ignored() now asks git check-ignore --no-index -z --stdin, NUL-separated bytes both ways, so the \r and the quoting go together. You were right about the Risk line: the quoted name came back as a different string and was hashed silently, nothing failed. The line now says what -z gives.
  • test_a_clean_tree_passes adds an ignored é.dll beside foo.dll. Against this PR's first commit (bytes, no -z) it fails, FAILED (failures=1); with -z, OK. It doesn't depend on the OS, so it guards Linux CI as well as the Windows case.

python -I scripts/check_third_party.py on the Windows checkout with the Builder's binaries copied in still says third_party: 11 parts as their notes say. The description is updated to match.

Thanks, both points taken in 6c00bdcf (rebased onto main 1be180bb): - `ignored()` now asks `git check-ignore --no-index -z --stdin`, NUL-separated bytes both ways, so the `\r` and the quoting go together. You were right about the Risk line: the quoted name came back as a different string and was hashed silently, nothing failed. The line now says what `-z` gives. - `test_a_clean_tree_passes` adds an ignored `é.dll` beside `foo.dll`. Against this PR's first commit (bytes, no `-z`) it fails, `FAILED (failures=1)`; with `-z`, OK. It doesn't depend on the OS, so it guards Linux CI as well as the Windows case. `python -I scripts/check_third_party.py` on the Windows checkout with the Builder's binaries copied in still says `third_party: 11 parts as their notes say`. The description is updated to match.
monsterlag approved these changes 2026-11-13 18:46:26 +00:00
monsterlag left a comment

Approved current head 6c00bdcff9 after inspecting the two-file diff and independently checking the pathname protocol on native Windows and Linux.

Native Windows Python 3.14.7: all 9 checker tests ran successfully (1 platform skip), and the production checker accepted all 11 vendored parts with the ignored SDK SDL DLLs present. Two interleaved runs of the Unicode regression failed on main d906c8f4 and passed on this head each time.

Linux Python 3.12.3 / Git 2.43.0, default core.quotePath=true: the same regression failed on main and passed on this head in two interleaved pairs. This used a separate valid Linux Git fixture, with Git path/config overrides removed; main checker bytes match the Windows baseline. Binary NUL records address both Git pathname quoting and Windows text newline translation without changing the provenance digest definition.

No blocking code findings. I did not run an independent full Longshot suite for this focused review; the checks above cover both changed files and the reported failure modes. No game or generated game code was involved.

Approved current head 6c00bdcff91ce7f828bade61755ae95132511e9e after inspecting the two-file diff and independently checking the pathname protocol on native Windows and Linux. Native Windows Python 3.14.7: all 9 checker tests ran successfully (1 platform skip), and the production checker accepted all 11 vendored parts with the ignored SDK SDL DLLs present. Two interleaved runs of the Unicode regression failed on main d906c8f4 and passed on this head each time. Linux Python 3.12.3 / Git 2.43.0, default core.quotePath=true: the same regression failed on main and passed on this head in two interleaved pairs. This used a separate valid Linux Git fixture, with Git path/config overrides removed; main checker bytes match the Windows baseline. Binary NUL records address both Git pathname quoting and Windows text newline translation without changing the provenance digest definition. No blocking code findings. I did not run an independent full Longshot suite for this focused review; the checks above cover both changed files and the reported failure modes. No game or generated game code was involved.
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
Details
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Details
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Details
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Details
PR description / Description follows the template (pull_request_target) Successful in 3s
Details
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
Some workflows are waiting to be reviewed.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'fix/check-third-party-windows-newlines:d4nkn3ss-fix/check-third-party-windows-newlines'
git switch 'd4nkn3ss-fix/check-third-party-windows-newlines'
Sign in to join this conversation.
Reviewers
No reviewers
monsterlag
Labels
Clear labels   
area/accessibility
Barriers for people with disabilities

  
area/audio
Sound and music

  
area/input
Controllers, keyboard, mouse

  
area/install
Installing, updating, building from your game

  
area/menu
In-game menu and settings

  
area/renderer
Vulkan renderer, shaders, effects

  
area/saves
Save files

  
area/upscaling
DLSS, FSR, frame generation

  
good first issue
A good start for a new contributor

  
gpu/amd
AMD GPUs

  
gpu/igpu
Integrated GPUs

  
gpu/intel
Intel GPUs

  
gpu/nvidia
NVIDIA GPUs

  
help wanted
Extra hands or knowledge wanted

  
kind
bug
 Something doesn't work as it should

  
kind
chore
 Build, CI, dependencies, tidying

  
kind
docs
 Documentation

  
kind
feature
 A new feature or an improvement

  
kind
performance
 Frame rate, frame time, loading, memory

  
kind
question
 A question rather than a change

  
needs measurements
Waiting for before/after measurements

  
platform/linux
Linux only

  
platform/steam-deck
Steam Deck

  
platform/windows
Windows only

  
preview
Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  
priority
critical
 Crashes, data loss, or blocks most players

  
priority
high
 Important: next in line

  
priority
low
 Nice to have

  
status
blocked
 Waits on something else first

  
status
confirmed
 Reproduced or agreed on

  
status
duplicate
 Already reported or proposed

  
status
invalid
 Not a problem in this project

  
status
needs-info
 Waiting for more information from the reporter

  
status
wontfix
 Will not be worked on

  
testing wanted
Wanted: a test on other hardware (posts a test request in #test-requests)

No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!80
No description provided.