From 6cc325601852cb822708c35c8bb2f3b9ef139c79 Mon Sep 17 00:00:00 2001 From: Derek Cormier Date: Thu, 13 Oct 2022 17:19:01 -0700 Subject: [PATCH] build(bazel): fix linking of local angular packages Npm angular deps were transitively being included, confusing the rules_nodejs linker. --- .bazelrc | 5 + .circleci/bazel.linux.rc | 4 +- aio/BUILD.bazel | 16 +-- aio/local_packages_util.bzl | 153 +++++++++++++++++++------ aio/package.json | 3 +- aio/scripts/local-workspace-status.mjs | 22 ++++ 6 files changed, 158 insertions(+), 45 deletions(-) create mode 100644 aio/scripts/local-workspace-status.mjs diff --git a/.bazelrc b/.bazelrc index a46a0b7514fa62..25ad99d7b55fde 100644 --- a/.bazelrc +++ b/.bazelrc @@ -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 diff --git a/.circleci/bazel.linux.rc b/.circleci/bazel.linux.rc index 4f6ebc4e43cec4..fa1b482ed8819e 100644 --- a/.circleci/bazel.linux.rc +++ b/.circleci/bazel.linux.rc @@ -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 diff --git a/aio/BUILD.bazel b/aio/BUILD.bazel index d792d9bfc2463a..b8324f4d98d224 100644 --- a/aio/BUILD.bazel +++ b/aio/BUILD.bazel @@ -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", @@ -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", @@ -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", @@ -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, @@ -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, }) @@ -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 = { @@ -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"], @@ -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, diff --git a/aio/local_packages_util.bzl b/aio/local_packages_util.bzl index 2ac46b005909f2..a7a53be1e25b4b 100644 --- a/aio/local_packages_util.bzl +++ b/aio/local_packages_util.bzl @@ -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"), + }, +) diff --git a/aio/package.json b/aio/package.json index da18c7df697390..441e73d3502ae4 100644 --- a/aio/package.json +++ b/aio/package.json @@ -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": { diff --git a/aio/scripts/local-workspace-status.mjs b/aio/scripts/local-workspace-status.mjs new file mode 100644 index 00000000000000..b486dec6560e3e --- /dev/null +++ b/aio/scripts/local-workspace-status.mjs @@ -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} +`);