Stringify Labels in display form in Args

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

@@ -25,6 +25,7 @@
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.MiddlemanFactory;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -166,4 +167,9 @@ Artifact.DerivedArtifact getDerivedArtifact(
   ImmutableSet<Artifact> getTreeArtifactsConflictingWithFiles();
 
   ActionKeyContext getActionKeyContext();
+
+  /**
+   * Returns and registers a Skyframe dependency on the {@link RepositoryMapping} of the main repo.
+   */
+  RepositoryMapping getMainRepoMapping() throws InterruptedException;
 }

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -411,6 +411,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_value_creation_exception",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:workspace_status_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe/config",
@@ -421,6 +422,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test",
+        "//src/main/java/com/google/devtools/build/lib/supplier",
         "//src/main/java/com/google/devtools/build/lib/unsafe:string",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
@@ -2450,9 +2452,11 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/actions:commandline_item",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
+        "//src/main/java/com/google/devtools/build/lib/supplier",
         "//src/main/java/net/starlark/java/eval",
         "//src/main/java/net/starlark/java/syntax",
         "//third_party:guava",

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

@@ -27,9 +27,12 @@
 import com.google.devtools.build.lib.actions.ArtifactFactory;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.MiddlemanFactory;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.skyframe.RepositoryMappingValue;
 import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue;
 import com.google.devtools.build.lib.skyframe.WorkspaceStatusValue;
 import com.google.devtools.build.lib.util.Pair;
@@ -368,6 +371,19 @@ private WorkspaceStatusValue getWorkspaceStatusValue() throws InterruptedExcepti
     return workspaceStatusValue;
   }
 
+  @Override
+  public RepositoryMapping getMainRepoMapping() throws InterruptedException {
+    var mainRepoMapping =
+        (RepositoryMappingValue)
+            skyframeEnv.getValue(RepositoryMappingValue.key(RepositoryName.MAIN));
+    if (mainRepoMapping == null) {
+      // This isn't expected to happen since the main repository mapping is computed before the
+      // analysis phase.
+      throw new MissingDepException("Restart due to missing main repository mapping");
+    }
+    return mainRepoMapping.getRepositoryMapping();
+  }
+
   @Override
   public ActionLookupKey getOwner() {
     return owner;

src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java

@@ -23,10 +23,13 @@
 import com.google.devtools.build.lib.actions.ParamFileInfo;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 import com.google.devtools.build.lib.actions.SingleStringArgFormatter;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.collect.nestedset.Depset;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.starlarkbuildapi.CommandLineArgsApi;
+import com.google.devtools.build.lib.supplier.InterruptibleSupplier;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -70,7 +73,8 @@ public void repr(Printer printer) {
   @Override
   public void debugPrint(Printer printer, StarlarkSemantics semantics) {
     try {
-      printer.append(Joiner.on(" ").join(build().arguments()));
+      printer.append(
+          Joiner.on(" ").join(build(/* mainRepoMappingSupplier= */ () -> null).arguments()));
     } catch (CommandLineExpansionException e) {
       printer.append("Cannot expand command line: " + e.getMessage());
     } catch (InterruptedException e) {
@@ -102,7 +106,8 @@ public void debugPrint(Printer printer, StarlarkSemantics semantics) {
   public abstract ImmutableSet<Artifact> getDirectoryArtifacts();
 
   /** Returns the command line built by this {@link Args} object. */
-  public abstract CommandLine build();
+  public abstract CommandLine build(
+      InterruptibleSupplier<RepositoryMapping> mainRepoMappingSupplier) throws InterruptedException;
 
   /**
    * Returns a frozen {@link Args} representation corresponding to an already-registered action.
@@ -157,7 +162,7 @@ public ImmutableSet<Artifact> getDirectoryArtifacts() {
     }
 
     @Override
-    public CommandLine build() {
+    public CommandLine build(InterruptibleSupplier<RepositoryMapping> mainRepoMappingSupplier) {
       return commandLine;
     }
 
@@ -259,6 +264,12 @@ private static class MutableArgs extends Args implements StarlarkValue, Mutabili
      */
     private boolean flagPerLine = false;
 
+    /**
+     * True if the command line needs to stringify any {@link Label}s without an explicit 'map_each'
+     * function.
+     */
+    private boolean mayStringifyExternalLabel = false;
+
     // May be set explicitly once -- if unset defaults to ParameterFileType.SHELL_QUOTED.
     private ParameterFileType parameterFileType = null;
     private String flagFormatString;
@@ -307,6 +318,9 @@ public CommandLineArgsApi addArgument(
             "Args.add() doesn't accept vectorized arguments. Please use Args.add_all() or"
                 + " Args.add_joined() instead.");
       }
+      if (value instanceof Label label && !label.getRepository().isMain()) {
+        mayStringifyExternalLabel = true;
+      }
       addSingleArg(value, format != Starlark.NONE ? (String) format : null);
       return this;
     }
@@ -436,8 +450,12 @@ private void addVectorArg(
       validateFormatString("format_each", formatEach);
       validateFormatString("format_joined", formatJoined);
       StarlarkCustomCommandLine.VectorArg.Builder vectorArg;
-      if (value instanceof Depset) {
-        Depset starlarkNestedSet = (Depset) value;
+      if (value instanceof Depset starlarkNestedSet) {
+        if (mapEach == null && Label.class.equals(starlarkNestedSet.getElementClass())) {
+          // We don't want to eagerly check whether all labels reference targets in the main repo,
+          // so just assume they might not. Nested sets of labels should be rare.
+          mayStringifyExternalLabel = true;
+        }
         NestedSet<?> nestedSet = starlarkNestedSet.getSet();
         if (nestedSet.isEmpty() && omitIfEmpty) {
           return;
@@ -451,8 +469,16 @@ private void addVectorArg(
         if (starlarkList.isEmpty() && omitIfEmpty) {
           return;
         }
-        if (expandDirectories) {
-          scanForDirectories(starlarkList);
+        for (Object object : starlarkList) {
+          if (expandDirectories && isDirectory(object)) {
+            directoryArtifacts.add((Artifact) object);
+          }
+          // Labels referencing targets in the main repo are stringified as //pkg:name and thus
+          // don't require a RepositoryMapping. If a map_each function is provided, default
+          // stringification via Label#toString() is not used.
+          if (mapEach == null && object instanceof Label label && !label.getRepository().isMain()) {
+            mayStringifyExternalLabel = true;
+          }
         }
         vectorArg = new StarlarkCustomCommandLine.VectorArg.Builder(starlarkList);
       }
@@ -573,8 +599,10 @@ private MutableArgs(@Nullable Mutability mutability, StarlarkSemantics starlarkS
     }
 
     @Override
-    public CommandLine build() {
-      return commandLine.build(flagPerLine);
+    public CommandLine build(InterruptibleSupplier<RepositoryMapping> mainRepoMappingSupplier)
+        throws InterruptedException {
+      return commandLine.build(
+          flagPerLine, mayStringifyExternalLabel ? mainRepoMappingSupplier.get() : null);
     }
 
     @Override
@@ -585,18 +613,15 @@ public Mutability mutability() {
     @Override
     public ImmutableSet<Artifact> getDirectoryArtifacts() {
       for (NestedSet<?> collection : potentialDirectoryArtifacts) {
-        scanForDirectories(collection.toList());
+        for (Object object : collection.toList()) {
+          if (isDirectory(object)) {
+            directoryArtifacts.add((Artifact) object);
+          }
+        }
       }
       potentialDirectoryArtifacts.clear();
       return ImmutableSet.copyOf(directoryArtifacts);
     }
 
-    private void scanForDirectories(Iterable<?> objects) {
-      for (Object object : objects) {
-        if (isDirectory(object)) {
-          directoryArtifacts.add((Artifact) object);
-        }
-      }
-    }
   }
 }

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java

@@ -51,6 +51,7 @@
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.collect.nestedset.Depset;
 import com.google.devtools.build.lib.collect.nestedset.Depset.TypeException;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -68,6 +69,7 @@
 import com.google.devtools.build.lib.starlarkbuildapi.FileApi;
 import com.google.devtools.build.lib.starlarkbuildapi.StarlarkActionFactoryApi;
 import com.google.devtools.build.lib.starlarkbuildapi.TemplateDictApi;
+import com.google.devtools.build.lib.supplier.InterruptibleSupplier;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.protobuf.GeneratedMessage;
@@ -332,7 +334,8 @@ public void symlink(
   }
 
   @Override
-  public void write(FileApi output, Object content, Boolean isExecutable) throws EvalException {
+  public void write(FileApi output, Object content, Boolean isExecutable)
+      throws EvalException, InterruptedException {
     context.checkMutable("actions.write");
     RuleContext ruleContext = getRuleContext();
 
@@ -347,7 +350,7 @@ public void write(FileApi output, Object content, Boolean isExecutable) throws E
               ruleContext.getActionOwner(),
               NestedSetBuilder.wrap(Order.STABLE_ORDER, args.getDirectoryArtifacts()),
               (Artifact) output,
-              args.build(),
+              args.build(getMainRepoMappingSupplier()),
               args.getParameterFileType());
     } else {
       throw new AssertionError("Unexpected type: " + content.getClass().getSimpleName());
@@ -373,7 +376,7 @@ public void run(
       Object shadowedActionUnchecked,
       Object resourceSetUnchecked,
       Object toolchainUnchecked)
-      throws EvalException {
+      throws EvalException, InterruptedException {
     context.checkMutable("actions.run");
     execGroupUnchecked = context.maybeOverrideExecGroup(execGroupUnchecked);
     toolchainUnchecked = context.maybeOverrideToolchain(toolchainUnchecked);
@@ -382,7 +385,7 @@ public void run(
     boolean useAutoExecGroups = ruleContext.useAutoExecGroups();
 
     StarlarkAction.Builder builder = new StarlarkAction.Builder();
-    buildCommandLine(builder, arguments);
+    buildCommandLine(builder, arguments, getMainRepoMappingSupplier());
     if (executableUnchecked instanceof Artifact) {
       Artifact executable = (Artifact) executableUnchecked;
       FilesToRunProvider provider = context.getExecutableRunfiles(executable, "executable");
@@ -560,15 +563,15 @@ public void runShell(
       Object shadowedActionUnchecked,
       Object resourceSetUnchecked,
       Object toolchainUnchecked)
-      throws EvalException {
+      throws EvalException, InterruptedException {
     context.checkMutable("actions.run_shell");
     execGroupUnchecked = context.maybeOverrideExecGroup(execGroupUnchecked);
     toolchainUnchecked = context.maybeOverrideToolchain(toolchainUnchecked);
 
     RuleContext ruleContext = getRuleContext();
 
     StarlarkAction.Builder builder = new StarlarkAction.Builder();
-    buildCommandLine(builder, arguments);
+    buildCommandLine(builder, arguments, getMainRepoMappingSupplier());
 
     // When we use a shell command, add an empty argument before other arguments.
     //   e.g.  bash -c "cmd" '' 'arg1' 'arg2'
@@ -631,8 +634,11 @@ public void runShell(
         builder);
   }
 
-  private static void buildCommandLine(SpawnAction.Builder builder, Sequence<?> argumentsList)
-      throws EvalException {
+  private static void buildCommandLine(
+      SpawnAction.Builder builder,
+      Sequence<?> argumentsList,
+      InterruptibleSupplier<RepositoryMapping> repoMappingSupplier)
+      throws EvalException, InterruptedException {
     ImmutableList.Builder<String> stringArgs = null;
     for (Object value : argumentsList) {
       if (value instanceof String) {
@@ -647,7 +653,7 @@ private static void buildCommandLine(SpawnAction.Builder builder, Sequence<?> ar
         }
         Args args = (Args) value;
         ParamFileInfo paramFileInfo = args.getParamFileInfo();
-        builder.addCommandLine(args.build(), paramFileInfo);
+        builder.addCommandLine(args.build(repoMappingSupplier), paramFileInfo);
       } else {
         throw Starlark.errorf(
             "expected list of strings or ctx.actions.args() for arguments instead of %s",
@@ -1046,6 +1052,10 @@ public void repr(Printer printer) {
     context.repr(printer);
   }
 
+  private InterruptibleSupplier<RepositoryMapping> getMainRepoMappingSupplier() {
+    return context.getRuleContext().getAnalysisEnvironment()::getMainRepoMapping;
+  }
+
   /** The analysis context for {@code Starlark} actions */
   // For now, this contains methods necessary for SubruleContext to begin using
   // StarlarkActionFactory without any invasive changes to the latter. It will be improved once the