Check for duplicates labels in multi-select statements after analysis.
Checking for duplicates in labels in the load phase results can reulst in false
positives where there is no duplicate for any particular configuration but the
same file is referenced in multiple select statements.
For label lists without a select or those that only have a single select, the
check remains in the loading phase.
For label lists with multiple selects, the check occurs after configurations
are resolved and only checks for duplicate labels in the configurations used
within a build rather than all potential configurations.
RELNOTES: selects() no longer produce irrelevant duplicate label checks
PiperOrigin-RevId: 470241662
Change-Id: I626518b96a66cc1fd80b8d355dbfd69506c061a9
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 4ce591a..d7fa514 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -1671,8 +1671,24 @@
}
}
+ /**
+ * Same as {@link #build}, except without some attribute checks.
+ *
+ * <p>Don't use this function outside of testing. The use should be limited to cases where
+ * specifying ConfigConditions.EMPTY, which can cause a noMatchError when accessing attributes
+ * within attribute checking.
+ */
+ @VisibleForTesting
+ public RuleContext unsafeBuild() throws InvalidExecGroupException {
+ return build(false);
+ }
+
@VisibleForTesting
public RuleContext build() throws InvalidExecGroupException {
+ return build(true);
+ }
+
+ private RuleContext build(boolean attributeChecks) throws InvalidExecGroupException {
Preconditions.checkNotNull(ruleClassProvider);
Preconditions.checkNotNull(hostConfiguration);
Preconditions.checkNotNull(configurationFragmentPolicy);
@@ -1681,14 +1697,21 @@
Preconditions.checkNotNull(configConditions);
Preconditions.checkNotNull(mutability);
Preconditions.checkNotNull(visibility);
- AttributeMap attributes =
+ ConfiguredAttributeMapper attributes =
ConfiguredAttributeMapper.of(
target.getAssociatedRule(), configConditions.asProviders(), configuration);
- checkAttributesNonEmpty(attributes);
ListMultimap<String, ConfiguredTargetAndData> targetMap = createTargetMap();
+ // These checks can fail in BuildViewForTesting.getRuleContextForTesting as it specifies
+ // ConfigConditions.EMPTY, resulting in noMatchError accessing attributes without a default
+ // condition.
+ if (attributeChecks) {
+ checkAttributesNonEmpty(attributes);
+ checkAttributesForDuplicateLabels(attributes);
+ }
// This conditionally checks visibility on config_setting rules based on
// --config_setting_visibility_policy. This should be removed as soon as it's deemed safe
- // to unconditionally check visibility. See https://github.com/bazelbuild/bazel/issues/12669.
+ // to unconditionally check visibility. See
+ // https://github.com/bazelbuild/bazel/issues/12669.
if (target.getPackage().getConfigSettingVisibilityPolicy()
!= ConfigSettingVisibilityPolicy.LEGACY_OFF) {
Attribute configSettingAttr = attributes.getAttributeDefinition("$config_dependencies");
@@ -1740,6 +1763,20 @@
}
}
+ private void checkAttributesForDuplicateLabels(ConfiguredAttributeMapper attributes) {
+ for (String attributeName : attributes.getAttributeNames()) {
+ Attribute attr = attributes.getAttributeDefinition(attributeName);
+ if (attr.getType() != BuildType.LABEL_LIST) {
+ continue;
+ }
+
+ Set<Label> duplicates = attributes.checkForDuplicateLabels(attr);
+ for (Label label : duplicates) {
+ reporter.attributeError(attr.getName(), String.format("Label '%s' is duplicated", label));
+ }
+ }
+ }
+
@CanIgnoreReturnValue
public Builder setRuleClassProvider(ConfiguredRuleClassProvider ruleClassProvider) {
this.ruleClassProvider = ruleClassProvider;
