Make it possible for aspects to provide FileProvider.
This will then be merged with the FileProvider of the associated configured target.
RELNOTES[NEW]: aspects can now return DefaultInfo, which will then be merged with that of the configured target they are applied to. Currently, only the files= field is supported.
PiperOrigin-RevId: 661221846
Change-Id: I15a95220b9ea263e3e2125f80c8acadd80078565
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectBaseTargetResolvedToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectBaseTargetResolvedToolchainContext.java
index e27fd48..3fb9985 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectBaseTargetResolvedToolchainContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectBaseTargetResolvedToolchainContext.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.MergingException;
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Provider;
@@ -47,7 +48,7 @@
UnloadedToolchainContext unloadedToolchainContext,
String targetDescription,
ImmutableMultimap<ToolchainTypeInfo, ConfiguredTargetAndData> toolchainTargets)
- throws DuplicateException {
+ throws MergingException {
ImmutableMap.Builder<ToolchainTypeInfo, ToolchainAspectsProviders> toolchainsBuilder =
new ImmutableMap.Builder<>();
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 59a75aa..177b241 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -89,7 +89,6 @@
":constraints/supported_environments_provider",
":constraints/top_level_constraint_semantics",
":dependency_kind",
- ":duplicate_exception",
":extra/extra_action_info_file_write_action",
":extra_action_artifacts_provider",
":file_provider",
@@ -305,7 +304,6 @@
":constraints/supported_environments",
":constraints/supported_environments_provider",
":dependency_kind",
- ":duplicate_exception",
":exec_group_collection",
":extra/extra_action_info_file_write_action",
":extra_action_artifacts_provider",
@@ -751,11 +749,6 @@
)
java_library(
- name = "duplicate_exception",
- srcs = ["DuplicateException.java"],
-)
-
-java_library(
name = "exec_group_collection",
srcs = ["ExecGroupCollection.java"],
deps = [
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DuplicateException.java b/src/main/java/com/google/devtools/build/lib/analysis/DuplicateException.java
deleted file mode 100644
index 3ad5cc1..0000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/DuplicateException.java
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright 2020 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.analysis;
-
-/**
- * This exception is thrown when configured targets and aspects being merged provide duplicate
- * things that they shouldn't (output groups or providers).
- */
-public final class DuplicateException extends Exception {
- public DuplicateException(String message) {
- super(message);
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
index e38e157..85de020 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
@@ -26,6 +26,7 @@
import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.MergingException;
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleConfiguredTargetUtil;
import com.google.devtools.build.lib.collect.ImmutableSharedKeyMap;
import com.google.devtools.build.lib.collect.nestedset.Depset;
@@ -178,7 +179,7 @@
* disjoint, except for the special validation output group, which is always merged.
*/
@Nullable
- public static OutputGroupInfo merge(List<OutputGroupInfo> providers) throws DuplicateException {
+ public static OutputGroupInfo merge(List<OutputGroupInfo> providers) throws MergingException {
if (providers.isEmpty()) {
return null;
}
@@ -193,7 +194,7 @@
continue;
}
if (outputGroups.put(group, provider.getOutputGroup(group)) != null) {
- throw new DuplicateException("Output group " + group + " provided twice");
+ throw new MergingException("Output group " + group + " provided twice");
}
}
}
@@ -204,7 +205,7 @@
validationOutputs.addTransitive(provider.getOutputGroup(VALIDATION));
} catch (IllegalArgumentException e) {
// Thrown if nested set orders aren't compatible.
- throw new DuplicateException(
+ throw new MergingException(
"Output group " + VALIDATION + " provided twice with incompatible depset orders");
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
index 4f64db4..801e48b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
@@ -16,11 +16,13 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionLookupKey;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.DuplicateException;
+import com.google.devtools.build.lib.analysis.DefaultInfo;
import com.google.devtools.build.lib.analysis.ExtraActionArtifactsProvider;
+import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
@@ -30,7 +32,9 @@
import com.google.devtools.build.lib.analysis.test.AnalysisFailureInfo;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.nestedset.Depset.TypeException;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.packages.Provider;
@@ -52,8 +56,23 @@
*/
@Immutable
public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
+ /**
+ * This exception is thrown when the providers of a configured target and the aspects applied to
+ * it cannot be merged.
+ */
+ public static final class MergingException extends Exception {
+ public MergingException(String message) {
+ super(message);
+ }
+
+ public MergingException(String message, Throwable cause) {
+ super(message, cause);
+ }
+ }
+
private final ConfiguredTarget base;
private final ImmutableList<ConfiguredAspect> aspects;
+
/**
* Providers that come from any source that isn't a pure pointer to the base rule's providers.
*
@@ -160,13 +179,50 @@
/** Creates an instance based on a configured target and a set of aspects. */
public static ConfiguredTarget of(ConfiguredTarget base, Collection<ConfiguredAspect> aspects)
- throws DuplicateException {
+ throws MergingException {
if (aspects.isEmpty()) {
return base; // If there are no aspects, don't bother with creating a proxy object.
}
TransitiveInfoProviderMapBuilder nonBaseProviders = new TransitiveInfoProviderMapBuilder();
+ // filesToBuild is special: for native aspects, it is returned in a FileProvider, for Starlark
+ // ones, in a DefaultInfo. Furthermore, DefaultInfo is not a "normal" provider on configured
+ // targets but is created on demand from FileProvider, etc. So we need to jump through some
+ // hoops here.
+ List<NestedSet<Artifact>> filesToBuild = new ArrayList<>();
+ filesToBuild.add(base.getProvider(FileProvider.class).getFilesToBuild());
+ for (ConfiguredAspect aspect : aspects) {
+ if (aspect.getProvider(FileProvider.class) != null) {
+ filesToBuild.add(aspect.getProvider(FileProvider.class).getFilesToBuild());
+ } else if (aspect.get(DefaultInfo.PROVIDER.getKey()) != null) {
+ DefaultInfo defaultInfo = (DefaultInfo) aspect.get(DefaultInfo.PROVIDER.getKey());
+ if (defaultInfo.getDataRunfiles() != null
+ || defaultInfo.getDefaultRunfiles() != null
+ || defaultInfo.getExecutable() != null
+ || defaultInfo.getFilesToRun() != null) {
+ throw new MergingException(
+ "Provider 'DefaultInfo' returned by an aspect not at top level must only have the "
+ + "'files' field set");
+ }
+
+ if (defaultInfo.getFiles() != null) {
+ try {
+ filesToBuild.add(defaultInfo.getFiles().getSet(Artifact.class));
+ } catch (TypeException e) {
+ throw new MergingException(
+ "'files' field of 'DefaultInfo' should contain a depset of files", e);
+ }
+ }
+ }
+ }
+
+ if (filesToBuild.size() > 1) {
+ nonBaseProviders.put(
+ FileProvider.class,
+ FileProvider.of(NestedSetBuilder.fromNestedSets(filesToBuild).build()));
+ }
+
// Merge output group providers.
OutputGroupInfo mergedOutputGroupInfo = mergeOutputGroupProviders(base, aspects);
if (mergedOutputGroupInfo != null) {
@@ -199,23 +255,27 @@
Object providerKey = providers.getProviderKeyAt(i);
if (OutputGroupInfo.STARLARK_CONSTRUCTOR.getKey().equals(providerKey)
|| AnalysisFailureInfo.STARLARK_CONSTRUCTOR.getKey().equals(providerKey)
+ || FileProvider.class.equals(providerKey)
|| ExtraActionArtifactsProvider.class.equals(providerKey)
|| RequiredConfigFragmentsProvider.class.equals(providerKey)) {
continue;
}
- if (providerKey instanceof Class<?>) {
+ if (providerKey.equals(DefaultInfo.PROVIDER.getKey())) {
+ // This was handled when creating FileProvider above.
+ continue;
+ } else if (providerKey instanceof Class<?>) {
@SuppressWarnings("unchecked")
Class<? extends TransitiveInfoProvider> providerClass =
(Class<? extends TransitiveInfoProvider>) providerKey;
if (base.getProvider(providerClass) != null || nonBaseProviders.contains(providerClass)) {
- throw new DuplicateException("Provider " + providerKey + " provided twice");
+ throw new MergingException("Provider " + providerKey + " provided twice");
}
nonBaseProviders.put(
providerClass, (TransitiveInfoProvider) providers.getProviderInstanceAt(i));
} else if (providerKey instanceof String legacyId) {
if (base.get(legacyId) != null || nonBaseProviders.contains(legacyId)) {
- throw new DuplicateException("Provider " + legacyId + " provided twice");
+ throw new MergingException("Provider " + legacyId + " provided twice");
}
nonBaseProviders.put(legacyId, providers.getProviderInstanceAt(i));
} else if (providerKey instanceof Provider.Key key) {
@@ -227,7 +287,7 @@
if ((!InstrumentedFilesInfo.STARLARK_CONSTRUCTOR.getKey().equals(key)
&& base.get(key) != null)
|| nonBaseProviders.contains(key)) {
- throw new DuplicateException("Provider " + key + " provided twice");
+ throw new MergingException("Provider " + key + " provided twice");
}
nonBaseProviders.put((Info) providers.getProviderInstanceAt(i));
}
@@ -237,8 +297,7 @@
}
private static OutputGroupInfo mergeOutputGroupProviders(
- @Nullable ConfiguredTarget base, Iterable<ConfiguredAspect> aspects)
- throws DuplicateException {
+ @Nullable ConfiguredTarget base, Iterable<ConfiguredAspect> aspects) throws MergingException {
ImmutableList.Builder<OutputGroupInfo> providers = ImmutableList.builder();
if (base != null) {
@@ -321,7 +380,7 @@
}
/** Returns only the providers from the aspects. */
- public TransitiveInfoProviderMap getAspectsProviders() throws DuplicateException {
+ public TransitiveInfoProviderMap getAspectsProviders() throws MergingException {
TransitiveInfoProviderMapBuilder aspectsProviders = new TransitiveInfoProviderMapBuilder();
// Merge output group providers of aspects only. Filtering the base target output
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD
index 2849660..a7bc9ed 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD
@@ -46,7 +46,6 @@
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
"//src/main/java/com/google/devtools/build/lib/analysis:constraints/incompatible_target_checker",
"//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind",
- "//src/main/java/com/google/devtools/build/lib/analysis:duplicate_exception",
"//src/main/java/com/google/devtools/build/lib/analysis:exec_group_collection",
"//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_aspect_order_exception",
"//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_null_config_exception",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredAspectProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredAspectProducer.java
index e34533c..14d8935 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredAspectProducer.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredAspectProducer.java
@@ -20,9 +20,9 @@
import com.google.devtools.build.lib.analysis.AspectCollection.AspectDeps;
import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
-import com.google.devtools.build.lib.analysis.DuplicateException;
import com.google.devtools.build.lib.analysis.TransitiveDependencyState;
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.MergingException;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.skyframe.AspectCreationException;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
@@ -42,7 +42,7 @@
void acceptConfiguredAspectError(AspectCreationException error);
- void acceptConfiguredAspectError(DuplicateException error);
+ void acceptConfiguredAspectError(MergingException error);
}
// -------------------- Input --------------------
@@ -117,7 +117,7 @@
outputIndex,
prerequisite.fromConfiguredTarget(
MergedConfiguredTarget.of(prerequisite.getConfiguredTarget(), configuredAspects)));
- } catch (DuplicateException e) {
+ } catch (MergingException e) {
sink.acceptConfiguredAspectError(e);
}
return DONE;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java
index bc2c047..ac198ed 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java
@@ -22,11 +22,11 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.AspectCollection;
-import com.google.devtools.build.lib.analysis.DuplicateException;
import com.google.devtools.build.lib.analysis.InconsistentAspectOrderException;
import com.google.devtools.build.lib.analysis.InconsistentNullConfigException;
import com.google.devtools.build.lib.analysis.InvalidVisibilityDependencyException;
import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException;
+import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.MergingException;
import com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Aspect;
@@ -243,7 +243,7 @@
}
@Override
- public void acceptConfiguredAspectError(DuplicateException error) {
+ public void acceptConfiguredAspectError(MergingException error) {
hasError = true;
sink.acceptPrerequisitesAspectError(
new DependencyEvaluationException(
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 18095b3..05a5cdc 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
@@ -35,7 +35,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
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.IncompatiblePlatformProvider;
@@ -50,6 +49,7 @@
import com.google.devtools.build.lib.analysis.config.StarlarkExecTransitionLoader;
import com.google.devtools.build.lib.analysis.config.StarlarkExecTransitionLoader.StarlarkExecTransitionLoadingException;
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.MergingException;
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.analysis.producers.DependencyContext;
import com.google.devtools.build.lib.analysis.producers.DependencyContextProducer;
@@ -338,7 +338,7 @@
Lists.transform(key.getBaseKeys(), k -> ((AspectValue) aspectValues.get(k)));
try {
associatedTarget = MergedConfiguredTarget.of(associatedTarget, directlyRequiredAspects);
- } catch (DuplicateException e) {
+ } catch (MergingException e) {
env.getListener().handle(Event.error(target.getLocation(), e.getMessage()));
throw new AspectFunctionException(
new AspectCreationException(e.getMessage(), target.getLabel(), configuration));
@@ -426,7 +426,7 @@
baseTargetToolchainContexts =
getBaseTargetToolchainContexts(
baseTargetUnloadedToolchainContexts, aspect, target, depValueMap);
- } catch (DuplicateException e) {
+ } catch (MergingException e) {
env.getListener().handle(Event.error(target.getLocation(), e.getMessage()));
throw new AspectFunctionException(
new AspectCreationException(e.getMessage(), target.getLabel(), configuration));
@@ -487,7 +487,7 @@
Aspect aspect,
Target target,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> depValueMap)
- throws DuplicateException {
+ throws MergingException {
if (baseTargetUnloadedToolchainContexts == null) {
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 7bf8943..f8da79a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -264,7 +264,6 @@
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
"//src/main/java/com/google/devtools/build/lib/analysis:constraints/incompatible_target_checker",
"//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind",
- "//src/main/java/com/google/devtools/build/lib/analysis:duplicate_exception",
"//src/main/java/com/google/devtools/build/lib/analysis:exec_group_collection",
"//src/main/java/com/google/devtools/build/lib/analysis:incompatible_platform_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_aspect_order_exception",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 7486f01..e28262a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -129,6 +129,18 @@
}
@Test
+ public void fileProviderMerged() throws Exception {
+ setRulesAvailableInTests(
+ TestAspects.BASE_RULE, TestAspects.FILE_PROVIDER_ASPECT_REQUIRING_RULE);
+ pkg("a", "file_provider_aspect(name='a', dep=':b')", "filegroup(name='b', srcs=['source'])");
+
+ ConfiguredTarget a = getConfiguredTarget("//a:a");
+ NestedSet<Artifact> filesToBuild = a.getProvider(FileProvider.class).getFilesToBuild();
+ assertThat(ActionsTestUtil.baseArtifactNames(filesToBuild))
+ .containsExactly("file_provider_aspect_file", "source");
+ }
+
+ @Test
public void providersOfAspectAreMergedIntoDependency() throws Exception {
setRulesAvailableInTests(TestAspects.BASE_RULE, TestAspects.ASPECT_REQUIRING_RULE);
pkg("a",
@@ -172,10 +184,6 @@
.containsExactly("rule //a:a", "aspect //a:b", "aspect //a:c");
}
-
-
-
-
@Test
public void aspectCreationWorksThroughBind() throws Exception {
if (getInternalTestExecutionMode() != TestConstants.InternalTestExecutionMode.NORMAL) {
@@ -1322,36 +1330,42 @@
scratch.file(
"aspect/build_defs.bzl",
"""
+ DuplicateInfo = provider(fields=[])
def _aspect_impl(target, ctx):
- return [DefaultInfo()]
+ return [DuplicateInfo()]
- returns_default_info_aspect = aspect(implementation = _aspect_impl)
+ returns_duplicate_aspect = aspect(implementation = _aspect_impl)
+
+ def _duplicate_rule_impl(ctx):
+ return [DefaultInfo(), DuplicateInfo()]
+
+ duplicate_rule = rule(implementation = _duplicate_rule_impl, attrs = {})
def _rule_impl(ctx):
pass
- duplicate_provider_aspect_applying_rule = rule(
+ duplicate_aspect_applying_rule = rule(
implementation = _rule_impl,
- attrs = {"to": attr.label(aspects = [returns_default_info_aspect])},
+ attrs = {"to": attr.label(aspects = [returns_duplicate_aspect])},
)
""");
scratch.file(
"aspect/BUILD",
"""
- load("build_defs.bzl", "duplicate_provider_aspect_applying_rule")
+ load("build_defs.bzl", "duplicate_aspect_applying_rule", "duplicate_rule")
- cc_library(name = "rule_target")
+ duplicate_rule(name = "duplicate")
- duplicate_provider_aspect_applying_rule(
+ duplicate_aspect_applying_rule(
name = "applies_aspect",
- to = ":rule_target",
+ to = ":duplicate",
)
""");
assertThat(
assertThrows(
AssertionError.class, () -> getConfiguredTarget("//aspect:applies_aspect")))
.hasMessageThat()
- .contains("Provider DefaultInfo provided twice");
+ .contains("Provider DuplicateInfo provided twice");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index d6707e0..f64710c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -710,6 +710,7 @@
TestAspects.EXTRA_ATTRIBUTE_ASPECT,
TestAspects.PACKAGE_GROUP_ATTRIBUTE_ASPECT,
TestAspects.COMPUTED_ATTRIBUTE_ASPECT,
+ TestAspects.FILE_PROVIDER_ASPECT,
TestAspects.FOO_PROVIDER_ASPECT,
TestAspects.ASPECT_REQUIRING_PROVIDER_SETS,
TestAspects.WARNING_ASPECT,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index 7e1dd40..50c8d83 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.PackageSpecificationProvider;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
@@ -39,6 +40,7 @@
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
+import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -170,6 +172,18 @@
return result.build();
}
+ public static class FileProviderForwardingRuleFactory implements RuleConfiguredTargetFactory {
+ @Override
+ public ConfiguredTarget create(RuleContext ruleContext)
+ throws InterruptedException, RuleErrorException, ActionConflictException {
+ return new RuleConfiguredTargetBuilder(ruleContext)
+ .setFilesToBuild(ruleContext.getPrerequisite("dep", FileProvider.class).getFilesToBuild())
+ .setRunfilesSupport(null, null)
+ .addProvider(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY))
+ .build();
+ }
+ }
+
/**
* A simple rule configured target factory that is used in all the mock rules in this class.
*/
@@ -273,7 +287,30 @@
}
}
+ public static class FileProviderAspect extends BaseAspect {
+ @Override
+ public ConfiguredAspect create(
+ Label targetLabel,
+ ConfiguredTarget ct,
+ RuleContext ruleContext,
+ AspectParameters parameters,
+ RepositoryName toolsRepository)
+ throws ActionConflictException, InterruptedException {
+ Artifact artifact = ruleContext.getBinArtifact("file_provider_aspect_file");
+ ruleContext.registerAction(FileWriteAction.create(ruleContext, artifact, "empty", false));
+ return new ConfiguredAspect.Builder(ruleContext)
+ .addProvider(FileProvider.of(NestedSetBuilder.create(Order.STABLE_ORDER, artifact)))
+ .build();
+ }
+
+ @Override
+ public AspectDefinition getDefinition(AspectParameters aspectParameters) {
+ return SIMPLE_ASPECT_DEFINITION;
+ }
+ }
+
public static final SimpleAspect SIMPLE_ASPECT = new SimpleAspect();
+ public static final FileProviderAspect FILE_PROVIDER_ASPECT = new FileProviderAspect();
public static final FooProviderAspect FOO_PROVIDER_ASPECT = new FooProviderAspect();
public static final BarProviderAspect BAR_PROVIDER_ASPECT = new BarProviderAspect();
public static final SimpleStarlarkNativeAspect SIMPLE_STARLARK_NATIVE_ASPECT =
@@ -803,6 +840,16 @@
public static final MockRule BASE_RULE = () ->
MockRule.factory(DummyRuleFactory.class).define("base");
+ public static final MockRule FILE_PROVIDER_ASPECT_REQUIRING_RULE =
+ () ->
+ MockRule.ancestor(BASE_RULE.getClass())
+ .factory(FileProviderForwardingRuleFactory.class)
+ .define(
+ "file_provider_aspect",
+ attr("dep", LABEL)
+ .allowedFileTypes(FileTypeSet.ANY_FILE)
+ .aspect(FILE_PROVIDER_ASPECT));
+
/**
* A rule that defines an aspect on one of its attributes.
*/
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
index 8569258..4006079 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
@@ -29,11 +29,13 @@
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.AnalysisResult;
import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
@@ -1091,6 +1093,127 @@
}
@Test
+ public void mergesFileProvider() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ """
+ def _asp_impl(target, ctx):
+ f = ctx.actions.declare_file('aspectfile')
+ ctx.actions.write(f, 'f')
+ return [DefaultInfo(files=depset([f]))]
+
+ asp = aspect(implementation=_asp_impl)
+ def _rule_impl(ctx):
+ return [DefaultInfo(files=depset(ctx.files.dep))]
+ my_rule = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [asp]) })
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load(':aspect.bzl', 'my_rule')
+ my_rule(name='a', dep=':b')
+ filegroup(name='b', srcs=['sourcefile'])
+ """);
+
+ AnalysisResult analysisResult = update("//test:a");
+ NestedSet<Artifact> filesToBuild =
+ Iterables.getOnlyElement(analysisResult.getTargetsToBuild())
+ .getProvider(FileProvider.class)
+ .getFilesToBuild();
+ assertThat(ActionsTestUtil.baseArtifactNames(filesToBuild))
+ .containsExactly("sourcefile", "aspectfile");
+ }
+
+ @Test
+ public void aspectDefaultInfoWithNonArtifacts() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ """
+ def _asp_impl(target, ctx):
+ return [DefaultInfo(files=depset([1]))]
+
+ asp = aspect(implementation=_asp_impl)
+ def _rule_impl(ctx):
+ return [DefaultInfo()]
+ my_rule = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [asp]) })
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load(':aspect.bzl', 'my_rule')
+ my_rule(name='a', dep=':b')
+ filegroup(name='b', srcs=['sourcefile'])
+ """);
+
+ reporter.removeHandler(failFastHandler);
+ try {
+ update("//test:a");
+ } catch (ViewCreationFailedException e) {
+ // expected.
+ }
+ assertContainsEvent("should contain a depset of files");
+ }
+
+ @Test
+ public void aspectDefaultInfoSomethingElseThanFiles() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ """
+ def _asp_impl(target, ctx):
+ return [DefaultInfo(runfiles=ctx.runfiles([]))]
+
+ asp = aspect(implementation=_asp_impl)
+ def _rule_impl(ctx):
+ return [DefaultInfo()]
+ my_rule = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [asp]) })
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load(':aspect.bzl', 'my_rule')
+ my_rule(name='a', dep=':b')
+ filegroup(name='b', srcs=['sourcefile'])
+ """);
+
+ reporter.removeHandler(failFastHandler);
+ try {
+ update("//test:a");
+ } catch (ViewCreationFailedException e) {
+ // expected.
+ }
+ assertContainsEvent("must only have the 'files' field set");
+ }
+
+ @Test
+ public void aspectEmptyDefaultInfo() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ """
+ def _asp_impl(target, ctx):
+ return [DefaultInfo()]
+
+ asp = aspect(implementation=_asp_impl)
+ def _rule_impl(ctx):
+ return [DefaultInfo(files=depset(ctx.files.dep))]
+ my_rule = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [asp]) })
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load(':aspect.bzl', 'my_rule')
+ my_rule(name='a', dep=':b')
+ filegroup(name='b', srcs=['sourcefile'])
+ """);
+
+ AnalysisResult analysisResult = update("//test:a");
+ NestedSet<Artifact> filesToBuild =
+ Iterables.getOnlyElement(analysisResult.getTargetsToBuild())
+ .getProvider(FileProvider.class)
+ .getFilesToBuild();
+ assertThat(ActionsTestUtil.baseArtifactNames(filesToBuild)).containsExactly("sourcefile");
+ }
+
+ @Test
public void outputGroupsFromOneAspect() throws Exception {
scratch.file(
"test/aspect.bzl",
