fix: self-contained Homebrew launcher, node back to :build - #11
Merged
Conversation
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.
Real fix for #497 (the extra-Node-install complaint) — not the caveats-text
option, the shell-wrapper option. Both were on the table after the revert in
#10; this is the one I picked, with reasoning below.
Why this, not documenting the waste as intentional
rook's published npm package shipsbin/rook.cjs: a small trampoline witha
#!/usr/bin/env nodeshebang, designed to bootstrap under whatever Nodelaunched it, resolve the bundled runtime via
require.resolve, and re-execinto it. Under the old,
:requireddepends_on "node", Homebrew's ownCleaner#rewrite_shebangs(language/node.rb) silently rewrote thatshebang to the absolute path of Homebrew's own node at build time — which
is what made the bottle work, and also exactly the extra install #497
complains about.
The trampoline's whole reason to exist is not knowing at publish time
whether a bundled runtime will be present. Homebrew's
def installdoesknow — it just finished downloading and chmod'ing that exact binary. So
instead of relying on npm's shim plus Homebrew's shebang-rewrite trick, this
PR has
def installwrite its own launcher directly:Only in the success path (bundled runtime installed and verified) — the
existing
opoo-and-fall-back-to-system-Node path is untouched, since npm'soriginal trampoline is exactly the right behavior there.
This is strictly better than either alternative on the table:
env nodeshebang left for Homebrew to rewrite (or fail to) —
depends_on "node"goes back to
=> :build, this time correctly, since nothing depends onthe rewrite happening.
bin/rook.cjsisuntouched; it's still exactly right for npm-direct/internal installs.
path:
execreplaces the shell's own process image, so a signal sentto the launcher's PID lands on the bundled node process natively, same
PID, no relay.
bin/rook.cjs's ~150 lines of SIGINT/SIGTERM/SIGHUPforwarding exist specifically because
spawn()creates a childprocess that needs that relay —
execin a shell script doesn't havethat problem to begin with.
Absolute paths baked into an installed script this way are ordinary,
relocatable Homebrew practice — any formula that writes a wrapper
referencing
#{libexec}does the same; nothing new here.Verification
Local
brew install --build-bottleturned out to be broken in myenvironment for reasons unrelated to this change — I confirmed this by
installing the already CI-verified, currently-live formula (pre-this-PR)
and getting the identical
FormulaUnavailableError. Rather than fight that,I pulled the real published
@testmuai/rookand bundled-runtime npmpackages, reconstructed
def install's exactpkg_dirlayout by hand, andran the actual generated wrapper end to end with
PATH=/usr/bin:/bin(nonode, no Homebrew, nothing):
All 6
scripts/test-*.shharnesses green,ruby -c,actionlintclean.What it takes to actually ship
A bottle pours a pre-built keg — it never runs
def install. Mergingthis alone changes nothing for real users;
def install's new code onlytakes effect once
build-bottles.ymlrebuilds and republishes the 0.1.0bottle. I'll dispatch that after merge and live-verify
brew-smokegreenon both platforms before considering this done — the same live-verification
discipline that caught the #9 regression, this time before declaring
victory instead of after.
New
test doassertion added as a structural guard on the actualmechanism (checks the launcher's shebang, not just that
rook --versionhappens to work) —
brew-smoke.yml's existing "node stays required" guardis flipped back too, this time for the right reason.
Agent-authored: Claude Sonnet 5, Claude Code CLI.