feat: add hermetic launcher as opt-in alternative to bash launcher by acozzette · Pull Request #3002 · aspect-build/rules_js

acozzette · 2026-09-02T21:45:20Z

This change introduces a new launcher based on hermetic_launcher. For now the bash launcher is still the default, but you can opt in to the new launcher with --@aspect_rules_js//js:hermetic_launcher.

This launcher consists of a small native binary produced with hermetic_launcher that execs a generated JS launcher which is currently just a transliteration of the bash launcher, minus a few deprecated features.

There is probably not much of a performance win right now (if any), because we are just replacing a bash invocation with a node invocation (now node is getting invoked twice). I plan to fix that in a follow-up, though. In the common case where expected_exit_code is not used, we should be able to have the generated JS launcher jump straight to the real entry point without needing to exec node again.

Changes are visible to end-users: yes

Searched for relevant documentation and updated as needed: yes
Breaking change (forces users to change their own code or config): no
Suggested release notes appear below: yes

An experimental new launcher based on hermetic_launcher is now available: opt in by passing --@aspect_rules_js//js:hermetic_launcher.

Test plan

Covered by existing test cases
New test cases added

Three fixes to the hermetic launcher added in the previous commit: - Drop the `NODE_V8_COVERAGE` export. #2993 removed these lines from `js_binary.sh.tpl` so that coverage is started from within node; the JS launcher was ported from a pre-#2993 base and still carried them. Output is byte-identical either way (node collects natively when the variable is preset), but the launcher sets it *after* chdir'ing to `BAZEL_BINDIR`, where `coverage.cjs` resolves `COVERAGE_DIR` against the execroot, so a relative `COVERAGE_DIR` would have landed profiles in the wrong place. - Normalize `RUNFILES_MANIFEST_FILE` before testing its suffix, as `bash.bzl` does. On Windows Bazel hands out a backslash-separated path, which would not match `/MANIFEST` and would take the fatal branch. - Accept a drive-letter or UNC prefix when deciding whether a `node_toolchain`'s `target_tool_path` is absolute. `startswith("/")` misses `C:\...`, which would then be embedded in the stub as the rlocation `_main/C:\...` and never resolve. The launcher itself already classified it correctly via `path.isAbsolute`. The latter two are Windows-only paths, which this repo does not exercise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The JS launcher was ported from a branch based on #2984, so the bash launcher changes that landed after it were missing: - #2995 dropped the `JS_BINARY__BINDIR` fallback in `resolve_execroot_bin_path`. `BAZEL_BINDIR` is already required by then, so the fallback only masked a misconfiguration. - #2991 made `NODE_DISABLE_COMPILE_CACHE=1` unconditional, leaving `bootstrap.cjs` to re-enable the cache when `NODE_COMPILE_CACHE` is set. The two forms agree on every combination the tests cover, but the conditional is the behavior #2991 deliberately replaced. The other two changes in that window were already reflected: #2994 moved chdir into `bootstrap.cjs` and #2984 moved coverage report generation into a node exit listener, and the JS launcher does neither. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The JavaScript launcher is a transliteration of the bash one, but nothing enforced that. A target gets one launcher per configuration, so every existing both-launcher test skips one side and CI covers the other by running the whole suite again with the flag on. That catches breakage but not drift: the three unported bash-launcher changes fixed in the last two commits were invisible to the suite, and were only found by hand-diffing js_binary.sh.tpl's history. //js/private/test/launcher_sync builds one js_binary twice through a configuration transition -- once with //js:hermetic_launcher off, once with it on -- runs both, and diffs the state node ends up in: process.env, the cwd, argv and execArgv. Comparing the launchers against each other rather than against a golden means there is no snapshot to regenerate, nothing to keep stable across the CI matrix, and the test fails on every leg rather than one. The transition is on the rule, so it reaches the tool down the cfg = "exec" edge, which does not reset Starlark build settings. Should that ever change, both variants would silently be the bash launcher and the diff would pass for the wrong reason, so the rule asserts on the launcher_js output group instead of assuming. Two divergences it found immediately, both fixed here: - process.chdir() does not maintain PWD, so a program under the JS launcher saw a PWD pointing at the execroot while its cwd was the bindir -- and so did every child process it spawned. bootstrap.cjs already does this fixup after its own chdir, but only when chdir is set. - The PATH prepend interpolated `${process.env.PATH}` unguarded, so with no PATH in the environment it put a directory literally named "undefined" on the path. Bash never hits this because it always has a PATH of its own. Also adds //js/private/test:write_launcher_js to update-snapshots.sh, which could not reach it before because it needs the flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two ways the generated launcher could differ from the bash one it transliterates, both found by a review of the launcher work. The paths substituted into js_binary.cjs.tpl -- the entry point, node, npm, the two wrappers and node-patches -- were spliced raw into JavaScript string literals. The bash launcher put them in double quotes, where neither a backslash nor an apostrophe is special, but in JavaScript both are: a Bazel label may contain an apostrophe, which ends the literal and makes the generated launcher a syntax error, and a node_toolchain target_tool_path on Windows is a backslash-separated path whose \n is read as a newline escape. Substitute them as JSON literals bound to consts instead. //js/private/test/entry_point_quoting covers the first case, which fails to build without this. fixed_args lost their quoting at analysis time but were expanded at run time, so the single quotes that told bash not to expand $VAR were gone by the time the launcher looked. `fixed_args = ["'$HOME/x'"]` reached the program as $HOME/x under the bash launcher and as the expanded path under this one. _shell_tokenize now emits each token as a list of [text, expand] segments so that decision survives. Both directions are covered by //js/private/test/fixed_args. Backslash escapes are still not interpreted, so `\$VAR` remains a divergence; that is deliberate, to keep a Windows-style path in a fixed_arg intact, and is now documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

js_image_layer rewrites the launcher for hermeticity, and which file that is depends on the launcher in use: with the bash launcher it is the executable itself, with the hermetic one it is the .cjs in runfiles, since the executable is a native stub with nothing patchable in it. The launcher_js output group is what tells the two apart. That group only exists if the rule that called js_binary_lib.create_launcher republishes it. js_binary and js_run_devserver do; the custom rule this repo documents as the example, js/private/test/create_launcher/custom_test.bzl, did not. So with --@aspect_rules_js//js:hermetic_launcher set, a layer built over such a rule took the bash branch and ran expand_template over the ELF stub, which decodes its input as UTF-8: the stub came out 3951 replacement characters and 7839 bytes larger, still recognizable as an ELF because \x7fELF is ASCII, and exec'd with "Exec format error". Nothing failed at build time -- expand_template does not require its substitutions to match. Fail at analysis instead of guessing, and name the four lines a custom rule needs. All three create_launcher callers in this repo publish the group and every js_image_layer here is over a js_binary, so nothing in tree trips the new check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jbedard · 2026-09-04T17:27:01Z

    # 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"],


If it's off by default why was this required? Or was this just bad practice and could be done in its own "refactor" PR beforehand?

jbedard · 2026-09-04T17:29:01Z

+`//js/private/test:write_launcher_js`, and diffing them is how a change to
+either is reviewed.
+
+## What is not implemented


Lets label this as "Unsupported Deprecated Features", and maybe declare these features on the bash launcher as deprecated? Changing behaviour at runtime via env vars should be deprecated and the use of run_binary should be used instead basically?

jbedard · 2026-09-04T17:31:08Z

+
+## Keeping the two launchers in sync
+
+The JavaScript launcher is a transliteration of the bash one and has to stay that


"The JavaScript launcher"? Shouldn't that be "The hermetic [JavaScript?] launcher" or something like that?

jbedard · 2026-09-04T17:33:12Z

+`JS_BINARY__STDERR_OUTPUT_FILE`, `JS_BINARY__EXIT_CODE_OUTPUT_FILE` and
+`JS_BINARY__SILENT_ON_SUCCESS` rather than honoring them.
+
+Nothing in rules_js asks the launcher for those anymore: `js_run_binary`


This paragraph doesn't seem belong in public docs? Would anyone care? I guess we do need to mention what users should do moving forward (use run_binary) if they want those deprecated features but this paragraph says far more then that.

jbedard · 2026-09-04T17:33:56Z

+outside rules_js that sets the variables by hand (#2955), and it stays in the
+bash launcher.
+
+`expected_exit_code` _is_ implemented, since it is a `js_binary` attribute with


jbedard · 2026-09-04T17:34:34Z

+`expected_exit_code` _is_ implemented, since it is a `js_binary` attribute with
+no other home.
+
+Everything else the bash launcher does is reproduced: the `--bazel-bindir` flag,


Is this paragraph necessary? I think just mentioning the difference implies "everything else is the same"

jbedard · 2026-09-04T17:35:29Z

+catches breakage but not drift -- a bash-launcher change that was never ported
+can potentially leave both launchers passing every test.
+
+`//js/private/test/launcher_sync` closes that gap. A configuration transition


We probably shouldn't be mentioning internal tests in a public doc?

jbedard · 2026-09-04T17:36:11Z

@@ -0,0 +1,22 @@
+load("@bazel_lib//lib:testing.bzl", "assert_contains")


Lets add this test in a separate PR ahead of time?

Along with a lot of others like js/private/test/fixed_args/BUILD.bazel I think?

jbedard · 2026-09-04T17:38:29Z

+# stamped by hermetic_launcher which execs node on a generated JavaScript launcher,
+# in place of the generated bash launcher script. See docs/hermetic_launcher.md.
+bool_flag(
+    name = "hermetic_launcher",


use_hermetic_launcher? I think we've used that terminology elsewhere, although idk what's best?

jbedard · 2026-09-04T17:41:46Z

@@ -0,0 +1,109 @@
+# The hermetic launcher


This entire doc seems to be slop written for a rules_js developer outlining the history the launcher. This should be written for a rules_js API user, not developer, someone who only knows the public APIs with little or not implementation details...

jbedard · 2026-09-04T17:49:47Z

@@ -0,0 +1,681 @@
+// This JavaScript file is the launcher for the NodeJS JavaScript file


WDYT about trying to reduce the size of the template to something minimal (just the templated info) that then does a require() on the larger file with the real code? Then it doesn't get cloned for every single js_binary() etc?

Or maybe more here is templated then I thought such as all the "Values baked in at analysis time"

acozzette added 2 commits September 2, 2026 16:08

feat: add hermetic launcher as opt-in alternative to bash launcher

c74e8f9

Simplify comments

4fc7669

acozzette force-pushed the new-launcher branch from e057496 to 4fc7669 Compare September 3, 2026 00:42

acozzette and others added 10 commits September 2, 2026 19:10

Simplify comments

26c7637

Simplify comments

d4016e5

buildifier

1451fe0

Clean up comments

1376eef

Clean up docs

aca0dfd

acozzette marked this pull request as ready for review September 4, 2026 03:39

acozzette requested a review from jbedard September 4, 2026 03:40

Simplifications from Claude

c47bb4f

jbedard reviewed Sep 4, 2026

View reviewed changes


		## Keeping the two launchers in sync

		The JavaScript launcher is a transliteration of the bash one and has to stay that

		@@ -0,0 +1,22 @@
		load("@bazel_lib//lib:testing.bzl", "assert_contains")

		@@ -0,0 +1,681 @@
		// This JavaScript file is the launcher for the NodeJS JavaScript file

Uh oh!

Uh oh!

Conversation

acozzette commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes are visible to end-users: yes

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

acozzette commented Sep 2, 2026 •

edited

Loading