Update AspectFunction to perform toolchain resolution for multiple exec groups.
RELNOTES: Aspects can now define and use exec groups using the same API as rules.
PiperOrigin-RevId: 453177050
Change-Id: Ia84ff906ede72cadfb6b6acf9deac7c55280bbe5
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index bd86160..af0c8b1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -490,7 +490,8 @@
Aspect aspect,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap,
ConfigConditions configConditions,
- @Nullable ResolvedToolchainContext toolchainContext,
+ @Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
+ @Nullable ExecGroupCollection.Builder execGroupCollectionBuilder,
BuildConfigurationValue aspectConfiguration,
BuildConfigurationValue hostConfiguration,
AspectKeyCreator.AspectKey aspectKey)
@@ -508,10 +509,8 @@
.setPrerequisites(transformPrerequisiteMap(prerequisiteMap))
.setAspectAttributes(mergeAspectAttributes(aspectPath))
.setConfigConditions(configConditions)
- .setToolchainContext(toolchainContext)
- // TODO(b/161222568): Implement the exec_properties attr for aspects and read its value
- // here.
- .setExecGroupCollectionBuilder(ExecGroupCollection.emptyBuilder())
+ .setToolchainContexts(toolchainContexts)
+ .setExecGroupCollectionBuilder(execGroupCollectionBuilder)
.setExecProperties(ImmutableMap.of())
.setRequiredConfigFragments(
RequiredFragmentsUtil.getAspectRequiredFragmentsIfEnabled(
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 5e40f16..ec41d70 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
@@ -1792,18 +1792,6 @@
return this;
}
- /** Sets the {@link ResolvedToolchainContext} used to access toolchains used by this rule. */
- public Builder setToolchainContext(ResolvedToolchainContext toolchainContext) {
- Preconditions.checkState(
- this.toolchainContexts == null,
- "toolchainContexts has already been set for this Builder");
- this.toolchainContexts =
- ToolchainCollection.<ResolvedToolchainContext>builder()
- .addDefaultContext(toolchainContext)
- .build();
- return this;
- }
-
/** Sets the collection of {@link ResolvedToolchainContext}s available to this rule. */
@VisibleForTesting
public Builder setToolchainContexts(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 326b851..bba79c9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.base.Preconditions;
+import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
@@ -35,6 +36,7 @@
import com.google.devtools.build.lib.analysis.DependencyKey;
import com.google.devtools.build.lib.analysis.DependencyKind;
import com.google.devtools.build.lib.analysis.DuplicateException;
+import com.google.devtools.build.lib.analysis.ExecGroupCollection;
import com.google.devtools.build.lib.analysis.ExecGroupCollection.InvalidExecGroupException;
import com.google.devtools.build.lib.analysis.InconsistentAspectOrderException;
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
@@ -78,6 +80,7 @@
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
import com.google.devtools.build.lib.skyframe.BzlLoadFunction.BzlLoadFailedException;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ComputeDependenciesState;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ComputedToolchainContexts;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -88,7 +91,9 @@
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkSemantics;
@@ -271,19 +276,28 @@
TargetAndConfiguration originalTargetAndConfiguration =
new TargetAndConfiguration(target, configuration);
try {
- UnloadedToolchainContext unloadedToolchainContext =
- getUnloadedToolchainContext(env, key, aspect, configuration);
+ ConfiguredTargetFunction.ComputedToolchainContexts computedToolchainContexts =
+ getUnloadedToolchainContexts(env, key, aspect, configuration);
if (env.valuesMissing()) {
return null;
}
+ ToolchainCollection<UnloadedToolchainContext> unloadedToolchainContexts = null;
+ ExecGroupCollection.Builder execGroupCollectionBuilder = null;
+ if (computedToolchainContexts != null) {
+ unloadedToolchainContexts = computedToolchainContexts.toolchainCollection;
+ execGroupCollectionBuilder = computedToolchainContexts.execGroupCollectionBuilder;
+ }
+
// Get the configuration targets that trigger this rule's configurable attributes.
ConfigConditions configConditions =
ConfiguredTargetFunction.getConfigConditions(
env,
originalTargetAndConfiguration,
state.transitivePackagesForPackageRootResolution,
- unloadedToolchainContext == null ? null : unloadedToolchainContext.targetPlatform(),
+ unloadedToolchainContexts == null
+ ? null
+ : unloadedToolchainContexts.getTargetPlatform(),
state.transitiveRootCauses);
if (configConditions == null) {
// Those targets haven't yet been resolved.
@@ -302,11 +316,9 @@
originalTargetAndConfiguration,
topologicalAspectPath,
configConditions.asProviders(),
- unloadedToolchainContext == null
+ unloadedToolchainContexts == null
? null
- : ToolchainCollection.builder()
- .addDefaultContext(unloadedToolchainContext)
- .build(),
+ : unloadedToolchainContexts.asToolchainContexts(),
ruleClassProvider,
buildViewProvider.getSkyframeBuildView().getHostConfiguration());
} catch (ConfiguredValueCreationException e) {
@@ -326,17 +338,23 @@
}
// Load the requested toolchains into the ToolchainContext, now that we have dependencies.
- ResolvedToolchainContext toolchainContext = null;
- if (unloadedToolchainContext != null) {
+ ToolchainCollection<ResolvedToolchainContext> toolchainContexts = null;
+ if (unloadedToolchainContexts != null) {
String targetDescription =
String.format(
"aspect %s applied to %s", aspect.getDescriptor().getDescription(), target);
- toolchainContext =
- ResolvedToolchainContext.load(
- unloadedToolchainContext,
- targetDescription,
- // TODO(161222568): Support exec groups on aspects.
- depValueMap.get(DependencyKind.defaultExecGroupToolchain()));
+ ToolchainCollection.Builder<ResolvedToolchainContext> contextsBuilder =
+ ToolchainCollection.builder();
+ for (Map.Entry<String, UnloadedToolchainContext> unloadedContext :
+ unloadedToolchainContexts.getContextMap().entrySet()) {
+ Set<ConfiguredTargetAndData> toolchainDependencies =
+ depValueMap.get(DependencyKind.forExecGroup(unloadedContext.getKey()));
+ contextsBuilder.addContext(
+ unloadedContext.getKey(),
+ ResolvedToolchainContext.load(
+ unloadedContext.getValue(), targetDescription, toolchainDependencies));
+ }
+ toolchainContexts = contextsBuilder.build();
}
return createAspect(
@@ -349,7 +367,8 @@
associatedTarget, target, configuration, /*transitionKeys=*/ null),
configuration,
configConditions,
- toolchainContext,
+ toolchainContexts,
+ execGroupCollectionBuilder,
depValueMap,
state.transitivePackagesForPackageRootResolution);
} catch (DependencyEvaluationException e) {
@@ -537,36 +556,34 @@
}
@Nullable
- private static UnloadedToolchainContext getUnloadedToolchainContext(
+ private static ComputedToolchainContexts getUnloadedToolchainContexts(
Environment env,
AspectKey key,
Aspect aspect,
@Nullable BuildConfigurationValue configuration)
throws InterruptedException, AspectCreationException {
- // Determine what toolchains are needed by this target.
- UnloadedToolchainContext unloadedToolchainContext = null;
- if (configuration != null) {
+ if (configuration == null) {
// Configuration can be null in the case of aspects applied to input files. In this case,
// there are no chances of toolchains being used, so skip it.
- try {
- unloadedToolchainContext =
- (UnloadedToolchainContext)
- env.getValueOrThrow(
- ToolchainContextKey.key()
- .configurationKey(configuration.getKey())
- .toolchainTypes(aspect.getDefinition().getToolchainTypes())
- .build(),
- ToolchainException.class);
- } catch (ToolchainException e) {
- // TODO(katre): better error handling
- throw new AspectCreationException(
- e.getMessage(), new LabelCause(key.getLabel(), e.getDetailedExitCode()));
- }
- }
- if (env.valuesMissing()) {
return null;
}
- return unloadedToolchainContext;
+ // Determine what toolchains are needed by this target.
+ try {
+ return ConfiguredTargetFunction.computeUnloadedToolchainContexts(
+ env,
+ key.getLabel(),
+ true,
+ Predicates.alwaysFalse(),
+ configuration.getKey(),
+ aspect.getDefinition().getToolchainTypes(),
+ aspect.getDefinition().execCompatibleWith(),
+ aspect.getDefinition().execGroups(),
+ null);
+ } catch (ToolchainException e) {
+ // TODO(katre): better error handling
+ throw new AspectCreationException(
+ e.getMessage(), new LabelCause(key.getLabel(), e.getDetailedExitCode()));
+ }
}
/**
@@ -613,12 +630,15 @@
// Compute the Dependency from originalTarget to aliasedLabel
Dependency dep;
try {
- UnloadedToolchainContext unloadedToolchainContext =
- getUnloadedToolchainContext(env, originalKey, aspect, originalTarget.getConfiguration());
+ ConfiguredTargetFunction.ComputedToolchainContexts computedToolchainContexts =
+ getUnloadedToolchainContexts(env, originalKey, aspect, originalTarget.getConfiguration());
if (env.valuesMissing()) {
return null;
}
+ ToolchainCollection<UnloadedToolchainContext> unloadedToolchainContexts =
+ computedToolchainContexts.toolchainCollection;
+
// See comment in compute() above for why we pair target with aspectConfiguration here
TargetAndConfiguration originalTargetAndAspectConfiguration =
new TargetAndConfiguration(originalTarget.getTarget(), aspectConfiguration);
@@ -629,7 +649,9 @@
env,
originalTargetAndAspectConfiguration,
transitivePackagesForPackageRootResolution,
- unloadedToolchainContext == null ? null : unloadedToolchainContext.targetPlatform(),
+ unloadedToolchainContexts == null
+ ? null
+ : unloadedToolchainContexts.getTargetPlatform(),
transitiveRootCauses);
if (configConditions == null) {
// Those targets haven't yet been resolved.
@@ -800,7 +822,8 @@
ConfiguredTargetAndData associatedTarget,
BuildConfigurationValue configuration,
ConfigConditions configConditions,
- ResolvedToolchainContext toolchainContext,
+ @Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
+ @Nullable ExecGroupCollection.Builder execGroupCollectionBuilder,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> directDeps,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution)
throws AspectFunctionException, InterruptedException {
@@ -842,7 +865,8 @@
aspect,
directDeps,
configConditions,
- toolchainContext,
+ toolchainContexts,
+ execGroupCollectionBuilder,
configuration,
view.getHostConfiguration(),
key);
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index deb7a23..c5149a8 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -806,6 +806,7 @@
":test-deps",
"@bazel_tools//tools/bash/runfiles",
],
+ shard_count = 5,
)
sh_test(
diff --git a/src/test/shell/integration/exec_group_test.sh b/src/test/shell/integration/exec_group_test.sh
index 7a93e73..9e1e648 100755
--- a/src/test/shell/integration/exec_group_test.sh
+++ b/src/test/shell/integration/exec_group_test.sh
@@ -681,6 +681,149 @@
EOF
}
+function test_aspect_exec_groups_inherit_toolchains() {
+ local -r pkg=${FUNCNAME[0]}
+ mkdir $pkg || fail "mkdir $pkg"
+
+ write_toolchains_for_exec_group_tests
+
+ # Add an aspect with exec groups.
+ mkdir -p ${pkg}/aspect
+ touch ${pkg}/aspect/BUILD
+ cat >> ${pkg}/aspect/aspect.bzl <<EOF
+def _impl(target, ctx):
+ toolchain = ctx.toolchains['//${pkg}/platform:toolchain_type']
+ print("hi from sample_aspect on %s, toolchain says %s" % (ctx.rule.attr.name, toolchain.message))
+
+ extra_toolchain = ctx.exec_groups["extra"].toolchains["//${pkg}/platform:toolchain_type"]
+ print("exec group extra: hi from sample_aspect on %s, toolchain says %s" % (ctx.rule.attr.name, extra_toolchain.message))
+
+ return []
+
+sample_aspect = aspect(
+ implementation = _impl,
+ exec_groups = {
+ # extra should inherit both the exec constraint and the toolchain.
+ 'extra': exec_group(copy_from_rule = True),
+ },
+ exec_compatible_with = ['//${pkg}/platform:value_foo'],
+ toolchains = ['//${pkg}/platform:toolchain_type'],
+)
+EOF
+
+ # Define a simple rule to put the aspect on.
+ mkdir -p ${pkg}/rule
+ touch ${pkg}/rule/BUILD
+ cat >> ${pkg}/rule/rule.bzl <<EOF
+def _impl(ctx):
+ pass
+
+sample_rule = rule(
+ implementation = _impl,
+)
+EOF
+
+ # Use the aspect and check the results.
+ mkdir -p ${pkg}/demo
+ cat >> ${pkg}/demo/BUILD <<EOF
+load('//${pkg}/rule:rule.bzl', 'sample_rule')
+
+sample_rule(
+ name = 'use',
+)
+EOF
+
+ # Build the target, using debug messages to verify the correct toolchain was selected.
+ bazel build --aspects=//${pkg}/aspect:aspect.bzl%sample_aspect //${pkg}/demo:use &> $TEST_log || fail "Build failed"
+ expect_log "hi from sample_aspect on use, toolchain says foo"
+ expect_log "exec group extra: hi from sample_aspect on use, toolchain says foo"
+}
+
+function test_aspect_exec_groups_different_toolchains() {
+ local -r pkg=${FUNCNAME[0]}
+ mkdir $pkg || fail "mkdir $pkg"
+
+ write_toolchains_for_exec_group_tests
+
+ # Also write a new toolchain.
+ mkdir -p ${pkg}/other
+ cat >> ${pkg}/other/BUILD <<EOF
+package(default_visibility = ['//visibility:public'])
+toolchain_type(name = 'toolchain_type')
+
+load('//${pkg}/platform:toolchain.bzl', 'test_toolchain')
+
+# Define the toolchains.
+test_toolchain(
+ name = 'test_toolchain_impl_other',
+ message = 'other',
+)
+
+# Declare the toolchains.
+toolchain(
+ name = 'test_toolchain_other',
+ toolchain_type = ':toolchain_type',
+ toolchain = ':test_toolchain_impl_other',
+)
+EOF
+
+ cat >> WORKSPACE <<EOF
+register_toolchains('//${pkg}/other:all')
+EOF
+
+ # Add an aspect with exec groups.
+ mkdir -p ${pkg}/aspect
+ touch ${pkg}/aspect/BUILD
+ cat >> ${pkg}/aspect/aspect.bzl <<EOF
+def _impl(target, ctx):
+ toolchain = ctx.toolchains['//${pkg}/platform:toolchain_type']
+ print("hi from sample_aspect on %s, toolchain says %s" % (ctx.rule.attr.name, toolchain.message))
+
+ other_toolchain = ctx.exec_groups["other"].toolchains["//${pkg}/other:toolchain_type"]
+ print("exec group other: hi from sample_aspect on %s, toolchain says %s" % (ctx.rule.attr.name, other_toolchain.message))
+
+ return []
+
+sample_aspect = aspect(
+ implementation = _impl,
+ exec_groups = {
+ # other defines new toolchain types.
+ 'other': exec_group(
+ toolchains = ['//${pkg}/other:toolchain_type'],
+ ),
+ },
+ toolchains = ['//${pkg}/platform:toolchain_type'],
+)
+EOF
+
+ # Define a simple rule to put the aspect on.
+ mkdir -p ${pkg}/rule
+ touch ${pkg}/rule/BUILD
+ cat >> ${pkg}/rule/rule.bzl <<EOF
+def _impl(ctx):
+ pass
+
+sample_rule = rule(
+ implementation = _impl,
+)
+EOF
+
+ # Use the aspect and check the results.
+ mkdir -p ${pkg}/demo
+ cat >> ${pkg}/demo/BUILD <<EOF
+load('//${pkg}/rule:rule.bzl', 'sample_rule')
+
+sample_rule(
+ name = 'use',
+)
+EOF
+
+ # Build the target, using debug messages to verify the correct toolchain was selected.
+ bazel build --aspects=//${pkg}/aspect:aspect.bzl%sample_aspect //${pkg}/demo:use &> $TEST_log || fail "Build failed"
+ expect_log "hi from sample_aspect on use, toolchain says bar"
+ expect_log "exec group other: hi from sample_aspect on use, toolchain says other"
+}
+
# Test basic inheritance of constraints and toolchains on a single rule.
function test_exec_group_rule_constraint_inheritance() {
local -r pkg=${FUNCNAME[0]}
