From 76ed014e77d7b862f6eb2894600ae525ea570f11 Mon Sep 17 00:00:00 2001 From: Klaus Aehlig Date: Fri, 23 Aug 2019 01:59:43 -0700 Subject: [PATCH] repository mapping lookup: convert to canonical name first By the way we propagate the information in which repository we are in, we always have a repository name that is valid in the main repository; however, it is not necessarily a canonical name. Our map of the repository mappings, however, is keyed by canonical names. So, add the missing indirection and first look up the canonical name for the repository name we're given by looking it up in the mapping of the main repository (for which we know its canonical name) and only then use that name as key. Fixes a bug related to #7130. Prerequisite to flip --incompatible_remap_main_repo. Therefore, consider for Bazel 0.29.0 (#8572). Change-Id: Icd426a18aaa406b614f4948a8122177463a72959 PiperOrigin-RevId: 265012106 --- .../devtools/build/lib/packages/Package.java | 11 +++- src/test/shell/bazel/workspace_test.sh | 60 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index 7dd00513ca5471..0eae9954100559 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -256,7 +256,16 @@ public ImmutableMap getRepositoryMapping( throw new UnsupportedOperationException("Can only access the external package repository" + "mappings from the //external package"); } - return externalPackageRepositoryMappings.getOrDefault(repository, ImmutableMap.of()); + + // We are passed a repository name as seen from the main repository, not necessarily + // a canonical repository name. So, we first have to find the canonical name for the + // repository in question before we can look up the mapping for it. + RepositoryName actualRepositoryName = + externalPackageRepositoryMappings + .getOrDefault(RepositoryName.MAIN, ImmutableMap.of()) + .getOrDefault(repository, repository); + + return externalPackageRepositoryMappings.getOrDefault(actualRepositoryName, ImmutableMap.of()); } /** Get the repository mapping for this package. */ diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh index 3e45fedbd5c4b2..29794a049394ea 100755 --- a/src/test/shell/bazel/workspace_test.sh +++ b/src/test/shell/bazel/workspace_test.sh @@ -994,5 +994,65 @@ EOF } +test_remap_toolchains_from_qualified_load() { + cat > WORKSPACE <<'EOF' +workspace(name = "my_ws") + +register_toolchains("@my_ws//toolchains:sample_toolchain") +EOF + mkdir toolchains + cat > toolchains/toolchain.bzl <<'EOF' +def _impl(ctx): + return [platform_common.ToolchainInfo()] + +mytoolchain = rule( + implementation = _impl, + attrs = {}, +) +EOF + cat > toolchains/rule.bzl <<'EOF' +def _impl(ctx): + # Ensure the toolchain is available under the requested (non-canonical) + # name + print("toolchain is %s" % + (ctx.toolchains["@my_ws//toolchains:my_toolchain_type"],)) + pass + +testrule = rule( + implementation = _impl, + attrs = {}, + toolchains = ["@my_ws//toolchains:my_toolchain_type"], +) +EOF + cat > toolchains/BUILD <<'EOF' +load("@my_ws//toolchains:toolchain.bzl", "mytoolchain") +load("@my_ws//toolchains:rule.bzl", "testrule") + +toolchain_type(name = "my_toolchain_type") +mytoolchain(name = "thetoolchain") + +toolchain( + name = "sample_toolchain", + toolchain = "@my_ws//toolchains:thetoolchain", + toolchain_type = "@my_ws//toolchains:my_toolchain_type", +) + +testrule( + name = "emptytoolchainconsumer", +) +EOF + + bazel build --incompatible_remap_main_repo=true @my_ws//toolchains/... \ + || fail "expected success" + + # Additionally check, that nothing goes wrong flipping the remapping + # off and on again. + bazel build --incompatible_remap_main_repo=false @my_ws//toolchains/... \ + || fail "expected success" + + bazel build --incompatible_remap_main_repo=true @my_ws//toolchains/... \ + || fail "expected success" +} + run_suite "workspace tests"