From dc140d0b6119950dd4a7d71b125b15a78bacc8ce Mon Sep 17 00:00:00 2001 From: jcater Date: Thu, 6 May 2021 07:56:27 -0700 Subject: [PATCH] Move DEFAULT_EXEC_GROUP_NAME from ToolchainCollection to ExecGroup. PiperOrigin-RevId: 372342837 --- .../com/google/devtools/build/lib/analysis/BUILD | 5 ++++- .../devtools/build/lib/analysis/DependencyKind.java | 5 +++-- .../devtools/build/lib/analysis/RuleContext.java | 2 +- .../build/lib/analysis/ToolchainCollection.java | 12 ++++-------- .../build/lib/analysis/actions/SpawnAction.java | 2 +- .../analysis/config/ExecutionTransitionFactory.java | 2 +- .../starlark/StarlarkExecGroupCollection.java | 2 +- .../com/google/devtools/build/lib/packages/BUILD | 1 - .../devtools/build/lib/packages/ExecGroup.java | 5 +++++ .../com/google/devtools/build/lib/analysis/BUILD | 1 - .../build/lib/analysis/StarlarkExecGroupTest.java | 2 +- .../google/devtools/build/lib/analysis/testing/BUILD | 1 + .../analysis/testing/ToolchainCollectionSubject.java | 3 ++- .../com/google/devtools/build/lib/starlark/BUILD | 1 - .../build/lib/starlark/StarlarkRuleContextTest.java | 2 +- 15 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 6b23175a20d6ad..bc5842d291068e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -746,6 +746,7 @@ java_library( deps = [ "toolchain_collection", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages:exec_group", "//third_party:auto_value", "//third_party:guava", "//third_party:jsr305", @@ -1066,6 +1067,7 @@ java_library( ":toolchain_context", "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/packages:exec_group", "//third_party:auto_value", "//third_party:guava", ], @@ -1651,10 +1653,10 @@ java_library( ":config/transitions/patch_transition", ":config/transitions/transition_factory", ":platform_options", - ":toolchain_collection", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages:exec_group", "//src/main/java/com/google/devtools/build/lib/rules/config:feature_flag_value", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi", "//third_party:guava", @@ -2152,6 +2154,7 @@ java_library( ":resolved_toolchain_context", ":starlark/starlark_toolchain_context", ":toolchain_collection", + "//src/main/java/com/google/devtools/build/lib/packages:exec_group", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform", "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java index 6227f234caafeb..99a3dbfefa2e42 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java @@ -17,6 +17,7 @@ import com.google.common.base.Preconditions; import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.Attribute; +import com.google.devtools.build.lib.packages.ExecGroup; import javax.annotation.Nullable; /** @@ -97,7 +98,7 @@ public AspectClass getOwningAspect() { /** Returns a {@link DependencyKind} for the given execution group. */ static DependencyKind forExecGroup(String execGroupName) { - if (ToolchainCollection.DEFAULT_EXEC_GROUP_NAME.equals(execGroupName)) { + if (ExecGroup.DEFAULT_EXEC_GROUP_NAME.equals(execGroupName)) { return defaultExecGroupToolchain(); } return new AutoValue_DependencyKind_ToolchainDependencyKind(execGroupName, false); @@ -106,7 +107,7 @@ static DependencyKind forExecGroup(String execGroupName) { /** Returns a {@link DependencyKind} for the default execution group. */ static DependencyKind defaultExecGroupToolchain() { return new AutoValue_DependencyKind_ToolchainDependencyKind( - ToolchainCollection.DEFAULT_EXEC_GROUP_NAME, true); + ExecGroup.DEFAULT_EXEC_GROUP_NAME, true); } /** Predicate to check if a dependency represents a toolchain. */ diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 44d90212dda57b..d5c027f3ba9411 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.analysis; -import static com.google.devtools.build.lib.analysis.ToolchainCollection.DEFAULT_EXEC_GROUP_NAME; +import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java index a7457f8861de60..5961ca7bded1aa 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java @@ -16,12 +16,12 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import com.google.auto.value.AutoValue; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.ExecGroup; import java.util.HashMap; import java.util.Map; @@ -34,15 +34,11 @@ @AutoValue public abstract class ToolchainCollection { - // This is intentionally a string that would fail {@code Identifier.isValid} so that - // users can't create a group with the same name. - @VisibleForTesting public static final String DEFAULT_EXEC_GROUP_NAME = "default-exec-group"; - /** A map of execution group names to toolchain contexts. */ public abstract ImmutableMap getContextMap(); public T getDefaultToolchainContext() { - return getContextMap().get(DEFAULT_EXEC_GROUP_NAME); + return getContextMap().get(ExecGroup.DEFAULT_EXEC_GROUP_NAME); } public boolean hasToolchainContext(String execGroup) { @@ -87,7 +83,7 @@ public static final class Builder { private final Map toolchainContexts = new HashMap<>(); public ToolchainCollection build() { - Preconditions.checkArgument(toolchainContexts.containsKey(DEFAULT_EXEC_GROUP_NAME)); + Preconditions.checkArgument(toolchainContexts.containsKey(ExecGroup.DEFAULT_EXEC_GROUP_NAME)); return new AutoValue_ToolchainCollection(ImmutableMap.copyOf(toolchainContexts)); } @@ -100,7 +96,7 @@ public void addContext(String execGroup, T context) { } public Builder addDefaultContext(T context) { - addContext(DEFAULT_EXEC_GROUP_NAME, context); + addContext(ExecGroup.DEFAULT_EXEC_GROUP_NAME, context); return this; } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index 500ca758a6b3ec..19a849dadc8a79 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.analysis.actions; -import static com.google.devtools.build.lib.analysis.ToolchainCollection.DEFAULT_EXEC_GROUP_NAME; +import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.CharMatcher; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java index 54c07569fc196d..94aa521cce1d77 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java @@ -15,7 +15,7 @@ package com.google.devtools.build.lib.analysis.config; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.devtools.build.lib.analysis.ToolchainCollection.DEFAULT_EXEC_GROUP_NAME; +import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java index 1c8e45f9b36227..506d9e5fc8250c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java @@ -13,7 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.analysis.starlark; -import static com.google.devtools.build.lib.analysis.ToolchainCollection.DEFAULT_EXEC_GROUP_NAME; +import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME; import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; diff --git a/src/main/java/com/google/devtools/build/lib/packages/BUILD b/src/main/java/com/google/devtools/build/lib/packages/BUILD index b0a072d9e59574..3a02fd6f387b87 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BUILD +++ b/src/main/java/com/google/devtools/build/lib/packages/BUILD @@ -74,7 +74,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi", "//third_party:auto_value", "//third_party:guava", - "//third_party:jsr305", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/packages/ExecGroup.java b/src/main/java/com/google/devtools/build/lib/packages/ExecGroup.java index 18d156f43572d3..f91c32aae703d6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/ExecGroup.java +++ b/src/main/java/com/google/devtools/build/lib/packages/ExecGroup.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.packages; import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.starlarkbuildapi.ExecGroupApi; @@ -28,6 +29,10 @@ public abstract class ExecGroup implements ExecGroupApi { public static final ExecGroup COPY_FROM_RULE_EXEC_GROUP = createCopied(ImmutableSet.of(), ImmutableSet.of()); + // This is intentionally a string that would fail {@code Identifier.isValid} so that + // users can't create a group with the same name. + @VisibleForTesting public static final String DEFAULT_EXEC_GROUP_NAME = "default-exec-group"; + // Create an exec group that is marked as copying from the rule. public static ExecGroup createCopied( Set