The build and the fetch tooling reach for python3 by name too - #52
Open
chongjiazhen wants to merge 1 commit into
Open
The build and the fetch tooling reach for python3 by name too#52chongjiazhen wants to merge 1 commit into
chongjiazhen wants to merge 1 commit into
Conversation
Follow-up to the suite fix in sqliteai#51, same finding one layer out: on Windows `python3` on PATH is usually the Microsoft Store App Execution Alias, a zero-byte reparse point that exists, exits 49, and prints an advert instead of running anything. run.sh can shim PATH for its own call sites, but that does not reach a recipe make runs itself, nor either tools script when run directly. Eleven call sites: three recipes, four in fetch_weights.sh, four in pipeline.sh. Makefile: serve-check, fuzz and fuzz-asan get $(PY). fetch_weights.sh and pipeline.sh get the same answer as $PY; pipeline.sh exports it so the fetch_weights.sh it drives inherits rather than resolving per stage. $(PY) is recursive rather than `:=` on purpose. Immediate assignment runs the probe on every make invocation, `make clean` and a no-op build included, and where python3 is the alias the first spawn is the alias itself: measured 196 ms per invocation on this box, which nearly triples a no-op make. Lazy, the probe runs only when a Python recipe expands it, and a no-op make is back to 0.664 s against 0.678 s on main. The fallback sits inside the shell for the same reason, since an ifeq on $(PY) would force the expansion at parse time. pipeline.sh is the one that misdiagnoses rather than merely failing: its first two probes end `2>/dev/null || echo 0`, so an interpreter that is not one yields 0 and the script dies "download (no index at $SRC)", naming a missing index that is present and readable. Nothing skips here. If no candidate answers, PY stays python3 and the recipe or script fails loudly at first use, which is right for a build target and for a 1.4 TB download: unlike the suite there is nothing to skip, and a run that cannot read its own index must stop. Verified on Windows 10, MinGW-w64 GCC 15.2. PY resolves to `python`; `make serve-check` reports OK (skipped=3) and `make fuzz FUZZ_RUNS=20` reports 20 cases, 0 crashed, 0 hung, both exit 0; with all three names shimmed to exit 49, serve-check fails loudly with Error 49 rather than skipping. The two tools scripts are not run end to end here, since that needs the full source weights: they carry bash -n plus a three-state check of the resolver. Assisted by AI.
chongjiazhen
force-pushed
the
fix/windows-python3-alias-tooling
branch
from
August 26, 2026 04:45
a866391 to
26ddf7c
Compare
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
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.
Follow-up to #51, offered there and kept separate so each is reviewable on its
own. Same finding one layer out: on Windows
python3on PATH is usually theMicrosoft Store App Execution Alias, a zero-byte reparse point that exists,
exits 49, and prints an advert for the Store instead of running anything.
tests/run.shcan shim PATH for its own call sites. That does not reach arecipe
makeruns itself, nor either tools script when run directly, so 11call sites are still exposed:
Makefileserve-check,fuzz,fuzz-asantools/fetch_weights.shtools/pipeline.shpipeline.shis the one that misdiagnoses rather than merely failing. Its firsttwo probes end
2>/dev/null || echo 0, so an interpreter that is not one yields0and the script dies withdownload (no index at $SRC), naming a missingindex that is present and perfectly readable.
The change
The Makefile resolves
$(PY)by running a candidate; the two scripts resolve$PYthe same way, andpipeline.shexports it so thefetch_weights.shitdrives inherits the answer instead of resolving once per stage.
Nothing skips here, deliberately. If no candidate answers,
PYstayspython3and the recipe or script fails loudly at first use. That is the right behaviour
for a build target and for a 1.4 TB download: unlike the suite there is nothing
to skip, and a run that cannot read its own index must stop rather than carry
on.
fetch_weights.shalready knew it runs on Windows, at thetr -d '\r'commentciting #36 gap 2. This is the same environment, one assumption earlier.
$(PY)is recursive rather than:=deliberately. Immediate assignment runsthe probe on every
makeinvocation,make cleanand a no-op build included,and where
python3is the alias the first spawn is the alias itself: measured196 ms added per invocation, which nearly triples a no-op make. Lazy, it
runs only when a Python recipe expands it. The fallback sits inside the shell
for the same reason, since an
ifeqon$(PY)would force the expansion atparse time and undo it.
Verification
Windows 10, MinGW-w64 GCC 15.2, Python 3.13 working under its own name.
make -preportsPY := python.The two tools scripts are not run end to end here, because that needs the
full source weights. They carry
bash -n, review, and a three-state check ofthe added resolver:
PY=python, runs 3.13.5PY=python3, fails loudly at first use (49)PYalready set in the environmentHappy to fold this into #51 instead, or to change the shape: a single shared
resolver sourced by both scripts would also work, I kept them self-contained to
match how they read today.
Assisted by AI.