Skip to content

Commit c40ceb9

Browse files
devversionmgechev
authored andcommitted
fix(npm_package): do not write external dep files to same location (#463)
Currently the `npm_package` acceps an arbitary list of targets that will be built as `dependencies`. Some of these dependencies are within the Bazel package that defines the `npm_package` and should be also copied into the `npm_package`. Transitive dependencies (or explicit deps) which are outside of the current package are currently passed to the `packager` and accidentally written to the same location (this causes exceptions) and since these output actions aren't having any effect, we should not even pass these files to the `packager` in order to improve performance and fix exceptions like: ``` ERROR: /home/circleci/ng/src/lib/schematics/BUILD.bazel:34:1: SkylarkAction src/lib/schematics/npm_package failed (Exit 1) Error: EEXIST: file already exists, mkdir 'bazel-out/k8-fastbuild/bin/src/external/matdeps/node_modules/@schematics/angular/component/files/__name@dasherize@if-flat__' at Object.mkdirSync (fs.js:731:3) at mkdirp (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/execroot/angular_material/bazel-out/host/bin/external/build_bazel_rules_nodejs/internal/npm_package/packager.runfiles/build_bazel_rules_nodejs/internal/npm_package/packager.js:23:8) at write (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/execroot/angular_material/bazel-out/host/bin/external/build_bazel_rules_nodejs/internal/npm_package/packager.runfiles/build_bazel_rules_nodejs/internal/npm_package/packager.js:28:3) at main (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/execroot/angular_material/bazel-out/host/bin/external/build_bazel_rules_nodejs/internal/npm_package/packager.runfiles/build_bazel_rules_nodejs/internal/npm_package/packager.js:101:5) ```
1 parent d554d80 commit c40ceb9

File tree

1 file changed

+30
-14
lines changed

1 file changed

+30
-14
lines changed

internal/npm_package/npm_package.bzl

+30-14
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,26 @@ to the `deps` of one of their targets.
88

99
load("//internal:node.bzl", "sources_aspect")
1010

11-
def create_package(ctx, devmode_sources, nested_packages):
11+
12+
# Takes a depset of files and returns a corresponding list of file paths without any files
13+
# that aren't part of the specified package path.
14+
def _filter_out_external_files(files, package_path):
15+
result = []
16+
for file in files:
17+
if file.short_path.startswith(package_path):
18+
result.append(file.path)
19+
return result
20+
21+
def create_package(ctx, deps_sources, nested_packages):
1222
"""Creates an action that produces the npm package.
1323
1424
It copies srcs and deps into the artifact and produces the .pack and .publish
1525
scripts.
1626
1727
Args:
1828
ctx: the skylark rule context
19-
devmode_sources: the .js files which belong in the package
29+
deps_sources: Files which have been specified as dependencies. Usually ".js" or ".d.ts"
30+
generated files.
2031
nested_packages: list of TreeArtifact outputs from other actions which are
2132
to be nested inside this package
2233
@@ -25,21 +36,26 @@ def create_package(ctx, devmode_sources, nested_packages):
2536
"""
2637

2738
package_dir = ctx.actions.declare_directory(ctx.label.name)
39+
package_path = ctx.label.package
40+
41+
# List of dependency sources which are local to the package that defines the current
42+
# target. We only want to package deps files which are inside of the current package.
43+
local_deps_sources = _filter_out_external_files(deps_sources, package_path)
2844

2945
args = ctx.actions.args()
3046
args.use_param_file("%s", use_always = True)
3147
args.add(package_dir.path)
32-
args.add(ctx.label.package)
48+
args.add(package_path)
3349
args.add_joined([s.path for s in ctx.files.srcs], join_with = ",", omit_if_empty = False)
3450
args.add(ctx.bin_dir.path)
3551
args.add(ctx.genfiles_dir.path)
36-
args.add_joined([s.path for s in devmode_sources], join_with = ",", omit_if_empty = False)
52+
args.add_joined(local_deps_sources, join_with = ",", omit_if_empty = False)
3753
args.add_joined([p.path for p in nested_packages], join_with = ",", omit_if_empty = False)
3854
args.add(ctx.attr.replacements)
3955
args.add_all([ctx.outputs.pack.path, ctx.outputs.publish.path])
4056
args.add(ctx.version_file.path if ctx.version_file else "")
4157

42-
inputs = ctx.files.srcs + devmode_sources + nested_packages + [ctx.file._run_npm_template]
58+
inputs = ctx.files.srcs + deps_sources + nested_packages + [ctx.file._run_npm_template]
4359

4460
# The version_file is an undocumented attribute of the ctx that lets us read the volatile-status.txt file
4561
# produced by the --workspace_status_command. That command will be executed whenever
@@ -65,24 +81,24 @@ def create_package(ctx, devmode_sources, nested_packages):
6581
return package_dir
6682

6783
def _npm_package(ctx):
68-
files = depset()
69-
for d in ctx.attr.deps:
84+
deps_sources = depset()
85+
for dep in ctx.attr.deps:
7086
transitive = [
71-
files,
87+
deps_sources,
7288
# Collect whatever is in the "data"
73-
d.data_runfiles.files,
89+
dep.data_runfiles.files,
7490
# For JavaScript-producing rules, gather up the devmode Node.js sources
75-
d.node_sources,
91+
dep.node_sources,
7692
]
7793

7894
# ts_library doesn't include .d.ts outputs in the runfiles
7995
# see comment in rules_typescript/internal/common/compilation.bzl
80-
if hasattr(d, "typescript"):
81-
transitive.append(d.typescript.transitive_declarations)
96+
if hasattr(dep, "typescript"):
97+
transitive.append(dep.typescript.transitive_declarations)
8298

83-
files = depset(transitive = transitive)
99+
deps_sources = depset(transitive = transitive)
84100

85-
package_dir = create_package(ctx, files.to_list(), ctx.files.packages)
101+
package_dir = create_package(ctx, deps_sources.to_list(), ctx.files.packages)
86102

87103
return [DefaultInfo(
88104
files = depset([package_dir]),

0 commit comments

Comments
 (0)