Changing SymlinkTreeStrategy not to use spawns.

RELNOTES: An internal action for symlinking runfiles will use Command instead of a Spawns. This should have no functional chages; the only user visible consequence should be that the internal action is no longer be included in statistics when calculating processes count.
PiperOrigin-RevId: 197131917

3 files changed, 24 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
index 5f231e4fd0..bd00f38500 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
@@ -21,15 +21,12 @@ import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
 import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.SimpleSpawn;
 import com.google.devtools.build.lib.actions.Spawn;
-import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.UserExecException;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.shell.CommandException;
 import com.google.devtools.build.lib.util.CommandBuilder;
 import com.google.devtools.build.lib.util.OsUtils;
@@ -63,8 +60,8 @@ public final class SymlinkTreeHelper {
    *
    * @param inputManifest exec path to the input runfiles manifest
    * @param symlinkTreeRoot the root of the symlink tree to be created
-   * @param filesetTree true if this is fileset symlink tree,
-   *                    false if this is a runfiles symlink tree.
+   * @param filesetTree true if this is fileset symlink tree, false if this is a runfiles symlink
+   *     tree.
    */
   public SymlinkTreeHelper(Path inputManifest, Path symlinkTreeRoot, boolean filesetTree) {
     this.inputManifest = inputManifest;
@@ -80,20 +77,23 @@ public final class SymlinkTreeHelper {
    * Creates a symlink tree using a CommandBuilder. This means that the symlink tree will always be
    * present on the developer's workstation. Useful when running commands locally.
    *
-   * Warning: this method REALLY executes the command on the box Bazel is running on, without any
+   * <p>Warning: this method REALLY executes the command on the box Bazel is running on, without any
    * kind of synchronization, locking, or anything else.
    *
    * @param config the configuration that is used for creating the symlink tree.
    * @throws CommandException
    */
   public void createSymlinksUsingCommand(
-      Path execRoot, BuildConfiguration config, BinTools binTools)
-          throws CommandException {
+      Path execRoot, BinTools binTools, ImmutableMap<String, String> shellEnvironment)
+      throws CommandException {
     List<String> argv = getSpawnArgumentList(execRoot, binTools.getExecPath(BUILD_RUNFILES));
-    CommandBuilder builder = new CommandBuilder();
-    builder.addArgs(argv);
-    builder.setWorkingDir(execRoot);
-    builder.build().execute();
+    Preconditions.checkNotNull(shellEnvironment);
+    new CommandBuilder()
+        .addArgs(argv)
+        .setWorkingDir(execRoot)
+        .setEnv(shellEnvironment)
+        .build()
+        .execute();
   }
 
   /**
@@ -105,27 +105,19 @@ public final class SymlinkTreeHelper {
    * @param enableRunfiles
    * @return a list of SpawnResults created during symlink creation, if any
    */
-  public List<SpawnResult> createSymlinks(
-      ActionExecutionMetadata owner,
+  public void createSymlinks(
       ActionExecutionContext actionExecutionContext,
       BinTools binTools,
       ImmutableMap<String, String> shellEnvironment,
-      Artifact inputManifestArtifact,
       boolean enableRunfiles)
-          throws ExecException, InterruptedException {
-    Preconditions.checkState(
-        actionExecutionContext.getInputPath(inputManifestArtifact).equals(inputManifest));
+      throws ExecException {
     if (enableRunfiles) {
-      Spawn spawn =
-          createSpawn(
-              owner,
-              actionExecutionContext.getExecRoot(),
-              binTools,
-              shellEnvironment,
-              inputManifestArtifact);
-      return actionExecutionContext
-          .getSpawnActionContext(spawn)
-          .exec(spawn, actionExecutionContext);
+      try {
+        createSymlinksUsingCommand(
+            actionExecutionContext.getExecRoot(), binTools, shellEnvironment);
+      } catch (CommandException e) {
+        throw new UserExecException(e.getMessage(), e);
+      }
     } else {
       // Pretend we created the runfiles tree by copying the manifest
       try {
@@ -134,7 +126,6 @@ public final class SymlinkTreeHelper {
       } catch (IOException e) {
         throw new UserExecException(e.getMessage(), e);
       }
-      return ImmutableList.of();
     }
   }
 
@@ -159,9 +150,7 @@ public final class SymlinkTreeHelper {
         RESOURCE_SET);
   }
 
-  /**
-   * Returns the complete argument list build-runfiles has to be called with.
-   */
+  /** Returns the complete argument list build-runfiles has to be called with. */
   private ImmutableList<String> getSpawnArgumentList(Path execRoot, PathFragment buildRunfiles) {
     List<String> args = Lists.newArrayList();
     args.add(buildRunfiles.getPathString());
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
index d672798e2c..82b24eca1a 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
@@ -13,18 +13,15 @@
 // limitations under the License.
 package com.google.devtools.build.lib.exec;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionStrategy;
-import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction;
 import com.google.devtools.build.lib.analysis.actions.SymlinkTreeActionContext;
 import com.google.devtools.build.lib.profiler.AutoProfiler;
 import com.google.devtools.build.lib.skyframe.OutputService;
-import java.util.List;
 import java.util.logging.Logger;
 
 /**
@@ -44,7 +41,7 @@ public final class SymlinkTreeStrategy implements SymlinkTreeActionContext {
   }
 
   @Override
-  public List<SpawnResult> createSymlinks(
+  public void createSymlinks(
       SymlinkTreeAction action,
       ActionExecutionContext actionExecutionContext,
       ImmutableMap<String, String> shellEnvironment,
@@ -60,7 +57,6 @@ public final class SymlinkTreeStrategy implements SymlinkTreeActionContext {
               actionExecutionContext.getInputPath(action.getOutputManifest()),
               action.isFilesetTree(),
               action.getOutputManifest().getExecPath().getParentDirectory());
-          return ImmutableList.of();
         } else {
           SymlinkTreeHelper helper =
               new SymlinkTreeHelper(
@@ -69,13 +65,7 @@ public final class SymlinkTreeStrategy implements SymlinkTreeActionContext {
                       .getInputPath(action.getOutputManifest())
                       .getParentDirectory(),
                   action.isFilesetTree());
-          return helper.createSymlinks(
-              action,
-              actionExecutionContext,
-              binTools,
-              shellEnvironment,
-              action.getInputManifest(),
-              enableRunfiles);
+          helper.createSymlinks(actionExecutionContext, binTools, shellEnvironment, enableRunfiles);
         }
       } catch (ExecException e) {
         throw e.toActionExecutionException(
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index ae65551927..fcd21ea316 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -409,13 +409,7 @@ public abstract class TestStrategy implements TestActionContext {
             actionExecutionContext.getInputPath(execSettings.getInputManifest()),
             runfilesDir,
             false)
-        .createSymlinks(
-            testAction,
-            actionExecutionContext,
-            binTools,
-            shellEnvironment,
-            execSettings.getInputManifest(),
-            enableRunfiles);
+        .createSymlinks(actionExecutionContext, binTools, shellEnvironment, enableRunfiles);
 
     actionExecutionContext.getEventHandler()
         .handle(Event.progress(testAction.getProgressMessage()));