forked from angular/angular
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
build(docs-infra): fix version mismatch of local built packages
There is quite some trickery going on with the adev build related to local packages: - Adev builds using npm packages from `/node_modules` - At runtime, we are adding `HEAD` packages for e.g. `@angular/core` to the bundles. - At build time, the CLI, or Angular devkit may accidentally resolve to `@angular/core` from `/node_modules/`— which is the core version from npm, transitively installed via `@angular/docs`. This causes a version mismatch, leading to issues like: - CLI throwing because of a mismatch. angular#54858 (comment) - Compiler changes not being picked up. angular#54858 (comment) This commit attempts to fix this by: - Linking all Angular `HEAD` packages into `adev/node_modules`. The current logic attempts to link into `/node_modules`, but this does not override existing `@angular/core`! - Linking all direct external NPM packages, like `@angular_devkit/build-angular` into `adev/node_modules` without their transitive deps. This allows proper resolution of e.g. compiler as node looks in `adev/node_modules` first, and falls back for the rest to the execroot `node_modules`, or symlink target destination (if `preserveSymlinks=false`). Note: This is still not 100% ideal because a direct external NPM dependency may have a transitive dependency that has another transitive dependency on `@angular/core`. In those cases, the may be a conflict that is not resolvable until we switch to a Bazel toolchain with better first party resolution support.
- Loading branch information
1 parent
cae993c
commit efd8235
Showing
5 changed files
with
162 additions
and
36 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| """Rule for filtering external NPM dependency targets to not include | ||
| transitive dependencies onto first-party linked `HEAD` dependencies.""" | ||
|
|
||
| load("@build_bazel_rules_nodejs//:providers.bzl", "ExternalNpmPackageInfo", "LinkablePackageInfo") | ||
|
|
||
| def _filter_external_npm_deps_impl(ctx): | ||
| problematic_paths = ["external/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 problematic_paths]): | ||
| filtered_deps.append(file) | ||
| filtered_depset = depset(filtered_deps) | ||
|
|
||
| providers = [ | ||
| DefaultInfo(files = filtered_depset), | ||
| ] | ||
|
|
||
| # Re-route all direct dependency external NPM packages into `adev/node_modules` without | ||
| # their transitive packages. This allows transitive dependency resolution to first look for | ||
| # e.g. `@angular/core` in `adev/node_modules`, and falls back to top-level node modules. | ||
| if ctx.attr.target.label.workspace_name == "npm": | ||
| providers.append(LinkablePackageInfo( | ||
| package_name = ctx.attr.target.label.package, | ||
| package_path = "adev", | ||
| path = "external/npm/node_modules/%s" % ctx.attr.target.label.package, | ||
| files = ctx.attr.target[ExternalNpmPackageInfo].direct_sources, | ||
| )) | ||
| else: | ||
| fail("Unknown workspace") | ||
|
|
||
| return providers | ||
|
|
||
| filter_external_npm_deps = rule( | ||
| doc = "Filter out transitive angular dependencies from a target", | ||
| implementation = _filter_external_npm_deps_impl, | ||
| attrs = { | ||
| "angular_packages": attr.string_list( | ||
| mandatory = True, | ||
| doc = "Angular packages to filter (useful for sandbox environments without linker)", | ||
| ), | ||
| "target": attr.label( | ||
| mandatory = True, | ||
| doc = "Target to filter", | ||
| providers = [ExternalNpmPackageInfo], | ||
| ), | ||
| }, | ||
| ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| load("//:packages.bzl", "ALL_PACKAGES", "to_package_label") | ||
| load("@build_bazel_rules_nodejs//internal/linker:npm_link.bzl", "npm_link") | ||
| load("//adev/tools/local_deps:filter_external_npm_deps.bzl", "filter_external_npm_deps") | ||
|
|
||
| def ensure_local_package_deps(deps): | ||
| """Replaces dependencies with their local-linked variants.""" | ||
| return [":%s" % _filtered_transitives_name(dep) for dep in deps] | ||
|
|
||
| def link_local_packages(all_deps): | ||
| """Create targets needed for building adev 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 adev. 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 `ensure_local_package_deps()` to translate a list of deps | ||
| to the equivalent "filtered" target that this rule creates. | ||
| Args: | ||
| all_deps: label list of all deps required to build and test adev | ||
| """ | ||
|
|
||
| local_angular_deps = [dep for dep in all_deps if _is_angular_dep(dep)] | ||
| local_angular_package_names = [_angular_dep_to_pkg_name(dep) for dep in local_angular_deps] | ||
|
|
||
| # Link local angular packages in place of their npm equivalent | ||
| for dep in local_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 = [ | ||
| "@npm//@angular/build-tooling/bazel/browsers/chromium", | ||
| ] | ||
|
|
||
| # Stamp a corresponding target for each dep that filters out transitive | ||
| # dependencies on external npm packages. This help the rules_nodejs linker, | ||
| # which fails to link local packages into transitive dependencies of npm deps. | ||
| for dep in all_deps: | ||
| target = dep | ||
| if dep in local_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_external_npm_deps( | ||
| name = _filtered_transitives_name(dep), | ||
| target = target, | ||
| testonly = True if dep in testonly_deps else False, | ||
| angular_packages = local_angular_package_names, | ||
| tags = ["manual"], | ||
| ) | ||
|
|
||
| def _is_angular_dep(dep): | ||
| """Check if a dep , e.g., @npm//@angular/core corresonds to a local Angular pacakge.""" | ||
| return dep.startswith("@npm//") and (_angular_dep_to_pkg_name(dep) in ALL_PACKAGES) | ||
|
|
||
| def _angular_dep_to_pkg_name(dep): | ||
| """E.g., @npm//@angular/core => '@angular/core'""" | ||
| label = Label(dep) | ||
| return label.package | ||
|
|
||
| def _npm_link_name(pkg_name): | ||
| return "local_head_%s" % pkg_name.replace("@", "_").replace("/", "_") | ||
|
|
||
| def _filtered_transitives_name(dep): | ||
| if dep.startswith(":"): | ||
| return "%s_without_transitive_deps" % dep[1:] | ||
| else: | ||
| label = Label(dep) | ||
| return "%s_without_transitive_deps" % label.package.replace("@", "_").replace("/", "_") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters