Skip to content

Commit 6cc3256

Browse files
kormidejosephperrott
authored andcommitted
build(bazel): fix linking of local angular packages
Npm angular deps were transitively being included, confusing the rules_nodejs linker.
1 parent 60058d2 commit 6cc3256

File tree

6 files changed

+158
-45
lines changed

6 files changed

+158
-45
lines changed

.bazelrc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ build --enable_runfiles
5252
build:release --workspace_status_command="yarn -s ng-dev release build-env-stamp --mode=release"
5353
build:release --stamp
5454

55+
# Building AIO against local Angular deps requires stamping
56+
# versions in Angular packages due to CLI version checks.
57+
build:aio_local_deps --stamp
58+
build:aio_local_deps --workspace_status_command="yarn -s --cwd aio local-workspace-status"
59+
5560
# Snapshots should also be stamped with version control information.
5661
build:snapshot-build --workspace_status_command="yarn -s ng-dev release build-env-stamp --mode=snapshot"
5762
build:snapshot-build --stamp

.circleci/bazel.linux.rc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ build --repository_cache=/home/circleci/bazel_repository_cache
1212

1313
# Workaround https://github.com/bazelbuild/bazel/issues/3645
1414
# Bazel doesn't calculate the memory ceiling correctly when running under Docker.
15-
# Limit Bazel to consuming resources that fit in CircleCI "xlarge" class
15+
# Limit Bazel to consuming resources that fit in CircleCI "2xlarge+" class
1616
# https://circleci.com/docs/2.0/configuration-reference/#resource_class
1717
build --local_cpu_resources=20
18-
build --local_ram_resources=32768
18+
build --local_ram_resources=40960
1919

2020
# All build executed remotely should be done using our RBE configuration.
2121
build:remote --google_default_credentials

aio/BUILD.bazel

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ load("@aio_npm//@angular-devkit/architect-cli:index.bzl", "architect", "architec
22
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin", "npm_package_bin")
33
load("//tools:defaults.bzl", "nodejs_binary")
44
load("@aspect_bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory")
5-
load(":local_packages_util.bzl", "link_local_packages", "substitute_local_packages")
5+
load(":local_packages_util.bzl", "link_local_packages", "substitute_local_package_deps")
66
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
77
load("//aio/scripts:local_server_test.bzl", "local_server_test")
88
load("@aio_npm//@angular/build-tooling/bazel/remote-execution:index.bzl", "ENABLE_NETWORK")
99
load(":aio_targets.bzl", "aio_test")
10+
load("@bazel_skylib//lib:collections.bzl", "collections")
1011

1112
exports_files([
1213
"firebase.json",
@@ -115,6 +116,7 @@ APPLICATION_DEPS = [
115116
"@aio_npm//@angular/cli",
116117
"@aio_npm//@angular/common",
117118
"@aio_npm//@angular/compiler",
119+
"@aio_npm//@angular/compiler-cli",
118120
"@aio_npm//@angular/core",
119121
"@aio_npm//@angular/elements",
120122
"@aio_npm//@angular/forms",
@@ -172,7 +174,7 @@ E2E_DEPS = APPLICATION_DEPS + [
172174

173175
# Stamp npm_link targets for all dependencies that correspond to a
174176
# first-party equivalent pacakge in angular.
175-
link_local_packages(deps = APPLICATION_DEPS)
177+
link_local_packages(all_aio_deps = collections.uniq(APPLICATION_DEPS + TEST_DEPS + E2E_DEPS))
176178

177179
copy_to_bin(
178180
name = "application_files_bin",
@@ -188,7 +190,7 @@ architect(
188190
chdir = "$(RULEDIR)",
189191
configuration_env_vars = ["NG_BUILD_CACHE"],
190192
data = [":application_files_bin"] + select({
191-
":aio_local_deps": substitute_local_packages(APPLICATION_DEPS),
193+
":aio_local_deps": substitute_local_package_deps(APPLICATION_DEPS),
192194
"//conditions:default": APPLICATION_DEPS,
193195
}),
194196
output_dir = True,
@@ -227,7 +229,7 @@ copy_to_directory(
227229
)
228230

229231
TEST_DATA = TEST_FILES + select({
230-
":aio_local_deps": substitute_local_packages(TEST_DEPS),
232+
":aio_local_deps": substitute_local_package_deps(TEST_DEPS),
231233
"//conditions:default": TEST_DEPS,
232234
})
233235

@@ -265,7 +267,7 @@ architect_test(
265267
chdir = package_name(),
266268
configuration_env_vars = ["NG_BUILD_CACHE"],
267269
data = E2E_FILES + select({
268-
":aio_local_deps": substitute_local_packages(E2E_DEPS),
270+
":aio_local_deps": substitute_local_package_deps(E2E_DEPS),
269271
"//conditions:default": E2E_DEPS,
270272
}),
271273
env = {
@@ -289,7 +291,7 @@ architect(
289291
],
290292
chdir = package_name(),
291293
data = APPLICATION_FILES + select({
292-
":aio_local_deps": substitute_local_packages(APPLICATION_DEPS),
294+
":aio_local_deps": substitute_local_package_deps(APPLICATION_DEPS),
293295
"//conditions:default": APPLICATION_DEPS,
294296
}),
295297
tags = ["ibazel_notify_changes"],
@@ -306,7 +308,7 @@ nodejs_binary(
306308
data = APPLICATION_FILES + [
307309
"//aio/scripts:fast-serve-and-watch",
308310
] + select({
309-
":aio_local_deps": substitute_local_packages(APPLICATION_DEPS),
311+
":aio_local_deps": substitute_local_package_deps(APPLICATION_DEPS),
310312
"//conditions:default": APPLICATION_DEPS,
311313
}),
312314
enable_linker = True,

aio/local_packages_util.bzl

Lines changed: 118 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,130 @@
11
load("//:packages.bzl", "ALL_PACKAGES", "to_package_label")
22
load("@build_bazel_rules_nodejs//internal/linker:npm_link.bzl", "npm_link")
3+
load("@build_bazel_rules_nodejs//:providers.bzl", "LinkablePackageInfo")
4+
load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "LinkerPackageMappingInfo")
35

4-
def link_local_packages(deps):
5-
"""Stamp npm_link targets for packages in deps that has a local package equivalent.
6+
def _is_angular_dep(dep):
7+
"""Check if a dep , e.g., @aio_npm//@angular/core corresonds to a local Angular pacakge."""
8+
return dep.startswith("@aio_npm//") and _angular_dep_to_pkg_name(dep) in ALL_PACKAGES
9+
10+
def _angular_dep_to_pkg_name(dep):
11+
"""E.g., @aio_npm//@angular/core => '@angular/core'"""
12+
label = Label(dep)
13+
return label.package
14+
15+
def link_local_packages(all_aio_deps):
16+
"""Create targets needed for building AIO against local angular packages.
17+
18+
Creates targets that link Angular packages, as well as targets to be used
19+
in place of any deps required to build and test AIO. These targets filter
20+
out any transitive deps on the npm packages and must be used in place of
21+
any original list of deps.
22+
23+
Use the helper `substitute_local_package_deps()` to translate a list of deps
24+
to the equivalent "filtered" target that this rule creates.
625
726
Args:
8-
deps: list of npm dependency labels
27+
all_aio_deps: label list of all deps required to build and test AIO
928
"""
10-
for dep in deps:
11-
if dep.startswith("@aio_npm//"):
12-
label = Label(dep)
13-
if label.package in ALL_PACKAGES:
14-
npm_link(
15-
name = _npm_link_name(dep),
16-
target = to_package_label(label.package),
17-
package_name = label.package,
18-
package_path = native.package_name(),
19-
tags = ["manual"],
20-
)
21-
22-
def substitute_local_packages(deps):
23-
"""Substitute npm dependencies for their local npm_link equivalent.
24-
25-
Assumes that link_local_packages() was already called on these dependencies.
26-
Dependencies that are not associated with a local package are left alone.
29+
30+
aio_angular_deps = [dep for dep in all_aio_deps if _is_angular_dep(dep)]
31+
angular_packages = [_angular_dep_to_pkg_name(dep) for dep in aio_angular_deps]
32+
33+
# Link local angular packages in place of their npm equivalent
34+
for dep in aio_angular_deps:
35+
pkg_name = _angular_dep_to_pkg_name(dep)
36+
npm_link(
37+
name = _npm_link_name(pkg_name),
38+
target = to_package_label(pkg_name),
39+
package_name = pkg_name,
40+
package_path = native.package_name(),
41+
tags = ["manual"],
42+
)
43+
44+
# Special case deps that must be testonly
45+
testonly_deps = [
46+
"@aio_npm//@angular/build-tooling/bazel/browsers/chromium",
47+
]
48+
49+
# Stamp a corresponding target for each AIO dep that filters out transitive
50+
# dependencies on angular npm packages. This help the rules_nodejs linker,
51+
# which fails to link local packages a transitive dependency on the npm
52+
# package exists.
53+
for dep in all_aio_deps:
54+
target = dep
55+
if dep in aio_angular_deps:
56+
pkg_name = _angular_dep_to_pkg_name(dep)
57+
58+
# We don't need to filter transitives on local packages as they depend
59+
# on each other locally.
60+
native.alias(
61+
name = _filtered_transitives_name(dep),
62+
actual = ":%s" % _npm_link_name(pkg_name),
63+
tags = ["manual"],
64+
)
65+
else:
66+
filter_transitive_angular_deps(
67+
name = _filtered_transitives_name(dep),
68+
target = target,
69+
angular_packages = angular_packages,
70+
testonly = True if dep in testonly_deps else False,
71+
tags = ["manual"],
72+
)
73+
74+
def substitute_local_package_deps(deps):
75+
"""Substitute AIO dependencies with an equivalent target that filters
76+
out any transitive npm dependencies. You should call link_local_packages()
77+
to actually stamp the targets first.
2778
2879
Args:
29-
deps: list of npm dependency labels
80+
deps: list of AIO dependencies
3081
3182
Returns:
3283
substituted list of dependencies
3384
"""
34-
substituted = []
35-
for dep in deps:
36-
if dep.startswith("@aio_npm//"):
37-
label = Label(dep)
38-
if label.package in ALL_PACKAGES:
39-
substituted.append(_npm_link_name(dep))
40-
continue
41-
42-
substituted.append(dep)
43-
return substituted
44-
45-
def _npm_link_name(dep):
46-
label = Label(dep)
47-
return "local_%s" % label.package.replace("@", "_").replace("/", "_")
85+
86+
return [":%s" % _filtered_transitives_name(dep) for dep in deps]
87+
88+
def _npm_link_name(pkg_name):
89+
return "local_%s" % pkg_name.replace("@", "_").replace("/", "_")
90+
91+
def _filtered_transitives_name(dep):
92+
if dep.startswith(":"):
93+
return "%s_filtered" % dep[1:]
94+
else:
95+
label = Label(dep)
96+
return "%s_filtered" % label.package.replace("@", "_").replace("/", "_")
97+
98+
def _filter_transitive_angular_deps_impl(ctx):
99+
paths = ["external/aio_npm/node_modules/%s" % pkg for pkg in ctx.attr.angular_packages]
100+
101+
filtered_deps = []
102+
103+
# Note: to_list() is expensive; we need to invoke it here to get the path
104+
# of each transitive dependency to check if it's an angular npm package.
105+
for file in ctx.attr.target[DefaultInfo].default_runfiles.files.to_list():
106+
if not any([file.path.startswith(path) for path in paths]):
107+
filtered_deps.append(file)
108+
109+
filtered_depset = depset(filtered_deps)
110+
providers = [
111+
DefaultInfo(
112+
files = filtered_depset,
113+
),
114+
]
115+
116+
if LinkerPackageMappingInfo in ctx.attr.target:
117+
providers.append(ctx.attr.target[LinkerPackageMappingInfo])
118+
if LinkablePackageInfo in ctx.attr.target:
119+
providers.append(ctx.attr.target[LinkablePackageInfo])
120+
121+
return providers
122+
123+
filter_transitive_angular_deps = rule(
124+
doc = "Filter out transitive angular dependencies from a target",
125+
implementation = _filter_transitive_angular_deps_impl,
126+
attrs = {
127+
"target": attr.label(mandatory = True, doc = "Target to filter"),
128+
"angular_packages": attr.string_list(default = [], doc = "Angular packages to filter"),
129+
},
130+
)

aio/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@
5050
"security-lint": "tsec -p tsconfig.app.json",
5151
"~~audit-web-app": "node scripts/audit-web-app.mjs",
5252
"~~check-env": "node scripts/check-environment",
53-
"~~light-server": "light-server --bind=localhost --historyindex=/index.html --no-reload"
53+
"~~light-server": "light-server --bind=localhost --historyindex=/index.html --no-reload",
54+
"local-workspace-status": "node scripts/local-workspace-status.mjs"
5455
},
5556
"//engines-comment": "If applicable, also update /package.json and /aio/tools/examples/shared/package.json",
5657
"engines": {
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import url from 'node:url';
4+
5+
/*
6+
Script to use as a Bazel workspace status command (https://bazel.build/docs/user-manual#workspace-status)
7+
when building AIO against local Angular packages (--config=aio_local_deps). This provides an Angular version
8+
stamp variable used to stamp the locally built Angular packages. We stamp the packages with whatever version
9+
of Angular AIO is currently using. In order for the architect build to succeed, we need to trick architect
10+
into thinking it's using compatible Angular versions even if the Angular version is actually futher ahead.
11+
*/
12+
13+
const __dirname = url.fileURLToPath(new URL('.', import.meta.url));
14+
const pkgJsonPath = path.join(__dirname, '..', 'package.json');
15+
const pkgJson = JSON.parse(fs.readFileSync(pkgJsonPath, 'utf8'));
16+
17+
const aioAngularVersion = pkgJson.dependencies['@angular/core'].replace(/^[\^~]/, '') + "+locallySubstituted";
18+
19+
// Output the workspace status variable format to stdout so Bazel can read it
20+
console.log(`\
21+
BUILD_SCM_VERSION ${aioAngularVersion}
22+
`);

0 commit comments

Comments
 (0)