Skip to content

Commit

Permalink
Limit API for rule context prerequisites map
Browse files Browse the repository at this point in the history
- remove `getConfiguredTargetAndDataMap()` and `getConfiguredTargetMap()` and replace their usages with calls to the attribute prerequisite getters methods.
- remove `getprerequisiteconfiguredtargetan()`

PiperOrigin-RevId: 569554909
Change-Id: I0bf5df8d04b7768933fca6cfaecae96ecd5c1006
  • Loading branch information
mai93 authored and copybara-github committed Sep 29, 2023
1 parent 80f7e52 commit da41c72
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ private static ImmutableList<NestedSet<AnalysisFailure>> depAnalysisFailures(
if (ruleContext.getConfiguration().allowAnalysisFailures()) {
ImmutableList.Builder<NestedSet<AnalysisFailure>> analysisFailures = ImmutableList.builder();
Iterable<? extends TransitiveInfoCollection> infoCollections =
Iterables.concat(ruleContext.getConfiguredTargetMap().values(), extraDeps);
Iterables.concat(ruleContext.getAllPrerequisites(), extraDeps);
for (TransitiveInfoCollection infoCollection : infoCollections) {
AnalysisFailureInfo failureInfo =
infoCollection.get(AnalysisFailureInfo.STARLARK_CONSTRUCTOR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ static ExtraActionArtifactsProvider createExtraActionProvider(
}

// Add extra action artifacts from dependencies
for (ExtraActionArtifactsProvider provider : AnalysisUtils.getProviders(
ruleContext.getConfiguredTargetMap().values(), ExtraActionArtifactsProvider.class)) {
for (ExtraActionArtifactsProvider provider :
AnalysisUtils.getProviders(
ruleContext.getAllPrerequisites(), ExtraActionArtifactsProvider.class)) {
builder.addTransitive(provider.getTransitiveExtraActionArtifacts());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.google.devtools.build.lib.analysis;

import com.google.common.collect.ListMultimap;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand All @@ -25,6 +24,7 @@
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.License;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import net.starlark.java.eval.StarlarkValue;

/** A {@link ConfiguredTarget} that has licensed targets in its transitive closure. */
Expand Down Expand Up @@ -72,9 +72,6 @@ public static LicensesProvider of(RuleContext ruleContext) {
builder.add(new TargetLicense(rule.getLabel(), rule.getLicense()));
}

ListMultimap<String, ? extends TransitiveInfoCollection> configuredMap =
ruleContext.getConfiguredTargetMap();

if (rule.getRuleClassObject().isPackageMetadataRule()) {
// Don't crawl a new-style license, it's effectively a leaf.
// The representation of the new-style rule is unfortunately hardcoded here,
Expand All @@ -84,8 +81,9 @@ public static LicensesProvider of(RuleContext ruleContext) {
// Only add the transitive licenses for the attributes that do not have the
// output_licenses.
Attribute attribute = attributes.getAttributeDefinition(depAttrName);
for (TransitiveInfoCollection dep : configuredMap.get(depAttrName)) {
LicensesProvider provider = dep.get(LicensesProvider.PROVIDER);
for (ConfiguredTargetAndData dep :
ruleContext.getPrerequisiteConfiguredTargets(depAttrName)) {
LicensesProvider provider = dep.getConfiguredTarget().get(LicensesProvider.PROVIDER);
if (provider == null) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,19 +356,11 @@ public boolean hasErrors() {
return reporter.hasErrors();
}

/**
* Returns an immutable map from attribute name to list of configured targets for that attribute.
*/
public ListMultimap<String, ? extends TransitiveInfoCollection> getConfiguredTargetMap() {
return Multimaps.transformValues(targetMap, ConfiguredTargetAndData::getConfiguredTarget);
}

/**
* Returns an immutable map from attribute name to list of {@link ConfiguredTargetAndData} objects
* for that attribute.
*/
public ListMultimap<String, ConfiguredTargetAndData> getConfiguredTargetAndDataMap() {
return targetMap;
/** Returns a list of all prerequisites as {@code ConfiguredTarget} objects. */
public ImmutableList<? extends TransitiveInfoCollection> getAllPrerequisites() {
return targetMap.values().stream()
.map(ConfiguredTargetAndData::getConfiguredTarget)
.collect(toImmutableList());
}

/** Returns the {@link ConfiguredTargetAndData} the given attribute. */
Expand Down Expand Up @@ -877,8 +869,14 @@ public <C extends TransitiveInfoProvider> C getPrerequisite(
*/
@Nullable
public TransitiveInfoCollection getPrerequisite(String attributeName) {
ConfiguredTargetAndData result = getPrerequisiteConfiguredTargetAndData(attributeName);
return result == null ? null : result.getConfiguredTarget();
checkAttributeIsDependency(attributeName);
List<ConfiguredTargetAndData> elements = getPrerequisiteConfiguredTargets(attributeName);
Preconditions.checkState(
elements.size() <= 1,
"%s attribute %s produces more than one prerequisite",
ruleClassNameForLogging,
attributeName);
return elements.isEmpty() ? null : elements.get(0).getConfiguredTarget();
}

/**
Expand All @@ -892,22 +890,6 @@ public <T extends Info> T getPrerequisite(String attributeName, BuiltinProvider<
return prerequisite == null ? null : prerequisite.get(starlarkKey);
}

/**
* Returns the {@link ConfiguredTargetAndData} that feeds ino this target through the specified
* attribute. Returns null if the attribute is empty.
*/
@Nullable
private ConfiguredTargetAndData getPrerequisiteConfiguredTargetAndData(String attributeName) {
checkAttributeIsDependency(attributeName);
List<ConfiguredTargetAndData> elements = getPrerequisiteConfiguredTargets(attributeName);
Preconditions.checkState(
elements.size() <= 1,
"%s attribute %s produces more than one prerequisite",
ruleClassNameForLogging,
attributeName);
return elements.isEmpty() ? null : elements.get(0);
}

/**
* For a given attribute, returns all declared provider provided by targets of that attribute.
* Each declared provider is keyed by the {@link BuildConfigurationValue} under which the provider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.devtools.build.lib.analysis;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -83,10 +82,9 @@ public static ImmutableSet<ConfiguredTargetKey> findImplicitDeps(RuleContext rul
Set<ConfiguredTargetKey> explicitDeps = CompactHashSet.create();
// Consider rule attribute dependencies.
AttributeMap attributes = ruleContext.attributes();
ListMultimap<String, ConfiguredTargetAndData> targetMap =
ruleContext.getConfiguredTargetAndDataMap();
for (String attrName : attributes.getAttributeNames()) {
List<ConfiguredTargetAndData> attrValues = targetMap.get(attrName);
List<ConfiguredTargetAndData> attrValues =
ruleContext.getPrerequisiteConfiguredTargets(attrName);
if (attrValues != null && !attrValues.isEmpty()) {
if (attributes.isAttributeValueExplicitlySpecified(attrName)) {
addLabelsAndConfigs(explicitDeps, attrValues);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static NativeLibs fromLinkedNativeDeps(
throws InterruptedException, RuleErrorException {
Map<String, ConfiguredTargetAndData> toolchainsByLibDir = new LinkedHashMap<>();
for (ConfiguredTargetAndData toolchainDep :
ruleContext.getConfiguredTargetAndDataMap().get("$cc_toolchain_split")) {
ruleContext.getPrerequisiteConfiguredTargets("$cc_toolchain_split")) {
toolchainsByLibDir.put(getLibDirName(toolchainDep), toolchainDep);
}

Expand All @@ -82,8 +82,7 @@ public static NativeLibs fromLinkedNativeDeps(
Multimap<String, ConfiguredTargetAndData> depsByLibDir =
MultimapBuilder.treeKeys().arrayListValues().build();
for (String depsAttr : depsAttributes) {
for (ConfiguredTargetAndData dep :
ruleContext.getConfiguredTargetAndDataMap().get(depsAttr)) {
for (ConfiguredTargetAndData dep : ruleContext.getPrerequisiteConfiguredTargets(depsAttr)) {
depsByLibDir.put(getLibDirName(dep), dep);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,9 @@ public synchronized void handle(Event e) {
*/
protected List<? extends TransitiveInfoCollection> getPrerequisites(
ConfiguredTarget target, String attributeName) throws Exception {
return getRuleContext(target).getConfiguredTargetMap().get(attributeName);
return Lists.transform(
getRuleContext(target).getPrerequisiteConfiguredTargets(attributeName),
ConfiguredTargetAndData::getConfiguredTarget);
}

/**
Expand Down

0 comments on commit da41c72

Please sign in to comment.