diff options
3 files changed, 89 insertions, 10 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java index fef7c2d31f..2e29fab1a4 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.collect.IterablesChain; import com.google.devtools.build.lib.util.Fingerprint; @@ -24,6 +25,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.ByteString; import java.io.IOException; import java.io.OutputStream; +import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -167,7 +169,12 @@ public class CommandLines { .replaceFirst("%s", paramFileExecPath.getPathString()); arguments.addElement(paramArg); cmdLineLength += paramArg.length() + 1; - paramFiles.add(new ParamFileActionInput(paramFileExecPath, args, paramFileInfo)); + paramFiles.add( + new ParamFileActionInput( + paramFileExecPath, + args, + paramFileInfo.getFileType(), + paramFileInfo.getCharset())); } } } @@ -221,22 +228,27 @@ public class CommandLines { } } - static final class ParamFileActionInput implements VirtualActionInput { + /** An in-memory param file virtual action input. */ + public static final class ParamFileActionInput implements VirtualActionInput { final PathFragment paramFileExecPath; final Iterable<String> arguments; - final ParamFileInfo paramFileInfo; + final ParameterFileType type; + final Charset charset; - ParamFileActionInput( - PathFragment paramFileExecPath, Iterable<String> arguments, ParamFileInfo paramFileInfo) { + public ParamFileActionInput( + PathFragment paramFileExecPath, + Iterable<String> arguments, + ParameterFileType type, + Charset charset) { this.paramFileExecPath = paramFileExecPath; this.arguments = arguments; - this.paramFileInfo = paramFileInfo; + this.type = type; + this.charset = charset; } @Override public void writeTo(OutputStream out) throws IOException { - ParameterFile.writeParameterFile( - out, arguments, paramFileInfo.getFileType(), paramFileInfo.getCharset()); + ParameterFile.writeParameterFile(out, arguments, type, charset); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java index e22da3ef85..cd4075f6bb 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java @@ -22,12 +22,15 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.io.ByteStreams; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.Spawns; +import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; @@ -241,6 +244,20 @@ public class LocalSpawnRunner implements SpawnRunner { context.prefetchInputs(); } + for (ActionInput input : spawn.getInputFiles()) { + if (input instanceof ParamFileActionInput) { + VirtualActionInput virtualActionInput = (VirtualActionInput) input; + Path outputPath = execRoot.getRelative(virtualActionInput.getExecPath()); + if (outputPath.exists()) { + outputPath.delete(); + } + outputPath.getParentDirectory().createDirectoryAndParents(); + try (OutputStream outputStream = outputPath.getOutputStream()) { + virtualActionInput.writeTo(outputStream); + } + } + } + stepLog(INFO, "running locally"); setState(State.LOCAL_ACTION_RUNNING); diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java index dea067dd79..5763f91eac 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.exec.local; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; import static org.mockito.Matchers.any; @@ -31,8 +32,10 @@ import com.google.common.io.Files; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.LocalHostCapacity; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.Spawn; @@ -62,7 +65,6 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.ArrayList; import java.util.List; @@ -340,6 +342,54 @@ public class LocalSpawnRunnerTest { } @Test + public void testParamFiles() throws Exception { + // TODO(#3536): Make this test work on Windows. + // The Command API implicitly absolutizes the path, and we get weird paths on Windows: + // T:\execroot\execroot\_bin\process-wrapper + assumeTrue(OS.getCurrent() != OS.WINDOWS); + + FileSystem fs = setupEnvironmentForFakeExecution(); + + SubprocessFactory factory = mock(SubprocessFactory.class); + when(factory.create(any())).thenReturn(new FinishedSubprocess(0)); + SubprocessBuilder.setSubprocessFactory(factory); + + LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class); + options.localSigkillGraceSeconds = 456; + Path execRoot = fs.getPath("/execroot"); + LocalSpawnRunner runner = + new TestedLocalSpawnRunner( + execRoot, options, resourceManager, USE_WRAPPER, OS.LINUX, LocalEnvProvider.UNMODIFIED); + ParamFileActionInput paramFileActionInput = + new ParamFileActionInput( + PathFragment.create("some/dir/params"), + ImmutableList.of("--foo", "--bar"), + ParameterFileType.UNQUOTED, + UTF_8); + Spawn spawn = + new SpawnBuilder("/bin/echo", "Hi!") + .withInput(paramFileActionInput) + .withEnvironment("VARIABLE", "value") + .build(); + FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); + policy.timeoutMillis = 123 * 1000L; + assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); + SpawnResult result = runner.exec(spawn, policy); + assertThat(result.status()).isEqualTo(SpawnResult.Status.SUCCESS); + assertThat(result.exitCode()).isEqualTo(0); + assertThat(result.setupSuccess()).isTrue(); + assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName()); + Path paramFile = execRoot.getRelative("some/dir/params"); + assertThat(paramFile.exists()).isTrue(); + try (InputStream inputStream = paramFile.getInputStream()) { + assertThat(new String(ByteStreams.toByteArray(inputStream), UTF_8).split("\n")) + .asList() + .containsExactly("--foo", "--bar"); + } + } + + @Test public void noProcessWrapper() throws Exception { // TODO(#3536): Make this test work on Windows. // The Command API implicitly absolutizes the path, and we get weird paths on Windows: @@ -467,7 +517,7 @@ public class LocalSpawnRunnerTest { assertThat(result.getSystemTime()).isEmpty(); assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName()); - assertThat(FileSystemUtils.readContent(fs.getPath("/out/stderr"), StandardCharsets.UTF_8)) + assertThat(FileSystemUtils.readContent(fs.getPath("/out/stderr"), UTF_8)) .isEqualTo("Action failed to execute: java.io.IOException: I'm sorry, Dave\n"); assertThat(policy.lockOutputFilesCalled).isTrue(); |