Skip to content

Commit 417711d

Browse files
authored
fix(builtin): entry point from sources used when used as tool (#3605)
Consider a `nodejs_binary` with a definition like the followed: ```bzl js_library( name = "lib", srcs = ["entrypoint.js", "constants.js"], ) nodejs_binary( name = "bin", entry_point = "entrypoint.js", data = [":lib"], ) ``` This seems very standanrd and also works perfectly when the binary is invoked with `bazel run`. Via Bazel run, the Node Bash launcher resolves the entrypoint via the actual runfiles using `rlocation`. If this binary is used as a tool, e.g. in `npm_package_bin`- then the entry point is resolved from the execroot, landing ultimately on the *actual source file*. This is wrong and breaks resolution in RBE (where only necessary sources are in the execroot). This happens because the source entrypoint file. i.e. not the `entrypoint.js` from `bazel-bin` is also ending up being included in the runfiles of `run_node` via `NodeRuntimeDepsInfo`. This mismatch breaks resolution, and also results in an incorrect/ unnecessary file being added to the action inputs. The entry point used in `NodeRuntimeDepsInfo` should be the one derived from the `data` sources of the rule, ensuring the entry-point can access its other files of the `js_library`. i.e. entry-point should come from the `data` preferred, and if it's not found- then the source, or `File` can be directly used. This fixes RBE for angular.io which started unveiling some issues when we attmepted to enable RBE via: angular/angular#48316
1 parent 10e828f commit 417711d

File tree

1 file changed

+18
-6
lines changed

1 file changed

+18
-6
lines changed

internal/node/node.bzl

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -353,10 +353,22 @@ if (process.cwd() !== __dirname) {
353353
else:
354354
executable = ctx.outputs.launcher_sh
355355

356-
# syntax sugar: allows you to avoid repeating the entry point in data
357-
# entry point is only needed in runfiles if it is a javascript file
358-
if len(ctx.files.entry_point) == 1 and is_javascript_file(ctx.files.entry_point[0]):
359-
runfiles.extend(ctx.files.entry_point)
356+
# Note: `to_list()` is expensive and should only be called once.
357+
sources_list = sources.to_list()
358+
entry_point_input_short_path = _ts_to_js(_get_entry_point_file(ctx).short_path)
359+
entry_point_script = None
360+
361+
for f in sources_list:
362+
if f.short_path == entry_point_input_short_path:
363+
entry_point_script = f
364+
break
365+
366+
if not entry_point_script and len(ctx.files.entry_point) == 1 and is_javascript_file(ctx.files.entry_point[0]):
367+
entry_point_script = ctx.files.entry_point[0]
368+
369+
# Convenience: We add the entry point to the runfiles. This means that users would not
370+
# need to explicitly repeat the entry point in the `data` attribute.
371+
runfiles.append(entry_point_script)
360372

361373
return [
362374
DefaultInfo(
@@ -371,14 +383,14 @@ if (process.cwd() !== __dirname) {
371383
# Calling the .to_list() method may have some perfs hits,
372384
# so we should be running this method only once per rule.
373385
# see: https://docs.bazel.build/versions/main/skylark/depsets.html#performance
374-
node_modules.to_list() + sources.to_list(),
386+
node_modules.to_list() + sources_list,
375387
collect_data = True,
376388
),
377389
),
378390
# TODO(alexeagle): remove sources and node_modules from the runfiles
379391
# when downstream usage is ready to rely on linker
380392
NodeRuntimeDepsInfo(
381-
deps = depset(ctx.files.entry_point, transitive = [node_modules, sources]),
393+
deps = depset([entry_point_script], transitive = [node_modules, sources]),
382394
pkgs = data,
383395
),
384396
# indicates that the this binary should be instrumented by coverage

0 commit comments

Comments
 (0)