diff options
5 files changed, 131 insertions, 12 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index 9f9e3351b4..ec006b9bbd 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -49,6 +49,16 @@ public class ExecutionOptions extends OptionsBase { public static final ExecutionOptions DEFAULTS = Options.getDefaults(ExecutionOptions.class); @Option( + name = "materialize_param_files", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "Writes intermediate parameter files to output tree even when using " + + "remote action execution. Useful when debugging actions. ") + public boolean materializeParamFiles; + + @Option( name = "verbose_failures", defaultValue = "false", documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index 4bcbb74ff1..e86b41ec6f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java @@ -80,6 +80,7 @@ final class RemoteActionContextProvider extends ActionContextProvider { new RemoteSpawnRunner( env.getExecRoot(), remoteOptions, + env.getOptions().getOptions(ExecutionOptions.class), createFallbackRunner(env), executionOptions.verboseFailures, env.getReporter(), diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 5806b67660..1198c2f5d8 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.MetadataProvider; @@ -35,6 +36,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.remote.Retrier.RetryException; @@ -59,6 +61,7 @@ import com.google.protobuf.TextFormat.ParseException; import io.grpc.Context; import io.grpc.Status.Code; import java.io.IOException; +import java.io.OutputStream; import java.time.Duration; import java.util.ArrayList; import java.util.Collection; @@ -77,7 +80,8 @@ class RemoteSpawnRunner implements SpawnRunner { private static final int POSIX_TIMEOUT_EXIT_CODE = /*SIGNAL_BASE=*/128 + /*SIGALRM=*/14; private final Path execRoot; - private final RemoteOptions options; + private final RemoteOptions remoteOptions; + private final ExecutionOptions executionOptions; private final SpawnRunner fallbackRunner; private final boolean verboseFailures; @@ -94,7 +98,8 @@ class RemoteSpawnRunner implements SpawnRunner { RemoteSpawnRunner( Path execRoot, - RemoteOptions options, + RemoteOptions remoteOptions, + ExecutionOptions executionOptions, SpawnRunner fallbackRunner, boolean verboseFailures, @Nullable Reporter cmdlineReporter, @@ -105,7 +110,8 @@ class RemoteSpawnRunner implements SpawnRunner { DigestUtil digestUtil, Path logDir) { this.execRoot = execRoot; - this.options = options; + this.remoteOptions = remoteOptions; + this.executionOptions = executionOptions; this.fallbackRunner = fallbackRunner; this.remoteCache = remoteCache; this.remoteExecutor = remoteExecutor; @@ -136,6 +142,7 @@ class RemoteSpawnRunner implements SpawnRunner { SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); + maybeWriteParamFilesLocally(spawn); Command command = buildCommand(spawn.getArguments(), spawn.getEnvironment()); Action action = buildAction( @@ -152,8 +159,8 @@ class RemoteSpawnRunner implements SpawnRunner { TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey); Context previous = withMetadata.attach(); try { - boolean acceptCachedResult = options.remoteAcceptCached && Spawns.mayBeCached(spawn); - boolean uploadLocalResults = options.remoteUploadLocalResults; + boolean acceptCachedResult = remoteOptions.remoteAcceptCached && Spawns.mayBeCached(spawn); + boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults; try { // Try to lookup the action in the action cache. @@ -199,7 +206,7 @@ class RemoteSpawnRunner implements SpawnRunner { try { ExecuteRequest.Builder request = ExecuteRequest.newBuilder() - .setInstanceName(options.remoteInstanceName) + .setInstanceName(remoteOptions.remoteInstanceName) .setAction(action) .setSkipCacheLookup(!acceptCachedResult); ExecuteResponse reply = remoteExecutor.executeRemotely(request.build()); @@ -223,6 +230,25 @@ class RemoteSpawnRunner implements SpawnRunner { } } + private void maybeWriteParamFilesLocally(Spawn spawn) throws IOException { + if (!executionOptions.materializeParamFiles) { + return; + } + for (ActionInput actionInput : spawn.getInputFiles()) { + if (actionInput instanceof ParamFileActionInput) { + ParamFileActionInput paramFileActionInput = (ParamFileActionInput) actionInput; + Path outputPath = execRoot.getRelative(paramFileActionInput.getExecPath()); + if (outputPath.exists()) { + outputPath.delete(); + } + outputPath.getParentDirectory().createDirectoryAndParents(); + try (OutputStream out = outputPath.getOutputStream()) { + paramFileActionInput.writeTo(out); + } + } + } + } + private void maybeDownloadServerLogs(ExecuteResponse resp, ActionKey actionKey) throws InterruptedException { ActionResult result = resp.getResult(); @@ -271,7 +297,7 @@ class RemoteSpawnRunner implements SpawnRunner { if (Thread.currentThread().isInterrupted()) { throw new InterruptedException(); } - if (options.remoteLocalFallback + if (remoteOptions.remoteLocalFallback && !(cause instanceof RetryException && RemoteRetrierUtils.causedByExecTimeout((RetryException) cause))) { return execLocally(spawn, context, inputMap, uploadLocalResults, remoteCache, actionKey); diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java index 5bb5f6ab3c..89962f173b 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.analysis.BlazeVersionInfo; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.authandtls.GoogleAuthUtils; import com.google.devtools.build.lib.clock.JavaClock; +import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; @@ -247,21 +248,25 @@ public class GrpcRemoteExecutionClientTest { FileSystemUtils.createDirectoryAndParents(stdout.getParentDirectory()); FileSystemUtils.createDirectoryAndParents(stderr.getParentDirectory()); outErr = new FileOutErr(stdout, stderr); - RemoteOptions options = Options.getDefaults(RemoteOptions.class); + RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); RemoteRetrier retrier = new RemoteRetrier( - options, RemoteRetrier.RETRIABLE_GRPC_ERRORS, retryService, Retrier.ALLOW_ALL_CALLS); + remoteOptions, + RemoteRetrier.RETRIABLE_GRPC_ERRORS, + retryService, + Retrier.ALLOW_ALL_CALLS); Channel channel = InProcessChannelBuilder.forName(fakeServerName).directExecutor().build(); GrpcRemoteExecutor executor = - new GrpcRemoteExecutor(channel, null, options.remoteTimeout, retrier); + new GrpcRemoteExecutor(channel, null, remoteOptions.remoteTimeout, retrier); CallCredentials creds = GoogleAuthUtils.newCallCredentials(Options.getDefaults(AuthAndTLSOptions.class)); GrpcRemoteCache remoteCache = - new GrpcRemoteCache(channel, creds, options, retrier, DIGEST_UTIL); + new GrpcRemoteCache(channel, creds, remoteOptions, retrier, DIGEST_UTIL); client = new RemoteSpawnRunner( execRoot, - options, + remoteOptions, + Options.getDefaults(ExecutionOptions.class), null, true, /*cmdlineReporter=*/ null, diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index 0b6e052c00..9eead3059d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.remote; import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.ISO_8859_1; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; @@ -28,12 +29,15 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.eventbus.EventBus; +import com.google.common.io.ByteStreams; import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.actions.ActionInput; 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.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.MetadataProvider; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.Spawn; @@ -44,6 +48,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner; @@ -69,6 +74,8 @@ import com.google.devtools.remoteexecution.v1test.LogFile; import com.google.protobuf.ByteString; import com.google.rpc.Code; import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Collection; import java.util.SortedMap; @@ -139,6 +146,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -198,6 +206,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -251,6 +260,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -300,6 +310,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -335,6 +346,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, false, reporter, @@ -392,6 +404,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -432,6 +445,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -469,6 +483,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -504,6 +519,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, Options.getDefaults(RemoteOptions.class), + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -544,6 +560,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, Options.getDefaults(RemoteOptions.class), + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -587,6 +604,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, Options.getDefaults(RemoteOptions.class), + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -625,6 +643,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, Options.getDefaults(RemoteOptions.class), + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -667,6 +686,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -709,6 +729,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -757,6 +778,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -803,6 +825,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -844,6 +867,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -881,6 +905,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner( execRoot, options, + Options.getDefaults(ExecutionOptions.class), localRunner, true, /*cmdlineReporter=*/ null, @@ -905,6 +930,58 @@ public class RemoteSpawnRunnerTest { } } + @Test + public void testMaterializeParamFiles() throws Exception { + ExecutionOptions executionOptions = + Options.parse(ExecutionOptions.class, "--materialize_param_files").getOptions(); + executionOptions.materializeParamFiles = true; + RemoteSpawnRunner runner = + new RemoteSpawnRunner( + execRoot, + Options.getDefaults(RemoteOptions.class), + executionOptions, + localRunner, + true, + /*cmdlineReporter=*/ null, + "build-req-id", + "command-id", + cache, + executor, + digestUtil, + logDir); + + ExecuteResponse succeeded = + ExecuteResponse.newBuilder() + .setResult(ActionResult.newBuilder().setExitCode(0).build()) + .build(); + when(executor.executeRemotely(any(ExecuteRequest.class))).thenReturn(succeeded); + + ImmutableList<String> args = ImmutableList.of("--foo", "--bar"); + ParamFileActionInput input = + new ParamFileActionInput( + PathFragment.create("out/param_file"), args, ParameterFileType.UNQUOTED, ISO_8859_1); + Spawn spawn = + new SimpleSpawn( + new FakeOwner("foo", "bar"), + /*arguments=*/ ImmutableList.of(), + /*environment=*/ ImmutableMap.of(), + /*executionInfo=*/ ImmutableMap.of(), + ImmutableList.of(input), + /*outputs=*/ ImmutableList.<ActionInput>of(), + ResourceSet.ZERO); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); + SpawnResult res = runner.exec(spawn, policy); + assertThat(res.status()).isEqualTo(Status.SUCCESS); + Path paramFile = execRoot.getRelative("out/param_file"); + assertThat(paramFile.exists()).isTrue(); + try (InputStream inputStream = paramFile.getInputStream()) { + assertThat( + new String(ByteStreams.toByteArray(inputStream), StandardCharsets.UTF_8).split("\n")) + .asList() + .containsExactly("--foo", "--bar"); + } + } + private static Spawn newSimpleSpawn() { return new SimpleSpawn( new FakeOwner("foo", "bar"), |