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