Allow repository rules to provide default bindings for external labels.

The idea is that an android_sdk_repository rule would by default bind @external:android/sdk to itself, thus avoiding an unnecessary roundtrip through //tools/android:sdk . If we then also eventually bind that external label to something, we can avoid having the //tools/android:sdk rule altogether.

--
MOS_MIGRATED_REVID=96285812

4 files changed, 80 insertions, 22 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
index 200de575ca..ae6a0f9af5 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
@@ -17,13 +17,21 @@ import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.Type.INTEGER;
 import static com.google.devtools.build.lib.packages.Type.STRING;
 
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.bazel.rules.workspace.WorkspaceBaseRule;
 import com.google.devtools.build.lib.bazel.rules.workspace.WorkspaceConfiguredTargetFactory;
+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.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.syntax.Label;
+
+import java.util.Map;
+
+import javax.annotation.Nullable;
 
 /**
  * Definition of the {@code android_sdk} repository rule.
@@ -31,11 +39,22 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 public class AndroidSdkRepositoryRule implements RuleDefinition {
   public static final String NAME = "android_sdk_repository";
 
+  public static final Function<? super Rule, Map<String, Label>> BINDINGS_FUNCTION =
+      new Function< Rule, Map<String, Label>>() {
+        @Nullable
+        @Override
+        public Map<String, Label> apply(Rule rule) {
+          return ImmutableMap.of(
+              "android/sdk", Label.parseAbsoluteUnchecked("@" + rule.getName() + "//:sdk"));
+        }
+      };
+
   @Override
   public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
     return builder
         .setUndocumented()
         .setWorkspaceOnly()
+        .setExternalBindingsFunction(BINDINGS_FUNCTION)
         .add(attr("path", STRING).mandatory().nonconfigurable("WORKSPACE rule"))
         .add(attr("build_tools_version", STRING).mandatory().nonconfigurable("WORKSPACE rule"))
         .add(attr("api_level", INTEGER).mandatory().nonconfigurable("WORKSPACE rule"))
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java b/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java
index f90d2a64d1..9a20f7b7d0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ExternalPackage.java
@@ -220,6 +220,11 @@ public class ExternalPackage extends Package {
           ast.getLocation());
       addEvents(eventHandler.getEvents());
       repositoryMap.put(RepositoryName.create("@" + tempRule.getName()), tempRule);
+      for (Map.Entry<String, Label> entry :
+          ruleClass.getExternalBindingsFunction().apply(tempRule).entrySet()) {
+          Label nameLabel = Label.parseAbsolute("//external:" + entry.getKey());
+          addBinding(nameLabel, new Binding(entry.getValue(), tempRule.getLocation()));
+      }
       return this;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index d2c1f569cc..05ad52af95 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -20,10 +20,13 @@ import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
 import static com.google.devtools.build.lib.packages.Type.LABEL_LIST;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Functions;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Ordering;
 import com.google.devtools.build.lib.events.EventHandler;
@@ -91,6 +94,8 @@ import javax.annotation.concurrent.Immutable;
  */
 @Immutable
 public final class RuleClass {
+  public static final Function<? super Rule, Map<String, Label>> NO_EXTERNAL_BINDINGS =
+        Functions.<Map<String, Label>>constant(ImmutableMap.<String, Label>of());
   /**
    * A constraint for the package name of the Rule instances.
    */
@@ -428,6 +433,8 @@ public final class RuleClass {
     private Predicate<String> preferredDependencyPredicate = Predicates.alwaysFalse();
     private List<Class<?>> advertisedProviders = new ArrayList<>();
     private BaseFunction configuredTargetFunction = null;
+    private Function<? super Rule, Map<String, Label>> externalBindingsFunction =
+        NO_EXTERNAL_BINDINGS;
     private SkylarkEnvironment ruleDefinitionEnvironment = null;
     private Set<Class<?>> configurationFragments = new LinkedHashSet<>();
     private boolean failIfMissingConfigurationFragment;
@@ -510,10 +517,13 @@ public final class RuleClass {
           == (configuredTargetFactory == null && configuredTargetFunction == null));
       Preconditions.checkState(skylarkExecutable == (configuredTargetFunction != null));
       Preconditions.checkState(skylarkExecutable == (ruleDefinitionEnvironment != null));
+      Preconditions.checkState(workspaceOnly || externalBindingsFunction == NO_EXTERNAL_BINDINGS);
+
       return new RuleClass(name, skylarkExecutable, documented, publicByDefault, binaryOutput,
           workspaceOnly, outputsDefaultExecutable, implicitOutputsFunction, configurator,
           configuredTargetFactory, validityPredicate, preferredDependencyPredicate,
           ImmutableSet.copyOf(advertisedProviders), configuredTargetFunction,
+          externalBindingsFunction,
           ruleDefinitionEnvironment, configurationFragments, failIfMissingConfigurationFragment,
           supportsConstraintChecking, attributes.values().toArray(new Attribute[0]));
     }
@@ -688,6 +698,11 @@ public final class RuleClass {
       return this;
     }
 
+    public Builder setExternalBindingsFunction(Function<? super Rule, Map<String, Label>> func) {
+      this.externalBindingsFunction = func;
+      return this;
+    }
+
     /**
      *  Sets the rule definition environment. Meant for Skylark usage.
      */
@@ -841,6 +856,11 @@ public final class RuleClass {
   @Nullable private final BaseFunction configuredTargetFunction;
 
   /**
+   * Returns the extra bindings a workspace function adds to the WORKSPACE file.
+   */
+  private final Function<? super Rule, Map<String, Label>> externalBindingsFunction;
+
+  /**
    * The Skylark rule definition environment of this RuleClass.
    * Null for non Skylark executable RuleClasses.
    */
@@ -900,6 +920,7 @@ public final class RuleClass {
       PredicateWithMessage<Rule> validityPredicate, Predicate<String> preferredDependencyPredicate,
       ImmutableSet<Class<?>> advertisedProviders,
       @Nullable BaseFunction configuredTargetFunction,
+      Function<? super Rule, Map<String, Label>> externalBindingsFunction,
       @Nullable SkylarkEnvironment ruleDefinitionEnvironment,
       Set<Class<?>> allowedConfigurationFragments, boolean failIfMissingConfigurationFragment,
       boolean supportsConstraintChecking,
@@ -917,6 +938,7 @@ public final class RuleClass {
     this.preferredDependencyPredicate = preferredDependencyPredicate;
     this.advertisedProviders = advertisedProviders;
     this.configuredTargetFunction = configuredTargetFunction;
+    this.externalBindingsFunction = externalBindingsFunction;
     this.ruleDefinitionEnvironment = ruleDefinitionEnvironment;
     // Do not make a defensive copy as builder does that already
     this.attributes = attributes;
@@ -1512,6 +1534,14 @@ public final class RuleClass {
   }
 
   /**
+   * Returns a function that computes the external bindings a repository function contributes to
+   * the WORKSPACE file.
+   */
+  public Function<? super Rule, Map<String, Label>> getExternalBindingsFunction() {
+    return externalBindingsFunction;
+  }
+
+  /**
    * Returns this RuleClass's rule definition environment.
    */
   @Nullable public SkylarkEnvironment getRuleDefinitionEnvironment() {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index 0eee09b298..0915e64850 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.substitutePlaceholderIntoTemplate;
+import static com.google.devtools.build.lib.packages.RuleClass.NO_EXTERNAL_BINDINGS;
 import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
 import static com.google.devtools.build.lib.packages.Type.INTEGER;
 import static com.google.devtools.build.lib.packages.Type.LABEL;
@@ -87,8 +88,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     return new RuleClass("ruleA", false, false, false, false, false, false,
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE,
         DUMMY_CONFIGURED_TARGET_FACTORY, PredicatesWithMessage.<Rule>alwaysTrue(),
-        PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null, null,
-        ImmutableSet.<Class<?>>of(), false, true,
+        PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null,
+        NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(), false, true,
         attr("my-string-attr", STRING).mandatory().build(),
         attr("my-label-attr", LABEL).mandatory().legacyAllowAnyFileType()
             .value(Label.parseAbsolute("//default:label")).build(),
@@ -106,8 +107,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     return new RuleClass("ruleB", false, false, false, false, false, false,
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
-        attributes.toArray(new Attribute[0]));
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true, attributes.toArray(new Attribute[0]));
   }
 
   public void testRuleClassBasics() throws Exception {
@@ -218,7 +219,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     RuleClass depsRuleClass = new RuleClass("ruleDeps", false, false, false, false, false, false,
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true,
         attr("list1", LABEL_LIST).mandatory().legacyAllowAnyFileType().build(),
         attr("list2", LABEL_LIST).mandatory().legacyAllowAnyFileType().build(),
         attr("list3", LABEL_LIST).mandatory().legacyAllowAnyFileType().build());
@@ -247,8 +249,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     RuleClass ruleClass = new RuleClass("ruleVis", false, false, false, false, false, false,
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
-        attr("visibility", LABEL_LIST).legacyAllowAnyFileType().build());
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true, attr("visibility", LABEL_LIST).legacyAllowAnyFileType().build());
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("visibility", Arrays.asList("//visibility:legacy_public"));
 
@@ -332,8 +334,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
                                               "stuff-%{outs}-bar"),
         RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
-        attr("outs", OUTPUT_LIST).build());
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true, attr("outs", OUTPUT_LIST).build());
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("outs", Collections.singletonList("explicit_out"));
@@ -353,8 +355,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     RuleClass ruleClass = new RuleClass("ruleClass", false, false, false, false, false, false,
         ImplicitOutputsFunction.fromTemplates("%{dirname}lib%{basename}.bar"), RuleClass.NO_CHANGE,
         DUMMY_CONFIGURED_TARGET_FACTORY, PredicatesWithMessage.<Rule>alwaysTrue(),
-        PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null, null,
-        ImmutableSet.<Class<?>>of(), false, true);
+        PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS,
+        null, ImmutableSet.<Class<?>>of(), false, true);
 
     Rule rule = createRule(ruleClass, "myRule", Collections.<String, Object>emptyMap(),
         testRuleLocation);
@@ -374,8 +376,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     return new RuleClass("ruleClass", false, false, false, false, false, false,
         ImplicitOutputsFunction.fromTemplates("empty"), RuleClass.NO_CHANGE,
         DUMMY_CONFIGURED_TARGET_FACTORY, PredicatesWithMessage.<Rule>alwaysTrue(),
-        PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null, null,
-        ImmutableSet.<Class<?>>of(), false, true,
+        PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS,
+        null, ImmutableSet.<Class<?>>of(), false, true,
         attr("condition", BOOLEAN).value(false).build(),
         attr("declared1", BOOLEAN).value(false).build(),
         attr("declared2", BOOLEAN).value(false).build(),
@@ -517,8 +519,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.fromTemplates("first-%{name}", "second-%{name}", "out-%{outs}"),
         RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
-        attr("outs", OUTPUT_LIST).build());
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true, attr("outs", OUTPUT_LIST).build());
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("outs", ImmutableList.of("third", "fourth"));
@@ -540,7 +542,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     RuleClass ruleClass = new RuleClass("ruleA", false, false, false, false, false, false,
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true,
         attr("a", STRING_LIST).mandatory().build(),
         attr("b", STRING_LIST).mandatory().build(),
         attr("c", STRING_LIST).mandatory().build(),
@@ -625,8 +628,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
         "ruleMNE", false, false, false, false, false, false,
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
-        attributes);
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true, attributes);
     return mandNonEmptyRuleClass;
   }
 
@@ -636,8 +639,9 @@ public class RuleClassTest extends PackageLoadingTestCase {
         "ruleMNE", false, false, false, false, false, false,
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(), false, true,
-        attr("list", LABEL_LIST).nonEmpty().legacyAllowAnyFileType().value(emptyList).build());
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        false, true, attr("list", LABEL_LIST)
+        .nonEmpty().legacyAllowAnyFileType().value(emptyList).build());
 
     Map<String, Object> attributeValues = new LinkedHashMap<>();
     reporter.removeHandler(failFastHandler);
@@ -721,8 +725,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
     RuleClass parentRuleClass = new RuleClass("parent_rule", false, false, false, false, false,
         false, ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
-        ImmutableSet.<Class<?>>of(), null, null, ImmutableSet.<Class<?>>of(DummyFragment.class),
-        false, true, attr("attr", STRING).build());
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null,
+        ImmutableSet.<Class<?>>of(DummyFragment.class), false, true, attr("attr", STRING).build());
     return parentRuleClass;
   }