Add a toolchains= attribute to *_binary, *_test, cc_library and extra_action rules to declare which Make variables they need.

The idea is that they would depend on the future java_runtime_alias / cc_toolchain_alias and similar rules and thus Bazel will know which Make variables they actually need instead of pulling in the whole BuildConfiguration and also making it possible to compute these Make variables during analysis instead of configuration creation.

RELNOTES: None.
PiperOrigin-RevId: 161785868

12 files changed, 68 insertions, 26 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index 2396c3edbb..b4553767c8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -48,6 +48,7 @@ import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.TestSize;
+import com.google.devtools.build.lib.rules.MakeVariableProvider;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import java.util.List;
@@ -156,7 +157,7 @@ public class BaseRuleClasses {
       return RuleDefinition.Metadata.builder()
           .name("$test_base_rule")
           .type(RuleClassType.ABSTRACT)
-          .ancestors(RootRule.class)
+          .ancestors(RootRule.class, MakeVariableExpandingRule.class)
           .build();
     }
   }
@@ -266,6 +267,33 @@ public class BaseRuleClasses {
   }
 
   /**
+   * A rule that contains a {@code variables=} attribute to allow referencing Make variables.
+   */
+  public static final class MakeVariableExpandingRule implements RuleDefinition {
+    @Override
+    public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
+      return builder
+          /* <!-- #BLAZE_RULE($make_variable_expanding_rule).ATTRIBUTE(toolchains) -->
+          The set of toolchains that supply <a href="${link make-variables}">"Make variables"</a>
+          that this target can use in some of its attributes. Some rules have toolchains whose Make
+          variables they can use by default.
+          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+          .add(attr("toolchains", LABEL_LIST)
+              .allowedFileTypes(FileTypeSet.NO_FILE)
+              .mandatoryProviders(ImmutableList.of(MakeVariableProvider.SKYLARK_CONSTRUCTOR.id())))
+          .build();
+    }
+
+    @Override
+    public Metadata getMetadata() {
+      return RuleDefinition.Metadata.builder()
+          .name("$make_variable_expanding_rule")
+          .type(RuleClassType.ABSTRACT)
+          .build();
+    }
+  }
+
+  /**
    * Common ancestor class for some rules.
    */
   public static final class RuleBase implements RuleDefinition {
@@ -311,7 +339,7 @@ public class BaseRuleClasses {
       return RuleDefinition.Metadata.builder()
           .name("$binary_base_rule")
           .type(RuleClassType.ABSTRACT)
-          .ancestors(RootRule.class)
+          .ancestors(RootRule.class, MakeVariableExpandingRule.class)
           .build();
     }
   }
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 2a17ddf838..3392aff327 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
@@ -38,13 +38,13 @@ public class ConfigurationMakeVariableContext implements MakeVariableExpander.Co
   // 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
   // pollute core with C++ specific constant.
-  private static final ImmutableList<String> defaultMakeVariableAttributes =
-      ImmutableList.of(":cc_toolchain");
+  protected static final ImmutableList<String> DEFAULT_MAKE_VARIABLE_ATTRIBUTES =
+      ImmutableList.of(":cc_toolchain", "toolchains");
 
   public ConfigurationMakeVariableContext(
       RuleContext ruleContext, Package pkg, BuildConfiguration configuration) {
     this(
-        ruleContext.getMakeVariables(defaultMakeVariableAttributes),
+        ruleContext.getMakeVariables(DEFAULT_MAKE_VARIABLE_ATTRIBUTES),
         pkg,
         configuration,
         ImmutableList.<MakeVariableSupplier>of());
@@ -63,7 +63,7 @@ public class ConfigurationMakeVariableContext implements MakeVariableExpander.Co
       BuildConfiguration configuration,
       Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
     this(
-        ruleContext.getMakeVariables(defaultMakeVariableAttributes),
+        ruleContext.getMakeVariables(DEFAULT_MAKE_VARIABLE_ATTRIBUTES),
         pkg,
         configuration,
         makeVariableSuppliers);
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 8987fedbd8..61daf33027 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
@@ -96,6 +96,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -1072,27 +1073,23 @@ 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 secretly stomp on
-    // each other.
-    ImmutableMap.Builder<String, String> makeVariableBuilder = ImmutableMap.builder();
-    ImmutableSet.Builder<MakeVariableProvider> makeVariableProvidersBuilder =
-        ImmutableSet.builder();
+    ArrayList<MakeVariableProvider> makeVariableProviders = new ArrayList<>();
 
     for (String attributeName : attributeNames) {
       // TODO(b/37567440): Remove this continue statement.
       if (!attributes().has(attributeName)) {
         continue;
       }
-      makeVariableProvidersBuilder.addAll(
+      Iterables.addAll(makeVariableProviders,
           getPrerequisites(attributeName, Mode.TARGET, MakeVariableProvider.SKYLARK_CONSTRUCTOR));
     }
 
-    for (MakeVariableProvider makeVariableProvider : makeVariableProvidersBuilder.build()) {
-      makeVariableBuilder.putAll(makeVariableProvider.getMakeVariables());
+    LinkedHashMap<String, String> makeVariables = new LinkedHashMap<>();
+    for (MakeVariableProvider makeVariableProvider : makeVariableProviders) {
+      makeVariables.putAll(makeVariableProvider.getMakeVariables());
     }
 
-    return makeVariableBuilder.build();
+    return ImmutableMap.copyOf(makeVariables);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 1da2b0dce4..a3a72bc31e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -242,6 +242,7 @@ public class BazelRuleClassProvider {
           builder.addRuleDefinition(new BaseRuleClasses.RootRule());
           builder.addRuleDefinition(new BaseRuleClasses.BaseRule());
           builder.addRuleDefinition(new BaseRuleClasses.RuleBase());
+          builder.addRuleDefinition(new BaseRuleClasses.MakeVariableExpandingRule());
           builder.addRuleDefinition(new BaseRuleClasses.BinaryBaseRule());
           builder.addRuleDefinition(new BaseRuleClasses.TestBaseRule());
           builder.addRuleDefinition(new BaseRuleClasses.ErrorRule());
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcLibraryRule.java
index cc553742b4..95906c5282 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcLibraryRule.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.bazel.rules.cpp;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
 
+import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses.CcLibraryBaseRule;
@@ -55,7 +56,7 @@ public final class BazelCcLibraryRule implements RuleDefinition {
   public Metadata getMetadata() {
     return RuleDefinition.Metadata.builder()
         .name("cc_library")
-        .ancestors(CcLibraryBaseRule.class)
+        .ancestors(CcLibraryBaseRule.class, BaseRuleClasses.MakeVariableExpandingRule.class)
         .factoryClass(BazelCcLibrary.class)
         .build();
   }
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 be35e55174..2b700f5c09 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
@@ -46,4 +46,14 @@ public final class MakeVariableProvider extends SkylarkClassObject {
   public ImmutableMap<String, String> getMakeVariables() {
     return makeVariables;
   }
+
+  @Override
+  public boolean equals(Object other) {
+    return other == this;
+  }
+
+  @Override
+  public int hashCode() {
+    return System.identityHashCode(this);
+  }
 }
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 8dbd03ce3c..88eb059c47 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
@@ -1035,7 +1035,11 @@ public final class SkylarkRuleContext implements SkylarkValue {
         attributeName,
         command,
         new ConfigurationMakeVariableContext(
-            ruleContext, ruleContext.getRule().getPackage(), ruleContext.getConfiguration()) {
+            // TODO(lberki): This should be removed. But only after either verifying that no one
+            // uses it or providing an alternative.
+            ruleContext.getMakeVariables(ImmutableList.of(":cc_toolchain")),
+            ruleContext.getRule().getPackage(),
+            ruleContext.getConfiguration()) {
           @Override
           public String lookupMakeVariable(String variableName) throws ExpansionException {
             if (additionalSubstitutions.containsKey(variableName)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
index 1fad6472b6..25b6f337ad 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.rules.MakeVariableProvider;
 import com.google.devtools.build.lib.rules.cpp.CppRuleClasses.LipoTransition;
 
 /**
@@ -62,6 +63,7 @@ public final class CcToolchainRule implements RuleDefinition {
     return builder
         .setUndocumented()
         .requiresConfigurationFragments(CppConfiguration.class)
+        .advertiseProvider(MakeVariableProvider.class)
         .add(attr("output_licenses", LICENSE))
         .add(attr("cpu", STRING).mandatory())
         .add(attr("all_files", LABEL).legacyAllowAnyFileType().cfg(HOST).mandatory())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionRule.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionRule.java
index 8acf714f1a..f2e5b26e03 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionRule.java
@@ -120,7 +120,7 @@ public final class ExtraActionRule implements RuleDefinition {
   public Metadata getMetadata() {
     return RuleDefinition.Metadata.builder()
         .name("extra_action")
-        .ancestors(BaseRuleClasses.RuleBase.class)
+        .ancestors(BaseRuleClasses.RuleBase.class, BaseRuleClasses.MakeVariableExpandingRule.class)
         .factoryClass(ExtraActionFactory.class)
         .build();
   }
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 8b90b48217..7b66683e4c 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
@@ -41,7 +41,6 @@ import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.AliasProvider;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
-import com.google.devtools.build.lib.rules.cpp.CcToolchain;
 import com.google.devtools.build.lib.rules.cpp.CppHelper;
 import com.google.devtools.build.lib.rules.java.JavaHelper;
 import com.google.devtools.build.lib.syntax.Type;
@@ -291,16 +290,13 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
     private final NestedSet<Artifact> resolvedSrcs;
     private final NestedSet<Artifact> filesToBuild;
 
-    private static final ImmutableList<String> makeVariableAttributes =
-        ImmutableList.of(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, "toolchains");
-
     public CommandResolverContext(
         RuleContext ruleContext,
         NestedSet<Artifact> resolvedSrcs,
         NestedSet<Artifact> filesToBuild,
         Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
       super(
-          ruleContext.getMakeVariables(makeVariableAttributes),
+          ruleContext,
           ruleContext.getRule().getPackage(),
           ruleContext.getConfiguration(),
           makeVariableSuppliers);
@@ -352,7 +348,8 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
         }
       } else if (JDK_MAKE_VARIABLE.matcher("$(" + variableName + ")").find()) {
         return new ConfigurationMakeVariableContext(
-                ruleContext.getMakeVariables(makeVariableAttributes),
+                ruleContext.getMakeVariables(
+                    ConfigurationMakeVariableContext.DEFAULT_MAKE_VARIABLE_ATTRIBUTES),
                 ruleContext.getTarget().getPackage(),
                 ruleContext.getHostConfiguration())
             .lookupMakeVariable(variableName);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeRule.java
index 9d4ae426cf..6b97768557 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeRule.java
@@ -24,6 +24,7 @@ import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
+import com.google.devtools.build.lib.rules.MakeVariableProvider;
 import com.google.devtools.build.lib.util.FileTypeSet;
 
 /** Rule definition for {@code java_runtime} */
@@ -31,6 +32,7 @@ public final class JavaRuntimeRule implements RuleDefinition {
   @Override
   public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
     return builder
+        .advertiseProvider(MakeVariableProvider.class)
         /* <!-- #BLAZE_RULE(java_runtime).ATTRIBUTE(srcs) -->
         All files in the runtime.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryRule.java
index 249f27ac7b..5aa9546fb3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryRule.java
@@ -51,7 +51,7 @@ public class ObjcLibraryRule implements RuleDefinition {
         .name("objc_library")
         .factoryClass(ObjcLibrary.class)
         .ancestors(BaseRuleClasses.BaseRule.class, ObjcRuleClasses.CompilingRule.class,
-            ObjcRuleClasses.AlwaysLinkRule.class)
+            ObjcRuleClasses.AlwaysLinkRule.class, BaseRuleClasses.MakeVariableExpandingRule.class)
         .build();
   }
 }