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 40e2d7f620f023374fd91a34f24f7159d03d9312
with fixing the needed additional renamings for tool chains, as to
not break skylib.
Change-Id: Ie194378b28961775742f24b271f5e7baa05a30b8
PiperOrigin-RevId: 258328800
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java
index 9ecc410..63e30d9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java
+++ b/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,7 +191,8 @@
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) {
@@ -197,8 +200,12 @@
} 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 @@
@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 @@
@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);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 2737aa2..2e384a7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/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 @@
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 @@
builder.addExecutionPlatformConstraints(
collectConstraintLabels(
execCompatibleWith.getContents(String.class, "exec_compatile_with"),
- ast.getLocation()));
+ ast.getLocation(),
+ repoMapping));
}
if (executionPlatformConstraintsAllowed) {
@@ -448,11 +455,14 @@
}
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> 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 @@
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()));
}
/**
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 78c88e9..7dd0051 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
@@ -806,7 +806,7 @@
// 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
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
index f4e02f3..b648f1f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
@@ -16,6 +16,7 @@
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;
@@ -23,6 +24,7 @@
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 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 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 @@
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 @@
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;
}
diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index 1783aa4..205119e 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -830,4 +830,99 @@
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"
