Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b5b6737

Browse filesBrowse files
devversiondevversion
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 5ae23f6 commit b5b6737
Copy full SHA for b5b6737

File tree

Expand file treeCollapse file tree

12 files changed

+1661
-38
lines changed
Filter options
Expand file treeCollapse file tree

12 files changed

+1661
-38
lines changed

‎.bazelrc

Copy file name to clipboardExpand all lines: .bazelrc
+7-8Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ build --action_env=NG_FORCE_TTY=false
44
# Required by `rules_ts`.
55
common --@aspect_rules_ts//ts:skipLibCheck=always
66
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
97

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

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
157+
# Fixes issues with browser archives and files with spaces. Could be
158+
# removed in Bazel 8 when Bazel runfiles supports spaces.
165159
build --experimental_inprocess_symlink_creation
166160

167161
####################################################
@@ -172,6 +166,11 @@ build --experimental_inprocess_symlink_creation
172166
# Prevents the npm package extract from occuring or caching on RBE which overwhelms our quota
173167
build --modify_execution_info=NpmPackageExtract=+no-remote
174168

169+
# Allow the Bazel server to check directory sources for changes. `rules_js` previously
170+
# heavily relied on this, but still uses directory "inputs" in some cases.
171+
# See: https://github.com/aspect-build/rules_js/issues/1408.
172+
startup --host_jvm_args=-DBAZEL_TRACK_SOURCE_DIRECTORIES=1
173+
175174
####################################################
176175
# User bazel configuration
177176
# NOTE: This needs to be the *last* entry in the config.

‎.bazelversion

Copy file name to clipboard
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
6.5.0
1+
7.4.0

‎.github/shared-actions/windows-bazel-test/action.yml

Copy file name to clipboardExpand all lines: .github/shared-actions/windows-bazel-test/action.yml
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ runs:
6363
6464
# Needed for resolution because Aspect/Bazel looks for repositories at `<workspace>/external`.
6565
# TODO(devversion): consult with Aspect on why this is needed.
66-
(cd $runfiles_dir/angular_cli && ${{steps.init_wsl.outputs.cmd_path}} /C "mklink /D external ..")
66+
(cd $runfiles_dir/_main && ${{steps.init_wsl.outputs.cmd_path}} /C "mklink /D external ..")
6767
6868
- name: Run tests
6969
# Note: This is Git Bash.

‎MODULE.bazel

Copy file name to clipboard
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# TODO(devversion): Investigate bzlmod and use it where possible.
2+
3+
module(
4+
name = "angular_cli",
5+
)

‎MODULE.bazel.lock

Copy file name to clipboardExpand all lines: MODULE.bazel.lock
+1,630Lines changed: 1630 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎WORKSPACE

Copy file name to clipboardExpand all lines: WORKSPACE
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,3 +280,10 @@ rules_angular_step3(
280280
angular_compiler_cli = "//:node_modules/@angular/compiler-cli",
281281
typescript = "//:node_modules/typescript",
282282
)
283+
284+
http_archive(
285+
name = "aspect_rules_rollup",
286+
sha256 = "c4062681968f5dcd3ce01e09e4ba73670c064744a7046211763e17c98ab8396e",
287+
strip_prefix = "rules_rollup-2.0.0",
288+
url = "https://github.com/aspect-build/rules_rollup/releases/download/v2.0.0/rules_rollup-v2.0.0.tar.gz",
289+
)

‎docs/DEVELOPER.md

Copy file name to clipboardExpand all lines: docs/DEVELOPER.md
+1-1Lines changed: 1 addition & 1 deletion
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.

‎scripts/windows-testing/convert-symlinks.mjs

Copy file name to clipboardExpand all lines: scripts/windows-testing/convert-symlinks.mjs
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const skipDirectories = [
2626
'_windows_amd64/bin/nodejs/node_modules',
2727
];
2828

29-
const workspaceRootPaths = [/.*\.runfiles\/angular_cli\//, /^.*-fastbuild\/bin\//];
29+
const workspaceRootPaths = [/.*\.runfiles\/_main\//, /^.*-fastbuild\/bin\//];
3030

3131
// Copying can be parallelized and doesn't cause any WSL flakiness (no exe is invoked).
3232
const parallelCopyTasks = [];

‎scripts/windows-testing/parallel-executor.mjs

Copy file name to clipboardExpand all lines: scripts/windows-testing/parallel-executor.mjs
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ async function main() {
99
const maxShards = 4;
1010

1111
const testEntrypoint = path.resolve(runfilesDir, '../', targetName);
12-
const testWorkingDir = path.resolve(runfilesDir, 'angular_cli');
12+
const testWorkingDir = path.resolve(runfilesDir, '_main');
1313
const tasks = [];
1414
const progress = {};
1515

‎tests/legacy-cli/e2e_runner.ts

Copy file name to clipboardExpand all lines: tests/legacy-cli/e2e_runner.ts
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createConsoleLogger } from '../../packages/angular_devkit/core/node';
33
import colors from 'ansi-colors';
44
import glob from 'fast-glob';
55
import * as path from 'node:path';
6+
import * as fs from 'node:fs';
67
import { getGlobalVariable, setGlobalVariable } from './e2e/utils/env';
78
import { gitClean } from './e2e/utils/git';
89
import { createNpmRegistry } from './e2e/utils/registry';
@@ -79,6 +80,11 @@ const argv = {
7980
Number(process.env.TEST_SHARD_INDEX ?? 0)),
8081
};
8182

83+
// Indicate sharding support for Bazel.
84+
if (process.env['TEST_SHARD_STATUS_FILE']) {
85+
fs.writeFileSync(process.env['TEST_SHARD_STATUS_FILE'], '', 'utf8');
86+
}
87+
8288
/**
8389
* Set the error code of the process to 255. This is to ensure that if something forces node
8490
* to exit without finishing properly, the error code will be 255. Right now that code is not used.

‎tools/BUILD.bazel

Copy file name to clipboardExpand all lines: tools/BUILD.bazel
-19Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
21
load("//tools:defaults.bzl", "js_binary")
32

43
package(default_visibility = ["//visibility:public"])
@@ -31,21 +30,3 @@ js_binary(
3130
],
3231
entry_point = "quicktype_runner.js",
3332
)
34-
35-
# TODO(devversion): Improve this by potentially sharing this common block.
36-
copy_file(
37-
name = "copy_worker_js",
38-
src = "@tsc_worker//file",
39-
out = "ts_worker.mjs",
40-
)
41-
42-
js_binary(
43-
name = "vanilla_ts_worker",
44-
data = [
45-
":copy_worker_js",
46-
"//:node_modules/@angular/compiler-cli",
47-
"//:node_modules/typescript",
48-
],
49-
entry_point = ":copy_worker_js",
50-
fixed_args = ["--vanilla-ts"],
51-
)

‎tools/defaults.bzl

Copy file name to clipboardExpand all lines: tools/defaults.bzl
+1-6Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
load("@aspect_bazel_lib//lib:copy_to_bin.bzl", _copy_to_bin = "copy_to_bin")
22
load("@aspect_rules_jasmine//jasmine:defs.bzl", _jasmine_test = "jasmine_test")
33
load("@aspect_rules_js//js:defs.bzl", _js_binary = "js_binary")
4-
load("@aspect_rules_ts//ts:defs.bzl", _ts_project = "ts_project")
54
load("@devinfra//bazel/ts_project:index.bzl", "strict_deps_test")
65
load("@rules_angular//src/ng_package:index.bzl", _ng_package = "ng_package")
6+
load("@rules_angular//src/ts_project:index.bzl", _ts_project = "ts_project")
77
load("//tools:substitutions.bzl", "substitutions")
88
load("//tools/bazel:npm_package.bzl", _npm_package = "npm_package")
99

@@ -24,11 +24,6 @@ def ts_project(
2424
declaration = True,
2525
tsconfig = tsconfig,
2626
visibility = visibility,
27-
# Use the worker from our own Angular rules, as the default worker
28-
# from `rules_ts` is incompatible with TS5+ and abandoned. We need
29-
# worker for efficient, fast DX and avoiding Windows no-sandbox issues.
30-
supports_workers = 1,
31-
tsc_worker = "//tools:vanilla_ts_worker",
3227
deps = deps,
3328
**kwargs
3429
)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.