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",
