diff options
author | 2018-04-19 05:41:44 -0700 | |
---|---|---|
committer | 2018-04-19 05:42:58 -0700 | |
commit | 29e306d66cc4c441f0321a62101077647a4c95dc (patch) | |
tree | cabcf23c53c9cdcfe501bc5f17531f73e81c5a30 /src | |
parent | 9566f677a1093e3a3c0ddaed3f9ab34dd98e5e26 (diff) |
Rename SpawnExecutionPolicy -> SpawnExecutionContext.
This class will be used to tie a Spawn to a SpawnRunner, and isn't really a policy object. It will carry state such as the expanded inputs and expanded command line.
Currently a context can be passed between different SpawnRunners. This will be addressed independently, so a context is tied to a particular spawn runner.
PiperOrigin-RevId: 193501918
Diffstat (limited to 'src')
18 files changed, 186 insertions, 191 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 0a0a2a811b..fee7f52e6b 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -33,7 +33,7 @@ import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.exec.SpawnCache.CacheHandle; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; import com.google.devtools.build.lib.util.CommandFailureUtils; import com.google.devtools.build.lib.util.io.FileOutErr; @@ -74,9 +74,8 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte actionExecutionContext.reportSubcommand(spawn); } final Duration timeout = Spawns.getTimeout(spawn); - SpawnExecutionPolicy policy = - new SpawnExecutionPolicyImpl( - spawn, actionExecutionContext, writeOutputFiles, timeout); + SpawnExecutionContext context = + new SpawnExecutionContextImpl(spawn, actionExecutionContext, writeOutputFiles, timeout); // TODO(ulfjack): Provide a way to disable the cache. We don't want the RemoteSpawnStrategy to // check the cache twice. Right now that can't happen because this is hidden behind an // experimental flag. @@ -88,12 +87,12 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte } SpawnResult spawnResult; try { - try (CacheHandle cacheHandle = cache.lookup(spawn, policy)) { + try (CacheHandle cacheHandle = cache.lookup(spawn, context)) { if (cacheHandle.hasResult()) { spawnResult = Preconditions.checkNotNull(cacheHandle.getResult()); } else { // Actual execution. - spawnResult = spawnRunner.exec(spawn, policy); + spawnResult = spawnRunner.exec(spawn, context); if (cacheHandle.willStore()) { cacheHandle.store( spawnResult, listExistingOutputFiles(spawn, actionExecutionContext.getExecRoot())); @@ -133,7 +132,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte return outputFiles; } - private final class SpawnExecutionPolicyImpl implements SpawnExecutionPolicy { + private final class SpawnExecutionContextImpl implements SpawnExecutionContext { private final Spawn spawn; private final ActionExecutionContext actionExecutionContext; private final AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles; @@ -144,7 +143,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte // TODO(ulfjack): Guard against client modification of this map. private SortedMap<PathFragment, ActionInput> lazyInputMapping; - public SpawnExecutionPolicyImpl( + public SpawnExecutionContextImpl( Spawn spawn, ActionExecutionContext actionExecutionContext, AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles, diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java index 1374b47fd8..afc9d9376b 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java @@ -18,7 +18,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.vfs.Path; import java.io.Closeable; import java.io.IOException; @@ -99,7 +99,7 @@ public interface SpawnCache extends ActionContext { ) public static class NoSpawnCache implements SpawnCache { @Override - public CacheHandle lookup(Spawn spawn, SpawnExecutionPolicy context) { + public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) { return SpawnCache.NO_RESULT_NO_STORE; } } @@ -156,12 +156,12 @@ public interface SpawnCache extends ActionContext { * Perform a spawn lookup. This method is similar to {@link SpawnRunner#exec}, taking the same * parameters and being allowed to throw the same exceptions. The intent for this method is to * compute a cache lookup key for the given spawn, looking it up in an implementation-dependent - * cache (can be either on the local or remote machine), and returning a non-null - * {@link CacheHandle} instance. + * cache (can be either on the local or remote machine), and returning a non-null {@link + * CacheHandle} instance. * * <p>If the lookup was successful, this method should write the cached outputs to their * corresponding output locations in the output tree, as well as stdout and stderr, after - * notifying {@link SpawnExecutionPolicy#lockOutputFiles}. + * notifying {@link SpawnExecutionContext#lockOutputFiles}. * * <p>If the lookup was unsuccessful, this method can return a {@link CacheHandle} instance that * has no result, but uploads the results of the execution to the cache. The reason for a callback @@ -173,6 +173,6 @@ public interface SpawnCache extends ActionContext { * <p>Note that cache stores may be disabled, in which case the returned {@link CacheHandle} * instance's {@link CacheHandle#store} is a no-op. */ - CacheHandle lookup(Spawn spawn, SpawnExecutionPolicy context) + CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index 36346a556d..5ed43cd14e 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -30,6 +30,7 @@ import java.util.SortedMap; * or without sandboxing, on a remote machine, or only consult a remote cache. * * <h2>Environment Variables</h2> + * * <ul> * <li>Implementations MUST set the specified environment variables. * <li>Implementations MAY add TMPDIR as an additional env variable, if it is not set already. @@ -38,12 +39,14 @@ import java.util.SortedMap; * </ul> * * <h2>Command line</h2> + * * <ul> * <li>Implementations MUST use the specified command line unmodified by default. * <li>Implementations MAY modify the specified command line if explicitly requested by the user. * </ul> * * <h2>Process</h2> + * * <ul> * <li>Implementations MUST be thread-safe. * <li>Implementations MUST ensure that all child processes (including transitive) exit in all @@ -53,30 +56,32 @@ import java.util.SortedMap; * <li>Implementations MUST be interruptible; they MUST throw {@link InterruptedException} from * {@link #exec} when interrupted * <li>Implementations MUST apply the specified timeout to the execution of the subprocess - * <ul> - * <li>If no timeout is specified, the implementation MAY apply an implementation-specific - * timeout - * <li>If the specified timeout is larger than an implementation-dependent maximum, then the - * implementation MUST throw {@link IllegalArgumentException}; it MUST not silently change - * the timeout to a smaller value - * <li>If the timeout is exceeded, the implementation MUST throw TimeoutException, with the - * timeout that was applied to the subprocess (TODO) - * </ul> + * <ul> + * <li>If no timeout is specified, the implementation MAY apply an implementation-specific + * timeout + * <li>If the specified timeout is larger than an implementation-dependent maximum, then the + * implementation MUST throw {@link IllegalArgumentException}; it MUST not silently + * change the timeout to a smaller value + * <li>If the timeout is exceeded, the implementation MUST throw TimeoutException, with the + * timeout that was applied to the subprocess (TODO) + * </ul> * </ul> * * <h2>Optimistic Concurrency</h2> + * * Bazel may choose to execute a spawn using multiple {@link SpawnRunner} implementations * simultaneously in order to minimize total latency. This is especially useful for builds with few * actions where remotely executing the actions incurs high round trip times. + * * <ul> - * <li>All implementations MUST call {@link SpawnExecutionPolicy#lockOutputFiles} before writing + * <li>All implementations MUST call {@link SpawnExecutionContext#lockOutputFiles} before writing * to any of the output files, but may write to stdout and stderr without calling it. Instead, * all callers must provide temporary locations for stdout & stderr if they ever call multiple * {@link SpawnRunner} implementations concurrently. Spawn runners that use the local machine * MUST either call it before starting the subprocess, or ensure that subprocesses write to * temporary locations (for example by running in a mount namespace) and then copy or move the * outputs into place. - * <li>Implementations SHOULD delay calling {@link SpawnExecutionPolicy#lockOutputFiles} until + * <li>Implementations SHOULD delay calling {@link SpawnExecutionContext#lockOutputFiles} until * just before writing. * </ul> */ @@ -113,15 +118,15 @@ public interface SpawnRunner { } /** - * A helper class to provide additional tools and methods to {@link SpawnRunner} implementations. + * A context that binds a {@link Spawn} to a {@link SpawnRunner}. * * <p>This interface may change without notice. * - * <p>Implementations must be at least thread-compatible, i.e., they must be safe as long as - * each instance is only used within a single thread. Different instances of the same class may - * be used by different threads, so they MUST not call any shared non-thread-safe objects. + * <p>Implementations must be at least thread-compatible, i.e., they must be safe as long as each + * instance is only used within a single thread. Different instances of the same class may be used + * by different threads, so they MUST not call any shared non-thread-safe objects. */ - public interface SpawnExecutionPolicy { + interface SpawnExecutionContext { /** * Returns a unique id for this spawn, to be used for logging. Note that a single spawn may be * passed to multiple {@link SpawnRunner} implementations, so any log entries should also @@ -192,17 +197,15 @@ public interface SpawnRunner { * Run the given spawn. * * @param spawn the spawn to run - * @param policy a helper that provides additional parameters + * @param context the spawn execution context * @return the result from running the spawn * @throws InterruptedException if the calling thread was interrupted, or if the runner could not - * lock the output files (see {@link SpawnExecutionPolicy#lockOutputFiles()}) + * lock the output files (see {@link SpawnExecutionContext#lockOutputFiles()}) * @throws IOException if something went wrong reading or writing to the local file system * @throws ExecException if the request is malformed */ - SpawnResult exec( - Spawn spawn, - SpawnExecutionPolicy policy) - throws InterruptedException, IOException, ExecException; + SpawnResult exec(Spawn spawn, SpawnExecutionContext context) + throws InterruptedException, IOException, ExecException; /* Name of the SpawnRunner. */ String getName(); 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 c9575d0679..e22da3ef85 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 @@ -118,16 +118,15 @@ public class LocalSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec( - Spawn spawn, - SpawnExecutionPolicy policy) throws IOException, InterruptedException { + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) + throws IOException, InterruptedException { ActionExecutionMetadata owner = spawn.getResourceOwner(); - policy.report(ProgressStatus.SCHEDULING, getName()); + context.report(ProgressStatus.SCHEDULING, getName()); try (ResourceHandle handle = resourceManager.acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, getName()); - policy.lockOutputFiles(); - return new SubprocessHandler(spawn, policy).run(); + context.report(ProgressStatus.EXECUTING, getName()); + context.lockOutputFiles(); + return new SubprocessHandler(spawn, context).run(); } } @@ -141,7 +140,7 @@ public class LocalSpawnRunner implements SpawnRunner { private final class SubprocessHandler { private final Spawn spawn; - private final SpawnExecutionPolicy policy; + private final SpawnExecutionContext context; private final long creationTime = System.currentTimeMillis(); private long stateStartTime = creationTime; @@ -150,13 +149,11 @@ public class LocalSpawnRunner implements SpawnRunner { private final int id; - public SubprocessHandler( - Spawn spawn, - SpawnExecutionPolicy policy) { + public SubprocessHandler(Spawn spawn, SpawnExecutionContext context) { Preconditions.checkArgument(!spawn.getArguments().isEmpty()); this.spawn = spawn; - this.policy = policy; - this.id = policy.getId(); + this.context = context; + this.id = context.getId(); setState(State.PARSING); } @@ -222,7 +219,7 @@ public class LocalSpawnRunner implements SpawnRunner { private SpawnResult start() throws InterruptedException, IOException { logger.info(String.format("starting local subprocess #%d, argv: %s", id, debugCmdString())); - FileOutErr outErr = policy.getFileOutErr(); + FileOutErr outErr = context.getFileOutErr(); String actionType = spawn.getResourceOwner().getMnemonic(); if (localExecutionOptions.allowedLocalAction != null && !localExecutionOptions.allowedLocalAction.matcher(actionType).matches()) { @@ -241,7 +238,7 @@ public class LocalSpawnRunner implements SpawnRunner { if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) { stepLog(INFO, "prefetching inputs for local execution"); setState(State.PREFETCHING_LOCAL_INPUTS); - policy.prefetchInputs(); + context.prefetchInputs(); } stepLog(INFO, "running locally"); @@ -270,7 +267,7 @@ public class LocalSpawnRunner implements SpawnRunner { ProcessWrapperUtil.commandLineBuilder(processWrapper, spawn.getArguments()) .setStdoutPath(getPathOrDevNull(outErr.getOutputPath())) .setStderrPath(getPathOrDevNull(outErr.getErrorPath())) - .setTimeout(policy.getTimeout()) + .setTimeout(context.getTimeout()) .setKillDelay(Duration.ofSeconds(localExecutionOptions.localSigkillGraceSeconds)); if (localExecutionOptions.collectLocalExecutionStatistics) { statisticsPath = tmpDir.getRelative("stats.out"); @@ -290,7 +287,7 @@ public class LocalSpawnRunner implements SpawnRunner { spawn.getArguments().toArray(new String[0]), environment, execRoot.getPathFile(), - policy.getTimeout()); + context.getTimeout()); } long startTime = System.currentTimeMillis(); @@ -326,7 +323,7 @@ public class LocalSpawnRunner implements SpawnRunner { Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime); boolean wasTimeout = commandResult.getTerminationStatus().timedOut() - || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTime)); + || (useProcessWrapper && wasTimeout(context.getTimeout(), wallTime)); int exitCode = wasTimeout ? POSIX_TIMEOUT_EXIT_CODE diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 0cb40b167b..72766ced3e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -26,7 +26,7 @@ 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.SpawnCache; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.DigestUtil.ActionKey; @@ -89,12 +89,12 @@ final class RemoteSpawnCache implements SpawnCache { } @Override - public CacheHandle lookup(Spawn spawn, SpawnExecutionPolicy policy) + public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) throws InterruptedException, IOException, ExecException { // Temporary hack: the TreeNodeRepository should be created and maintained upstream! TreeNodeRepository repository = - new TreeNodeRepository(execRoot, policy.getActionInputFileCache(), digestUtil); - SortedMap<PathFragment, ActionInput> inputMap = policy.getInputMapping(); + new TreeNodeRepository(execRoot, context.getActionInputFileCache(), digestUtil); + SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); Command command = RemoteSpawnRunner.buildCommand(spawn.getArguments(), spawn.getEnvironment()); @@ -104,7 +104,7 @@ final class RemoteSpawnCache implements SpawnCache { digestUtil.compute(command), repository.getMerkleDigest(inputRoot), spawn.getExecutionPlatform(), - policy.getTimeout(), + context.getTimeout(), Spawns.mayBeCached(spawn)); // Look up action cache, and reuse the action output if it is found. @@ -123,7 +123,7 @@ final class RemoteSpawnCache implements SpawnCache { // We don't cache failed actions, so we know the outputs exist. // For now, download all outputs locally; in the future, we can reuse the digests to // just update the TreeNodeRepository and continue the build. - remoteCache.download(result, execRoot, policy.getFileOutErr()); + remoteCache.download(result, execRoot, context.getFileOutErr()); SpawnResult spawnResult = new SpawnResult.Builder() .setStatus(Status.SUCCESS) @@ -177,7 +177,7 @@ final class RemoteSpawnCache implements SpawnCache { && result.exitCode() == 0; Context previous = withMetadata.attach(); try { - remoteCache.upload(actionKey, execRoot, files, policy.getFileOutErr(), uploadAction); + remoteCache.upload(actionKey, execRoot, files, context.getFileOutErr(), uploadAction); } catch (IOException e) { if (verboseFailures) { report(Event.debug("Upload to remote cache failed: " + e.getMessage())); @@ -199,7 +199,7 @@ final class RemoteSpawnCache implements SpawnCache { if (input instanceof VirtualActionInput) { continue; } - Metadata metadata = policy.getActionInputFileCache().getMetadata(input); + Metadata metadata = context.getActionInputFileCache().getMetadata(input); if (metadata instanceof FileArtifactValue) { FileArtifactValue artifactValue = (FileArtifactValue) metadata; Path path = execRoot.getRelative(input.getExecPath()); 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 cbbae37dfa..df20b4220d 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 @@ -120,17 +120,17 @@ class RemoteSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException, IOException { if (!Spawns.mayBeExecutedRemotely(spawn) || remoteCache == null) { - return fallbackRunner.exec(spawn, policy); + return fallbackRunner.exec(spawn, context); } - policy.report(ProgressStatus.EXECUTING, getName()); + context.report(ProgressStatus.EXECUTING, getName()); // Temporary hack: the TreeNodeRepository should be created and maintained upstream! - ActionInputFileCache inputFileCache = policy.getActionInputFileCache(); + ActionInputFileCache inputFileCache = context.getActionInputFileCache(); TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil); - SortedMap<PathFragment, ActionInput> inputMap = policy.getInputMapping(); + SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); Command command = buildCommand(spawn.getArguments(), spawn.getEnvironment()); @@ -140,7 +140,7 @@ class RemoteSpawnRunner implements SpawnRunner { digestUtil.compute(command), repository.getMerkleDigest(inputRoot), spawn.getExecutionPlatform(), - policy.getTimeout(), + context.getTimeout(), Spawns.mayBeCached(spawn)); // Look up action cache, and reuse the action output if it is found. @@ -165,7 +165,7 @@ class RemoteSpawnRunner implements SpawnRunner { + actionKey.getDigest()); } try { - return downloadRemoteResults(cachedResult, policy.getFileOutErr()) + return downloadRemoteResults(cachedResult, context.getFileOutErr()) .setCacheHit(true) .setRunnerName("remote cache hit") .build(); @@ -176,19 +176,19 @@ class RemoteSpawnRunner implements SpawnRunner { } } } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } if (remoteExecutor == null) { // Remote execution is disabled and so execute the spawn on the local machine. - return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey); + return execLocally(spawn, context, inputMap, uploadLocalResults, remoteCache, actionKey); } try { // Upload the command and all the inputs into the remote cache. remoteCache.ensureInputsPresent(repository, execRoot, inputRoot, command); } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } final ActionResult result; @@ -204,16 +204,16 @@ class RemoteSpawnRunner implements SpawnRunner { result = reply.getResult(); remoteCacheHit = reply.getCachedResult(); } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } try { - return downloadRemoteResults(result, policy.getFileOutErr()) + return downloadRemoteResults(result, context.getFileOutErr()) .setRunnerName(remoteCacheHit ? "remote cache hit" : getName()) .setCacheHit(remoteCacheHit) .build(); } catch (IOException e) { - return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + return execLocallyOrFail(spawn, context, inputMap, actionKey, uploadLocalResults, e); } } finally { withMetadata.detach(previous); @@ -257,7 +257,7 @@ class RemoteSpawnRunner implements SpawnRunner { private SpawnResult execLocallyOrFail( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, SortedMap<PathFragment, ActionInput> inputMap, ActionKey actionKey, boolean uploadLocalResults, @@ -271,9 +271,9 @@ class RemoteSpawnRunner implements SpawnRunner { if (options.remoteLocalFallback && !(cause instanceof RetryException && RemoteRetrierUtils.causedByExecTimeout((RetryException) cause))) { - return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey); + return execLocally(spawn, context, inputMap, uploadLocalResults, remoteCache, actionKey); } - return handleError(cause, policy.getFileOutErr(), actionKey); + return handleError(cause, context.getFileOutErr(), actionKey); } private SpawnResult handleError(IOException exception, FileOutErr outErr, ActionKey actionKey) @@ -412,28 +412,28 @@ class RemoteSpawnRunner implements SpawnRunner { */ private SpawnResult execLocally( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, SortedMap<PathFragment, ActionInput> inputMap, boolean uploadToCache, @Nullable AbstractRemoteActionCache remoteCache, @Nullable ActionKey actionKey) throws ExecException, IOException, InterruptedException { if (uploadToCache && remoteCache != null && actionKey != null) { - return execLocallyAndUpload(spawn, policy, inputMap, remoteCache, actionKey); + return execLocallyAndUpload(spawn, context, inputMap, remoteCache, actionKey); } - return fallbackRunner.exec(spawn, policy); + return fallbackRunner.exec(spawn, context); } @VisibleForTesting SpawnResult execLocallyAndUpload( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, SortedMap<PathFragment, ActionInput> inputMap, AbstractRemoteActionCache remoteCache, ActionKey actionKey) throws ExecException, IOException, InterruptedException { Map<Path, Long> ctimesBefore = getInputCtimes(inputMap); - SpawnResult result = fallbackRunner.exec(spawn, policy); + SpawnResult result = fallbackRunner.exec(spawn, context); Map<Path, Long> ctimesAfter = getInputCtimes(inputMap); for (Map.Entry<Path, Long> e : ctimesBefore.entrySet()) { // Skip uploading to remote cache, because an input was modified during execution. @@ -447,7 +447,7 @@ class RemoteSpawnRunner implements SpawnRunner { Spawns.mayBeCached(spawn) && Status.SUCCESS.equals(result.status()) && result.exitCode() == 0; - remoteCache.upload(actionKey, execRoot, outputFiles, policy.getFileOutErr(), uploadAction); + remoteCache.upload(actionKey, execRoot, outputFiles, context.getFileOutErr(), uploadAction); } catch (IOException e) { if (verboseFailures) { report(Event.debug("Upload to remote cache failed: " + e.getMessage())); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 9fee6544dd..5610242eaa 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java @@ -62,14 +62,14 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException { ActionExecutionMetadata owner = spawn.getResourceOwner(); - policy.report(ProgressStatus.SCHEDULING, getName()); + context.report(ProgressStatus.SCHEDULING, getName()); try (ResourceHandle ignored = ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, getName()); - return actuallyExec(spawn, policy); + context.report(ProgressStatus.EXECUTING, getName()); + return actuallyExec(spawn, context); } catch (IOException e) { throw new UserExecException("I/O exception during sandboxed execution", e); } @@ -78,25 +78,25 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { // TODO(laszlocsomor): refactor this class to make `actuallyExec`'s contract clearer: the caller // of `actuallyExec` should not depend on `actuallyExec` calling `runSpawn` because it's easy to // forget to do so in `actuallyExec`'s implementations. - protected abstract SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected abstract SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException, IOException; protected SpawnResult runSpawn( Spawn originalSpawn, SandboxedSpawn sandbox, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, Path execRoot, Duration timeout, Path statisticsPath) throws IOException, InterruptedException { try { sandbox.createFileSystem(); - OutErr outErr = policy.getFileOutErr(); - policy.prefetchInputs(); + OutErr outErr = context.getFileOutErr(); + context.prefetchInputs(); SpawnResult result = run(originalSpawn, sandbox, outErr, timeout, execRoot, statisticsPath); - policy.lockOutputFiles(); + context.lockOutputFiles(); try { // We copy the outputs even when the command failed. sandbox.copyOutputs(execRoot); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java index d20eec70b1..86631443bf 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java @@ -193,10 +193,10 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } @Override - protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws IOException, InterruptedException { // Each invocation of "exec" gets its own sandbox. - Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(context.getId())); sandboxPath.createDirectory(); // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like @@ -215,7 +215,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb"); - Duration timeout = policy.getTimeout(); + Duration timeout = context.getTimeout(); ProcessWrapperUtil.CommandLineBuilder processWrapperCommandLineBuilder = ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) @@ -241,7 +241,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { boolean allowNetworkForThisSpawn = allowNetwork || Spawns.requiresNetwork(spawn); - Map<PathFragment, Path> inputs = SandboxHelpers.getInputFiles(spawn, policy, execRoot); + Map<PathFragment, Path> inputs = SandboxHelpers.getInputFiles(spawn, context, execRoot); SandboxedSpawn sandbox; if (sandboxfsProcess != null) { @@ -287,7 +287,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } }; } - return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); + return runSpawn(spawn, sandbox, context, execRoot, timeout, statisticsPath); } private void writeConfig( diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index f2aedf8615..888f9abab2 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -117,10 +117,10 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } @Override - protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws IOException, ExecException, InterruptedException { // Each invocation of "exec" gets its own sandbox base, execroot and temporary directory. - Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(context.getId())); sandboxPath.createDirectory(); // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like @@ -134,7 +134,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); - Duration timeout = policy.getTimeout(); + Duration timeout = context.getTimeout(); LinuxSandboxUtil.CommandLineBuilder commandLineBuilder = LinuxSandboxUtil.commandLineBuilder(linuxSandbox, spawn.getArguments()) @@ -170,7 +170,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { sandboxPath, commandLineBuilder.build(), environment, - SandboxHelpers.getInputFiles(spawn, policy, execRoot), + SandboxHelpers.getInputFiles(spawn, context, execRoot), outputs, ImmutableSet.of()); } else { @@ -180,12 +180,12 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { sandboxExecRoot, commandLineBuilder.build(), environment, - SandboxHelpers.getInputFiles(spawn, policy, execRoot), + SandboxHelpers.getInputFiles(spawn, context, execRoot), outputs, writableDirs); } - return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); + return runSpawn(spawn, sandbox, context, execRoot, timeout, statisticsPath); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index e79b2abcd7..b3865a73a5 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -63,10 +63,10 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne } @Override - protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException { // Each invocation of "exec" gets its own sandbox. - Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId())); + Path sandboxPath = sandboxBase.getRelative(Integer.toString(context.getId())); sandboxPath.createDirectory(); // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like @@ -78,7 +78,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne Map<String, String> environment = localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp"); - Duration timeout = policy.getTimeout(); + Duration timeout = context.getTimeout(); ProcessWrapperUtil.CommandLineBuilder commandLineBuilder = ProcessWrapperUtil.commandLineBuilder(processWrapper.getPathString(), spawn.getArguments()) .setTimeout(timeout); @@ -97,11 +97,11 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne sandboxExecRoot, commandLineBuilder.build(), environment, - SandboxHelpers.getInputFiles(spawn, policy, execRoot), + SandboxHelpers.getInputFiles(spawn, context, execRoot), SandboxHelpers.getOutputFiles(spawn), getWritableDirs(sandboxExecRoot, environment)); - return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath); + return runSpawn(spawn, sandbox, context, execRoot, timeout, statisticsPath); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java index cee91cebe1..0acbe064b4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java @@ -127,12 +127,12 @@ final class SandboxActionContextProvider extends ActionContextProvider { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws InterruptedException, IOException, ExecException { if (!Spawns.mayBeSandboxed(spawn)) { - return fallbackSpawnRunner.exec(spawn, policy); + return fallbackSpawnRunner.exec(spawn, context); } else { - return sandboxSpawnRunner.exec(spawn, policy); + return sandboxSpawnRunner.exec(spawn, context); } } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java index 31f2dcdfd1..5894680a1e 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java @@ -27,7 +27,7 @@ import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput; import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.exec.SpawnInputExpander; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -65,11 +65,8 @@ public final class SandboxHelpers { * host filesystem where the input files can be found. */ public static Map<PathFragment, Path> getInputFiles( - Spawn spawn, - SpawnExecutionPolicy policy, - Path execRoot) - throws IOException { - return postProcess(policy.getInputMapping(), spawn, policy.getArtifactExpander(), execRoot); + Spawn spawn, SpawnExecutionContext context, Path execRoot) throws IOException { + return postProcess(context.getInputMapping(), spawn, context.getArtifactExpander(), execRoot); } /** diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index a615cf76cd..7efe410b19 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -96,7 +96,7 @@ final class WorkerSpawnRunner implements SpawnRunner { } @Override - public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException { if (!spawn.getExecutionInfo().containsKey(ExecutionRequirements.SUPPORTS_WORKERS) || !spawn.getExecutionInfo().get(ExecutionRequirements.SUPPORTS_WORKERS).equals("1")) { @@ -105,19 +105,19 @@ final class WorkerSpawnRunner implements SpawnRunner { reporter.handle( Event.warn( String.format(ERROR_MESSAGE_PREFIX + REASON_NO_EXECUTION_INFO, spawn.getMnemonic()))); - return fallbackRunner.exec(spawn, policy); + return fallbackRunner.exec(spawn, context); } - policy.report(ProgressStatus.SCHEDULING, getName()); + context.report(ProgressStatus.SCHEDULING, getName()); ActionExecutionMetadata owner = spawn.getResourceOwner(); try (ResourceHandle handle = ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, getName()); - return actuallyExec(spawn, policy); + context.report(ProgressStatus.EXECUTING, getName()); + return actuallyExec(spawn, context); } } - private SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy) + private SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException, InterruptedException { if (Iterables.isEmpty(spawn.getToolFiles())) { throw new UserExecException( @@ -132,15 +132,15 @@ final class WorkerSpawnRunner implements SpawnRunner { ImmutableList<String> workerArgs = splitSpawnArgsIntoWorkerArgsAndFlagFiles(spawn, flagFiles); ImmutableMap<String, String> env = spawn.getEnvironment(); - ActionInputFileCache inputFileCache = policy.getActionInputFileCache(); + ActionInputFileCache inputFileCache = context.getActionInputFileCache(); SortedMap<PathFragment, HashCode> workerFiles = WorkerFilesHash.getWorkerFilesWithHashes( - spawn, policy.getArtifactExpander(), policy.getActionInputFileCache()); + spawn, context.getArtifactExpander(), context.getActionInputFileCache()); HashCode workerFilesCombinedHash = WorkerFilesHash.getCombinedHash(workerFiles); - Map<PathFragment, Path> inputFiles = SandboxHelpers.getInputFiles(spawn, policy, execRoot); + Map<PathFragment, Path> inputFiles = SandboxHelpers.getInputFiles(spawn, context, execRoot); Set<PathFragment> outputFiles = SandboxHelpers.getOutputFiles(spawn); WorkerKey key = @@ -151,15 +151,15 @@ final class WorkerSpawnRunner implements SpawnRunner { spawn.getMnemonic(), workerFilesCombinedHash, workerFiles, - policy.speculating()); + context.speculating()); - WorkRequest workRequest = createWorkRequest(spawn, policy, flagFiles, inputFileCache); + WorkRequest workRequest = createWorkRequest(spawn, context, flagFiles, inputFileCache); long startTime = System.currentTimeMillis(); - WorkResponse response = execInWorker(key, workRequest, policy, inputFiles, outputFiles); + WorkResponse response = execInWorker(key, workRequest, context, inputFiles, outputFiles); Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime); - FileOutErr outErr = policy.getFileOutErr(); + FileOutErr outErr = context.getFileOutErr(); response.getOutputBytes().writeTo(outErr.getErrorStream()); int exitCode = response.getExitCode(); @@ -202,7 +202,7 @@ final class WorkerSpawnRunner implements SpawnRunner { private WorkRequest createWorkRequest( Spawn spawn, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, List<String> flagfiles, ActionInputFileCache inputFileCache) throws IOException { @@ -212,7 +212,7 @@ final class WorkerSpawnRunner implements SpawnRunner { } List<ActionInput> inputs = - ActionInputHelper.expandArtifacts(spawn.getInputFiles(), policy.getArtifactExpander()); + ActionInputHelper.expandArtifacts(spawn.getInputFiles(), context.getArtifactExpander()); for (ActionInput input : inputs) { byte[] digestBytes = inputFileCache.getMetadata(input).getDigest(); @@ -258,7 +258,7 @@ final class WorkerSpawnRunner implements SpawnRunner { private WorkResponse execInWorker( WorkerKey key, WorkRequest request, - SpawnExecutionPolicy policy, + SpawnExecutionContext context, Map<PathFragment, Path> inputFiles, Set<PathFragment> outputFiles) throws InterruptedException, ExecException { @@ -327,7 +327,7 @@ final class WorkerSpawnRunner implements SpawnRunner { .toString()); } - policy.lockOutputFiles(); + context.lockOutputFiles(); if (response == null) { throw new UserExecException( diff --git a/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java index e9731210d3..43f6e65ecd 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java @@ -27,7 +27,7 @@ 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.exec.SpawnCache.CacheHandle; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.testutil.Suite; import com.google.devtools.build.lib.testutil.TestSpec; @@ -72,7 +72,7 @@ public class AbstractSpawnStrategyTest { when(actionExecutionContext.getExecRoot()).thenReturn(fs.getPath("/execroot")); SpawnResult spawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build(); - when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class))) + when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))) .thenReturn(spawnResult); List<SpawnResult> spawnResults = @@ -81,7 +81,7 @@ public class AbstractSpawnStrategyTest { assertThat(spawnResults).containsExactly(spawnResult); // Must only be called exactly once. - verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionPolicy.class)); + verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class)); } @Test @@ -94,8 +94,7 @@ public class AbstractSpawnStrategyTest { .setExitCode(1) .setRunnerName("test") .build(); - when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class))) - .thenReturn(result); + when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(result); try { // Ignoring the List<SpawnResult> return value. @@ -105,7 +104,7 @@ public class AbstractSpawnStrategyTest { assertThat(e.getSpawnResult()).isSameAs(result); } // Must only be called exactly once. - verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionPolicy.class)); + verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class)); } @Test @@ -113,7 +112,7 @@ public class AbstractSpawnStrategyTest { SpawnCache cache = mock(SpawnCache.class); SpawnResult spawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build(); - when(cache.lookup(any(Spawn.class), any(SpawnExecutionPolicy.class))) + when(cache.lookup(any(Spawn.class), any(SpawnExecutionContext.class))) .thenReturn(SpawnCache.success(spawnResult)); when(actionExecutionContext.getContext(eq(SpawnCache.class))).thenReturn(cache); when(actionExecutionContext.getExecRoot()).thenReturn(fs.getPath("/execroot")); @@ -121,7 +120,7 @@ public class AbstractSpawnStrategyTest { List<SpawnResult> spawnResults = new TestedSpawnStrategy(execRoot, spawnRunner).exec(SIMPLE_SPAWN, actionExecutionContext); assertThat(spawnResults).containsExactly(spawnResult); - verify(spawnRunner, never()).exec(any(Spawn.class), any(SpawnExecutionPolicy.class)); + verify(spawnRunner, never()).exec(any(Spawn.class), any(SpawnExecutionContext.class)); } @SuppressWarnings("unchecked") @@ -129,7 +128,7 @@ public class AbstractSpawnStrategyTest { public void testCacheMiss() throws Exception { SpawnCache cache = mock(SpawnCache.class); CacheHandle entry = mock(CacheHandle.class); - when(cache.lookup(any(Spawn.class), any(SpawnExecutionPolicy.class))).thenReturn(entry); + when(cache.lookup(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(entry); when(entry.hasResult()).thenReturn(false); when(entry.willStore()).thenReturn(true); @@ -137,7 +136,7 @@ public class AbstractSpawnStrategyTest { when(actionExecutionContext.getExecRoot()).thenReturn(fs.getPath("/execroot")); SpawnResult spawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build(); - when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class))) + when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))) .thenReturn(spawnResult); List<SpawnResult> spawnResults = @@ -146,7 +145,7 @@ public class AbstractSpawnStrategyTest { assertThat(spawnResults).containsExactly(spawnResult); // Must only be called exactly once. - verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionPolicy.class)); + verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class)); verify(entry).store(eq(spawnResult), any(Collection.class)); } @@ -155,7 +154,7 @@ public class AbstractSpawnStrategyTest { public void testCacheMissWithNonZeroExit() throws Exception { SpawnCache cache = mock(SpawnCache.class); CacheHandle entry = mock(CacheHandle.class); - when(cache.lookup(any(Spawn.class), any(SpawnExecutionPolicy.class))).thenReturn(entry); + when(cache.lookup(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(entry); when(entry.hasResult()).thenReturn(false); when(entry.willStore()).thenReturn(true); @@ -167,7 +166,7 @@ public class AbstractSpawnStrategyTest { .setExitCode(1) .setRunnerName("test") .build(); - when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class))).thenReturn(result); + when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(result); try { // Ignoring the List<SpawnResult> return value. @@ -177,7 +176,7 @@ public class AbstractSpawnStrategyTest { assertThat(e.getSpawnResult()).isSameAs(result); } // Must only be called exactly once. - verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionPolicy.class)); + verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class)); verify(entry).store(eq(result), any(Collection.class)); } } 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 11312c8b2a..dea067dd79 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 @@ -38,7 +38,7 @@ import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.shell.JavaSubprocessFactory; import com.google.devtools.build.lib.shell.Subprocess; @@ -185,7 +185,7 @@ public class LocalSpawnRunnerTest { } } - private final class SpawnExecutionPolicyForTesting implements SpawnExecutionPolicy { + private final class SpawnExecutionContextForTesting implements SpawnExecutionContext { private final List<ProgressStatus> reportedStatus = new ArrayList<>(); private final TreeMap<PathFragment, ActionInput> inputMapping = new TreeMap<>(); @@ -194,7 +194,7 @@ public class LocalSpawnRunnerTest { private boolean lockOutputFilesCalled; private FileOutErr fileOutErr; - public SpawnExecutionPolicyForTesting(FileOutErr fileOutErr) { + public SpawnExecutionContextForTesting(FileOutErr fileOutErr) { this.fileOutErr = fileOutErr; } @@ -311,7 +311,7 @@ public class LocalSpawnRunnerTest { LocalEnvProvider.UNMODIFIED); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); policy.timeoutMillis = 123 * 1000L; assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); SpawnResult result = runner.exec(SIMPLE_SPAWN, policy); @@ -365,7 +365,7 @@ public class LocalSpawnRunnerTest { LocalEnvProvider.UNMODIFIED); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); policy.timeoutMillis = 123 * 1000L; assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); SpawnResult result = runner.exec(SIMPLE_SPAWN, policy); @@ -410,7 +410,7 @@ public class LocalSpawnRunnerTest { assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); SpawnResult result = runner.exec(SIMPLE_SPAWN, policy); verify(factory).create(any(SubprocessBuilder.class)); assertThat(result.status()).isEqualTo(SpawnResult.Status.NON_ZERO_EXIT); @@ -456,7 +456,7 @@ public class LocalSpawnRunnerTest { assertThat(fs.getPath("/out").createDirectory()).isTrue(); assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); SpawnResult result = runner.exec(SIMPLE_SPAWN, policy); verify(factory).create(any(SubprocessBuilder.class)); assertThat(result.status()).isEqualTo(SpawnResult.Status.EXECUTION_FAILED); @@ -490,7 +490,7 @@ public class LocalSpawnRunnerTest { assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); FileOutErr fileOutErr = new FileOutErr(); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); SpawnResult reply = runner.exec(SIMPLE_SPAWN, policy); assertThat(reply.status()).isEqualTo(SpawnResult.Status.EXECUTION_DENIED); assertThat(reply.exitCode()).isEqualTo(-1); @@ -539,7 +539,7 @@ public class LocalSpawnRunnerTest { LocalEnvProvider.UNMODIFIED); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); try { runner.exec(SIMPLE_SPAWN, policy); @@ -570,7 +570,7 @@ public class LocalSpawnRunnerTest { LocalEnvProvider.UNMODIFIED); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); policy.timeoutMillis = 123 * 1000L; assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); runner.exec(SIMPLE_SPAWN, policy); @@ -596,7 +596,7 @@ public class LocalSpawnRunnerTest { LocalEnvProvider.UNMODIFIED); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); policy.timeoutMillis = 123 * 1000L; Spawn spawn = new SpawnBuilder("/bin/echo", "Hi!") @@ -626,7 +626,7 @@ public class LocalSpawnRunnerTest { localEnvProvider); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); policy.timeoutMillis = 123 * 1000L; assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); @@ -664,7 +664,7 @@ public class LocalSpawnRunnerTest { LocalEnvProvider.UNMODIFIED); FileOutErr fileOutErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr")); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); policy.timeoutMillis = 321 * 1000L; assertThat(fs.getPath("/execroot").createDirectory()).isTrue(); SpawnResult result = runner.exec(SIMPLE_SPAWN, policy); @@ -795,7 +795,7 @@ public class LocalSpawnRunnerTest { .build(); FileOutErr fileOutErr = new FileOutErr(); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); SpawnResult spawnResult = runner.exec(spawn, policy); @@ -856,7 +856,7 @@ public class LocalSpawnRunnerTest { .build(); FileOutErr fileOutErr = new FileOutErr(); - SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting(fileOutErr); + SpawnExecutionContextForTesting policy = new SpawnExecutionContextForTesting(fileOutErr); SpawnResult spawnResult = runner.exec(spawn, policy); 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 3bc5b37b1e..1e52b0bbbe 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 @@ -42,7 +42,7 @@ import com.google.devtools.build.lib.clock.JavaClock; 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; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; @@ -128,8 +128,8 @@ public class GrpcRemoteExecutionClientTest { private FileOutErr outErr; private Server fakeServer; - private final SpawnExecutionPolicy simplePolicy = - new SpawnExecutionPolicy() { + private final SpawnExecutionContext simplePolicy = + new SpawnExecutionContext() { @Override public int getId() { return 0; diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 0374e3dc38..5304562f99 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -42,7 +42,7 @@ import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.exec.SpawnCache.CacheHandle; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.remote.util.DigestUtil; @@ -95,8 +95,8 @@ public class RemoteSpawnCacheTest { private StoredEventHandler eventHandler = new StoredEventHandler(); - private final SpawnExecutionPolicy simplePolicy = - new SpawnExecutionPolicy() { + private final SpawnExecutionContext simplePolicy = + new SpawnExecutionContext() { @Override public int getId() { return 0; 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 e16f8e4874..ebf3f5a0d0 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 @@ -47,7 +47,7 @@ import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.DigestUtil.ActionKey; @@ -161,7 +161,7 @@ public class RemoteSpawnRunnerTest { /*outputs=*/ ImmutableList.<ActionInput>of(), ResourceSet.ZERO); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); runner.exec(spawn, policy); @@ -219,7 +219,7 @@ public class RemoteSpawnRunnerTest { /*outputs=*/ ImmutableList.<ActionInput>of(), ResourceSet.ZERO); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); runner.exec(spawn, policy); @@ -261,7 +261,7 @@ public class RemoteSpawnRunnerTest { logDir)); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = Mockito.mock(SpawnResult.class); when(res.exitCode()).thenReturn(1); @@ -292,7 +292,7 @@ public class RemoteSpawnRunnerTest { when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(failedAction); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); RemoteSpawnRunner runner = spy( @@ -345,7 +345,7 @@ public class RemoteSpawnRunnerTest { logDir); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); when(cache.getCachedActionResult(any(ActionKey.class))) .thenThrow(new IOException("cache down")); @@ -402,7 +402,7 @@ public class RemoteSpawnRunnerTest { logDir); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -442,7 +442,7 @@ public class RemoteSpawnRunnerTest { logDir); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); when(cache.getCachedActionResult(any(ActionKey.class))).thenThrow(new IOException()); @@ -482,7 +482,7 @@ public class RemoteSpawnRunnerTest { when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException()); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); IOException err = new IOException("local execution error"); when(localRunner.exec(eq(spawn), eq(policy))).thenThrow(err); @@ -524,7 +524,7 @@ public class RemoteSpawnRunnerTest { .build()); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); @@ -564,7 +564,7 @@ public class RemoteSpawnRunnerTest { "", 1, new ExecutionStatusException(resp.getStatus(), resp))); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.TIMEOUT); @@ -602,7 +602,7 @@ public class RemoteSpawnRunnerTest { .build()); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); @@ -642,7 +642,7 @@ public class RemoteSpawnRunnerTest { .build()); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.SUCCESS); @@ -686,7 +686,7 @@ public class RemoteSpawnRunnerTest { Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); @@ -733,7 +733,7 @@ public class RemoteSpawnRunnerTest { Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.TIMEOUT); @@ -781,7 +781,7 @@ public class RemoteSpawnRunnerTest { Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.TIMEOUT); @@ -818,7 +818,7 @@ public class RemoteSpawnRunnerTest { Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); @@ -855,7 +855,7 @@ public class RemoteSpawnRunnerTest { when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException()); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); try { runner.exec(spawn, policy); @@ -891,7 +891,7 @@ public class RemoteSpawnRunnerTest { when(cache.getCachedActionResult(any(ActionKey.class))).thenThrow(new IOException()); Spawn spawn = newSimpleSpawn(); - SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn); try { runner.exec(spawn, policy); @@ -914,14 +914,14 @@ public class RemoteSpawnRunnerTest { } // TODO(buchgr): Extract a common class to be used for testing. - class FakeSpawnExecutionPolicy implements SpawnExecutionPolicy { + class FakeSpawnExecutionContext implements SpawnExecutionContext { private final ArtifactExpander artifactExpander = (artifact, output) -> output.add(artifact); private final Spawn spawn; - FakeSpawnExecutionPolicy(Spawn spawn) { + FakeSpawnExecutionContext(Spawn spawn) { this.spawn = spawn; } |