Skip to content

Commit 137d3f1

Browse files
mai93copybara-github
mai93
authored and
copybara-github
committed
Simplify prerequisites query functions in RuleContext
- keep only one function to get split prerequisites that returns `ConfiguredTargetAndData` and modify users to get `ConfiguredTarget` from it. - remove `getPrerequisiteMap()` PiperOrigin-RevId: 573886458 Change-Id: Ic4dbf5add0ccebd39e4c00492c4fd51d283d8f09
1 parent cadbaa5 commit 137d3f1

File tree

6 files changed

+58
-56
lines changed

6 files changed

+58
-56
lines changed

‎src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java‎

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import com.google.common.collect.Iterables;
3434
import com.google.common.collect.ListMultimap;
3535
import com.google.common.collect.Lists;
36-
import com.google.common.collect.Maps;
3736
import com.google.common.collect.Multimaps;
3837
import com.google.common.collect.Streams;
3938
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -795,44 +794,12 @@ public boolean isAttrDefined(String attrName, Type<?> type) {
795794
return attributes().has(attrName, type);
796795
}
797796

798-
/**
799-
* Returns the dependencies through a {@code LABEL_DICT_UNARY} attribute as a map from a string to
800-
* a {@link TransitiveInfoCollection}.
801-
*/
802-
public Map<String, TransitiveInfoCollection> getPrerequisiteMap(String attributeName) {
803-
Preconditions.checkState(attributes().has(attributeName, BuildType.LABEL_DICT_UNARY));
804-
805-
ImmutableMap.Builder<String, TransitiveInfoCollection> result = ImmutableMap.builder();
806-
Map<String, Label> dict = attributes().get(attributeName, BuildType.LABEL_DICT_UNARY);
807-
Map<Label, ConfiguredTarget> labelToDep = new HashMap<>();
808-
for (ConfiguredTargetAndData dep : targetMap.get(attributeName)) {
809-
labelToDep.put(dep.getTargetLabel(), dep.getConfiguredTarget());
810-
}
811-
812-
for (Map.Entry<String, Label> entry : dict.entrySet()) {
813-
result.put(entry.getKey(), Preconditions.checkNotNull(labelToDep.get(entry.getValue())));
814-
}
815-
816-
return result.buildOrThrow();
817-
}
818-
819-
/**
820-
* Returns the prerequisites keyed by their configuration transition keys. If the split transition
821-
* is not active (e.g. split() returned an empty list), the key is an empty Optional.
822-
*/
823-
public Map<Optional<String>, ? extends List<? extends TransitiveInfoCollection>>
824-
getSplitPrerequisites(String attributeName) {
825-
return Maps.transformValues(
826-
getSplitPrerequisiteConfiguredTargetAndTargets(attributeName),
827-
(ctatList) -> Lists.transform(ctatList, ConfiguredTargetAndData::getConfiguredTarget));
828-
}
829-
830797
/**
831798
* Returns the prerequisites keyed by their transition keys. If the split transition is not active
832799
* (e.g. split() returned an empty list), the key is an empty Optional.
833800
*/
834-
public Map<Optional<String>, List<ConfiguredTargetAndData>>
835-
getSplitPrerequisiteConfiguredTargetAndTargets(String attributeName) {
801+
public Map<Optional<String>, List<ConfiguredTargetAndData>> getSplitPrerequisites(
802+
String attributeName) {
836803
checkAttributeIsDependency(attributeName);
837804
// Use an ImmutableListMultimap.Builder here to preserve ordering.
838805
ImmutableListMultimap.Builder<Optional<String>, ConfiguredTargetAndData> result =
@@ -939,7 +906,7 @@ && attributes().getAttributeDefinition(attributeName).getTransitionFactory().isS
939906
// portion of the split transition.
940907
// Callers should be identified, cleaned up, and this check removed.
941908
Map<Optional<String>, List<ConfiguredTargetAndData>> map =
942-
getSplitPrerequisiteConfiguredTargetAndTargets(attributeName);
909+
getSplitPrerequisites(attributeName);
943910
prerequisiteConfiguredTargets =
944911
map.isEmpty() ? ImmutableList.of() : map.entrySet().iterator().next().getValue();
945912
} else {

‎src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.analysis.starlark;
1616

17+
import static com.google.common.collect.ImmutableList.toImmutableList;
1718
import static com.google.common.collect.ImmutableSet.toImmutableSet;
1819
import static com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition.PATCH_TRANSITION_KEY;
1920
import static com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions.ALLOWLIST_EXTEND_RULE;
@@ -78,6 +79,7 @@
7879
import com.google.devtools.build.lib.packages.Type.LabelClass;
7980
import com.google.devtools.build.lib.shell.ShellUtils;
8081
import com.google.devtools.build.lib.shell.ShellUtils.TokenizationException;
82+
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
8183
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkRuleContextApi;
8284
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkSubruleApi;
8385
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
@@ -501,12 +503,12 @@ private static StructImpl buildSplitAttributeInfo(
501503
if (!attr.getTransitionFactory().isSplit()) {
502504
continue;
503505
}
504-
Map<Optional<String>, ? extends List<? extends TransitiveInfoCollection>> splitPrereqs =
506+
Map<Optional<String>, List<ConfiguredTargetAndData>> splitPrereqs =
505507
ruleContext.getSplitPrerequisites(attr.getName());
506508

507509
Map<Object, Object> splitPrereqsMap = new LinkedHashMap<>();
508-
for (Map.Entry<Optional<String>, ? extends List<? extends TransitiveInfoCollection>>
509-
splitPrereq : splitPrereqs.entrySet()) {
510+
for (Map.Entry<Optional<String>, List<ConfiguredTargetAndData>> splitPrereq :
511+
splitPrereqs.entrySet()) {
510512

511513
// Skip a split with an empty dependency list.
512514
// TODO(jungjw): Figure out exactly which cases trigger this and see if this can be made
@@ -518,10 +520,14 @@ private static StructImpl buildSplitAttributeInfo(
518520
Object value;
519521
if (attr.getType() == BuildType.LABEL) {
520522
Preconditions.checkState(splitPrereq.getValue().size() == 1);
521-
value = splitPrereq.getValue().get(0);
523+
value = splitPrereq.getValue().get(0).getConfiguredTarget();
522524
} else {
523525
// BuildType.LABEL_LIST
524-
value = StarlarkList.immutableCopyOf(splitPrereq.getValue());
526+
value =
527+
StarlarkList.immutableCopyOf(
528+
splitPrereq.getValue().stream()
529+
.map(ConfiguredTargetAndData::getConfiguredTarget)
530+
.collect(toImmutableList()));
525531
}
526532

527533
if (splitPrereq.getKey().isPresent()

‎src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import com.google.auto.value.AutoValue;
2525
import com.google.common.base.Function;
2626
import com.google.common.base.Functions;
27-
import com.google.common.base.Optional;
2827
import com.google.common.base.Preconditions;
2928
import com.google.common.base.Predicates;
3029
import com.google.common.collect.ImmutableList;
@@ -51,7 +50,6 @@
5150
import com.google.devtools.build.lib.analysis.RuleContext;
5251
import com.google.devtools.build.lib.analysis.Runfiles;
5352
import com.google.devtools.build.lib.analysis.RunfilesProvider;
54-
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
5553
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
5654
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
5755
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
@@ -88,6 +86,7 @@
8886
import com.google.devtools.build.lib.rules.java.OneVersionCheckActionBuilder;
8987
import com.google.devtools.build.lib.rules.java.ProguardSpecProvider;
9088
import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code;
89+
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
9190
import com.google.devtools.build.lib.vfs.PathFragment;
9291
import java.io.Serializable;
9392
import java.util.ArrayList;
@@ -970,10 +969,10 @@ public static RuleConfiguredTargetBuilder createAndroidBinary(
970969
attr -> !"deps".equals(attr),
971970
validations -> builder.addOutputGroup(OutputGroupInfo.VALIDATION_TRANSITIVE, validations));
972971
boolean filterSplitValidations = false; // propagate validations from first split unfiltered
973-
for (List<? extends TransitiveInfoCollection> deps :
974-
ruleContext.getSplitPrerequisites("deps").values()) {
972+
for (List<ConfiguredTargetAndData> deps : ruleContext.getSplitPrerequisites("deps").values()) {
975973
for (OutputGroupInfo provider :
976-
AnalysisUtils.getProviders(deps, OutputGroupInfo.STARLARK_CONSTRUCTOR)) {
974+
AnalysisUtils.getProviders(
975+
getConfiguredTargets(deps), OutputGroupInfo.STARLARK_CONSTRUCTOR)) {
977976
NestedSet<Artifact> validations = provider.getOutputGroup(OutputGroupInfo.VALIDATION);
978977
if (filterSplitValidations) {
979978
// Filter out Android Lint validations by name: we know these validations are expensive
@@ -1038,16 +1037,22 @@ public static NestedSet<Artifact> getTransitiveNativeLibs(RuleContext ruleContex
10381037
// libraries across multiple architectures, e.g. x86 and armeabi-v7a, and need to be packed
10391038
// into the APK.
10401039
NestedSetBuilder<Artifact> transitiveNativeLibs = NestedSetBuilder.naiveLinkOrder();
1041-
for (Map.Entry<Optional<String>, ? extends List<? extends TransitiveInfoCollection>> entry :
1042-
ruleContext.getSplitPrerequisites("deps").entrySet()) {
1040+
for (List<ConfiguredTargetAndData> deps : ruleContext.getSplitPrerequisites("deps").values()) {
10431041
for (AndroidNativeLibsInfo provider :
1044-
AnalysisUtils.getProviders(entry.getValue(), AndroidNativeLibsInfo.PROVIDER)) {
1042+
AnalysisUtils.getProviders(getConfiguredTargets(deps), AndroidNativeLibsInfo.PROVIDER)) {
10451043
transitiveNativeLibs.addTransitive(provider.getNativeLibs());
10461044
}
10471045
}
10481046
return transitiveNativeLibs.build();
10491047
}
10501048

1049+
private static ImmutableList<ConfiguredTarget> getConfiguredTargets(
1050+
List<ConfiguredTargetAndData> prerequisitesList) {
1051+
return prerequisitesList.stream()
1052+
.map(ConfiguredTargetAndData::getConfiguredTarget)
1053+
.collect(toImmutableList());
1054+
}
1055+
10511056
static class Java8LegacyDexOutput {
10521057
private final Artifact dex;
10531058
private final Artifact map;

‎src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuite.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.rules.cpp;
1515

16+
import static com.google.common.collect.ImmutableMap.toImmutableMap;
17+
18+
import com.google.common.base.Preconditions;
1619
import com.google.common.base.Strings;
1720
import com.google.common.collect.ImmutableList;
1821
import com.google.common.collect.ImmutableMap;
@@ -43,6 +46,7 @@
4346
* com.google.devtools.build.lib.rules.cpp.CppConfiguration}.
4447
*/
4548
public class CcToolchainSuite implements RuleConfiguredTargetFactory {
49+
private static final String TOOLCHAIN_ATTRIBUTE_NAME = "toolchains";
4650

4751
private static TemplateVariableInfo createMakeVariableProvider(
4852
CcToolchainProvider toolchainProvider, Location location) {
@@ -68,7 +72,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
6872
String compiler = cppConfiguration.getCompilerFromOptions();
6973
String key = transformedCpu + (Strings.isNullOrEmpty(compiler) ? "" : ("|" + compiler));
7074
Map<String, Label> toolchains =
71-
ruleContext.attributes().get("toolchains", BuildType.LABEL_DICT_UNARY);
75+
ruleContext.attributes().get(TOOLCHAIN_ATTRIBUTE_NAME, BuildType.LABEL_DICT_UNARY);
7276
Label selectedCcToolchain = toolchains.get(key);
7377
CcToolchainProvider ccToolchainProvider;
7478

@@ -134,6 +138,28 @@ public ConfiguredTarget create(RuleContext ruleContext)
134138
return builder.build();
135139
}
136140

141+
/**
142+
* Returns the toolchains defined through a {@code LABEL_DICT_UNARY} attribute as a map from a
143+
* string to a {@link TransitiveInfoCollection}.
144+
*/
145+
private ImmutableMap<String, TransitiveInfoCollection> getToolchainsMap(RuleContext ruleContext) {
146+
Preconditions.checkState(
147+
ruleContext.attributes().has(TOOLCHAIN_ATTRIBUTE_NAME, BuildType.LABEL_DICT_UNARY));
148+
149+
ImmutableMap.Builder<String, TransitiveInfoCollection> result = ImmutableMap.builder();
150+
Map<String, Label> dict =
151+
ruleContext.attributes().get(TOOLCHAIN_ATTRIBUTE_NAME, BuildType.LABEL_DICT_UNARY);
152+
ImmutableMap<Label, ConfiguredTarget> labelToDep =
153+
ruleContext.getPrerequisiteConfiguredTargets(TOOLCHAIN_ATTRIBUTE_NAME).stream()
154+
.collect(toImmutableMap(dep -> dep.getTargetLabel(), dep -> dep.getConfiguredTarget()));
155+
156+
for (Map.Entry<String, Label> entry : dict.entrySet()) {
157+
result.put(entry.getKey(), Preconditions.checkNotNull(labelToDep.get(entry.getValue())));
158+
}
159+
160+
return result.buildOrThrow();
161+
}
162+
137163
private <T extends HasCcToolchainLabel> T selectCcToolchain(
138164
BuiltinProvider<T> providerType,
139165
RuleContext ruleContext,
@@ -142,7 +168,7 @@ private <T extends HasCcToolchainLabel> T selectCcToolchain(
142168
Label selectedCcToolchain)
143169
throws RuleErrorException {
144170
T selectedAttributes = null;
145-
for (TransitiveInfoCollection dep : ruleContext.getPrerequisiteMap("toolchains").values()) {
171+
for (TransitiveInfoCollection dep : getToolchainsMap(ruleContext).values()) {
146172
T attributes = dep.get(providerType);
147173
if (attributes != null && attributes.getCcToolchainLabel().equals(selectedCcToolchain)) {
148174
selectedAttributes = attributes;

‎src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,9 @@ public static AppleLinkingOutputs linkMultiArchBinary(
7575
UserVariablesExtension userVariablesExtension)
7676
throws InterruptedException, RuleErrorException, EvalException {
7777
Map<Optional<String>, List<ConfiguredTargetAndData>> splitDeps =
78-
ruleContext.getSplitPrerequisiteConfiguredTargetAndTargets("deps");
78+
ruleContext.getSplitPrerequisites("deps");
7979
Map<Optional<String>, List<ConfiguredTargetAndData>> splitToolchains =
80-
ruleContext.getSplitPrerequisiteConfiguredTargetAndTargets(
81-
ObjcRuleClasses.CHILD_CONFIG_ATTR);
80+
ruleContext.getSplitPrerequisites(ObjcRuleClasses.CHILD_CONFIG_ATTR);
8281

8382
Preconditions.checkState(
8483
splitDeps.keySet().isEmpty() || splitDeps.keySet().equals(splitToolchains.keySet()),

‎src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,8 +335,7 @@ public StructImpl linkMultiArchStaticLibrary(
335335
ruleContext.getStarlarkDefinedBuiltin("link_multi_arch_static_library");
336336
Dict<String, StructImpl> splitTargetTriplets =
337337
MultiArchBinarySupport.getSplitTargetTripletFromCtads(
338-
ruleContext.getSplitPrerequisiteConfiguredTargetAndTargets(
339-
ObjcRuleClasses.CHILD_CONFIG_ATTR));
338+
ruleContext.getSplitPrerequisites(ObjcRuleClasses.CHILD_CONFIG_ATTR));
340339
return (StructImpl)
341340
ruleContext.callStarlarkOrThrowRuleError(
342341
linkMultiArchLibrary,

0 commit comments

Comments
 (0)