Repository navigation
Decide the Sorbet binary in one place (fixes #41) - #154
Open
jiro-by-nori[bot] wants to merge 1 commit into
Open
jiro-by-nori[bot] wants to merge 1 commit into
jiro-by-nori[bot] wants to merge 1 commit into
Conversation
`find_sorbet()` in `books/scripts/srb_sigs.py` and the Sorbet block of `scripts/check-prereqs.sh` were two copies of the same lookup. They also disagreed about `$SORBET`: the script only used it if it existed and was executable, while `find_sorbet()` returned it unconditionally, so a typo reached `subprocess` and failed later in the corpus stage instead of saying which binary was wrong -- and a fallback could silently grade with a different Sorbet. The lookup now lives in `find_sorbet()` alone: * `$SORBET`, when set, must resolve to an executable file (a bare name is looked up on PATH, as before); otherwise it is an error naming the value, so an intended binary is never silently replaced by another one. * the `sorbet-static` gem binary is used only if it is executable, then `srb` on PATH; when none is found the message says so and names `make deps` rather than returning a bare `srb` that cannot run. `scripts/check-prereqs.sh` calls the new `--which-sorbet` mode instead of repeating the RubyGems lookup, and reports the same path, so `make prereqs` and the gate cannot name different binaries. The CI copies of the workaround are already gone, so the duplication was the two sites above. `srb_sigs.py` already records the binary in each manifest's `"sorbet"` field, so a run still records which Sorbet produced its signatures. Tests: `books/scripts/test/test_srb_sigs.rb` pins the gem lookup, an executable `$SORBET`, a bare name on PATH, a missing path, a non-executable file, and that `check-prereqs.sh` reports exactly the path `--which-sorbet` prints (the drift #41 is about).
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.
What this fixes
Issue #41: "where is Sorbet" was written down twice, and the two copies
disagreed.
books/scripts/srb_sigs.py'sfind_sorbet()returned$SORBETunconditionally, so a stale or misspelled value reached
subprocessand failedlater in the corpus stage;
scripts/check-prereqs.shonly accepted$SORBETifit existed and otherwise repeated the RubyGems lookup. Since the binary decides
which programs reach the validator, the wrong one moves every verdict without an
error naming the cause.
The issue also names three
find ... sorbet-staticcopies in.github/workflows/ci.yml; those were removed by an earlier refactor, so theduplication left was the two sites above. (
srb_sigs.pyhas also moved fromruby-lean/scripts/tobooks/scripts/since the issue was filed.)The change
find_sorbet()is now the single definition, and it validates what itreturns:
$SORBET, when set, must resolve to an executable file (a bare name islooked up on PATH, which is what it meant before). A path that does not
exist, or a file without the execute bit, is an error naming the value
instead of a silent fallback to a different Sorbet.
sorbet-staticgem binary is used only when executable, thensrbonPATH; when none is found the message names the cause and
make depsinstead of returning a bare
srbthat cannot run.scripts/check-prereqs.shcalls the newsrb_sigs.py --which-sorbetandreports that path, so
make prereqsand the gate cannot name differentbinaries. The Python check moves above it because the lookup runs through it.
srb_sigs.pyalready records the chosen binary in each manifest's"sorbet"field, so a run still records which Sorbet produced its signatures.
Tests
New
books/scripts/test/test_srb_sigs.rb(6 tests, runs undermake emitter-unit) pins the contract:$SORBETis unset, and it is executable;$SORBETis honoured, and a bare name resolves through PATH;$SORBETthat does not exist, and one without the execute bit, are errorsthat name the value and print nothing on stdout;
scripts/check-prereqs.shreports exactly the path--which-sorbetprints,which is the drift Sorbet-location logic is duplicated in four places and can drift (follow-up to #32) #41 describes.
Validation
Run locally on the branch from a cold toolchain (Lean
v4.32.2, CRuby4.0.5):make prereqs— prints the gem's Sorbet;SORBET=/nope/sorbet scripts/check-prereqs.shreportssorbet MISSING — $SORBET is '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/nope/sorbet', which is not an executable fileand exits 1;srb_sigs.py --which-sorbetagrees with the script's line.make conformance—ran 1287, agree 1087, sut_unsupported 194, disagree 0,exactly the recorded baseline.
make emitter-unit— 373 runs, 444 assertions, 0 failures; coverage holds(
line 98.17%,branch 82.33%).make lint— ruff (format + check), RuboCop, shellcheck and actionlint allclean;
make gen-check,make docs,make books,make soundness,make comparatorall pass as part ofmake check.added.
Authored by Jiro, an autonomous coding agent: both the code and this
description.