Genrules do not override c++ toolchain Make variables using the toolchains attribute.

This fixes an issue where CC_FLAGS was being overwritten.

PiperOrigin-RevId: 181463694

2 files changed, 30 insertions, 16 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
index ec27c3acc0..3d97d2cf53 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.rules.genrule;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
@@ -56,13 +57,22 @@ import java.util.regex.Pattern;
  */
 public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
 
-  private static final Pattern CROSSTOOL_MAKE_VARIABLE =
-      Pattern.compile("\\$\\((CC|CC_FLAGS|AR|NM|OBJCOPY|STRIP|GCOVTOOL)\\)");
-  private static final Pattern JDK_MAKE_VARIABLE =
-      Pattern.compile("\\$\\((JAVABASE|JAVA)\\)");
+  private static final ImmutableList<String> CROSSTOOL_MAKE_VARIABLES = ImmutableList.of("CC",
+      "CC_FLAGS", "AR", "NM", "OBJCOPY", "STRIP", "GCOVTOOL");
+
+  private static final ImmutableList<String> JDK_MAKE_VARIABLES = ImmutableList.of("JAVABASE",
+      "JAVA");
+
+  private static Pattern matchesMakeVariables(Iterable<String> variables) {
+    return Pattern.compile("\\$\\((" + Joiner.on("|").join(variables) + ")\\)");
+  }
+
+  private static final Pattern CROSSTOOL_MAKE_VARIABLE_PATTERN =
+      matchesMakeVariables(CROSSTOOL_MAKE_VARIABLES);
+  private static final Pattern JDK_MAKE_VARIABLE = matchesMakeVariables(JDK_MAKE_VARIABLES);
 
   protected static boolean requiresCrosstool(String command) {
-    return CROSSTOOL_MAKE_VARIABLE.matcher(command).find();
+    return CROSSTOOL_MAKE_VARIABLE_PATTERN.matcher(command).find();
   }
 
   protected boolean requiresJdk(String command) {
@@ -341,9 +351,13 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
         }
       }
 
-      String valueFromToolchains = resolveVariableFromToolchains(variableName);
-      if (valueFromToolchains != null) {
-        return valueFromToolchains;
+      // Make variables provided by the :cc_toolchain attributes should not be overridden by
+      // those provided by the toolchains attribute.
+      if (!CROSSTOOL_MAKE_VARIABLES.contains(variableName)) {
+        String valueFromToolchains = resolveVariableFromToolchains(variableName);
+        if (valueFromToolchains != null) {
+          return valueFromToolchains;
+        }
       }
 
       return super.lookupVariable(variableName);
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
index e5d1e88dc0..c4a040069c 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
@@ -104,23 +104,23 @@ public class GenRuleConfiguredTargetTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testToolchainMakeVariableExpansion() throws Exception {
+  public void testToolchainOverridesJavabase() throws Exception {
     scratch.file("a/BUILD",
-        "genrule(name='gr', srcs=[], outs=['out'], cmd='$(FOO)', toolchains=[':v'])",
-        "make_variable_tester(name='v', variables={'FOO': 'FOOBAR'})");
+        "genrule(name='gr', srcs=[], outs=['out'], cmd='JAVABASE=$(JAVABASE)', toolchains=[':v'])",
+        "make_variable_tester(name='v', variables={'JAVABASE': 'REPLACED'})");
 
     String cmd = getCommand("//a:gr");
-    assertThat(cmd).endsWith("FOOBAR");
+    assertThat(cmd).endsWith("JAVABASE=REPLACED");
   }
 
   @Test
-  public void testToolchainOverridesConfiguration() throws Exception {
+  public void testToolchainDoesNotOverrideCcFlags() throws Exception {
     scratch.file("a/BUILD",
-        "genrule(name='gr', srcs=[], outs=['out'], cmd='JAVABASE=$(JAVABASE)', toolchains=[':v'])",
-        "make_variable_tester(name='v', variables={'JAVABASE': 'REPLACED'})");
+        "genrule(name='gr', srcs=[], outs=['out'], cmd='CC_FLAGS=$(CC_FLAGS)', toolchains=[':v'])",
+        "make_variable_tester(name='v', variables={'CC_FLAGS': 'REPLACED'})");
 
     String cmd = getCommand("//a:gr");
-    assertThat(cmd).endsWith("JAVABASE=REPLACED");
+    assertThat(cmd).doesNotContain("CC_FLAGS=REPLACED");
   }
 
   @Test