Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions .github/workflows/ci-workflows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ jobs:
version: '9.x',
flags: '--bazel-flag=--test_tag_filters=-skip-on-bazel9 --bazel-flag=--@aspect_rules_js//js:use_execroot_entry_point=False',
}
- {
id: 'bazel-9-hermetic-launcher',
version: '9.x',
flags: '--bazel-flag=--test_tag_filters=-skip-on-bazel9 --bazel-flag=--@aspect_rules_js//js:hermetic_launcher=True',
}
exclude:
# e2e/js_image_oci pulls in the `llvm` module, whose hermetic toolchain requires Bazel 8+.
- {
Expand All @@ -198,12 +203,21 @@ jobs:
workspace: { slug: 'e2e-patch_from_repo' },
bazel: { id: 'bazel-9-no-execroot-entry-point' },
}
- {
workspace: { slug: 'e2e-patch_from_repo' },
bazel: { id: 'bazel-9-hermetic-launcher' },
}
# e2e/repo_mapping renames aspect_rules_js, so a flag beginning with
# --@aspect_rules_js// (as used by the no-execroot-entry-point variant) doesn't resolve.
# --@aspect_rules_js// (as used by the no-execroot-entry-point and
# hermetic-launcher variants) doesn't resolve.
- {
workspace: { slug: 'e2e-repo_mapping' },
bazel: { id: 'bazel-9-no-execroot-entry-point' },
}
- {
workspace: { slug: 'e2e-repo_mapping' },
bazel: { id: 'bazel-9-hermetic-launcher' },
}
env:
USE_BAZEL_VERSION: ${{ matrix.bazel.version }}
ASPECT_GH_PACKAGES_AUTH_TOKEN: ${{ secrets.ASPECT_GH_PACKAGES_AUTH_TOKEN }}
Expand Down Expand Up @@ -251,11 +265,11 @@ jobs:
if: matrix.workspace.path == '.' && matrix.bazel.id == 'bazel-7'
run: aspect test --task-key=coverage-split-${{ matrix.workspace.slug }}-${{ matrix.bazel.id }} ${{ matrix.bazel.flags }} --bazel-flag=--collect_code_coverage --bazel-flag=--instrument_test_targets --bazel-flag=--nocache_test_results --bazel-flag=--experimental_split_coverage_postprocessing --bazel-flag=--experimental_fetch_all_coverage_outputs -- //js/private/test/coverage/...

# Skipped on the no-execroot-entry-point variant: test.sh scripts invoke plain
# `bazel` without matrix.bazel.flags, so that leg wouldn't exercise the flag —
# it would only duplicate the bazel-9 run.
# Skipped on the flag-flip variants: test.sh scripts invoke plain `bazel`
# without matrix.bazel.flags, so those legs wouldn't exercise the flag — they
# would only duplicate the bazel-9 run.
- name: Optional ./test.sh
if: matrix.bazel.id != 'bazel-9-no-execroot-entry-point'
if: matrix.bazel.id != 'bazel-9-no-execroot-entry-point' && matrix.bazel.id != 'bazel-9-hermetic-launcher'
working-directory: ${{ matrix.workspace.path }}
env:
ASPECT_RULES_JS_FROZEN_PNPM_LOCK: 1
Expand Down
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ min/
npm/private/test/vendored/
js/private/worker/worker.js
js/private/worker/src/worker_protocol.ts
js/private/test/snapshots/launcher.cjs
1 change: 1 addition & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ bazel_dep(name = "bazel_features", version = "1.41.0")
bazel_dep(name = "bazel_skylib", version = "1.5.0")
bazel_dep(name = "platforms", version = "1.0.0")
bazel_dep(name = "rules_nodejs", version = "6.7.3")
bazel_dep(name = "hermetic_launcher", version = "0.0.15")

# Changes ensured by rules_js:
# 3.2.2: https://github.com/bazel-contrib/bazel-lib/commit/cac2d7855949d1b222fa26888892fbbe1d31015d
Expand Down
109 changes: 109 additions & 0 deletions docs/hermetic_launcher.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# The hermetic launcher

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...


A `js_binary` is normally invoked through a generated bash script
(`js/private/js_binary.sh.tpl`), which works out where node, the fs patches and
the entry point are, exports a set of `JS_BINARY__*` variables, changes into the
root of the output tree, and finally execs node. That is a shell process and a
few hundred lines of path resolution on every invocation, and it cannot run at
all where there is no bash.

The hermetic launcher is an experimental alternative, off by default:

```sh
bazel build //... --@aspect_rules_js//js:hermetic_launcher
```

With the flag on, a `js_binary`'s executable is a small native binary stamped by
[hermetic_launcher](https://github.com/hermeticbuild/hermetic-launcher) which
does nothing but resolve its runfiles and `execve` node on a generated
JavaScript launcher, `<name>_/<name>.cjs`. No shell is involved. The flag
applies everywhere: `bazel run`, `bazel test` and `js_run_binary` all go through
it, and a target gets one launcher or the other, never both.

This is the first step towards replacing the bash launcher outright. The
JavaScript launcher is deliberately an almost literal translation of the bash
one -- same order, same messages, same decisions -- so that the two can be read
side by side. `js/private/test/snapshots/launcher.sh` and
`js/private/test/snapshots/launcher.cjs` are checked-in expansions of both, kept
up to date by `//js/private/test:write_launcher` and
`//js/private/test:write_launcher_js`, and diffing them is how a change to
either is reviewed.

## What is not implemented

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


The JavaScript launcher does not implement stdout capture, stderr capture, exit
code capture or `silent_on_success`. It ignores `JS_BINARY__STDOUT_OUTPUT_FILE`,
`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`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

forwards `stdout`, `stderr`, `exit_code_out` and `silent_on_success` to
bazel-lib's `run_binary`, which captures through its own spawn wrapper -- a
process that outlives the program and can do the work they need once it has
exited. The launcher's implementation of them is legacy compatibility for code
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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spam

no other home.

Everything else the bash launcher does is reproduced: the `--bazel-bindir` flag,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

the execroot derivation, the `cd` into `BAZEL_BINDIR`, entry point / node / npm /
wrapper resolution, `node_options`, `fixed_args`, `JS_BINARY__FS_PATCH_ROOTS`,
coverage, the node wrapper on the `PATH`, the `JS_BINARY__*` per-target
constants, signal forwarding, and the debug and info logging.

## How many processes it costs

When there is no `expected_exit_code` (almost always) the launcher replaces
itself with node through `process.execve`, exactly as the bash launcher's `exec`
did, so no launcher process survives. It is still one more node startup than the
bash launcher paid, which is the price of this step; collapsing it is the point
of the next one.

`process.execve` is POSIX-only and was added in node 22.15. On an older node, on
Windows, and whenever `expected_exit_code` is set, the launcher spawns node and
waits for it, forwarding `SIGTERM` and `SIGINT` -- the bash launcher's
fork-and-wait path.

## Differences you may notice

- **`fixed_args` are tokenized at analysis time.** The bash launcher spliced
them into `ALL_ARGS=(... "$@")`, so the shell word-split them and removed
quotes. `_shell_tokenize` in `js/private/js_binary.bzl` reproduces that
splitting; backslash escapes are deliberately not interpreted, so a
Windows-style path survives intact. That last point shows in one place: bash
passes `\$VAR` through literally where this launcher expands it. Use
`'$VAR'` for a literal `$`.
- **`$VAR` expansion in `env`, `node_options` and `fixed_args` is done by the
launcher, not a shell.** `$VAR` and `${VAR}` are expanded against the
environment as it is built up; command substitution is not reproduced, and the
result is not re-split on whitespace. A single-quoted segment of a `fixed_arg`
is left alone, as bash would have left it.
- **A custom rule built on `js_binary_lib.create_launcher` must republish
`launcher_js`.** That output group is how `js_image_layer` tells a
hermetic-launcher binary from a bash-launcher one: the two keep the values
that have to be rewritten for hermeticity in different files.

## Keeping the two launchers in sync

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

way until it replaces it. The rest of the suite cannot check that: a target gets
one launcher per configuration, so every other 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 -- 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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

builds one `js_binary` twice, once with the flag off and once with it on, runs
both, and diffs the state node ends up in: `process.env`, the working directory,
`argv` and `execArgv`. Because it compares the two launchers against each other
rather than against a recorded golden, it needs no snapshot to regenerate and it
fails on every CI leg rather than just the one with the flag on.

## Status

Windows is wired up untested: the repo's Windows smoke job only runs on `main`.

CI runs the whole test suite against the flag on the `bazel-9-hermetic-launcher`
matrix leg.
5 changes: 2 additions & 3 deletions e2e/js_image_oci/src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,12 @@ oci_image(
# 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"],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

entrypoint = ["/usr/bin/bash"],
entrypoint = ["/app/src/bin"],
tars = [
":layers",
],
visibility = ["//visibility:public"],
# This is `cmd` + `.runfiles/[workspace name]`
# This is `entrypoint` + `.runfiles/[workspace name]`
workdir = "/app/src/bin.runfiles/_main",
)

Expand Down
6 changes: 2 additions & 4 deletions e2e/js_image_oci/src/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ schemaVersion: 2.0.0

commandTests:
- name: 'smoke'
command: '/usr/bin/bash'
args: ['/app/src/bin']
command: '/app/src/bin'
expectedOutput:
[
'OS',
Expand All @@ -23,8 +22,7 @@ commandTests:
' REPO NPM CHECK true',
]
- name: 'smoke2'
command: '/usr/bin/bash'
args: ['/app/src/bin']
command: '/app/src/bin'
expectedOutput:
[
'OS',
Expand Down
15 changes: 15 additions & 0 deletions js/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,21 @@ config_setting(
visibility = ["//visibility:public"],
)

# This flag selects the experimental hermetic launcher: a native launcher binary
# 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",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

build_setting_default = False,
visibility = ["//visibility:public"],
)

config_setting(
name = "_hermetic_launcher_true",
flag_values = {"hermetic_launcher": "True"},
visibility = ["//js/private/test:__subpackages__"],
)

bzl_library(
name = "defs",
srcs = ["defs.bzl"],
Expand Down
3 changes: 3 additions & 0 deletions js/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package(default_visibility = ["//visibility:public"])

exports_files([
"js_binary.sh.tpl",
"js_binary.cjs.tpl",
"node_bin/node",
"node_bin_windows/node.bat",
"npm_bin/npm",
Expand Down Expand Up @@ -49,7 +50,9 @@ bzl_library(
"@bazel_lib//lib:paths",
"@bazel_lib//lib:windows_utils",
"@bazel_skylib//lib:dicts",
"@bazel_skylib//rules:common_settings",
"@bazel_tools//tools/build_defs/repo:cache.bzl",
"@hermetic_launcher//launcher:lib_bzl",
],
)

Expand Down
Loading
Loading