register_{toolchains, execution_platforms}: honor renaming

When handling the registration of toolchains or execution platforms,
take the applicable renaming into account. This, in particular, includes
the renaming from the name of the main workspace to the canonical '@'.

Fixes #7773.

This is a roll-forward of 40e2d7f
with fixing the needed additional renamings for tool chains, as to
not break skylib.

Change-Id: Ie194378b28961775742f24b271f5e7baa05a30b8
PiperOrigin-RevId: 258328800

src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java

@@ -24,8 +24,10 @@
 import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
 import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
 import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
+import com.google.devtools.build.lib.analysis.skylark.BazelStarlarkContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.Location;
@@ -189,16 +191,21 @@ public void repr(SkylarkPrinter printer) {
     printer.append(">");
   }
 
-  private Label transformKey(Object key, Location loc) throws EvalException {
+  private static Label transformKey(Object key, Location loc, StarlarkContext context)
+      throws EvalException {
     if (key instanceof Label) {
       return (Label) key;
     } else if (key instanceof ToolchainTypeInfo) {
       return ((ToolchainTypeInfo) key).typeLabel();
     } else if (key instanceof String) {
       Label toolchainType;
       String rawLabel = (String) key;
+      ImmutableMap<RepositoryName, RepositoryName> repoMapping = ImmutableMap.of();
+      if (context instanceof BazelStarlarkContext) {
+        repoMapping = ((BazelStarlarkContext) context).getRepoMapping();
+      }
       try {
-        toolchainType = Label.parseAbsolute(rawLabel, ImmutableMap.of());
+        toolchainType = Label.parseAbsolute(rawLabel, repoMapping);
       } catch (LabelSyntaxException e) {
         throw new EvalException(
             loc, String.format("Unable to parse toolchain %s: %s", rawLabel, e.getMessage()), e);
@@ -216,7 +223,7 @@ private Label transformKey(Object key, Location loc) throws EvalException {
   @Override
   public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
       throws EvalException {
-    Label toolchainTypeLabel = transformKey(key, loc);
+    Label toolchainTypeLabel = transformKey(key, loc, context);
 
     if (!containsKey(key, loc, context)) {
       // TODO(bazel-configurability): The list of available toolchain types is confusing in the
@@ -239,7 +246,7 @@ public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
   @Override
   public boolean containsKey(Object key, Location loc, StarlarkContext context)
       throws EvalException {
-    Label toolchainTypeLabel = transformKey(key, loc);
+    Label toolchainTypeLabel = transformKey(key, loc, context);
     return requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
   }
 

src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java

@@ -43,6 +43,7 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.LabelValidator;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.AttributeMap;
@@ -354,9 +355,14 @@ public BaseFunction rule(
     builder.setConfiguredTargetFunction(implementation);
     builder.setRuleDefinitionEnvironmentLabelAndHashCode(
         funcallEnv.getGlobals().getLabel(), funcallEnv.getTransitiveContentHashCode());
+
+    ImmutableMap<RepositoryName, RepositoryName> repoMapping = ImmutableMap.of();
+    if (context instanceof BazelStarlarkContext) {
+      repoMapping = ((BazelStarlarkContext) context).getRepoMapping();
+    }
     builder.addRequiredToolchains(
         collectToolchainLabels(
-            toolchains.getContents(String.class, "toolchains"), ast.getLocation()));
+            toolchains.getContents(String.class, "toolchains"), ast.getLocation(), repoMapping));
 
     if (!buildSetting.equals(Runtime.NONE) && !cfg.equals(Runtime.NONE)) {
       throw new EvalException(
@@ -397,7 +403,8 @@ public BaseFunction rule(
       builder.addExecutionPlatformConstraints(
           collectConstraintLabels(
               execCompatibleWith.getContents(String.class, "exec_compatile_with"),
-              ast.getLocation()));
+              ast.getLocation(),
+              repoMapping));
     }
 
     if (executionPlatformConstraintsAllowed) {
@@ -448,11 +455,14 @@ private static void addAttribute(
   }
 
   private static ImmutableList<Label> collectToolchainLabels(
-      Iterable<String> rawLabels, Location loc) throws EvalException {
+      Iterable<String> rawLabels,
+      Location loc,
+      ImmutableMap<RepositoryName, RepositoryName> mapping)
+      throws EvalException {
     ImmutableList.Builder<Label> requiredToolchains = new ImmutableList.Builder<>();
     for (String rawLabel : rawLabels) {
       try {
-        Label toolchainLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
+        Label toolchainLabel = Label.parseAbsolute(rawLabel, mapping);
         requiredToolchains.add(toolchainLabel);
       } catch (LabelSyntaxException e) {
         throw new EvalException(
@@ -464,11 +474,14 @@ private static ImmutableList<Label> collectToolchainLabels(
   }
 
   private static ImmutableList<Label> collectConstraintLabels(
-      Iterable<String> rawLabels, Location loc) throws EvalException {
+      Iterable<String> rawLabels,
+      Location loc,
+      ImmutableMap<RepositoryName, RepositoryName> mapping)
+      throws EvalException {
     ImmutableList.Builder<Label> constraintLabels = new ImmutableList.Builder<>();
     for (String rawLabel : rawLabels) {
       try {
-        Label constraintLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
+        Label constraintLabel = Label.parseAbsolute(rawLabel, mapping);
         constraintLabels.add(constraintLabel);
       } catch (LabelSyntaxException e) {
         throw new EvalException(
@@ -580,7 +593,11 @@ public SkylarkAspect aspect(
         HostTransition.INSTANCE,
         ImmutableSet.copyOf(hostFragments.getContents(String.class, "host_fragments")),
         collectToolchainLabels(
-            toolchains.getContents(String.class, "toolchains"), ast.getLocation()));
+            toolchains.getContents(String.class, "toolchains"),
+            ast.getLocation(),
+            // TODO(https://github.com/bazelbuild/bazel/issues/7773): Update to get the
+            // repository mapping from a context, like in rule().
+            ImmutableMap.of()));
   }
 
   /**

src/main/java/com/google/devtools/build/lib/packages/Package.java

@@ -806,7 +806,7 @@ public void onLoadingComplete(
 
     // The map from each repository to that repository's remappings map.
     // This is only used in the //external package, it is an empty map for all other packages.
-    private final HashMap<RepositoryName, HashMap<RepositoryName, RepositoryName>>
+    public final HashMap<RepositoryName, HashMap<RepositoryName, RepositoryName>>
         externalPackageRepositoryMappings = new HashMap<>();
     // The map of repository reassignments for BUILD packages loaded within external repositories.
     // It contains an entry from "@<main workspace name>" to "@" for packages within

src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java

@@ -16,13 +16,15 @@
 
 import static com.google.devtools.build.lib.syntax.Runtime.NONE;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.LabelValidator;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
@@ -39,6 +41,7 @@
 import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import javax.annotation.Nullable;
 
 /** A collection of global skylark build API functions that apply to WORKSPACE files. */
 public class WorkspaceGlobals implements WorkspaceGlobalsApi {
@@ -136,7 +139,8 @@ private void parseManagedDirectories(
     }
   }
 
-  private RepositoryName createRepositoryName(String key, Location location) throws EvalException {
+  private static RepositoryName createRepositoryName(String key, Location location)
+      throws EvalException {
     if (!key.startsWith("@")) {
       throw new EvalException(
           location,
@@ -150,7 +154,7 @@ private RepositoryName createRepositoryName(String key, Location location) throw
     }
   }
 
-  private List<PathFragment> getManagedDirectoriesPaths(
+  private static List<PathFragment> getManagedDirectoriesPaths(
       Object directoriesList, Location location, Map<PathFragment, String> nonNormalizedPathsMap)
       throws EvalException {
     if (!(directoriesList instanceof SkylarkList)) {
@@ -195,15 +199,33 @@ public Map<PathFragment, RepositoryName> getManagedDirectories() {
     return managedDirectoriesMap;
   }
 
+  private static RepositoryName getRepositoryName(@Nullable Label label) {
+    if (label == null) {
+      // registration happened directly in the main WORKSPACE
+      return RepositoryName.MAIN;
+    }
+
+    // registeration happened in a loaded bzl file
+    return label.getPackageIdentifier().getRepository();
+  }
+
+  private static List<String> renamePatterns(
+      List<String> patterns, Package.Builder builder, Environment env) {
+    RepositoryName myName = getRepositoryName(env.getGlobals().getLabel());
+    Map<RepositoryName, RepositoryName> renaming = builder.getRepositoryMappingFor(myName);
+    return patterns.stream()
+        .map(patternEntry -> TargetPattern.renameRepository(patternEntry, renaming))
+        .collect(ImmutableList.toImmutableList());
+  }
+
   @Override
   public NoneType registerExecutionPlatforms(
       SkylarkList<?> platformLabels, Location location, Environment env)
       throws EvalException, InterruptedException {
     // Add to the package definition for later.
     Package.Builder builder = PackageFactory.getContext(env, location).pkgBuilder;
-    builder.addRegisteredExecutionPlatforms(
-        platformLabels.getContents(String.class, "platform_labels"));
-
+    List<String> patterns = platformLabels.getContents(String.class, "platform_labels");
+    builder.addRegisteredExecutionPlatforms(renamePatterns(patterns, builder, env));
     return NONE;
   }
 
@@ -213,8 +235,8 @@ public NoneType registerToolchains(
       throws EvalException, InterruptedException {
     // Add to the package definition for later.
     Package.Builder builder = PackageFactory.getContext(env, location).pkgBuilder;
-    builder.addRegisteredToolchains(toolchainLabels.getContents(String.class, "toolchain_labels"));
-
+    List<String> patterns = toolchainLabels.getContents(String.class, "toolchain_labels");
+    builder.addRegisteredToolchains(renamePatterns(patterns, builder, env));
     return NONE;
   }
 

src/test/shell/bazel/workspace_test.sh

@@ -830,4 +830,99 @@ EOF
   expect_log '//external:true.*build rule.*expected'
 }
 
+function test_remap_execution_platform() {
+    # Regression test for issue https://github.com/bazelbuild/bazel/issues/7773,
+    # using the reproduction case as reported
+    cat > WORKSPACE <<'EOF'
+workspace(name = "my_ws")
+
+register_execution_platforms("@my_ws//platforms:my_host_platform")
+EOF
+    mkdir platforms
+    cat > platforms/BUILD <<'EOF'
+package(default_visibility = ["//visibility:public"])
+
+constraint_setting(name = "machine_size")
+constraint_value(name = "large_machine", constraint_setting = ":machine_size")
+constraint_value(name = "small_machine", constraint_setting = ":machine_size")
+
+platform(
+    name = "my_host_platform",
+    parents = ["@bazel_tools//platforms:host_platform"],
+    constraint_values = [
+        ":large_machine"
+    ]
+)
+EOF
+    mkdir code
+    cat > code/BUILD <<'EOF'
+sh_library(
+	name = "foo",
+	srcs = ["foo.sh"],
+	exec_compatible_with = ["@my_ws//platforms:large_machine"]
+)
+EOF
+    echo exit 0 > code/foo.sh
+    chmod u+x code/foo.sh
+
+
+    bazel build --incompatible_remap_main_repo=true //code/... \
+          > "${TEST_log}" 2>&1 || fail "expected success"
+}
+
+function test_remap_toolchain_deps() {
+    # Regression test for the registration pattern of toolchains used in
+    # bazel-skylib, where the failure to handle it correctly caused the
+    # roll back of the first attempt of enabling renaming in tool chains.
+    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(":toolchain.bzl", "mytoolchain")
+load(":rule.bzl", "testrule")
+
+toolchain_type(name = "my_toolchain_type")
+mytoolchain(name = "thetoolchain")
+
+toolchain(
+  name = "sample_toolchain",
+  toolchain = ":thetoolchain",
+  toolchain_type = "@my_ws//toolchains:my_toolchain_type",
+)
+
+testrule(
+  name = "emptytoolchainconsumer",
+)
+EOF
+
+    bazel build --incompatible_remap_main_repo=true //toolchains/... \
+          || fail "expected success"
+}
+
 run_suite "workspace tests"