Skip to content

Commit b864223

Browse files
authored
fix(builtin): fix logic error in linker conflict resolution (#1597)
Fixes #1595. The `elif` path in ``` if mappings[k][0] == "runfiles": return True elif v[0] == "runfiles": return False ``` is now exercised by the linker integration tests.
1 parent 5c36bd7 commit b864223

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

internal/linker/link_node_modules.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def _link_mapping(label, mappings, k, v):
4343
# v = ["bin", "angular/packages/compiler"]
4444
if mappings[k][0] == "runfiles":
4545
return True
46-
elif v[0] != "runfiles":
46+
elif v[0] == "runfiles":
4747
return False
4848
fail(("conflicting mapping at %s: %s maps to both %s and %s" % (label, k, mappings[k], v)), "deps")
4949
else:

internal/linker/test/integration/BUILD.bazel

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,13 @@ linked(
7777
testonly = True,
7878
out = "actual_with_conflicts",
7979
program = ":run_program",
80+
# do not sort
8081
deps = [
8182
# NB: reference the copy of index.js in the output folder
8283
"//%s/absolute_import:copy_to_bin" % package_name(),
84+
# Intentinally include this before static_linked_pkg as order matters for the linker.
85+
# The order here exercises a different code path in the linker conflict resolution logic
86+
# than `example_with_conflicts_alt`.
8387
":run_program",
8488
# NB: static_linked maps to both
8589
# ["runfiles", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_pkg"] and
@@ -97,6 +101,35 @@ linked(
97101
],
98102
)
99103

104+
linked(
105+
name = "example_with_conflicts_alt",
106+
testonly = True,
107+
out = "actual_with_conflicts_alt",
108+
program = ":run_program",
109+
# do not sort
110+
deps = [
111+
# NB: reference the copy of index.js in the output folder
112+
"//%s/absolute_import:copy_to_bin" % package_name(),
113+
# NB: static_linked maps to both
114+
# ["runfiles", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_pkg"] and
115+
# ["bin", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_pkg"]
116+
# as the "runfiles" mapping comes from `:run_program`
117+
"//internal/linker/test/integration/static_linked_pkg",
118+
# NB: @linker_scoped/static_linked maps to both
119+
# ["runfiles", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_scoped_pkg"] and
120+
# ["src", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_scoped_pkg"]
121+
# as the "runfiles" mapping comes from `:run_program`
122+
"//internal/linker/test/integration/static_linked_scoped_pkg",
123+
# Intentinally include this before static_linked_pkg as order matters for the linker.
124+
# The order here exercises a different code path in the linker conflict resolution logic
125+
# than `example_with_conflicts`.
126+
":run_program",
127+
"//internal/linker/test/integration/dynamic_linked_pkg",
128+
"//internal/linker/test/integration/dynamic_linked_scoped_pkg",
129+
"@npm//semver",
130+
],
131+
)
132+
100133
golden_file_test(
101134
# default rule in this package
102135
name = "integration",
@@ -110,3 +143,10 @@ golden_file_test(
110143
actual = "actual_with_conflicts",
111144
golden = "golden.txt",
112145
)
146+
147+
golden_file_test(
148+
# default rule in this package
149+
name = "integration_conflicts_alt",
150+
actual = "actual_with_conflicts_alt",
151+
golden = "golden.txt",
152+
)

0 commit comments

Comments
 (0)