Skip to content

Commit

Permalink
Make it possible for aspects to provide FileProvider.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lberki authored and copybara-github committed Aug 9, 2024
1 parent 6205568 commit 60203aa
Show file tree
Hide file tree
Showing 14 changed files with 281 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,7 +48,7 @@ public static AspectBaseTargetResolvedToolchainContext load(
UnloadedToolchainContext unloadedToolchainContext,
String targetDescription,
ImmutableMultimap<ToolchainTypeInfo, ConfiguredTargetAndData> toolchainTargets)
throws DuplicateException {
throws MergingException {

ImmutableMap.Builder<ToolchainTypeInfo, ToolchainAspectsProviders> toolchainsBuilder =
new ImmutableMap.Builder<>();
Expand Down
7 changes: 0 additions & 7 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ java_library(
":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",
Expand Down Expand Up @@ -305,7 +304,6 @@ java_library(
":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",
Expand Down Expand Up @@ -750,11 +748,6 @@ java_library(
],
)

java_library(
name = "duplicate_exception",
srcs = ["DuplicateException.java"],
)

java_library(
name = "exec_group_collection",
srcs = ["ExecGroupCollection.java"],
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -178,7 +179,7 @@ public static OutputGroupInfo get(ProviderCollection collection) {
* 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;
}
Expand All @@ -193,7 +194,7 @@ public static OutputGroupInfo merge(List<OutputGroupInfo> providers) throws Dupl
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");
}
}
}
Expand All @@ -204,7 +205,7 @@ public static OutputGroupInfo merge(List<OutputGroupInfo> providers) throws Dupl
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");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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.
*
Expand Down Expand Up @@ -160,13 +179,50 @@ protected Object rawGetStarlarkProvider(String providerKey) {

/** 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) {
Expand Down Expand Up @@ -199,23 +255,27 @@ public static ConfiguredTarget of(ConfiguredTarget base, Collection<ConfiguredAs
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) {
Expand All @@ -227,7 +287,7 @@ public static ConfiguredTarget of(ConfiguredTarget base, Collection<ConfiguredAs
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));
}
Expand All @@ -237,8 +297,7 @@ public static ConfiguredTarget of(ConfiguredTarget base, Collection<ConfiguredAs
}

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) {
Expand Down Expand Up @@ -321,7 +380,7 @@ public ConfiguredTarget unwrapIfMerged() {
}

/** 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ java_library(
"//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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,7 +42,7 @@ interface ResultSink {

void acceptConfiguredAspectError(AspectCreationException error);

void acceptConfiguredAspectError(DuplicateException error);
void acceptConfiguredAspectError(MergingException error);
}

// -------------------- Input --------------------
Expand Down Expand Up @@ -117,7 +117,7 @@ private StateMachine processResult(Tasks tasks) {
outputIndex,
prerequisite.fromConfiguredTarget(
MergedConfiguredTarget.of(prerequisite.getConfiguredTarget(), configuredAspects)));
} catch (DuplicateException e) {
} catch (MergingException e) {
sink.acceptConfiguredAspectError(e);
}
return DONE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -243,7 +243,7 @@ public void acceptConfiguredAspectMergedTarget(
}

@Override
public void acceptConfiguredAspectError(DuplicateException error) {
public void acceptConfiguredAspectError(MergingException error) {
hasError = true;
sink.acceptPrerequisitesAspectError(
new DependencyEvaluationException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -338,7 +338,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
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));
Expand Down Expand Up @@ -426,7 +426,7 @@ && canAspectsPropagateToToolchains(topologicalAspectPath, target)) {
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));
Expand Down Expand Up @@ -487,7 +487,7 @@ && canAspectsPropagateToToolchains(topologicalAspectPath, target)) {
Aspect aspect,
Target target,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> depValueMap)
throws DuplicateException {
throws MergingException {
if (baseTargetUnloadedToolchainContexts == null) {
return null;
}
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ java_library(
"//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",
Expand Down
Loading

0 comments on commit 60203aa

Please sign in to comment.