diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html index e9f10d7575fb91..4011a0f6969fb6 100644 --- a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html +++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html @@ -28,13 +28,25 @@
  • no-cache keyword results in the action or test never being - cached. + cached (remotely or locally)
  • -
  • no-remote keyword results in the action or test never being +
  • no-remote-cache keyword results in the action or test never being + cached remotely (but it may be cached locally; it may also be executed remotely). + Note: for the purposes of this tag, the disk-cache is considered a local cache, whereas + the http and gRPC caches are considered remote. + If a combined cache is specified (i.e. a cache with local and remote components), + it's treated as a remote cache and disabled entirely. +
  • + +
  • no-remote-exec keyword results in the action or test never being executed remotely (but it may be cached remotely).
  • +
  • no-remote keyword prevents the action or test from being executed remotely + or cached remotely. This is equivalent to using both no-remote-cache and no-remote-exec. +
  • +
  • local keyword precludes the action or test from being remotely cached, remotely executed, or run inside the sandbox. diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java index 330dffa665e930..9f0645c2b41d70 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java @@ -176,14 +176,24 @@ public String parseIfMatches(String tag) throws ValidationException { */ public static final String NO_CACHE = "no-cache"; + /** Disables remote caching of a spawn. Note: does not disable remote execution */ + public static final String NO_REMOTE_CACHE = "no-remote-cache"; + + /** Disables remote execution of a spawn. Note: does not disable remote caching */ + public static final String NO_REMOTE_EXEC = "no-remote-exec"; + + /** + * Disables both remote execution and remote caching of a spawn. This is the equivalent of using + * no-remote-cache and no-remote-exec together. + */ + public static final String NO_REMOTE = "no-remote"; + /** Disables local sandboxing of a spawn. */ public static final String LEGACY_NOSANDBOX = "nosandbox"; /** Disables local sandboxing of a spawn. */ public static final String NO_SANDBOX = "no-sandbox"; - /** Disables remote execution of a spawn. */ - public static final String NO_REMOTE = "no-remote"; /** * Enables networking for a spawn if possible (only if sandboxing is enabled and if the sandbox diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java index 3b86bd7b4dca25..6bd5ee65c6750c 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java @@ -33,6 +33,20 @@ public static boolean mayBeCached(Spawn spawn) { && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL); } + /** Returns {@code true} if the result of {@code spawn} may be cached remotely. */ + public static boolean mayBeCachedRemotely(Spawn spawn) { + return mayBeCached(spawn) + && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE) + && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE_CACHE); + } + + /** Returns {@code true} if {@code spawn} may be executed remotely. */ + public static boolean mayBeExecutedRemotely(Spawn spawn) { + return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL) + && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE) + && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE_EXEC); + } + /** Returns whether a Spawn can be executed in a sandbox environment. */ public static boolean mayBeSandboxed(Spawn spawn) { return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LEGACY_NOSANDBOX) @@ -52,15 +66,6 @@ public static boolean requiresNetwork(Spawn spawn, boolean defaultSandboxDisallo return defaultSandboxDisallowNetwork; } - /** - * Returns whether a Spawn claims to support being executed remotely according to its execution - * info tags. - */ - public static boolean mayBeExecutedRemotely(Spawn spawn) { - return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL) - && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE); - } - /** * Returns whether a Spawn claims to support being executed with the persistent worker strategy * according to its execution info tags. 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 48bdaf5c81c664..b9220e74eb7f3b 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 @@ -162,7 +162,8 @@ interface CacheHandle extends Closeable { * object is for the cache to store expensive intermediate values (such as the cache key) that are * needed both for the lookup and the subsequent store operation. * - *

    The lookup must not succeed for non-cachable spawns. See {@link Spawns#mayBeCached()}. + *

    The lookup must not succeed for non-cachable spawns. See {@link Spawns#mayBeCached()} and + * {@link Spawns#mayBeCachedRemotely}. * *

    Note that cache stores may be disabled, in which case the returned {@link CacheHandle} * instance's {@link CacheHandle#store} is a no-op. 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 06f4a040759f21..fa7b9c3558733d 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 @@ -111,11 +111,13 @@ final class RemoteSpawnCache implements SpawnCache { this.topLevelOutputs = Preconditions.checkNotNull(topLevelOutputs, "topLevelOutputs"); } - @Override public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) throws InterruptedException, IOException, ExecException { - if (!Spawns.mayBeCached(spawn) || !Spawns.mayBeExecutedRemotely(spawn)) { + if (!Spawns.mayBeCached(spawn) + || (!Spawns.mayBeCachedRemotely(spawn) && isRemoteCache(options))) { + // returning SpawnCache.NO_RESULT_NO_STORE in case the caching is disabled or in case + // the remote caching is disabled and the only configured cache is remote. return SpawnCache.NO_RESULT_NO_STORE; } boolean checkCache = options.remoteAcceptCached; @@ -291,4 +293,8 @@ private void report(Event evt) { cmdlineReporter.handle(evt); } } + + private static boolean isRemoteCache(RemoteOptions options) { + return !isNullOrEmpty(options.remoteCache); + } } 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 d8f9d8048703be..e5873a4d04be90 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 @@ -156,10 +156,10 @@ public String getName() { @Override public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException, IOException { - if (!Spawns.mayBeExecutedRemotely(spawn)) { - return execLocally(spawn, context); - } - boolean spawnCachable = Spawns.mayBeCached(spawn); + + boolean spawnCacheableRemotely = Spawns.mayBeCachedRemotely(spawn); + boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCacheableRemotely; + boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCacheableRemotely; context.report(ProgressStatus.EXECUTING, getName()); RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode; @@ -177,21 +177,20 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) Digest commandHash = digestUtil.compute(command); Action action = buildAction( - commandHash, - merkleTree.getRootDigest(), - context.getTimeout(), - Spawns.mayBeCached(spawn)); + commandHash, merkleTree.getRootDigest(), context.getTimeout(), spawnCacheableRemotely); ActionKey actionKey = digestUtil.computeActionKey(action); + if (!Spawns.mayBeExecutedRemotely(spawn)) { + return execLocallyAndUpload( + spawn, context, inputMap, actionKey, action, command, uploadLocalResults); + } + // Look up action cache, and reuse the action output if it is found. Context withMetadata = TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey); Context previous = withMetadata.attach(); Profiler prof = Profiler.instance(); try { - boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCachable; - boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCachable; - try { // Try to lookup the action in the action cache. ActionResult cachedResult; @@ -223,7 +222,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) if (remoteExecutor == null) { // Remote execution is disabled and so execute the spawn on the local machine. return execLocallyAndUpload( - spawn, context, inputMap, remoteCache, actionKey, action, command, uploadLocalResults); + spawn, context, inputMap, actionKey, action, command, uploadLocalResults); } ExecuteRequest.Builder requestBuilder = @@ -246,13 +245,17 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) () -> { ExecuteRequest request = requestBuilder.build(); - // Upload the command and all the inputs into the remote cache. - try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) { - Map additionalInputs = Maps.newHashMapWithExpectedSize(2); - additionalInputs.put(actionKey.getDigest(), action); - additionalInputs.put(commandHash, command); - remoteCache.ensureInputsPresent(merkleTree, additionalInputs, execRoot); + // Upload the command and all the inputs into the remote cache, if remote caching + // is enabled and not disabled using tags, {@see Spawns#mayBeCachedRemotely} + if (spawnCacheableRemotely) { + try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) { + Map additionalInputs = Maps.newHashMapWithExpectedSize(2); + additionalInputs.put(actionKey.getDigest(), action); + additionalInputs.put(commandHash, command); + remoteCache.ensureInputsPresent(merkleTree, additionalInputs, execRoot); + } } + ExecuteResponse reply; try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) { reply = remoteExecutor.executeRemotely(request); @@ -398,7 +401,7 @@ private SpawnResult execLocallyAndUploadOrFail( } if (remoteOptions.remoteLocalFallback && !RemoteRetrierUtils.causedByExecTimeout(cause)) { return execLocallyAndUpload( - spawn, context, inputMap, remoteCache, actionKey, action, command, uploadLocalResults); + spawn, context, inputMap, actionKey, action, command, uploadLocalResults); } return handleError(cause, context.getFileOutErr(), actionKey, context); } @@ -522,7 +525,6 @@ SpawnResult execLocallyAndUpload( Spawn spawn, SpawnExecutionContext context, SortedMap inputMap, - AbstractRemoteActionCache remoteCache, ActionKey actionKey, Action action, Command command, 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 efdf4eb987edd9..9f32fb0958cab0 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 @@ -205,6 +205,30 @@ public void injectDigest(ActionInput output, FileStatus statNoFollow, byte[] dig } }; + private static SimpleSpawn simpleSpawnWithExecutionInfo( + ImmutableMap executionInfo) { + return new SimpleSpawn( + new FakeOwner("Mnemonic", "Progress Message"), + ImmutableList.of("/bin/echo", "Hi!"), + ImmutableMap.of("VARIABLE", "value"), + executionInfo, + /* inputs= */ ImmutableList.of(ActionInputHelper.fromPath("input")), + /* outputs= */ ImmutableList.of(ActionInputHelper.fromPath("/random/file")), + ResourceSet.ZERO); + } + + private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { + return new RemoteSpawnCache( + execRoot, + options, + remoteCache, + "build-req-id", + "command-id", + reporter, + digestUtil, + /* topLevelOutputs= */ ImmutableSet.of()); + } + @Before public final void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -213,15 +237,7 @@ public final void setUp() throws Exception { execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); - simpleSpawn = - new SimpleSpawn( - new FakeOwner("Mnemonic", "Progress Message"), - ImmutableList.of("/bin/echo", "Hi!"), - ImmutableMap.of("VARIABLE", "value"), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ ImmutableList.of(ActionInputHelper.fromPath("input")), - /*outputs=*/ ImmutableList.of(ActionInputHelper.fromPath("/random/file")), - ResourceSet.ZERO); + simpleSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of()); Path stdout = fs.getPath("/tmp/stdout"); Path stderr = fs.getPath("/tmp/stderr"); @@ -232,16 +248,7 @@ public final void setUp() throws Exception { reporter = new Reporter(new EventBus()); eventHandler = new StoredEventHandler(); reporter.addHandler(eventHandler); - cache = - new RemoteSpawnCache( - execRoot, - options, - remoteCache, - "build-req-id", - "command-id", - reporter, - digestUtil, - /* topLevelOutputs= */ ImmutableSet.of()); + cache = remoteSpawnCacheWithOptions(options); fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz"); } @@ -341,20 +348,83 @@ public Void answer(InvocationOnMock invocation) { @Test public void noCacheSpawns() throws Exception { - // Checks that spawns satisfying Spawns.mayBeCached false are not looked up in the remote cache, - // and also that their result and artifacts are not uploaded to the remote cache. + // Checks that spawns satisfying Spawns.mayBeCached=false are not looked up in the cache + // (even if it is a local cache) and that the results/artifacts are not uploaded to the cache. + + RemoteOptions withLocalCache = Options.getDefaults(RemoteOptions.class); + withLocalCache.diskCache = PathFragment.create("/etc/something/cache/here"); + for (RemoteSpawnCache remoteSpawnCache : + ImmutableList.of(cache, remoteSpawnCacheWithOptions(withLocalCache))) { + for (String requirement : + ImmutableList.of(ExecutionRequirements.NO_CACHE, ExecutionRequirements.LOCAL)) { + SimpleSpawn uncacheableSpawn = + simpleSpawnWithExecutionInfo(ImmutableMap.of(requirement, "")); + CacheHandle entry = remoteSpawnCache.lookup(uncacheableSpawn, simplePolicy); + verify(remoteCache, never()).getCachedActionResult(any(ActionKey.class)); + assertThat(entry.hasResult()).isFalse(); + SpawnResult result = + new SpawnResult.Builder() + .setExitCode(0) + .setStatus(Status.SUCCESS) + .setRunnerName("test") + .build(); + entry.store(result); + verifyNoMoreInteractions(remoteCache); + assertThat(progressUpdates).isEmpty(); + } + } + } + + @Test + public void noRemoteCacheSpawns_remoteCache() throws Exception { + // Checks that spawns satisfying Spawns.mayBeCachedRemotely=false are not looked up in the + // remote cache, and that the results/artifacts are not uploaded to the remote cache. + + RemoteOptions remoteCacheOptions = Options.getDefaults(RemoteOptions.class); + remoteCacheOptions.remoteCache = "https://somecache.com"; + RemoteSpawnCache remoteSpawnCache = remoteSpawnCacheWithOptions(remoteCacheOptions); + + for (String requirement : + ImmutableList.of( + ExecutionRequirements.NO_CACHE, + ExecutionRequirements.LOCAL, + ExecutionRequirements.NO_REMOTE_CACHE, + ExecutionRequirements.NO_REMOTE)) { + SimpleSpawn uncacheableSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of(requirement, "")); + CacheHandle entry = remoteSpawnCache.lookup(uncacheableSpawn, simplePolicy); + verify(remoteCache, never()).getCachedActionResult(any(ActionKey.class)); + assertThat(entry.hasResult()).isFalse(); + SpawnResult result = + new SpawnResult.Builder() + .setExitCode(0) + .setStatus(Status.SUCCESS) + .setRunnerName("test") + .build(); + entry.store(result); + verifyNoMoreInteractions(remoteCache); + assertThat(progressUpdates).isEmpty(); + } + } + + @Test + public void noRemoteCacheSpawns_combinedCache() throws Exception { + // Checks that spawns satisfying Spawns.mayBeCachedRemotely=false are not looked up in the + // remote cache, and that the results/artifacts are not uploaded to the remote cache. + // For the purposes of execution requirements, a combined cache with a remote component + // is considered a remote cache + RemoteOptions combinedCacheOptions = Options.getDefaults(RemoteOptions.class); + combinedCacheOptions.remoteCache = "https://somecache.com"; + combinedCacheOptions.diskCache = PathFragment.create("/etc/something/cache/here"); + RemoteSpawnCache remoteSpawnCache = remoteSpawnCacheWithOptions(combinedCacheOptions); + for (String requirement : - ImmutableList.of(ExecutionRequirements.NO_CACHE, ExecutionRequirements.LOCAL)) { - SimpleSpawn uncacheableSpawn = - new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - ImmutableMap.of(requirement, ""), - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.of(ActionInputHelper.fromPath("/random/file")), - ResourceSet.ZERO); - CacheHandle entry = cache.lookup(uncacheableSpawn, simplePolicy); + ImmutableList.of( + ExecutionRequirements.NO_CACHE, + ExecutionRequirements.LOCAL, + ExecutionRequirements.NO_REMOTE_CACHE, + ExecutionRequirements.NO_REMOTE)) { + SimpleSpawn uncacheableSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of(requirement, "")); + CacheHandle entry = remoteSpawnCache.lookup(uncacheableSpawn, simplePolicy); verify(remoteCache, never()).getCachedActionResult(any(ActionKey.class)); assertThat(entry.hasResult()).isFalse(); SpawnResult result = @@ -365,10 +435,30 @@ public void noCacheSpawns() throws Exception { .build(); entry.store(result); verifyNoMoreInteractions(remoteCache); - assertThat(progressUpdates).containsExactly(); + assertThat(progressUpdates).isEmpty(); } } + @Test + public void noRemoteCacheStillUsesLocalCache() throws Exception { + RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); + remoteOptions.diskCache = PathFragment.create("/etc/something/cache/here"); + cache = remoteSpawnCacheWithOptions(remoteOptions); + + SimpleSpawn cacheableSpawn = + simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_CACHE, "")); + cache.lookup(cacheableSpawn, simplePolicy); + verify(remoteCache).getCachedActionResult(any(ActionKey.class)); + } + + @Test + public void noRemoteExecStillUsesCache() throws Exception { + SimpleSpawn cacheableSpawn = + simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_EXEC, "")); + cache.lookup(cacheableSpawn, simplePolicy); + verify(remoteCache).getCachedActionResult(any(ActionKey.class)); + } + @Test public void failedActionsAreNotUploaded() throws Exception { // Only successful action results are uploaded to the remote cache. @@ -566,16 +656,7 @@ public void testDownloadMinimal() throws Exception { // arrange RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL; - cache = - new RemoteSpawnCache( - execRoot, - remoteOptions, - remoteCache, - "build-req-id", - "command-id", - reporter, - digestUtil, - /* topLevelOutputs= */ ImmutableSet.of()); + cache = remoteSpawnCacheWithOptions(remoteOptions); ActionResult success = ActionResult.newBuilder().setExitCode(0).build(); when(remoteCache.getCachedActionResult(any())).thenReturn(success); @@ -594,16 +675,7 @@ public void testDownloadMinimalIoError() throws Exception { // arrange RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL; - cache = - new RemoteSpawnCache( - execRoot, - remoteOptions, - remoteCache, - "build-req-id", - "command-id", - reporter, - digestUtil, - /* topLevelOutputs= */ ImmutableSet.of()); + cache = remoteSpawnCacheWithOptions(remoteOptions); IOException downloadFailure = new IOException("downloadMinimal failed"); 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 384657e42ccd92..21f880594f42bd 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 @@ -173,16 +173,7 @@ public void nonCachableSpawnsShouldNotBeCached_remote() throws Exception { .build(); when(executor.executeRemotely(any(ExecuteRequest.class))).thenReturn(succeeded); - Spawn spawn = - new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - NO_CACHE, - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.of(), - ResourceSet.ZERO); - + Spawn spawn = simpleSpawnWithExecutionInfo(NO_CACHE); SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn, fakeFileCache, execRoot, outErr); @@ -201,40 +192,100 @@ public void nonCachableSpawnsShouldNotBeCached_remote() throws Exception { } @Test - public void nonCachableSpawnsShouldNotBeCached_local() throws Exception { - // Test that if a spawn is executed locally, due to the local fallback, that its result is not - // uploaded to the remote cache. + public void nonCachableSpawnsShouldNotBeCached_localFallback() throws Exception { + // Test that if a non-cachable spawn is executed locally due to the local fallback, + // that its result is not uploaded to the remote cache. remoteOptions.remoteAcceptCached = true; remoteOptions.remoteLocalFallback = true; remoteOptions.remoteUploadLocalResults = true; - RemoteSpawnRunner runner = newSpawnRunnerWithoutExecutor(); + RemoteSpawnRunner runner = newSpawnRunner(); // Throw an IOException to trigger the local fallback. when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(IOException.class); - Spawn spawn = - new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - NO_CACHE, - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.of(), - ResourceSet.ZERO); - + Spawn spawn = simpleSpawnWithExecutionInfo(NO_CACHE); SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn, fakeFileCache, execRoot, outErr); runner.exec(spawn, policy); verify(localRunner).exec(spawn, policy); - - verify(cache, never()).getCachedActionResult(any(ActionKey.class)); + verify(cache, never()).upload(any(), any(), any(), any(), any(), any()); verifyNoMoreInteractions(cache); } + @Test + public void cachableSpawnsShouldBeCached_localFallback() throws Exception { + // Test that if a cachable spawn is executed locally due to the local fallback, + // that its result is uploaded to the remote cache. + + remoteOptions.remoteAcceptCached = true; + remoteOptions.remoteLocalFallback = true; + remoteOptions.remoteUploadLocalResults = true; + + RemoteSpawnRunner runner = spy(newSpawnRunner()); + + // Throw an IOException to trigger the local fallback. + when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(IOException.class); + + SpawnResult res = + new SpawnResult.Builder() + .setStatus(Status.SUCCESS) + .setExitCode(0) + .setRunnerName("test") + .build(); + when(localRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(res); + + Spawn spawn = newSimpleSpawn(); + SpawnExecutionContext policy = + new FakeSpawnExecutionContext(spawn, fakeFileCache, execRoot, outErr); + + SpawnResult result = runner.exec(spawn, policy); + assertThat(result.exitCode()).isEqualTo(0); + assertThat(result.status()).isEqualTo(Status.SUCCESS); + verify(localRunner).exec(eq(spawn), eq(policy)); + verify(runner) + .execLocallyAndUpload( + eq(spawn), eq(policy), any(), any(), any(), any(), /* uploadLocalResults= */ eq(true)); + verify(cache).upload(any(), any(), any(), any(), any(), any()); + } + + @Test + public void noRemoteExecExecutesLocallyButUsesCache() throws Exception { + // Test that if the NO_REMOTE_EXEC execution requirement is specified, the spawn is executed + // locally, but its result is still uploaded to the remote cache. + + remoteOptions.remoteAcceptCached = true; + remoteOptions.remoteUploadLocalResults = true; + + RemoteSpawnRunner runner = spy(newSpawnRunner()); + + SpawnResult res = + new SpawnResult.Builder() + .setStatus(Status.SUCCESS) + .setExitCode(0) + .setRunnerName("test") + .build(); + when(localRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(res); + + Spawn spawn = + simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_EXEC, "")); + SpawnExecutionContext policy = + new FakeSpawnExecutionContext(spawn, fakeFileCache, execRoot, outErr); + + SpawnResult result = runner.exec(spawn, policy); + + assertThat(result.exitCode()).isEqualTo(0); + assertThat(result.status()).isEqualTo(Status.SUCCESS); + verify(localRunner).exec(eq(spawn), eq(policy)); + verify(runner) + .execLocallyAndUpload( + eq(spawn), eq(policy), any(), any(), any(), any(), /* uploadLocalResults= */ eq(true)); + verify(cache).upload(any(), any(), any(), any(), any(), any()); + } + @Test public void failedLocalActionShouldNotBeUploaded() throws Exception { // Test that the outputs of a locally executed action that failed are not uploaded. @@ -260,7 +311,6 @@ public void failedLocalActionShouldNotBeUploaded() throws Exception { eq(spawn), eq(policy), any(), - eq(cache), any(), any(), any(), @@ -298,7 +348,6 @@ public void treatFailedCachedActionAsCacheMiss_local() throws Exception { eq(spawn), eq(policy), any(), - eq(cache), any(), any(), any(), @@ -800,7 +849,7 @@ public void testMaterializeParamFiles() throws Exception { /*arguments=*/ ImmutableList.of(), /*environment=*/ ImmutableMap.of(), /*executionInfo=*/ ImmutableMap.of(), - ImmutableList.of(input), + /*inputs=*/ ImmutableList.of(input), /*outputs=*/ ImmutableList.of(), ResourceSet.ZERO); SpawnExecutionContext policy = @@ -927,11 +976,16 @@ public void testDownloadTopLevel() throws Exception { } private static Spawn newSimpleSpawn(Artifact... outputs) { + return simpleSpawnWithExecutionInfo(ImmutableMap.of(), outputs); + } + + private static SimpleSpawn simpleSpawnWithExecutionInfo( + ImmutableMap executionInfo, Artifact... outputs) { return new SimpleSpawn( new FakeOwner("foo", "bar"), /*arguments=*/ ImmutableList.of(), /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), + /*executionInfo=*/ executionInfo, /*inputs=*/ ImmutableList.of(), /*outputs=*/ ImmutableList.copyOf(outputs), ResourceSet.ZERO);