bazel package: consolidate all calls to rule.setAttributeValue
This change refactors the logic for setting test_suite.$implicit_tests
so that all calls to rule.setAttributeValue occur within
populateRuleAttributeValues
A follow-up will optimize AttributeContainer for space;
see prior prep in CL 317189967.
PiperOrigin-RevId: 318895312
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 646d068..0e77871 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -892,6 +892,11 @@
return generatorNameByLocation;
}
+ // Value of '$implicit_tests' attribute shared by all test_suite rules in the
+ // package that don't specify an explicit 'tests' attribute value.
+ // It contains the label of each non-manual test in the package, in label order.
+ final List<Label> testSuiteImplicitTests = new ArrayList<>();
+
@ThreadCompatible
private static class ThreadCompatibleInterner<T> implements Interner<T> {
private final Map<T, T> interns = new HashMap<>();
@@ -1497,7 +1502,7 @@
// current instance here.
buildFile = (InputFile) Preconditions.checkNotNull(targets.get(buildFileLabel.getName()));
- List<Label> labelsOfTestTargets = new ArrayList<>();
+ List<Label> tests = new ArrayList<>();
List<Rule> implicitTestSuiteRuleInstances = new ArrayList<>();
Map<Label, InputFile> newInputFiles = new HashMap<>();
for (final Rule rule : getTargets(Rule.class)) {
@@ -1519,32 +1524,19 @@
// since clearly this information isn't available at Rule construction
// time, as forward references are permitted.
if (TargetUtils.isTestRule(rule) && !TargetUtils.hasManualTag(rule)) {
- labelsOfTestTargets.add(rule.getLabel());
- }
-
- AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
- if (rule.getRuleClass().equals("test_suite")
- && attributes.get("tests", BuildType.LABEL_LIST).isEmpty()) {
- implicitTestSuiteRuleInstances.add(rule);
+ // Update the testSuiteImplicitTests list shared
+ // by all test_suite.$implicit_test attributes.
+ tests.add(rule.getLabel());
}
}
+ Collections.sort(tests); // (for determinism)
+ this.testSuiteImplicitTests.addAll(tests);
+
for (InputFile inputFile : newInputFiles.values()) {
addInputFile(inputFile);
}
- if (!implicitTestSuiteRuleInstances.isEmpty()) {
- Collections.sort(labelsOfTestTargets);
- for (Rule rule : implicitTestSuiteRuleInstances) {
- // Pretend the test_suite.$implicit_tests attribute
- // (which is synthesized during package loading)
- // is explicitly set so that it appears in query output.
- rule.setAttributeValue(
- rule.getRuleClassObject().getAttributeByName("$implicit_tests"),
- labelsOfTestTargets,
- /*explicit=*/ true);
- }
- }
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index aad0239..be7a417 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -2235,6 +2235,17 @@
}
}
+ // An instance of the built-in 'test_suite' rule with no explicit 'tests'
+ // attribute gets an '$implicit_tests' attribute, whose value is a
+ // shared per-package list of all test labels, populated later.
+ if (this.name.equals("test_suite")) {
+ Attribute implicitTests = this.getAttributeByName("$implicit_tests");
+ if (implicitTests != null && !definedAttrIndices.get(this.getAttributeIndex("tests"))) {
+ boolean explicit = true; // so that it appears in query output
+ rule.setAttributeValue(implicitTests, pkgBuilder.testSuiteImplicitTests, explicit);
+ }
+ }
+
// Set computed default attribute values now that all other (i.e. non-computed) default values
// have been set.
for (Attribute attr : attrsWithComputedDefaults) {
