Skip to content

Commit

Permalink
fix: fix logic error in linker conflict resolution
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gregmagolan committed Jan 31, 2020
1 parent 5c36bd7 commit fe72f51
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/linker/link_node_modules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def _link_mapping(label, mappings, k, v):
# v = ["bin", "angular/packages/compiler"]
if mappings[k][0] == "runfiles":
return True
elif v[0] != "runfiles":
elif v[0] == "runfiles":
return False
fail(("conflicting mapping at %s: %s maps to both %s and %s" % (label, k, mappings[k], v)), "deps")
else:
Expand Down
40 changes: 40 additions & 0 deletions internal/linker/test/integration/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,13 @@ linked(
testonly = True,
out = "actual_with_conflicts",
program = ":run_program",
# do not sort
deps = [
# NB: reference the copy of index.js in the output folder
"//%s/absolute_import:copy_to_bin" % package_name(),
# Intentinally include this before static_linked_pkg as order matters for the linker.
# The order here exercises a different code path in the linker conflict resolution logic
# than `example_with_conflicts_alt`.
":run_program",
# NB: static_linked maps to both
# ["runfiles", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_pkg"] and
Expand All @@ -97,6 +101,35 @@ linked(
],
)

linked(
name = "example_with_conflicts_alt",
testonly = True,
out = "actual_with_conflicts_alt",
program = ":run_program",
# do not sort
deps = [
# NB: reference the copy of index.js in the output folder
"//%s/absolute_import:copy_to_bin" % package_name(),
# NB: static_linked maps to both
# ["runfiles", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_pkg"] and
# ["bin", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_pkg"]
# as the "runfiles" mapping comes from `:run_program`
"//internal/linker/test/integration/static_linked_pkg",
# NB: @linker_scoped/static_linked maps to both
# ["runfiles", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_scoped_pkg"] and
# ["src", "build_bazel_rules_nodejs/internal/linker/test/integration/static_linked_scoped_pkg"]
# as the "runfiles" mapping comes from `:run_program`
"//internal/linker/test/integration/static_linked_scoped_pkg",
# Intentinally include this before static_linked_pkg as order matters for the linker.
# The order here exercises a different code path in the linker conflict resolution logic
# than `example_with_conflicts`.
":run_program",
"//internal/linker/test/integration/dynamic_linked_pkg",
"//internal/linker/test/integration/dynamic_linked_scoped_pkg",
"@npm//semver",
],
)

golden_file_test(
# default rule in this package
name = "integration",
Expand All @@ -110,3 +143,10 @@ golden_file_test(
actual = "actual_with_conflicts",
golden = "golden.txt",
)

golden_file_test(
# default rule in this package
name = "integration_conflicts_alt",
actual = "actual_with_conflicts_alt",
golden = "golden.txt",
)

0 comments on commit fe72f51

Please sign in to comment.