Automated rollback of commit be25bbf38d9337432c1e79e3f027ad5d0122bb40.
*** Reason for rollback ***
Breaks bazel-skylib
See https://github.com/bazelbuild/bazel-skylib/issues/173
*** Original change description ***
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.
RELNOTES: None.
PiperOrigin-RevId: 258776285
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 63e30d9..9ecc410 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,10 +24,8 @@
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;
@@ -191,8 +189,7 @@
printer.append(">");
}
- private static Label transformKey(Object key, Location loc, StarlarkContext context)
- throws EvalException {
+ private Label transformKey(Object key, Location loc) throws EvalException {
if (key instanceof Label) {
return (Label) key;
} else if (key instanceof ToolchainTypeInfo) {
@@ -200,12 +197,8 @@
} 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, repoMapping);
+ toolchainType = Label.parseAbsolute(rawLabel, ImmutableMap.of());
} catch (LabelSyntaxException e) {
throw new EvalException(
loc, String.format("Unable to parse toolchain %s: %s", rawLabel, e.getMessage()), e);
@@ -223,7 +216,7 @@
@Override
public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
throws EvalException {
- Label toolchainTypeLabel = transformKey(key, loc, context);
+ Label toolchainTypeLabel = transformKey(key, loc);
if (!containsKey(key, loc, context)) {
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
@@ -246,7 +239,7 @@
@Override
public boolean containsKey(Object key, Location loc, StarlarkContext context)
throws EvalException {
- Label toolchainTypeLabel = transformKey(key, loc, context);
+ Label toolchainTypeLabel = transformKey(key, loc);
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 2e384a7..2737aa2 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,7 +43,6 @@
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;
@@ -355,14 +354,9 @@
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(), repoMapping));
+ toolchains.getContents(String.class, "toolchains"), ast.getLocation()));
if (!buildSetting.equals(Runtime.NONE) && !cfg.equals(Runtime.NONE)) {
throw new EvalException(
@@ -403,8 +397,7 @@
builder.addExecutionPlatformConstraints(
collectConstraintLabels(
execCompatibleWith.getContents(String.class, "exec_compatile_with"),
- ast.getLocation(),
- repoMapping));
+ ast.getLocation()));
}
if (executionPlatformConstraintsAllowed) {
@@ -455,14 +448,11 @@
}
private static ImmutableList<Label> collectToolchainLabels(
- Iterable<String> rawLabels,
- Location loc,
- ImmutableMap<RepositoryName, RepositoryName> mapping)
- throws EvalException {
+ Iterable<String> rawLabels, Location loc) throws EvalException {
ImmutableList.Builder<Label> requiredToolchains = new ImmutableList.Builder<>();
for (String rawLabel : rawLabels) {
try {
- Label toolchainLabel = Label.parseAbsolute(rawLabel, mapping);
+ Label toolchainLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
requiredToolchains.add(toolchainLabel);
} catch (LabelSyntaxException e) {
throw new EvalException(
@@ -474,14 +464,11 @@
}
private static ImmutableList<Label> collectConstraintLabels(
- Iterable<String> rawLabels,
- Location loc,
- ImmutableMap<RepositoryName, RepositoryName> mapping)
- throws EvalException {
+ Iterable<String> rawLabels, Location loc) throws EvalException {
ImmutableList.Builder<Label> constraintLabels = new ImmutableList.Builder<>();
for (String rawLabel : rawLabels) {
try {
- Label constraintLabel = Label.parseAbsolute(rawLabel, mapping);
+ Label constraintLabel = Label.parseAbsolute(rawLabel, ImmutableMap.of());
constraintLabels.add(constraintLabel);
} catch (LabelSyntaxException e) {
throw new EvalException(
@@ -593,11 +580,7 @@
HostTransition.INSTANCE,
ImmutableSet.copyOf(hostFragments.getContents(String.class, "host_fragments")),
collectToolchainLabels(
- 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()));
+ toolchains.getContents(String.class, "toolchains"), ast.getLocation()));
}
/**
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 7dd0051..78c88e9 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.
- public final HashMap<RepositoryName, HashMap<RepositoryName, RepositoryName>>
+ private 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 b648f1f..f4e02f3 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,7 +16,6 @@
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;
@@ -24,7 +23,6 @@
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;
@@ -41,7 +39,6 @@
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 {
@@ -139,8 +136,7 @@
}
}
- private static RepositoryName createRepositoryName(String key, Location location)
- throws EvalException {
+ private RepositoryName createRepositoryName(String key, Location location) throws EvalException {
if (!key.startsWith("@")) {
throw new EvalException(
location,
@@ -154,7 +150,7 @@
}
}
- private static List<PathFragment> getManagedDirectoriesPaths(
+ private List<PathFragment> getManagedDirectoriesPaths(
Object directoriesList, Location location, Map<PathFragment, String> nonNormalizedPathsMap)
throws EvalException {
if (!(directoriesList instanceof SkylarkList)) {
@@ -199,33 +195,15 @@
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;
- List<String> patterns = platformLabels.getContents(String.class, "platform_labels");
- builder.addRegisteredExecutionPlatforms(renamePatterns(patterns, builder, env));
+ builder.addRegisteredExecutionPlatforms(
+ platformLabels.getContents(String.class, "platform_labels"));
+
return NONE;
}
@@ -235,8 +213,8 @@
throws EvalException, InterruptedException {
// Add to the package definition for later.
Package.Builder builder = PackageFactory.getContext(env, location).pkgBuilder;
- List<String> patterns = toolchainLabels.getContents(String.class, "toolchain_labels");
- builder.addRegisteredToolchains(renamePatterns(patterns, builder, env));
+ builder.addRegisteredToolchains(toolchainLabels.getContents(String.class, "toolchain_labels"));
+
return NONE;
}
diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index 205119e..1783aa4 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -830,99 +830,4 @@
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"
