Make skylark repository functions exportable

In this way, their name is one they can actually be referred to by.
This is necessary, as Skylark repository rules end up in the resolved
file when freezing the versions of external dependencies---and the call
to the repository function might be indirect.

Change-Id: Ie1b5a28fac4f690c424de848b42d0c4a217d494d
PiperOrigin-RevId: 203466286

2 files changed, 83 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
index 9e8e84c172..1a4ec433d3 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
@@ -22,6 +22,7 @@ import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
 
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.skylark.SkylarkAttr.Descriptor;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.packages.AttributeValueSource;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
@@ -30,8 +31,10 @@ import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
+import com.google.devtools.build.lib.packages.SkylarkExportable;
 import com.google.devtools.build.lib.packages.WorkspaceFactoryHelper;
 import com.google.devtools.build.lib.skylarkbuildapi.repository.RepositoryModuleApi;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.syntax.BaseFunction;
 import com.google.devtools.build.lib.syntax.DotExpression;
 import com.google.devtools.build.lib.syntax.EvalException;
@@ -85,8 +88,11 @@ public class SkylarkRepositoryModule implements RepositoryModuleApi {
     return new RepositoryRuleFunction(builder);
   }
 
-  private static final class RepositoryRuleFunction extends BaseFunction {
+  private static final class RepositoryRuleFunction extends BaseFunction
+      implements SkylarkExportable {
     private final RuleClass.Builder builder;
+    private Label extensionLabel;
+    private String exportedName;
 
     public RepositoryRuleFunction(RuleClass.Builder builder) {
       super("repository_rule", FunctionSignature.KWARGS);
@@ -94,12 +100,40 @@ public class SkylarkRepositoryModule implements RepositoryModuleApi {
     }
 
     @Override
+    public void export(Label extensionLabel, String exportedName) {
+      this.extensionLabel = extensionLabel;
+      this.exportedName = exportedName;
+    }
+
+    @Override
+    public boolean isExported() {
+      return extensionLabel != null;
+    }
+
+    @Override
+    public void repr(SkylarkPrinter printer) {
+      if (exportedName == null) {
+        printer.append("<anonymous skylark repository rule>");
+      } else {
+        printer.append("<skylark repository rule " + extensionLabel + "%" + exportedName + ">");
+      }
+    }
+
+    @Override
     public Object call(
         Object[] args, FuncallExpression ast, com.google.devtools.build.lib.syntax.Environment env)
         throws EvalException, InterruptedException {
       String ruleClassName = null;
       Expression function = ast.getFunction();
-      if (function instanceof Identifier) {
+      // If the function ever got exported (the common case), we take the name
+      // it was exprted to. Only in the not intended case of calling an unexported
+      // repository function through an exported macro, we fall back, for lack of
+      // alternatives, to the name in the local context.
+      // TODO(b/111199163): we probably should disallow the use of non-exported
+      // repository rules anyway.
+      if (isExported()) {
+        ruleClassName = exportedName;
+      } else if (function instanceof Identifier) {
         ruleClassName = ((Identifier) function).getName();
       } else if (function instanceof DotExpression) {
         ruleClassName = ((DotExpression) function).getField().getName();
diff --git a/src/test/shell/bazel/workspace_resolved_test.sh b/src/test/shell/bazel/workspace_resolved_test.sh
index cc9d9ec6d8..b013c6dcaf 100755
--- a/src/test/shell/bazel/workspace_resolved_test.sh
+++ b/src/test/shell/bazel/workspace_resolved_test.sh
@@ -388,4 +388,51 @@ EOF
   expect_log "Failure-message"
 }
 
+test_indirect_call() {
+  rm -rf fetchrepo
+  mkdir fetchrepo
+  cd fetchrepo
+  touch BUILD
+  cat > rule.bzl <<'EOF'
+def _trivial_rule_impl(ctx):
+  ctx.file("BUILD","genrule(name='hello', outs=['hello.txt'], cmd=' echo hello world > $@')")
+
+trivial_rule = repository_rule(
+  implementation = _trivial_rule_impl,
+  attrs = {},
+)
+EOF
+  cat > indirect.bzl <<'EOF'
+def call(fn_name, **args):
+  fn_name(**args)
+EOF
+  cat > WORKSPACE <<'EOF'
+load("//:rule.bzl", "trivial_rule")
+load("//:indirect.bzl", "call")
+
+call(trivial_rule, name="foo")
+EOF
+  bazel sync --experimental_repository_resolved_file=../repo.bzl
+  bazel shutdown; sync; sleep 10
+
+  cd ..
+  echo; cat repo.bzl; echo
+  touch WORKSPACE
+  cat > BUILD <<'EOF'
+load("//:repo.bzl", "resolved")
+
+ruleclass = "".join([entry["original_rule_class"] for entry in resolved if entry["original_attributes"]["name"]=="foo"])
+
+genrule(
+  name = "ruleclass",
+  outs = ["ruleclass.txt"],
+  cmd = "echo %s > $@" % (ruleclass,)
+)
+EOF
+  bazel build //:ruleclass
+  cat `bazel info bazel-genfiles`/ruleclass.txt > ${TEST_log}
+  expect_log '//:rule.bzl%trivial_rule'
+  expect_not_log 'fn_name'
+}
+
 run_suite "workspace_resolved_test tests"