Skip to content

Commit d98ee70

Browse files
committed
build: fix bazel run invocations of js_binary/js_test
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
1 parent 95d16dc commit d98ee70

File tree

3 files changed

+7
-10
lines changed

3 files changed

+7
-10
lines changed

.bazelrc

+5-8
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1+
# TODO(devversion): enable bzlmod and consider using it.
2+
build --enable_bzlmod=false
3+
14
# Disable NG CLI TTY mode
25
build --action_env=NG_FORCE_TTY=false
36

47
# Required by `rules_ts`.
58
common --@aspect_rules_ts//ts:skipLibCheck=always
69
common --@aspect_rules_ts//ts:default_to_tsc_transpiler
7-
# TODO: remove this flag once we get to bazel version >7.
8-
common --incompatible_merge_fixed_and_default_shell_env
910

1011
# Make TypeScript compilation fast, by keeping a few copies of the compiler
1112
# running as daemons, and cache SourceFile AST's to reduce parse time.
@@ -156,12 +157,8 @@ build:remote-cache --google_default_credentials
156157
# Additional flags added when running a "trusted build" with additional access
157158
build:trusted-build --remote_upload_local_results=true
158159

159-
###############################
160-
# NodeJS rules settings
161-
# These settings are required for rules_nodejs
162-
###############################
163-
164-
# Fixes use of npm paths with spaces such as some within the puppeteer module
160+
# Fixes issues with browser archives and files with spaces. Could be
161+
# removed in Bazel 8 when Bazel runfiles supports spaces.
165162
build --experimental_inprocess_symlink_creation
166163

167164
####################################################

.bazelversion

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
6.5.0
1+
7.4.0

docs/DEVELOPER.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ You can find more info about debugging [tests with Bazel in the docs.](https://g
9090

9191
- For a complete list of test targets use the following Bazel query: `pnpm bazel query "tests(//tests/...)"`
9292
- Run a subset of the tests: `pnpm bazel test //tests/legacy-cli:e2e_node22 --config=e2e --test_filter="tests/i18n/ivy-localize-*"`
93-
- Use `bazel run` to debug failing tests debugging: `JS_BINARY__PATCH_NODE_FS=0 pnpm bazel run //tests/legacy-cli:e2e_node22 --config=e2e --test_arg="--glob=tests/basic/aot.ts"`
93+
- Use `bazel run` to debug failing tests debugging: `pnpm bazel run //tests/legacy-cli:e2e_node22 --config=e2e --test_arg="--glob=tests/basic/aot.ts"`
9494
- Provide additional `e2e_runner` options using `--test_arg`: `--test_arg="--package-manager=yarn"`
9595

9696
When running the debug commands, Node will stop and wait for a debugger to attach.

0 commit comments

Comments
 (0)