Use FeatureConfiguration to compute value of CC_FLAGS make variable

This cl introduces new action_config type for Crosstool named 'generic'. This
can be used to set the value of CC_FLAGS make variable using much more
expressive mechanism (action_configs + features) than previous make_variable
Crosstool messages. This has been requested by the C++ LPT.

However, as FeatureConfiguration needs RuleContext, CC_FLAGS cannot be
computed using configuration only anymore. Also, FeatureConfiguration is C++
rules specific class, and Bazel build-base cannot depend on it. Therefore we
cannot use FeatureConfiguration for ExtraActions, for example. Because it cannot
be made perfect, this cl is not updating all the possible places that expand
make variables but limits the scope to:

* genrule (the only widely used rule that often expands make variables)
* *_test (CC_FLAGS is used by Tensorflow in the 'args' attribute)
* cc_rules (people sometimes set 'copts' to something like:
  "-DCC_STRING=\\\"$(CC)\\\" -DCC_FLAGS_STRING=\"$(CC_FLAGS)\""

The long term plan is to use Skylark C++ API to get C++ command lines, so
CC_FLAGS together with this inconsistent solution will be removed.

RELNOTES: CC_FLAGS can be defined using 'generic' action_config in CROSSTOOL
PiperOrigin-RevId: 157202883

src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java

@@ -17,23 +17,23 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.MakeVariableExpander.ExpansionException;
+import com.google.devtools.build.lib.analysis.MakeVariableSupplier.MapBackedMakeVariableSupplier;
+import com.google.devtools.build.lib.analysis.MakeVariableSupplier.PackageBackedMakeVariableSupplier;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.syntax.SkylarkDict;
+import com.google.devtools.build.lib.util.Preconditions;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
 /**
- * Implements make variable expansion for make variables that depend on the
- * configuration and the target (not on behavior of the
- * {@link ConfiguredTarget} implementation)
+ * Implements make variable expansion for make variables that depend on the configuration and the
+ * target (not on behavior of the {@link ConfiguredTarget} implementation). Retrieved Make variable
+ * value can be modified using {@link MakeVariableSupplier}
  */
 public class ConfigurationMakeVariableContext implements MakeVariableExpander.Context {
-  private final Package pkg;
-  private final ImmutableMap<String, String> ruleEnv;
-  private final ImmutableMap<String, String> commandLineEnv;
-  private final ImmutableMap<String, String> globalEnv;
-  private final String platform;
+
+  private final ImmutableList<? extends MakeVariableSupplier> allMakeVariableSuppliers;
 
   // TODO(b/37567440): Remove when Skylark callers can be updated to get this from
   // CcToolchainProvider. We should use CcCommon.CC_TOOLCHAIN_ATTRIBUTE_NAME, but we didn't want to
@@ -43,47 +43,65 @@ public class ConfigurationMakeVariableContext implements MakeVariableExpander.Co
 
   public ConfigurationMakeVariableContext(
       RuleContext ruleContext, Package pkg, BuildConfiguration configuration) {
-    this(ruleContext.getMakeVariables(defaultMakeVariableAttributes), pkg, configuration);
+    this(
+        ruleContext.getMakeVariables(defaultMakeVariableAttributes),
+        pkg,
+        configuration,
+        ImmutableList.<MakeVariableSupplier>of());
   }
 
   public ConfigurationMakeVariableContext(
       ImmutableMap<String, String> ruleMakeVariables,
       Package pkg,
       BuildConfiguration configuration) {
-    this.pkg = pkg;
-    this.ruleEnv = ruleMakeVariables;
-    this.commandLineEnv = ImmutableMap.copyOf(configuration.getCommandLineBuildVariables());
-    this.globalEnv = ImmutableMap.copyOf(configuration.getGlobalMakeEnvironment());
-    this.platform = configuration.getPlatformName();
+    this(ruleMakeVariables, pkg, configuration, ImmutableList.<MakeVariableSupplier>of());
+  }
+
+  public ConfigurationMakeVariableContext(
+      RuleContext ruleContext,
+      Package pkg,
+      BuildConfiguration configuration,
+      Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
+    this(
+        ruleContext.getMakeVariables(defaultMakeVariableAttributes),
+        pkg,
+        configuration,
+        makeVariableSuppliers);
+  }
+
+  public ConfigurationMakeVariableContext(
+      ImmutableMap<String, String> ruleMakeVariables,
+      Package pkg,
+      BuildConfiguration configuration,
+      Iterable<? extends MakeVariableSupplier> extraMakeVariableSuppliers) {
+    this.allMakeVariableSuppliers =
+        ImmutableList.<MakeVariableSupplier>builder()
+            .addAll(Preconditions.checkNotNull(extraMakeVariableSuppliers))
+            .add(new MapBackedMakeVariableSupplier(ruleMakeVariables))
+            .add(new MapBackedMakeVariableSupplier(configuration.getCommandLineBuildVariables()))
+            .add(new PackageBackedMakeVariableSupplier(pkg, configuration.getPlatformName()))
+            .add(new MapBackedMakeVariableSupplier(configuration.getGlobalMakeEnvironment()))
+            .build();
   }
 
   @Override
-  public String lookupMakeVariable(String var) throws ExpansionException {
-    String value = ruleEnv.get(var);
-    if (value == null) {
-      value = commandLineEnv.get(var);
-    }
-    if (value == null) {
-      value = pkg.lookupMakeVariable(var, platform);
-    }
-    if (value == null) {
-      value = globalEnv.get(var);
+  public String lookupMakeVariable(String variableName) throws ExpansionException {
+    for (MakeVariableSupplier supplier : allMakeVariableSuppliers) {
+      String variableValue = supplier.getMakeVariable(variableName);
+      if (variableValue != null) {
+        return variableValue;
+      }
     }
-    if (value == null) {
-      throw new MakeVariableExpander.ExpansionException("$(" + var + ") not defined");
-    }
-
-    return value;
+    throw new MakeVariableExpander.ExpansionException("$(" + variableName + ") not defined");
   }
 
   public SkylarkDict<String, String> collectMakeVariables() {
     Map<String, String> map = new LinkedHashMap<>();
     // Collect variables in the reverse order as in lookupMakeVariable
     // because each update is overwriting.
-    map.putAll(pkg.getAllMakeVariables(platform));
-    map.putAll(globalEnv);
-    map.putAll(commandLineEnv);
-    map.putAll(ruleEnv);
+    for (MakeVariableSupplier supplier : allMakeVariableSuppliers.reverse()) {
+      map.putAll(supplier.getAllMakeVariables());
+    }
     return SkylarkDict.<String, String>copyOf(null, map);
   }
 }

src/main/java/com/google/devtools/build/lib/analysis/MakeVariableSupplier.java

@@ -0,0 +1,78 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.analysis;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.util.Preconditions;
+import javax.annotation.Nullable;
+
+/**
+ * Instances of {@link MakeVariableSupplier} passed to {@link ConfigurationMakeVariableContext} will
+ * be called before getting value from {@link ConfigurationMakeVariableContext} itself.
+ */
+public interface MakeVariableSupplier {
+
+  /** Returns Make variable value or null if value is not supplied. */
+  @Nullable
+  String getMakeVariable(String variableName);
+
+  /** Returns all Make variables that it supplies */
+  ImmutableMap<String, String> getAllMakeVariables();
+
+  /** {@link MakeVariableSupplier} that reads variables it supplies from a map. */
+  class MapBackedMakeVariableSupplier implements MakeVariableSupplier {
+
+    private final ImmutableMap<String, String> makeVariables;
+
+    public MapBackedMakeVariableSupplier(ImmutableMap<String, String> makeVariables) {
+      this.makeVariables = Preconditions.checkNotNull(makeVariables);
+    }
+
+    @Nullable
+    @Override
+    public String getMakeVariable(String variableName) {
+      return makeVariables.get(variableName);
+    }
+
+    @Override
+    public ImmutableMap<String, String> getAllMakeVariables() {
+      return makeVariables;
+    }
+  }
+
+  /** {@link MakeVariableSupplier} that reads variables it supplies from a {@link Package} */
+  class PackageBackedMakeVariableSupplier implements MakeVariableSupplier {
+
+    private final String platform;
+    private final Package pkg;
+
+    public PackageBackedMakeVariableSupplier(Package pkg, String platform) {
+      this.pkg = pkg;
+      this.platform = platform;
+    }
+
+    @Nullable
+    @Override
+    public String getMakeVariable(String variableName) {
+      return pkg.lookupMakeVariable(variableName, platform);
+    }
+
+    @Override
+    public ImmutableMap<String, String> getAllMakeVariables() {
+      return pkg.getAllMakeVariables(platform);
+    }
+  }
+}