Automated g4 rollback of commit ce33ab7a49126a513d7d5a6bc16f86154d9a85b6.

PiperOrigin-RevId: 154931201

8 files changed, 47 insertions, 122 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java
index 4d3b3b6be0..aa795bbea2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java
@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.analysis;
 
-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.config.BuildConfiguration;
@@ -30,35 +29,27 @@ import java.util.Map;
  */
 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 Map<String, String> toolchainEnv;
+  private final Map<String, String> commandLineEnv;
+  private final Map<String, String> globalEnv;
   private final String platform;
 
-  // TODO(b/37567440): Remove when Skylark callers can be updated to get this from
-  // CcToolchainProvider.
-  private static final ImmutableList<String> defaultMakeVariableAttributes =
-      ImmutableList.of(":cc_toolchain");
-
-  public ConfigurationMakeVariableContext(
-      RuleContext ruleContext, Package pkg, BuildConfiguration configuration) {
-    this(ruleContext.getMakeVariables(defaultMakeVariableAttributes), pkg, configuration);
+  public ConfigurationMakeVariableContext(Package pkg, BuildConfiguration configuration) {
+    this(pkg, configuration, ImmutableMap.<String, String>of());
   }
 
-  public ConfigurationMakeVariableContext(
-      ImmutableMap<String, String> ruleMakeVariables,
-      Package pkg,
-      BuildConfiguration configuration) {
+  public ConfigurationMakeVariableContext(Package pkg, BuildConfiguration configuration,
+      ImmutableMap<String, String> toolchainEnv) {
     this.pkg = pkg;
-    this.ruleEnv = ruleMakeVariables;
-    this.commandLineEnv = ImmutableMap.copyOf(configuration.getCommandLineBuildVariables());
-    this.globalEnv = ImmutableMap.copyOf(configuration.getGlobalMakeEnvironment());
-    this.platform = configuration.getPlatformName();
+    this.toolchainEnv = toolchainEnv;
+    commandLineEnv = configuration.getCommandLineBuildVariables();
+    globalEnv = configuration.getGlobalMakeEnvironment();
+    platform = configuration.getPlatformName();
   }
 
   @Override
   public String lookupMakeVariable(String var) throws ExpansionException {
-    String value = ruleEnv.get(var);
+    String value = toolchainEnv.get(var);
     if (value == null) {
       value = commandLineEnv.get(var);
     }
@@ -82,7 +73,6 @@ public class ConfigurationMakeVariableContext implements MakeVariableExpander.Co
     map.putAll(pkg.getAllMakeVariables(platform));
     map.putAll(globalEnv);
     map.putAll(commandLineEnv);
-    map.putAll(ruleEnv);
     return SkylarkDict.<String, String>copyOf(null, map);
   }
 }
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 a6c43d4689..13ed25c1db 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
@@ -77,7 +77,6 @@ import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.AliasProvider;
-import com.google.devtools.build.lib.rules.MakeVariableProvider;
 import com.google.devtools.build.lib.rules.fileset.FilesetProvider;
 import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.syntax.ClassObject;
@@ -1018,30 +1017,6 @@ public final class RuleContext extends TargetContext
     }
   }
 
-  public ImmutableMap<String, String> getMakeVariables(Iterable<String> attributeNames) {
-    // Using an ImmutableBuilder to complain about duplicate keys. This traversal order of
-    // getPrerequisites isn't well-defined, so this makes sure providers don't seceretly stomp on
-    // each other.
-    ImmutableMap.Builder<String, String> makeVariableBuilder = ImmutableMap.builder();
-    ImmutableSet.Builder<MakeVariableProvider> makeVariableProvidersBuilder =
-        ImmutableSet.builder();
-
-    for (String attributeName : attributeNames) {
-      // TODO(b/37567440): Remove this continue statement.
-      if (!attributes().has(attributeName)) {
-        continue;
-      }
-      makeVariableProvidersBuilder.addAll(
-          getPrerequisites(attributeName, Mode.TARGET, MakeVariableProvider.class));
-    }
-
-    for (MakeVariableProvider makeVariableProvider : makeVariableProvidersBuilder.build()) {
-      makeVariableBuilder.putAll(makeVariableProvider.getMakeVariables());
-    }
-
-    return makeVariableBuilder.build();
-  }
-
   /**
    * Return a context that maps Make variable names (string) to values (string).
    *
@@ -1049,8 +1024,8 @@ public final class RuleContext extends TargetContext
    **/
   public ConfigurationMakeVariableContext getConfigurationMakeVariableContext() {
     if (configurationMakeVariableContext == null) {
-      configurationMakeVariableContext =
-          new ConfigurationMakeVariableContext(this, getRule().getPackage(), getConfiguration());
+      configurationMakeVariableContext = new ConfigurationMakeVariableContext(
+          getRule().getPackage(), getConfiguration());
     }
     return configurationMakeVariableContext;
   }
@@ -1113,8 +1088,8 @@ public final class RuleContext extends TargetContext
    */
   public String expandSingleMakeVariable(String attrName, String expression) {
     try {
-      return MakeVariableExpander.expandSingleVariable(
-          expression, getConfigurationMakeVariableContext());
+      return MakeVariableExpander.expandSingleVariable(expression,
+          new ConfigurationMakeVariableContext(getRule().getPackage(), getConfiguration()));
     } catch (MakeVariableExpander.ExpansionException e) {
       attributeError(attrName, e.getMessage());
       return expression;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/MakeVariableProvider.java b/src/main/java/com/google/devtools/build/lib/rules/MakeVariableProvider.java
index 15d4998c7f..1c56c20bfb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/MakeVariableProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/MakeVariableProvider.java
@@ -15,37 +15,34 @@
 package com.google.devtools.build.lib.rules;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.packages.ClassObjectConstructor;
-import com.google.devtools.build.lib.packages.NativeClassObjectConstructor;
-import com.google.devtools.build.lib.packages.SkylarkClassObject;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import java.util.TreeMap;
 
 /** Provides access to make variables from the current fragments. */
-@SkylarkModule(name = "MakeVariables", doc = "Make variables exposed by the current target.")
 @Immutable
-public final class MakeVariableProvider extends SkylarkClassObject
-    implements TransitiveInfoProvider {
-  public static final String SKYLARK_NAME = "MakeVariableInfo";
-
-  public static final ClassObjectConstructor SKYLARK_CONSTRUCTOR =
-      new NativeClassObjectConstructor(SKYLARK_NAME) {};
-
+public final class MakeVariableProvider implements TransitiveInfoProvider {
   private final ImmutableMap<String, String> makeVariables;
 
   public MakeVariableProvider(ImmutableMap<String, String> makeVariables) {
-    super(SKYLARK_CONSTRUCTOR, ImmutableMap.<String, Object>of());
     this.makeVariables = makeVariables;
   }
 
-  @SkylarkCallable(
-    name = "make_variables",
-    doc = "Returns the make variables defined by this target.",
-    structField = true
-  )
   public ImmutableMap<String, String> getMakeVariables() {
     return makeVariables;
   }
+
+  public static ImmutableMap<String, String> getToolchainMakeVariables(
+      RuleContext ruleContext, String attributeName) {
+    // Cannot be an ImmutableMap.Builder because we want to support duplicate keys
+    TreeMap<String, String> result = new TreeMap<>();
+    for (MakeVariableProvider provider :
+        ruleContext.getPrerequisites(attributeName, Mode.TARGET, MakeVariableProvider.class)) {
+      result.putAll(provider.getMakeVariables());
+    }
+
+    return ImmutableMap.copyOf(result);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
index 7ab8097ed9..897d77ac01 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
@@ -986,11 +986,9 @@ public final class SkylarkRuleContext implements SkylarkValue {
   public String expandMakeVariables(String attributeName, String command,
       final Map<String, String> additionalSubstitutions) throws EvalException {
     checkMutable("expand_make_variables");
-    return ruleContext.expandMakeVariables(
-        attributeName,
-        command,
-        new ConfigurationMakeVariableContext(
-            ruleContext, ruleContext.getRule().getPackage(), ruleContext.getConfiguration()) {
+    return ruleContext.expandMakeVariables(attributeName,
+        command, new ConfigurationMakeVariableContext(ruleContext.getRule().getPackage(),
+            ruleContext.getConfiguration()) {
           @Override
           public String lookupMakeVariable(String name) throws ExpansionException {
             if (additionalSubstitutions.containsKey(name)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
index 14da7d1b21..69924f7da1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
@@ -41,7 +41,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.License;
-import com.google.devtools.build.lib.rules.MakeVariableProvider;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException;
 import com.google.devtools.build.lib.util.Pair;
@@ -53,7 +52,6 @@ import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -284,7 +282,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
       coverage = crosstool;
     }
 
-    CcToolchainProvider ccProvider =
+    CcToolchainProvider provider =
         new CcToolchainProvider(
             cppConfiguration,
             crosstool,
@@ -311,19 +309,10 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
             ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST),
             getEnvironment(ruleContext),
             cppConfiguration.getBuiltInIncludeDirectories());
-
-    // TODO(kmensah): Remove sysroot from cppConfiguration and calculate it here.
-    PathFragment sysroot = cppConfiguration.getSysroot();
-
-    MakeVariableProvider makeVariableProvider =
-        createMakeVariableProvider(cppConfiguration, sysroot);
-
     RuleConfiguredTargetBuilder builder =
         new RuleConfiguredTargetBuilder(ruleContext)
-            .addProvider(ccProvider)
-            .addNativeDeclaredProvider(ccProvider)
-            .addProvider(makeVariableProvider)
-            .addNativeDeclaredProvider(makeVariableProvider)
+            .addProvider(provider)
+            .addNativeDeclaredProvider(provider)
             .addProvider(
                 fdoSupport.getFdoSupport().createFdoSupportProvider(ruleContext, profileArtifact))
             .setFilesToBuild(new NestedSetBuilder<Artifact>(Order.STABLE_ORDER).build())
@@ -444,22 +433,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
         : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
   }
 
-  private MakeVariableProvider createMakeVariableProvider(
-      CppConfiguration cppConfiguration, PathFragment sysroot) {
-
-    HashMap<String, String> makeVariables =
-        new HashMap<>(cppConfiguration.getAdditionalMakeVariables());
-
-    // Overwrite the CC_FLAGS variable to include sysroot, if it's available.
-    if (sysroot != null) {
-      String sysrootFlag = "--sysroot=" + sysroot;
-      String ccFlags = makeVariables.get("CC_FLAGS");
-      ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag;
-      makeVariables.put("CC_FLAGS", ccFlags);
-    }
-    return new MakeVariableProvider(ImmutableMap.copyOf(makeVariables));
-  }
-
   /**
    * Returns a map that should be templated into the crosstool as build variables. Is meant to
    * be overridden by subclasses of CcToolchain.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 04be2553e1..96df247dd2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -725,7 +725,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     for (CrosstoolConfig.MakeVariable variable : toolchain.getMakeVariableList()) {
       makeVariablesBuilder.put(variable.getName(), variable.getValue());
     }
-    // TODO(kmensah): Remove once targets can depend on the cc_toolchain in skylark.
     if (sysrootFlag != null) {
       String ccFlags = makeVariablesBuilder.get("CC_FLAGS");
       ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag;
@@ -1663,6 +1662,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * <p>The returned map must contain an entry for {@code STACK_FRAME_UNLIMITED},
    * though the entry may be an empty string.
    */
+  @VisibleForTesting
   public ImmutableMap<String, String> getAdditionalMakeVariables() {
     return additionalMakeVariables;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
index f0bc1c7679..ef7062303b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
@@ -64,11 +64,9 @@ public final class ExtraActionFactory implements RuleConfiguredTargetFactory {
     command = command.replace("$(OWNER_LABEL_DIGEST)", "$$(OWNER_LABEL_DIGEST)");
     command = command.replace("$(output ", "$$(output ");
     try {
-      command =
-          MakeVariableExpander.expand(
-              command,
-              new ConfigurationMakeVariableContext(
-                  context, context.getTarget().getPackage(), context.getConfiguration()));
+      command = MakeVariableExpander.expand(
+          command, new ConfigurationMakeVariableContext(
+              context.getTarget().getPackage(), context.getConfiguration()));
     } catch (MakeVariableExpander.ExpansionException e) {
       context.ruleError(String.format("Unable to expand make variables: %s",
           e.getMessage()));
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 49372a9c3a..ca50836a59 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
@@ -39,6 +39,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.AliasProvider;
+import com.google.devtools.build.lib.rules.MakeVariableProvider;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.cpp.CppHelper;
 import com.google.devtools.build.lib.rules.java.JavaHelper;
@@ -289,17 +290,12 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
     private final NestedSet<Artifact> resolvedSrcs;
     private final NestedSet<Artifact> filesToBuild;
 
-    private static final ImmutableList<String> makeVariableAttributes =
-        ImmutableList.of(":cc_toolchain", "toolchains");
-
-    public CommandResolverContext(
-        RuleContext ruleContext,
-        NestedSet<Artifact> resolvedSrcs,
+    public CommandResolverContext(RuleContext ruleContext, NestedSet<Artifact> resolvedSrcs,
         NestedSet<Artifact> filesToBuild) {
       super(
-          ruleContext.getMakeVariables(makeVariableAttributes),
           ruleContext.getRule().getPackage(),
-          ruleContext.getConfiguration());
+          ruleContext.getConfiguration(),
+          MakeVariableProvider.getToolchainMakeVariables(ruleContext, "toolchains"));
       this.ruleContext = ruleContext;
       this.resolvedSrcs = resolvedSrcs;
       this.filesToBuild = filesToBuild;
@@ -348,9 +344,7 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
         }
       } else if (JDK_MAKE_VARIABLE.matcher("$(" + name + ")").find()) {
         return new ConfigurationMakeVariableContext(
-                ruleContext.getMakeVariables(makeVariableAttributes),
-                ruleContext.getTarget().getPackage(),
-                ruleContext.getHostConfiguration())
+            ruleContext.getTarget().getPackage(), ruleContext.getHostConfiguration())
             .lookupMakeVariable(name);
       } else {
         return super.lookupMakeVariable(name);