Fix #757: Bazel does not copy xml test output from sandbox.
--
MOS_MIGRATED_REVID=112404257
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
index 0a09555..8e34389 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
@@ -17,6 +17,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
import com.google.devtools.build.lib.actions.extra.SpawnInfo;
@@ -41,6 +42,7 @@
private final ImmutableMap<String, String> environment;
private final ImmutableMap<String, String> executionInfo;
private final ImmutableMap<PathFragment, Artifact> runfilesManifests;
+ private final ImmutableSet<PathFragment> optionalOutputFiles;
private final RunfilesSupplier runfilesSupplier;
private final ActionMetadata action;
private final ResourceSet localResources;
@@ -49,13 +51,15 @@
// policy on runfilesManifests and runfilesSupplier being non-empty (ie: are overlapping mappings
// allowed?).
@VisibleForTesting
- BaseSpawn(List<String> arguments,
+ BaseSpawn(
+ List<String> arguments,
Map<String, String> environment,
Map<String, String> executionInfo,
Map<PathFragment, Artifact> runfilesManifests,
RunfilesSupplier runfilesSupplier,
ActionMetadata action,
- ResourceSet localResources) {
+ ResourceSet localResources,
+ Collection<PathFragment> optionalOutputFiles) {
this.arguments = ImmutableList.copyOf(arguments);
this.environment = ImmutableMap.copyOf(environment);
this.executionInfo = ImmutableMap.copyOf(executionInfo);
@@ -63,6 +67,7 @@
this.runfilesSupplier = runfilesSupplier;
this.action = action;
this.localResources = localResources;
+ this.optionalOutputFiles = ImmutableSet.copyOf(optionalOutputFiles);
}
/**
@@ -75,8 +80,15 @@
RunfilesSupplier runfilesSupplier,
ActionMetadata action,
ResourceSet localResources) {
- this(arguments, environment, executionInfo, ImmutableMap.<PathFragment, Artifact>of(),
- runfilesSupplier, action, localResources);
+ this(
+ arguments,
+ environment,
+ executionInfo,
+ ImmutableMap.<PathFragment, Artifact>of(),
+ runfilesSupplier,
+ action,
+ localResources,
+ ImmutableSet.<PathFragment>of());
}
/**
@@ -89,8 +101,15 @@
Map<PathFragment, Artifact> runfilesManifests,
ActionMetadata action,
ResourceSet localResources) {
- this(arguments, environment, executionInfo, runfilesManifests, EmptyRunfilesSupplier.INSTANCE,
- action, localResources);
+ this(
+ arguments,
+ environment,
+ executionInfo,
+ runfilesManifests,
+ EmptyRunfilesSupplier.INSTANCE,
+ action,
+ localResources,
+ ImmutableSet.<PathFragment>of());
}
/**
@@ -101,8 +120,32 @@
Map<String, String> executionInfo,
ActionMetadata action,
ResourceSet localResources) {
- this(arguments, environment, executionInfo,
- ImmutableMap.<PathFragment, Artifact>of(), action, localResources);
+ this(
+ arguments,
+ environment,
+ executionInfo,
+ ImmutableMap.<PathFragment, Artifact>of(),
+ action,
+ localResources);
+ }
+
+ public BaseSpawn(
+ List<String> arguments,
+ Map<String, String> environment,
+ Map<String, String> executionInfo,
+ RunfilesSupplier runfilesSupplier,
+ ActionMetadata action,
+ ResourceSet localResources,
+ Collection<PathFragment> optionalOutputFiles) {
+ this(
+ arguments,
+ environment,
+ executionInfo,
+ ImmutableMap.<PathFragment, Artifact>of(),
+ runfilesSupplier,
+ action,
+ localResources,
+ optionalOutputFiles);
}
public static PathFragment runfilesForFragment(PathFragment pathFragment) {
@@ -210,6 +253,11 @@
}
@Override
+ public Collection<PathFragment> getOptionalOutputFiles() {
+ return optionalOutputFiles;
+ }
+
+ @Override
public ActionMetadata getResourceOwner() {
return action;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
index 2c0a0ad..acb9cd6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
@@ -95,6 +95,11 @@
}
@Override
+ public Collection<PathFragment> getOptionalOutputFiles() {
+ return spawn.getOptionalOutputFiles();
+ }
+
+ @Override
public ActionMetadata getResourceOwner() {
return spawn.getResourceOwner();
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
index d027f70..b4ed5c7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
@@ -121,6 +121,12 @@
Collection<? extends ActionInput> getOutputFiles();
/**
+ * Instructs the spawn strategy to try to fetch these optional output files in addition to the
+ * usual output artifacts. The PathFragments should be relative to the exec root.
+ */
+ Collection<PathFragment> getOptionalOutputFiles();
+
+ /**
* Returns the resource owner for local fallback.
*/
ActionMetadata getResourceOwner();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
index 5d7739f..bb5ffa7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.test;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.BaseSpawn;
@@ -79,8 +80,8 @@
.getChild(getTmpDirName(action.getExecutionSettings().getExecutable().getExecPath()));
Path workingDirectory = runfilesDir.getRelative(action.getRunfilesPrefix());
- TestRunnerAction.ResolvedPaths resolvedPaths =
- action.resolve(actionExecutionContext.getExecutor().getExecRoot());
+ Path execRoot = actionExecutionContext.getExecutor().getExecRoot();
+ TestRunnerAction.ResolvedPaths resolvedPaths = action.resolve(execRoot);
Map<String, String> env = getEnv(action, runfilesDir, testTmpDir, resolvedPaths);
Map<String, String> info = new HashMap<>();
@@ -100,9 +101,8 @@
new RunfilesSupplierImpl(
runfilesDir.asFragment(), action.getExecutionSettings().getRunfiles()),
action,
- action
- .getTestProperties()
- .getLocalResourceUsage(executionOptions.usingLocalTestJobs()));
+ action.getTestProperties().getLocalResourceUsage(executionOptions.usingLocalTestJobs()),
+ ImmutableSet.of(resolvedPaths.getXmlOutputPath().relativeTo(execRoot)));
Executor executor = actionExecutionContext.getExecutor();
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
index eea7f17..132b457 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -130,6 +130,15 @@
int timeout = getTimeout(spawn);
+ ImmutableSet.Builder<PathFragment> outputFiles = ImmutableSet.<PathFragment>builder();
+ for (PathFragment optionalOutput : spawn.getOptionalOutputFiles()) {
+ Preconditions.checkArgument(!optionalOutput.isAbsolute());
+ outputFiles.add(optionalOutput);
+ }
+ for (ActionInput output : spawn.getOutputFiles()) {
+ outputFiles.add(new PathFragment(output.getExecPathString()));
+ }
+
try {
final NamespaceSandboxRunner runner =
new NamespaceSandboxRunner(
@@ -140,7 +149,7 @@
spawn.getEnvironment(),
execRoot.getPathFile(),
outErr,
- spawn.getOutputFiles(),
+ outputFiles.build(),
timeout,
!spawn.getExecutionInfo().containsKey("requires-network"));
} finally {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java
index 7fae775..239d3ac 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java
@@ -18,7 +18,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.io.ByteStreams;
import com.google.common.io.Files;
-import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.config.BinTools;
@@ -115,6 +114,7 @@
* @param env - environment to run sandbox in
* @param cwd - current working directory
* @param outErr - error output to capture sandbox's and command's stderr
+ * @param outputs - files to extract from the sandbox, paths are relative to the exec root
* @throws ExecException
*/
public void run(
@@ -122,7 +122,7 @@
ImmutableMap<String, String> env,
File cwd,
FileOutErr outErr,
- Collection<? extends ActionInput> outputs,
+ Collection<PathFragment> outputs,
int timeout,
boolean blockNetwork)
throws IOException, ExecException {
@@ -205,21 +205,20 @@
copyOutputs(outputs);
}
- private void createFileSystem(Collection<? extends ActionInput> outputs) throws IOException {
+ private void createFileSystem(Collection<PathFragment> outputs) throws IOException {
FileSystemUtils.createDirectoryAndParents(sandboxPath);
// Prepare the output directories in the sandbox.
- for (ActionInput output : outputs) {
- PathFragment parentDirectory =
- new PathFragment(output.getExecPathString()).getParentDirectory();
- FileSystemUtils.createDirectoryAndParents(sandboxExecRoot.getRelative(parentDirectory));
+ for (PathFragment output : outputs) {
+ FileSystemUtils.createDirectoryAndParents(
+ sandboxExecRoot.getRelative(output.getParentDirectory()));
}
}
- private void copyOutputs(Collection<? extends ActionInput> outputs) throws IOException {
- for (ActionInput output : outputs) {
- Path source = sandboxExecRoot.getRelative(output.getExecPathString());
- Path target = execRoot.getRelative(output.getExecPathString());
+ private void copyOutputs(Collection<PathFragment> outputs) throws IOException {
+ for (PathFragment output : outputs) {
+ Path source = sandboxExecRoot.getRelative(output);
+ Path target = execRoot.getRelative(output);
FileSystemUtils.createDirectoryAndParents(target.getParentDirectory());
if (source.isFile() || source.isSymbolicLink()) {
Files.move(source.getPathFile(), target.getPathFile());
