Symlink creation: simplify interfaces
- throw a higher-level exception so callers don't need to
- remove the enableRunfiles parameter from SymlinkTreeActionContext
- move manifest copying into a separate method
I'm considering adding an in-process symlink creation implementation, which is easier after this change.
PiperOrigin-RevId: 281280393
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java
index db515ae..04ff823 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java
@@ -123,6 +123,10 @@
return filesetTree;
}
+ public boolean enableRunfiles() {
+ return enableRunfiles;
+ }
+
@Override
public String getMnemonic() {
return "SymlinkTree";
@@ -159,7 +163,7 @@
throws ActionExecutionException, InterruptedException {
actionExecutionContext
.getContext(SymlinkTreeActionContext.class)
- .createSymlinks(this, actionExecutionContext, enableRunfiles);
+ .createSymlinks(this, actionExecutionContext);
return ActionResult.EMPTY;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java
index e7e3280..21f43ec 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java
@@ -22,12 +22,7 @@
*/
public interface SymlinkTreeActionContext extends ActionContext {
- /**
- * Creates the symlink tree.
- */
- void createSymlinks(
- SymlinkTreeAction action,
- ActionExecutionContext actionExecutionContext,
- boolean enableRunfiles)
+ /** Creates the symlink tree. */
+ void createSymlinks(SymlinkTreeAction action, ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException;
}
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 37d6f2b..d96d408 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
@@ -16,8 +16,8 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.shell.Command;
import com.google.devtools.build.lib.shell.CommandException;
import com.google.devtools.build.lib.util.CommandBuilder;
@@ -61,27 +61,6 @@
}
/**
- * 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.
- *
- * <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, BinTools binTools, Map<String, String> shellEnvironment, OutErr outErr)
- throws CommandException {
- Command command = createCommand(execRoot, binTools, shellEnvironment);
- if (outErr != null) {
- command.execute(outErr.getOutputStream(), outErr.getErrorStream());
- } else {
- command.execute();
- }
- }
-
- /**
* Creates symlink tree and output manifest using the {@code build-runfiles.cc} tool.
*
* @param enableRunfiles If {@code false} only the output manifest is created.
@@ -94,19 +73,42 @@
boolean enableRunfiles)
throws ExecException {
if (enableRunfiles) {
- try {
- createSymlinksUsingCommand(execRoot, binTools, shellEnvironment, outErr);
- } catch (CommandException e) {
- throw new UserExecException(CommandUtils.describeCommandFailure(true, e), e);
- }
+ createSymlinksUsingCommand(execRoot, binTools, shellEnvironment, outErr);
} else {
- // Pretend we created the runfiles tree by copying the manifest
- try {
- symlinkTreeRoot.createDirectoryAndParents();
- FileSystemUtils.copyFile(inputManifest, symlinkTreeRoot.getChild("MANIFEST"));
- } catch (IOException e) {
- throw new UserExecException(e.getMessage(), e);
+ copyManifest();
+ }
+ }
+
+ /** Copies the input manifest to the output manifest. */
+ public void copyManifest() throws ExecException {
+ // Pretend we created the runfiles tree by copying the manifest
+ try {
+ symlinkTreeRoot.createDirectoryAndParents();
+ FileSystemUtils.copyFile(inputManifest, symlinkTreeRoot.getChild("MANIFEST"));
+ } catch (IOException e) {
+ throw new EnvironmentalExecException(e);
+ }
+ }
+
+ /**
+ * 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.
+ *
+ * <p>Warning: this method REALLY executes the command on the box Bazel is running on, without any
+ * kind of synchronization, locking, or anything else.
+ */
+ public void createSymlinksUsingCommand(
+ Path execRoot, BinTools binTools, Map<String, String> shellEnvironment, OutErr outErr)
+ throws EnvironmentalExecException {
+ Command command = createCommand(execRoot, binTools, shellEnvironment);
+ try {
+ if (outErr != null) {
+ command.execute(outErr.getOutputStream(), outErr.getErrorStream());
+ } else {
+ command.execute();
}
+ } catch (CommandException e) {
+ throw new EnvironmentalExecException(CommandUtils.describeCommandFailure(true, e), e);
}
}
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 5ed9764..bd1c7a8 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
@@ -44,9 +44,7 @@
@Override
public void createSymlinks(
- SymlinkTreeAction action,
- ActionExecutionContext actionExecutionContext,
- boolean enableRunfiles)
+ SymlinkTreeAction action, ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
actionExecutionContext.getEventHandler().post(new RunningActionEvent(action, "local"));
try (AutoProfiler p =
@@ -59,22 +57,17 @@
actionExecutionContext.getInputPath(action.getOutputManifest()),
action.isFilesetTree(),
action.getOutputManifest().getExecPath().getParentDirectory());
+ } else if (!action.enableRunfiles()) {
+ createSymlinkTreeHelper(action, actionExecutionContext).copyManifest();
} else {
Map<String, String> resolvedEnv = new LinkedHashMap<>();
action.getEnvironment().resolve(resolvedEnv, actionExecutionContext.getClientEnv());
- SymlinkTreeHelper helper =
- new SymlinkTreeHelper(
- actionExecutionContext.getInputPath(action.getInputManifest()),
- actionExecutionContext
- .getInputPath(action.getOutputManifest())
- .getParentDirectory(),
- action.isFilesetTree());
- helper.createSymlinks(
- actionExecutionContext.getExecRoot(),
- actionExecutionContext.getFileOutErr(),
- binTools,
- resolvedEnv,
- enableRunfiles);
+ createSymlinkTreeHelper(action, actionExecutionContext)
+ .createSymlinksUsingCommand(
+ actionExecutionContext.getExecRoot(),
+ binTools,
+ resolvedEnv,
+ actionExecutionContext.getFileOutErr());
}
} catch (ExecException e) {
throw e.toActionExecutionException(
@@ -82,4 +75,12 @@
}
}
}
+
+ private static SymlinkTreeHelper createSymlinkTreeHelper(
+ SymlinkTreeAction action, ActionExecutionContext actionExecutionContext) {
+ return new SymlinkTreeHelper(
+ actionExecutionContext.getInputPath(action.getInputManifest()),
+ actionExecutionContext.getInputPath(action.getOutputManifest()).getParentDirectory(),
+ action.isFilesetTree());
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index 3a89854..1256a20 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
@@ -66,7 +67,6 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.server.CommandProtos.EnvironmentVariable;
import com.google.devtools.build.lib.server.CommandProtos.ExecRequest;
-import com.google.devtools.build.lib.shell.CommandException;
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.util.CommandDescriptionForm;
import com.google.devtools.build.lib.util.CommandFailureUtils;
@@ -389,7 +389,7 @@
runfilesDir = ensureRunfilesBuilt(env, runfilesSupport,
env.getSkyframeExecutor().getConfiguration(env.getReporter(),
targetToRun.getConfigurationKey()));
- } catch (CommandException e) {
+ } catch (EnvironmentalExecException e) {
env.getReporter().handle(Event.error("Error creating runfiles: " + e.getMessage()));
return BlazeCommandResult.exitCode(ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
}
@@ -567,11 +567,12 @@
}
/**
- * Ensures that runfiles are built for the specified target. If they already
- * are, does nothing, otherwise builds them.
+ * Ensures that runfiles are built for the specified target. If they already are, does nothing,
+ * otherwise builds them.
*/
- private Path ensureRunfilesBuilt(CommandEnvironment env, RunfilesSupport runfilesSupport,
- BuildConfiguration configuration) throws CommandException {
+ private static Path ensureRunfilesBuilt(
+ CommandEnvironment env, RunfilesSupport runfilesSupport, BuildConfiguration configuration)
+ throws EnvironmentalExecException {
Artifact manifest = Preconditions.checkNotNull(runfilesSupport.getRunfilesManifest());
PathFragment runfilesDir = runfilesSupport.getRunfilesDirectoryExecPath();
Path workingDir = env.getExecRoot().getRelative(runfilesDir);
