diff options
8 files changed, 121 insertions, 26 deletions
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 0a0955538e..8e34389185 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 @@ package com.google.devtools.build.lib.actions; 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 @@ public class BaseSpawn implements Spawn { 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 @@ public class BaseSpawn implements Spawn { // 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 @@ public class BaseSpawn implements Spawn { this.runfilesSupplier = runfilesSupplier; this.action = action; this.localResources = localResources; + this.optionalOutputFiles = ImmutableSet.copyOf(optionalOutputFiles); } /** @@ -75,8 +80,15 @@ public class BaseSpawn implements Spawn { 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 @@ public class BaseSpawn implements Spawn { 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 @@ public class BaseSpawn implements Spawn { 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 @@ public class BaseSpawn implements Spawn { } @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 2c0a0ad5ca..acb9cd64d4 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 @@ public class DelegateSpawn implements Spawn { } @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 d027f70570..b4ed5c79fa 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 @@ public interface Spawn { 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 5d7739f664..bb5ffa7cb9 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 @@ public class StandaloneTestStrategy extends TestStrategy { .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 @@ public class StandaloneTestStrategy extends TestStrategy { 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 eea7f170b1..132b457365 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 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { 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 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { 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 7fae775e33..239d3acef6 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.ImmutableMap; 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 @@ public class NamespaceSandboxRunner { * @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 @@ public class NamespaceSandboxRunner { 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 @@ public class NamespaceSandboxRunner { 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()); diff --git a/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java b/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java index 72fc7e9d25..07abe5283a 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl; import com.google.devtools.build.lib.testutil.Scratch; @@ -130,7 +131,9 @@ public class BaseSpawnTest { ImmutableMap.<String, String>of(), runfilesManifests, runfilesSupplier, - null, null); + null, + null, + ImmutableSet.<PathFragment>of()); } private static Artifact mkArtifact(String path, Root rootDir) { diff --git a/src/test/shell/bazel/bazel_test_test.sh b/src/test/shell/bazel/bazel_test_test.sh index 9e7f609fe8..92c57e8c4b 100755 --- a/src/test/shell/bazel/bazel_test_test.sh +++ b/src/test/shell/bazel/bazel_test_test.sh @@ -243,4 +243,29 @@ EOF done } +# Tests that the test.xml is extracted from the sandbox correctly. +function test_xml_is_present() { + mkdir -p dir + + cat <<'EOF' > dir/test.sh +#!/bin/sh +echo HELLO > $XML_OUTPUT_FILE +exit 0 +EOF + + chmod +x dir/test.sh + + cat <<'EOF' > dir/BUILD + sh_test( + name = "test", + srcs = [ "test.sh" ], + ) +EOF + + bazel test -s --test_output=streamed //dir:test &> $TEST_log || fail "expected success" + + xml_log=bazel-testlogs/dir/test/test.xml + [ -s $xml_log ] || fail "$xml_log was not present after test" +} + run_suite "test tests" |