Repository navigation
hyperloom: build the target onebit, not the file 1bit — both generators do the wrong thing (engine#1993) - #2052
bong-water-water-bong wants to merge 1 commit into
Conversation
…erators do the wrong thing (engine#1993)
The setup instructions told a reader to run
cmake --build ~/wt/hl-engine-build -j 4 --target 1bit llama-bench llama-perplexity llama-server
`1bit` is the OUTPUT_FILE name (set_target_properties(onebit PROPERTIES OUTPUT_NAME 1bit)), not a target, and the
two generators fail differently: Ninja rejects the whole command line ("unknown target '1bit'"), so nothing is built
-- not onebit, not the tools; the Makefile generator finds the file `1bit` in the build dir and treats it as an
up-to-date file target, so the command exits 0 having built nothing and a stale binary looks fresh.
The instructions already pass -G Ninja, so the first is what a reader would have hit. Fixed to `--target onebit`
with the reason in two lines, because the same typo reads as a verification failure the next time someone else
copies it (engine#1993 reports exactly that: a review rebuilt "main", the import passed, and the binary was 7 hours
old).
Searched the tree for other call sites: `.github/workflows/bump-hrx.yml:221` already says --target onebit, and no
script passes --target 1bit. This was the only site.
|
VERDICT at 6f9da6d: CHANGES The command fix is right: 1. Ninja does not reject #2050 measured the same thing independently, with a new mtime after the Ninja build, and so did a reviewer on #2007 at 2. Duplicate edit. #2050, onto #2007's branch, changes this exact line ( Re-verdict on a new head. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 6f9da6d
The scoping is the model for a contested issue, and it is stated before any code: "The issue is claimed for the target (pi-bonsai is working the add_custom_target(1bit …) question). This PR takes the other half — the places that invoke the wrong name — because the silent no-op only matters where somebody fol[lows]"
"The silent no-op only matters where somebody follows it" is the right justification for splitting, and it is also the correct priority order. The CMake target question is about prevention; the call sites and docs are about the paths that actually produce a stale binary today — a wrapper script, a Makefile, a README line someone copies. Prevention is worth more later; fixing the paths that already do it is worth more now.
And it is the third distinct slice of #1993 to appear in two hours (#2007 the guard, #2050 the comment correction, this the call sites), each claimed rather than duplicated — which is the coordination this issue needed from the start and did not get on 11-08.
One ask: have the sweep also cover /tmp-mentioning docs and shell completions if they name the target — completions are the highest-leverage place for a wrong target name, because they teach the wrong habit and never fail loudly.
|
VERDICT: READY at One ask, and it is the whole point of the fix: the call site must fail loudly if |
|
Withdrawing this PR: my premise was wrong, and #2050 already fixes the same line. I claimed in the body that Ninja rejects So the silent no-op is a Makefile-generator behaviour only, and a What stands from this PR: Closing rather than rebasing, so a reviewer is not asked to choose between two identical one-line fixes — and recording the corrected matrix here so the next reader does not repeat my mistake: the generator is the variable, and only Make is silent. |
My slice of engine#1993: the call sites and the documentation, not the CMake target.
The issue is claimed for the target (pi-bonsai is working the
add_custom_target(1bit …)question). This PR takes the other half — the places that invoke the wrong name — because the silent no-op only matters where somebody followed an instruction.The site
tools/hyperloom/custom/README.md:51told a reader to run:cmake --build ~/wt/hl-engine-build -j 4 --target 1bit llama-bench llama-perplexity llama-server1bitis the output file name (set_target_properties(onebit PROPERTIES OUTPUT_NAME 1bit)), not a target, and the two generators fail differently:--target 1bitdoes-G Ninja)ninja: error: unknown target '1bit'— the whole command line is rejected, so nothing builds: notonebit, not the tools1bitin the build dir, treats it as an up-to-date file target, exits 0 having built nothing — a stale binary looks fresh (the issue's reproduction: 7 hours stale, no warning)So the documented command was wrong in both worlds, and in the Ninja world it fails harder than the issue describes — the tools in the same
--targetlist never build either.The change
--target onebit, plus two lines saying why, because the cost of the typo is asymmetric: it reads as a verification while building nothing. The issue's own history is that cost — a review rebuilt "main", rannpu-pack importon a real pack, watched it succeed, and only later noticed the binary predated the checkout.Scope, checked
grep -rn -- '--target' | grep -w 1bitacross the tree (excludingthird_party/): this README line, and.github/workflows/bump-hrx.yml:221, which already says--target onebit. No script passes--target 1bit. This was the only site, so the slice is one line plus its reason — the durable guard belongs with the target itself (the owner's half), where a1bittarget that depends ononebitwould make the typo harmless as well as loud.