Skip to content

Commit

Permalink
build(bazel): fix linking of local angular packages
Browse files Browse the repository at this point in the history
Npm angular deps were transitively being included, confusing the
rules_nodejs linker.
  • Loading branch information
kormide authored and josephperrott committed Nov 22, 2022
1 parent 60058d2 commit 6cc3256
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 45 deletions.
5 changes: 5 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ build --enable_runfiles
build:release --workspace_status_command="yarn -s ng-dev release build-env-stamp --mode=release"
build:release --stamp

# Building AIO against local Angular deps requires stamping
# versions in Angular packages due to CLI version checks.
build:aio_local_deps --stamp
build:aio_local_deps --workspace_status_command="yarn -s --cwd aio local-workspace-status"

# Snapshots should also be stamped with version control information.
build:snapshot-build --workspace_status_command="yarn -s ng-dev release build-env-stamp --mode=snapshot"
build:snapshot-build --stamp
Expand Down
4 changes: 2 additions & 2 deletions .circleci/bazel.linux.rc
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ build --repository_cache=/home/circleci/bazel_repository_cache

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

# All build executed remotely should be done using our RBE configuration.
build:remote --google_default_credentials
Expand Down
16 changes: 9 additions & 7 deletions aio/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ load("@aio_npm//@angular-devkit/architect-cli:index.bzl", "architect", "architec
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin", "npm_package_bin")
load("//tools:defaults.bzl", "nodejs_binary")
load("@aspect_bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory")
load(":local_packages_util.bzl", "link_local_packages", "substitute_local_packages")
load(":local_packages_util.bzl", "link_local_packages", "substitute_local_package_deps")
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
load("//aio/scripts:local_server_test.bzl", "local_server_test")
load("@aio_npm//@angular/build-tooling/bazel/remote-execution:index.bzl", "ENABLE_NETWORK")
load(":aio_targets.bzl", "aio_test")
load("@bazel_skylib//lib:collections.bzl", "collections")

exports_files([
"firebase.json",
Expand Down Expand Up @@ -115,6 +116,7 @@ APPLICATION_DEPS = [
"@aio_npm//@angular/cli",
"@aio_npm//@angular/common",
"@aio_npm//@angular/compiler",
"@aio_npm//@angular/compiler-cli",
"@aio_npm//@angular/core",
"@aio_npm//@angular/elements",
"@aio_npm//@angular/forms",
Expand Down Expand Up @@ -172,7 +174,7 @@ E2E_DEPS = APPLICATION_DEPS + [

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

copy_to_bin(
name = "application_files_bin",
Expand All @@ -188,7 +190,7 @@ architect(
chdir = "$(RULEDIR)",
configuration_env_vars = ["NG_BUILD_CACHE"],
data = [":application_files_bin"] + select({
":aio_local_deps": substitute_local_packages(APPLICATION_DEPS),
":aio_local_deps": substitute_local_package_deps(APPLICATION_DEPS),
"//conditions:default": APPLICATION_DEPS,
}),
output_dir = True,
Expand Down Expand Up @@ -227,7 +229,7 @@ copy_to_directory(
)

TEST_DATA = TEST_FILES + select({
":aio_local_deps": substitute_local_packages(TEST_DEPS),
":aio_local_deps": substitute_local_package_deps(TEST_DEPS),
"//conditions:default": TEST_DEPS,
})

Expand Down Expand Up @@ -265,7 +267,7 @@ architect_test(
chdir = package_name(),
configuration_env_vars = ["NG_BUILD_CACHE"],
data = E2E_FILES + select({
":aio_local_deps": substitute_local_packages(E2E_DEPS),
":aio_local_deps": substitute_local_package_deps(E2E_DEPS),
"//conditions:default": E2E_DEPS,
}),
env = {
Expand All @@ -289,7 +291,7 @@ architect(
],
chdir = package_name(),
data = APPLICATION_FILES + select({
":aio_local_deps": substitute_local_packages(APPLICATION_DEPS),
":aio_local_deps": substitute_local_package_deps(APPLICATION_DEPS),
"//conditions:default": APPLICATION_DEPS,
}),
tags = ["ibazel_notify_changes"],
Expand All @@ -306,7 +308,7 @@ nodejs_binary(
data = APPLICATION_FILES + [
"//aio/scripts:fast-serve-and-watch",
] + select({
":aio_local_deps": substitute_local_packages(APPLICATION_DEPS),
":aio_local_deps": substitute_local_package_deps(APPLICATION_DEPS),
"//conditions:default": APPLICATION_DEPS,
}),
enable_linker = True,
Expand Down
153 changes: 118 additions & 35 deletions aio/local_packages_util.bzl
Original file line number Diff line number Diff line change
@@ -1,47 +1,130 @@
load("//:packages.bzl", "ALL_PACKAGES", "to_package_label")
load("@build_bazel_rules_nodejs//internal/linker:npm_link.bzl", "npm_link")
load("@build_bazel_rules_nodejs//:providers.bzl", "LinkablePackageInfo")
load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "LinkerPackageMappingInfo")

def link_local_packages(deps):
"""Stamp npm_link targets for packages in deps that has a local package equivalent.
def _is_angular_dep(dep):
"""Check if a dep , e.g., @aio_npm//@angular/core corresonds to a local Angular pacakge."""
return dep.startswith("@aio_npm//") and _angular_dep_to_pkg_name(dep) in ALL_PACKAGES

def _angular_dep_to_pkg_name(dep):
"""E.g., @aio_npm//@angular/core => '@angular/core'"""
label = Label(dep)
return label.package

def link_local_packages(all_aio_deps):
"""Create targets needed for building AIO against local angular packages.
Creates targets that link Angular packages, as well as targets to be used
in place of any deps required to build and test AIO. These targets filter
out any transitive deps on the npm packages and must be used in place of
any original list of deps.
Use the helper `substitute_local_package_deps()` to translate a list of deps
to the equivalent "filtered" target that this rule creates.
Args:
deps: list of npm dependency labels
all_aio_deps: label list of all deps required to build and test AIO
"""
for dep in deps:
if dep.startswith("@aio_npm//"):
label = Label(dep)
if label.package in ALL_PACKAGES:
npm_link(
name = _npm_link_name(dep),
target = to_package_label(label.package),
package_name = label.package,
package_path = native.package_name(),
tags = ["manual"],
)

def substitute_local_packages(deps):
"""Substitute npm dependencies for their local npm_link equivalent.
Assumes that link_local_packages() was already called on these dependencies.
Dependencies that are not associated with a local package are left alone.

aio_angular_deps = [dep for dep in all_aio_deps if _is_angular_dep(dep)]
angular_packages = [_angular_dep_to_pkg_name(dep) for dep in aio_angular_deps]

# Link local angular packages in place of their npm equivalent
for dep in aio_angular_deps:
pkg_name = _angular_dep_to_pkg_name(dep)
npm_link(
name = _npm_link_name(pkg_name),
target = to_package_label(pkg_name),
package_name = pkg_name,
package_path = native.package_name(),
tags = ["manual"],
)

# Special case deps that must be testonly
testonly_deps = [
"@aio_npm//@angular/build-tooling/bazel/browsers/chromium",
]

# Stamp a corresponding target for each AIO dep that filters out transitive
# dependencies on angular npm packages. This help the rules_nodejs linker,
# which fails to link local packages a transitive dependency on the npm
# package exists.
for dep in all_aio_deps:
target = dep
if dep in aio_angular_deps:
pkg_name = _angular_dep_to_pkg_name(dep)

# We don't need to filter transitives on local packages as they depend
# on each other locally.
native.alias(
name = _filtered_transitives_name(dep),
actual = ":%s" % _npm_link_name(pkg_name),
tags = ["manual"],
)
else:
filter_transitive_angular_deps(
name = _filtered_transitives_name(dep),
target = target,
angular_packages = angular_packages,
testonly = True if dep in testonly_deps else False,
tags = ["manual"],
)

def substitute_local_package_deps(deps):
"""Substitute AIO dependencies with an equivalent target that filters
out any transitive npm dependencies. You should call link_local_packages()
to actually stamp the targets first.
Args:
deps: list of npm dependency labels
deps: list of AIO dependencies
Returns:
substituted list of dependencies
"""
substituted = []
for dep in deps:
if dep.startswith("@aio_npm//"):
label = Label(dep)
if label.package in ALL_PACKAGES:
substituted.append(_npm_link_name(dep))
continue

substituted.append(dep)
return substituted

def _npm_link_name(dep):
label = Label(dep)
return "local_%s" % label.package.replace("@", "_").replace("/", "_")

return [":%s" % _filtered_transitives_name(dep) for dep in deps]

def _npm_link_name(pkg_name):
return "local_%s" % pkg_name.replace("@", "_").replace("/", "_")

def _filtered_transitives_name(dep):
if dep.startswith(":"):
return "%s_filtered" % dep[1:]
else:
label = Label(dep)
return "%s_filtered" % label.package.replace("@", "_").replace("/", "_")

def _filter_transitive_angular_deps_impl(ctx):
paths = ["external/aio_npm/node_modules/%s" % pkg for pkg in ctx.attr.angular_packages]

filtered_deps = []

# Note: to_list() is expensive; we need to invoke it here to get the path
# of each transitive dependency to check if it's an angular npm package.
for file in ctx.attr.target[DefaultInfo].default_runfiles.files.to_list():
if not any([file.path.startswith(path) for path in paths]):
filtered_deps.append(file)

filtered_depset = depset(filtered_deps)
providers = [
DefaultInfo(
files = filtered_depset,
),
]

if LinkerPackageMappingInfo in ctx.attr.target:
providers.append(ctx.attr.target[LinkerPackageMappingInfo])
if LinkablePackageInfo in ctx.attr.target:
providers.append(ctx.attr.target[LinkablePackageInfo])

return providers

filter_transitive_angular_deps = rule(
doc = "Filter out transitive angular dependencies from a target",
implementation = _filter_transitive_angular_deps_impl,
attrs = {
"target": attr.label(mandatory = True, doc = "Target to filter"),
"angular_packages": attr.string_list(default = [], doc = "Angular packages to filter"),
},
)
3 changes: 2 additions & 1 deletion aio/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@
"security-lint": "tsec -p tsconfig.app.json",
"~~audit-web-app": "node scripts/audit-web-app.mjs",
"~~check-env": "node scripts/check-environment",
"~~light-server": "light-server --bind=localhost --historyindex=/index.html --no-reload"
"~~light-server": "light-server --bind=localhost --historyindex=/index.html --no-reload",
"local-workspace-status": "node scripts/local-workspace-status.mjs"
},
"//engines-comment": "If applicable, also update /package.json and /aio/tools/examples/shared/package.json",
"engines": {
Expand Down
22 changes: 22 additions & 0 deletions aio/scripts/local-workspace-status.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import fs from 'node:fs';
import path from 'node:path';
import url from 'node:url';

/*
Script to use as a Bazel workspace status command (https://bazel.build/docs/user-manual#workspace-status)
when building AIO against local Angular packages (--config=aio_local_deps). This provides an Angular version
stamp variable used to stamp the locally built Angular packages. We stamp the packages with whatever version
of Angular AIO is currently using. In order for the architect build to succeed, we need to trick architect
into thinking it's using compatible Angular versions even if the Angular version is actually futher ahead.
*/

const __dirname = url.fileURLToPath(new URL('.', import.meta.url));
const pkgJsonPath = path.join(__dirname, '..', 'package.json');
const pkgJson = JSON.parse(fs.readFileSync(pkgJsonPath, 'utf8'));

const aioAngularVersion = pkgJson.dependencies['@angular/core'].replace(/^[\^~]/, '') + "+locallySubstituted";

// Output the workspace status variable format to stdout so Bazel can read it
console.log(`\
BUILD_SCM_VERSION ${aioAngularVersion}
`);

0 comments on commit 6cc3256

Please sign in to comment.