refactor: replace the bash launcher with a JavaScript launcher - #2989
Draft
acozzette wants to merge 10 commits into
Draft
refactor: replace the bash launcher with a JavaScript launcher#2989acozzette wants to merge 10 commits into
acozzette wants to merge 10 commits into
Conversation
Move the js_binary/js_test launcher logic out of js_binary.sh.tpl and into a
new JavaScript template, js_binary.cjs.tpl. The bash template shrinks from 578
lines to a stub that resolves node and the JavaScript launcher out of the
runfiles tree and execs it; removing that stub is a follow-up.
The translation is close to literal, with a few points worth calling out:
- Where bash did `exec node`, the launcher calls process.execve, so the process
is replaced rather than spawning a child: same PID, no extra node startup.
Node <22.15 and the capture paths (no dup2 in JavaScript) fall back to
child_process.spawn with signal forwarding.
- The spawn fallback re-raises a fatal signal on itself after mop up, so it
reports signal termination rather than bash's 128+N. Both paths now behave
identically; the bash non-exec path did not.
- Env values, node options and fixed args used to be spliced into double-quoted
bash, so shell parameter expansion happened at launch time and callers depend
on it (examples/stack_traces). expandEnvRefs() reproduces $VAR / ${VAR};
command substitution is not reproduced and the result is not re-word-split.
- fixed_args tokenizing moves to Starlark (_shell_tokenize), since bash is no
longer there to word split and remove quotes. Backslash escapes are
deliberately not interpreted, which leaves Windows-style paths intact.
- Launcher log lines keep bash's `echo -e $(printf ...)` whitespace collapsing,
which is what renders the multi-line BAZEL_BINDIR diagnostic on one line.
The generated JavaScript launcher is exposed in a launcher_js output group so
that it can be asserted on and snapshotted. The image listing goldens gain the
new per-target .cjs runfile, whose size is masked like the bash launcher's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit left js_binary.sh.tpl as a 40-line stub whose only job was to resolve the runfiles tree, find node and the generated JavaScript launcher in it, and exec node on the launcher. hermetic_launcher does exactly that natively, so the stub is replaced by a ~19KB static binary stamped from a prebuilt template. js_binary.sh.tpl, js/private/bash.bzl (BASH_INITIALIZE_RUNFILES), the snapshots/launcher.sh golden, create_windows_native_launcher_script and the @bazel_tools//tools/sh:toolchain_type dependency are all deleted: there is no shell anywhere on the js_binary launch path now, on any platform. This stays small because the JavaScript launcher is generated per target, so env, node options, fixed args and the entry point are already baked into it. The embedded argv is just [node, launcher.cjs], two of the stub's ten slots, and no per-target constants are lost. Both entries are marked for runfiles resolution, and their rlocation paths carry no output tree configuration segment, so the launcher stays byte-identical across configurations and path mapping keeps sharing cache entries. Runfiles discovery moves into the launcher: a port of the remaining half of BASH_INITIALIZE_RUNFILES, minus the $0 walk, since the stub self-locates and exports whichever source it settled on. compile_stub is reimplemented locally rather than calling the upstream helper, which names its toolchain types with string labels. Those resolve against the repo mapping of whatever repo the js_binary is instantiated in, and a user repo has no visibility on @hermetic_launcher. hermetic_launcher registers stub toolchains for linux x86_64/aarch64/s390x, macOS x86_64/aarch64 and Windows x86_64 only. Rather than fail analysis elsewhere, the template toolchain is optional and a target platform without a stub gets a placeholder that reports the problem when run -- a js_binary is routinely built for a platform it can never run on, and `bazel build //...` should keep working there (#2347). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
js_image_layer patched the js_binary launcher by expand_template-ing the executable, which used to be the bash script: it injected an `export BAZEL_BINDIR="."` preamble and rewrote JS_BINARY__BINDIR and JS_BINARY__TARGET_CPU to runtime-computed values. The executable is now a native launcher, so none of those substitutions matched and the "patched" launcher was a byte-identical copy. Containers got no BAZEL_BINDIR and the launcher aborted with the "BAZEL_BINDIR must be set" fatal. Patch the .cjs launcher instead, via the launcher_js output group. The native launcher embeds only runfiles-relative paths so it needs no sanitizing; the container binary path now maps to it directly, and it is the runfiles copy of the .cjs that gets swapped for the patched one. Targets that are not a js_binary have no launcher_js group and skip patching, rather than getting a no-op copy. The e2e image invoked the launcher as `/usr/bin/bash /app/src/bin`, which exits 126 on a native binary; run it as the entrypoint directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The native launcher embedded `node <launcher>.cjs`, so the launcher was
node's main module and the only way it could reach the entry point was to
exec node a second time. Embed the entry point too:
node --require <launcher>.cjs -- <entry point> [args...]
The launcher is now a preload of a node process that is already set up to
run the entry point, so it will be able to just return instead of exec'ing
and leave one node process per launch instead of two.
This change is the plumbing only: the launcher still always execs, so
behavior is unchanged. The launch site carries the list of conditions that
have to hold before the exec can be skipped.
Node fixes which file it runs as main before any preload executes, and
assigning to process.argv[1] only changes what the program reads out of
argv. So the launcher can no longer simply return on the paths where it
has to outlive the program -- a capture with a declared output file, an
expected exit code, or Node before 22.15 -- because the entry point would
then run here as well as in the spawned child. Those paths throw a
sentinel out of the preload instead, which skips node's main while leaving
the event loop running for the child's exit handler.
The entry point is embedded as the File rather than as entry_point_path,
which for a DirectoryPathInfo entry point reaches into the directory and
is not a runfiles manifest key. If either path is too long for one of the
stub's 256-byte arg slots, the launcher falls back to the previous
`node <launcher>.cjs` shape, which is correct but can never skip the exec.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`node --require <launcher>.cjs -- <entry point>` broke every js_binary
launched with a relative runfiles dir. The stub resolves a runfile to
`<runfiles dir>/<rlocation path>`, and the runfiles dir is relative
whenever the caller's is -- an sh_binary running a js_binary, which is
`examples/js_binary` use case 10 and what CI caught. Node resolves a
`--require` value that is neither absolute nor ./-prefixed as a bare
package specifier, so the launcher failed with MODULE_NOT_FOUND before
any of it ran. hermetic_launcher 0.0.15 is the newest release and can
neither emit an absolute path nor prefix a transformed arg, so there is
no fix on the stub side. As node's main the same relative path is fine,
since node resolves argv[1] as a path relative to cwd.
So the stub argv goes back to `node <launcher>.cjs` and the launcher
stays node's main module. To reach one node process per launch it will
hand off to the entry point in this process instead of exec'ing:
process.argv = [process.argv[0], entryPoint, ...args]
process.execArgv = ['--require', nodePatches, ...nodeOptions]
require(process.env.JS_BINARY__NODE_PATCHES)
require('node:module').runMain()
runMain is what node's own startup calls for the file it was given, so
`require.main === module` holds and an ESM entry point is detected the
same way. That consumes no stub arg slots, needs no absolute runfiles
dir, and needs nothing to stop node running a main of its own, so the
sentinel-throw the preload shape required is gone too.
This leaves no behavior change from before the preload attempt: the
launch site records the conditions the in-process path needs, and the
rule records why the entry point is not embedded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
acozzette
marked this pull request as ready for review
August 26, 2026 22:14
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f045478f76
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+32
to
+34
| return p | ||
| .replace(/^(.):/, (_match, drive) => '/' + drive.toLowerCase()) | ||
| .replace(/\\/g, '/') |
There was a problem hiding this comment.
In the Windows --enable_runfiles configuration used by e2e/bzlmod/.bazelrc:6, TEST_SRCDIR or RUNFILES_DIR is a native drive path such as C:\...; converting it to /c/... was required by the former MSYS/bash launcher but is not understood as the same path by native Windows Node. The launcher consequently constructs its entry-point and tool paths beneath an invalid runfiles root and fails the isFile checks before starting the target.
Useful? React with 👍 / 👎.
| // ============================================================================== | ||
|
|
||
| let bazelOutSegment | ||
| if (process.cwd().includes('/bazel-out/')) { |
There was a problem hiding this comment.
On native Windows, process.cwd() uses backslashes, so a runfiles working directory such as C:\...\bazel-out\...runfiles\_main never matches this forward-slash check. The launcher then treats that directory as an execroot and reaches the fatal BAZEL_BINDIR must be set branch during ordinary bazel run/bazel test invocations where that variable is absent; this affects the repository's checked --enable_runfiles Windows flow.
Useful? React with 👍 / 👎.
| // A stream that is only subject to silent_on_success has no final destination, so | ||
| // it is buffered in a temp file and replayed on failure. | ||
| function mktemp(name) { | ||
| const p = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'js_binary-')), name) |
There was a problem hiding this comment.
Whenever the legacy direct-capture path enables JS_BINARY__SILENT_ON_SUCCESS, each captured stream creates a new js_binary-* directory, but mopUp only unlinks the file inside it and never removes the directory. Repeated actions therefore leave one or two empty directories in the system temp directory per invocation, causing unbounded filesystem clutter in long-running CI workers.
Useful? React with 👍 / 👎.
Found by code review of the bash-to-JavaScript port. Each is a regression
against the launcher this PR replaces, not a pre-existing issue.
js_image_layer set BAZEL_BINDIR="." from a preamble that replaced the
shebang, so it ran ahead of the environment block. The port folded it into
the JS_BINARY__BINDIR patch, which js_binary emits after `env` and
`fixed_env`. Since setEnv expands $VAR references as it goes, a
js_binary(env = {"FOO": "$BAZEL_BINDIR/x"}) inside an image expanded
against an empty value. Split the two patches again so BAZEL_BINDIR is set
at the top of the launcher, where bash set it. e2e/js_image_oci now asserts
on such an env value; without the fix it reads "/marker", not "./marker".
mktemp buffers a stream for silent_on_success. bash mktemp made a bare
file, but the race-free equivalent here is mkdtempSync, so each call also
made a directory that mopUp never removed: two stray directories in TMPDIR
per run, and silent_on_success is now the default in more places. Share one
directory between the streams and remove it in mopUp.
The bash launcher ran under `set -o errexit` with `trap _exit EXIT`, so a
failure anywhere still replayed the captured streams and removed the temp
files. Nothing does that by default in JavaScript: process.chdir() on a
missing directory printed a raw stack trace, discarded the buffered stderr
holding the launcher's own diagnostics, and leaked the temp files. Add an
uncaughtException handler that reports through logfFatal and exits via
exitWith. Whatever comes to run the entry point in this process has to
remove it first.
_launcher_js returned None for a target with no launcher_js output group,
which skipped sanitizing entirely -- no BAZEL_BINDIR, no TARGET_CPU rewrite
-- with no build-time signal and a container-run-time failure that says
nothing about the layer. js_run_devserver was such a target: it shares
create_launcher but returned only DefaultInfo. Give it the output group and
make a missing one a build error.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jbedard
reviewed
Aug 26, 2026
| # Since js_binary depends on bash we have to bring in a base image that has bash | ||
| base = "@debian", | ||
| # This is `/[js_image_layer 'root']/[package name of js_image_layer 'binary' target]/[name of js_image_layer 'binary' target]` | ||
| cmd = ["/app/src/bin"], |
Member
There was a problem hiding this comment.
This looks like a breaking change already...
acozzette
marked this pull request as draft
August 27, 2026 21:32
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
2 participants
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.
This change takes our bash launcher, translates it more-or-less literally into JavaScript, and puts hermetic_launcher in front of that, removing our dependence on bash for launching Node executables. (Technically bash may still be involved in some cases, because we have a tiny bash wrapper for node at
js/private/node_bin/nodeand a similar one for npm. These wrappers are made available on the PATH, but we do not call them ourselves.)Currently hermetic_launcher invokes node to run the generated JS launcher, which in turn execs node again to run the real entry point. In certain situations we will need to keep exec'ing node twice, but I think in the common case we can put in a fast path that skips the second exec and goes straight to the main entry point without that.
Changes are visible to end-users: yes
Replace the generated bash launcher with a JavaScript launcher
Test plan