Allow multiple matching select branches if they resolve to the same v…

…alue
bazelbuild · Feb 20, 2022 · 2f19014 · 2f19014
1 parent ac48e65
commit 2f19014
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java
@@ -215,17 +215,18 @@ private <T> ConfigKeyAndValue<T> resolveSelector(String attributeName, Selector<
       }
     }
 
-    if (matchingConditions.size() > 1) {
+    if (matchingConditions.values().stream().map(s -> s.value).distinct().count() > 1) {
       throw new ValidationException(
           "Illegal ambiguous match on configurable attribute \""
               + attributeName
               + "\" in "
               + getLabel()
               + ":\n"
               + Joiner.on("\n").join(matchingConditions.keySet())
-              + "\nMultiple matches are not allowed unless one is unambiguously more specialized.");
-    } else if (matchingConditions.size() == 1) {
-      return Iterables.getOnlyElement(matchingConditions.values());
+              + "\nMultiple matches are not allowed unless one is unambiguously "
+              + "more specialized or they resolve to the same value.");
+    } else if (matchingConditions.size() > 0) {
+      return Iterables.getFirst(matchingConditions.values(), null);
     }
 
     // If nothing matched, choose the default condition.

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java
@@ -681,6 +681,37 @@ public void multipleMatchesConditionAndSubcondition() throws Exception {
             "bin java/a/libgeneric.jar", "bin java/a/libprecise.jar"));
   }
 
+  /**
+   * Tests that multiple matches are  allowed for conditions where the value is the same.
+   */
+  @Test
+  public void multipleMatchesSameValue() throws Exception {
+    reporter.removeHandler(failFastHandler); // Expect errors.
+    scratch.file("conditions/BUILD",
+            "config_setting(",
+            "    name = 'dup1',",
+            "    values = {'compilation_mode': 'opt'})",
+            "config_setting(",
+            "    name = 'dup2',",
+            "    values = {'define': 'foo=bar'})");
+    scratch.file("a/BUILD",
+            "genrule(",
+            "    name = 'gen',",
+            "    cmd = '',",
+            "    outs = ['gen.out'],",
+            "    srcs = select({",
+            "        '//conditions:dup1': ['a.in'],",
+            "        '//conditions:dup2': ['a.in'],",
+            "        '" + BuildType.Selector.DEFAULT_CONDITION_KEY + "': [':default.in'],",
+            "    }))");
+    checkRule(
+            "//a:gen",
+            "srcs",
+            ImmutableList.of("-c", "opt", "--define", "foo=bar"),
+            /*expected:*/ ImmutableList.of("src a/a.in"),
+            /*not expected:*/ ImmutableList.of("src a/default.in"));
+  }
+
   /**
    * Tests that when multiple conditions match but one condition is more specialized than the
    * others, it is chosen and there is no error.