Revert "[7.3.0] Deduplicate locally executed path mapped spawns (#230… (#23290)
…69)"
This reverts commit ffe1df57c3ccbbff815df0068634b881da8c1501.
Causes race conditions, see #23288 for details.
Fixes https://github.com/bazelbuild/bazel/issues/23289
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
index 95633d1..60c14b8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
@@ -488,149 +488,6 @@
}
}
- /**
- * A helper class for wrapping an existing {@link SpawnResult} and modifying a subset of its
- * methods.
- */
- class DelegateSpawnResult implements SpawnResult {
- private final SpawnResult delegate;
-
- public DelegateSpawnResult(SpawnResult delegate) {
- this.delegate = delegate;
- }
-
- @Override
- public boolean setupSuccess() {
- return delegate.setupSuccess();
- }
-
- @Override
- public boolean isCatastrophe() {
- return delegate.isCatastrophe();
- }
-
- @Override
- public Status status() {
- return delegate.status();
- }
-
- @Override
- public int exitCode() {
- return delegate.exitCode();
- }
-
- @Override
- @Nullable
- public FailureDetail failureDetail() {
- return delegate.failureDetail();
- }
-
- @Override
- @Nullable
- public String getExecutorHostName() {
- return delegate.getExecutorHostName();
- }
-
- @Override
- public String getRunnerName() {
- return delegate.getRunnerName();
- }
-
- @Override
- public String getRunnerSubtype() {
- return delegate.getRunnerSubtype();
- }
-
- @Override
- @Nullable
- public Instant getStartTime() {
- return delegate.getStartTime();
- }
-
- @Override
- public int getWallTimeInMs() {
- return delegate.getWallTimeInMs();
- }
-
- @Override
- public int getUserTimeInMs() {
- return delegate.getUserTimeInMs();
- }
-
- @Override
- public int getSystemTimeInMs() {
- return delegate.getSystemTimeInMs();
- }
-
- @Override
- @Nullable
- public Long getNumBlockOutputOperations() {
- return delegate.getNumBlockOutputOperations();
- }
-
- @Override
- @Nullable
- public Long getNumBlockInputOperations() {
- return delegate.getNumBlockInputOperations();
- }
-
- @Override
- @Nullable
- public Long getNumInvoluntaryContextSwitches() {
- return delegate.getNumInvoluntaryContextSwitches();
- }
-
- @Override
- @Nullable
- public Long getMemoryInKb() {
- return delegate.getMemoryInKb();
- }
-
- @Override
- public SpawnMetrics getMetrics() {
- return delegate.getMetrics();
- }
-
- @Override
- public boolean isCacheHit() {
- return delegate.isCacheHit();
- }
-
- @Override
- public String getFailureMessage() {
- return delegate.getFailureMessage();
- }
-
- @Override
- @Nullable
- public InputStream getInMemoryOutput(ActionInput output) {
- return delegate.getInMemoryOutput(output);
- }
-
- @Override
- public String getDetailMessage(
- String message, boolean catastrophe, boolean forciblyRunRemotely) {
- return delegate.getDetailMessage(message, catastrophe, forciblyRunRemotely);
- }
-
- @Override
- @Nullable
- public MetadataLog getActionMetadataLog() {
- return delegate.getActionMetadataLog();
- }
-
- @Override
- public boolean wasRemote() {
- return delegate.wasRemote();
- }
-
- @Override
- @Nullable
- public Digest getDigest() {
- return delegate.getDigest();
- }
- }
-
/** Builder class for {@link SpawnResult}. */
final class Builder {
private int exitCode;
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 8e1a1aa..05bf139 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
@@ -221,7 +221,7 @@
? resultMessage
: CommandFailureUtils.describeCommandFailure(
executionOptions.verboseFailures, cwd, spawn);
- throw new SpawnExecException(message, spawnResult, /* forciblyRunRemotely= */ false);
+ throw new SpawnExecException(message, spawnResult, /*forciblyRunRemotely=*/ false);
}
return ImmutableList.of(spawnResult);
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD
index ef276bd..37870bd 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD
@@ -225,7 +225,6 @@
deps = [
":spawn_runner",
"//src/main/java/com/google/devtools/build/lib/actions",
- "//src/main/java/com/google/devtools/build/lib/profiler",
],
)
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 694e079..9b46802 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,6 @@
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
-import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import java.io.Closeable;
import java.io.IOException;
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
index 893132e..31d7897 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -17,7 +17,6 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Strings.isNullOrEmpty;
-import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.util.concurrent.Futures.immediateFailedFuture;
import static com.google.common.util.concurrent.Futures.immediateFuture;
import static com.google.common.util.concurrent.Futures.transform;
@@ -63,7 +62,6 @@
import com.google.common.eventbus.Subscribe;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
-import com.google.common.util.concurrent.SettableFuture;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
@@ -130,7 +128,6 @@
import io.reactivex.rxjava3.core.SingleObserver;
import io.reactivex.rxjava3.disposables.Disposable;
import io.reactivex.rxjava3.schedulers.Schedulers;
-import java.io.FileNotFoundException;
import java.io.IOException;
import java.time.Instant;
import java.util.ArrayList;
@@ -146,11 +143,9 @@
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.TreeSet;
-import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ConcurrentLinkedQueue;
-import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.Phaser;
import java.util.concurrent.Semaphore;
@@ -868,12 +863,15 @@
}
}
- /** Moves the locally created outputs from their temporary location to their declared location. */
+ /**
+ * Copies moves the downloaded outputs from their download location to their declared location.
+ */
private void moveOutputsToFinalLocation(
- Iterable<Path> localOutputs, Map<Path, Path> realToTmpPath) throws IOException {
+ List<FileMetadata> finishedDownloads, Map<Path, Path> realToTmpPath) throws IOException {
// Move the output files from their temporary name to the actual output file name. Executable
// bit is ignored since the file permission will be changed to 0555 after execution.
- for (Path realPath : localOutputs) {
+ for (FileMetadata outputFile : finishedDownloads) {
+ Path realPath = outputFile.path();
Path tmpPath = Preconditions.checkNotNull(realToTmpPath.get(realPath));
realPath.getParentDirectory().createDirectoryAndParents();
FileSystemUtils.moveFile(tmpPath, realPath);
@@ -1272,8 +1270,7 @@
// TODO(chiwang): Stage directories directly
((BazelOutputService) outputService).stageArtifacts(finishedDownloads);
} else {
- moveOutputsToFinalLocation(
- Iterables.transform(finishedDownloads, FileMetadata::path), realToTmpPath);
+ moveOutputsToFinalLocation(finishedDownloads, realToTmpPath);
}
List<SymlinkMetadata> symlinksInDirectories = new ArrayList<>();
@@ -1330,152 +1327,6 @@
return null;
}
- /** An ongoing local execution of a spawn. */
- public static final class LocalExecution {
- private final RemoteAction action;
- private final SettableFuture<SpawnResult> spawnResultFuture;
-
- private LocalExecution(RemoteAction action) {
- this.action = action;
- this.spawnResultFuture = SettableFuture.create();
- }
-
- /**
- * Creates a new {@link LocalExecution} instance tracking the potential local execution of the
- * given {@link RemoteAction} if there is a chance that the same action will be executed by a
- * different Spawn.
- *
- * <p>This is only done for local (as in, non-remote) execution as remote executors are expected
- * to already have deduplication mechanisms for actions in place, perhaps even across different
- * builds and clients.
- */
- @Nullable
- public static LocalExecution createIfDeduplicatable(RemoteAction action) {
- if (action.getSpawn().getPathMapper().isNoop()) {
- return null;
- }
- return new LocalExecution(action);
- }
-
- /**
- * Signals to all potential consumers of the {@link #spawnResultFuture} that this execution has
- * been cancelled and that the result will not be available.
- */
- public void cancel() {
- spawnResultFuture.cancel(true);
- }
- }
-
- /**
- * Makes the {@link SpawnResult} available to all parallel {@link Spawn}s for the same {@link
- * RemoteAction} waiting for it or notifies them that the spawn failed.
- *
- * @return Whether the spawn result should be uploaded to the cache.
- */
- public boolean commitResultAndDecideWhetherToUpload(
- SpawnResult result, @Nullable LocalExecution execution) {
- if (result.status().equals(SpawnResult.Status.SUCCESS) && result.exitCode() == 0) {
- if (execution != null) {
- execution.spawnResultFuture.set(result);
- }
- return true;
- } else {
- if (execution != null) {
- execution.spawnResultFuture.cancel(true);
- }
- return false;
- }
- }
-
- /**
- * Reuses the outputs of a concurrent local execution of the same RemoteAction in a different
- * spawn.
- *
- * <p>Since each output file is generated by a unique action and actions generally take care to
- * run a unique spawn for each output file, this method is only useful with path mapping enabled,
- * which allows different spawns in a single build to have the same RemoteAction.ActionKey.
- *
- * @return The {@link SpawnResult} of the previous execution if it was successful, otherwise null.
- */
- @Nullable
- public SpawnResult waitForAndReuseOutputs(RemoteAction action, LocalExecution previousExecution)
- throws InterruptedException, IOException {
- checkState(!shutdown.get(), "shutdown");
-
- SpawnResult previousSpawnResult;
- try {
- previousSpawnResult = previousExecution.spawnResultFuture.get();
- } catch (CancellationException | ExecutionException e) {
- if (e.getCause() != null) {
- Throwables.throwIfInstanceOf(e.getCause(), InterruptedException.class);
- Throwables.throwIfUnchecked(e.getCause());
- }
- // The spawn this action was deduplicated against failed due to an exception or
- // non-zero exit code. Since it isn't possible to transparently replay its failure for the
- // current spawn, we rerun the action instead.
- return null;
- }
-
- Preconditions.checkArgument(
- action.getActionKey().equals(previousExecution.action.getActionKey()));
-
- ImmutableMap<Path, ActionInput> previousOutputs =
- previousExecution.action.getSpawn().getOutputFiles().stream()
- .collect(toImmutableMap(output -> execRoot.getRelative(output.getExecPath()), o -> o));
- Map<Path, Path> realToTmpPath = new HashMap<>();
- for (String output : action.getCommand().getOutputPathsList()) {
- Path sourcePath =
- previousExecution
- .action
- .getRemotePathResolver()
- .outputPathToLocalPath(encodeBytestringUtf8(output));
- ActionInput outputArtifact = previousOutputs.get(sourcePath);
- Path tmpPath = tempPathGenerator.generateTempPath();
- tmpPath.getParentDirectory().createDirectoryAndParents();
- try {
- if (outputArtifact.isDirectory()) {
- tmpPath.createDirectory();
- FileSystemUtils.copyTreesBelow(sourcePath, tmpPath, Symlinks.NOFOLLOW);
- } else if (outputArtifact.isSymlink()) {
- FileSystemUtils.ensureSymbolicLink(tmpPath, sourcePath.readSymbolicLink());
- } else {
- FileSystemUtils.copyFile(sourcePath, tmpPath);
- }
- } catch (FileNotFoundException e) {
- // The spawn this action was deduplicated against failed to create an output file. If the
- // output is mandatory, we cannot reuse the previous execution.
- if (action.getSpawn().isMandatoryOutput(outputArtifact)) {
- return null;
- }
- }
-
- Path targetPath =
- action.getRemotePathResolver().outputPathToLocalPath(encodeBytestringUtf8(output));
- realToTmpPath.put(targetPath, tmpPath);
- }
-
- // TODO: FileOutErr is action-scoped, not spawn-scoped, but this is not a problem for the
- // current use case of supporting deduplication of path mapped spawns:
- // 1. Starlark and C++ compilation actions always create a single spawn.
- // 2. Java compilation actions may run a fallback spawn, but reset the FileOutErr before
- // running it.
- // If this changes, we will need to introduce a spawn-scoped OutErr.
- FileOutErr.dump(
- previousExecution.action.getSpawnExecutionContext().getFileOutErr(),
- action.getSpawnExecutionContext().getFileOutErr());
-
- action
- .getSpawnExecutionContext()
- .lockOutputFiles(
- previousSpawnResult.exitCode(),
- previousSpawnResult.getFailureMessage(),
- action.getSpawnExecutionContext().getFileOutErr());
- // All outputs are created locally.
- moveOutputsToFinalLocation(realToTmpPath.keySet(), realToTmpPath);
-
- return previousSpawnResult;
- }
-
private boolean shouldDownload(RemoteActionResult result, PathFragment execPath) {
if (outputService instanceof BazelOutputService) {
return false;
@@ -1550,7 +1401,7 @@
}
/** Upload outputs of a remote action which was executed locally to remote cache. */
- public void uploadOutputs(RemoteAction action, SpawnResult spawnResult, Runnable onUploadComplete)
+ public void uploadOutputs(RemoteAction action, SpawnResult spawnResult)
throws InterruptedException, ExecException {
checkState(!shutdown.get(), "shutdown");
checkState(
@@ -1586,7 +1437,6 @@
Profiler.instance()
.completeTask(startTime, ProfilerTask.UPLOAD_TIME, "upload outputs");
backgroundTaskPhaser.arriveAndDeregister();
- onUploadComplete.run();
}
@Override
@@ -1595,7 +1445,6 @@
.completeTask(startTime, ProfilerTask.UPLOAD_TIME, "upload outputs");
backgroundTaskPhaser.arriveAndDeregister();
reportUploadError(e);
- onUploadComplete.run();
}
});
} else {
@@ -1605,8 +1454,6 @@
manifest.upload(action.getRemoteActionExecutionContext(), remoteCache, reporter);
} catch (IOException e) {
reportUploadError(e);
- } finally {
- onUploadComplete.run();
}
}
}
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 3224f56..ad0c51c 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,6 +26,7 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnMetrics;
import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.actions.SpawnResult.Status;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.Event;
@@ -34,11 +35,9 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
-import com.google.devtools.build.lib.remote.RemoteExecutionService.LocalExecution;
import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult;
import com.google.devtools.build.lib.remote.common.BulkTransferException;
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
-import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.Utils;
@@ -46,7 +45,6 @@
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import java.util.NoSuchElementException;
-import java.util.concurrent.ConcurrentHashMap;
/** A remote {@link SpawnCache} implementation. */
@ThreadSafe // If the RemoteActionCache implementation is thread-safe.
@@ -57,8 +55,6 @@
private final RemoteExecutionService remoteExecutionService;
private final DigestUtil digestUtil;
private final boolean verboseFailures;
- private final ConcurrentHashMap<RemoteCacheClient.ActionKey, LocalExecution> inFlightExecutions =
- new ConcurrentHashMap<>();
RemoteSpawnCache(
Path execRoot,
@@ -100,19 +96,7 @@
context.setDigest(digestUtil.asSpawnLogProto(action.getActionKey()));
Profiler prof = Profiler.instance();
- LocalExecution thisExecution = null;
if (shouldAcceptCachedResult) {
- // With path mapping enabled, different Spawns in a single build can have the same ActionKey.
- // When their result isn't in the cache and two of them are scheduled concurrently, neither
- // will result in a cache hit before the other finishes and uploads its result, which results
- // in unnecessary work. To avoid this, we keep track of in-flight executions as long as their
- // results haven't been uploaded to the cache yet and deduplicate all of them against the
- // first one.
- LocalExecution previousExecution = null;
- thisExecution = LocalExecution.createIfDeduplicatable(action);
- if (shouldUploadLocalResults && thisExecution != null) {
- previousExecution = inFlightExecutions.putIfAbsent(action.getActionKey(), thisExecution);
- }
// Metadata will be available in context.current() until we detach.
// This is done via a thread-local variable.
try {
@@ -162,41 +146,9 @@
remoteExecutionService.report(Event.warn(errorMessage));
}
}
- if (previousExecution != null) {
- Stopwatch fetchTime = Stopwatch.createStarted();
- SpawnResult previousResult;
- try (SilentCloseable c = prof.profile(REMOTE_DOWNLOAD, "reuse outputs")) {
- previousResult = remoteExecutionService.waitForAndReuseOutputs(action, previousExecution);
- }
- if (previousResult != null) {
- spawnMetrics
- .setFetchTimeInMs((int) fetchTime.elapsed().toMillis())
- .setTotalTimeInMs((int) totalTime.elapsed().toMillis())
- .setNetworkTimeInMs((int) action.getNetworkTime().getDuration().toMillis());
- SpawnMetrics buildMetrics = spawnMetrics.build();
- return SpawnCache.success(
- new SpawnResult.DelegateSpawnResult(previousResult) {
- @Override
- public String getRunnerName() {
- return "deduplicated";
- }
-
- @Override
- public SpawnMetrics getMetrics() {
- return buildMetrics;
- }
- });
- }
- // If we reach here, the previous execution was not successful (it encountered an exception
- // or the spawn had an exit code != 0). Since it isn't possible to accurately recreate the
- // failure without rerunning the action, we fall back to running the action locally. This
- // means that we have introduced an unnecessary wait, but that can only happen in the case
- // of a failing build with --keep_going.
- }
}
if (shouldUploadLocalResults) {
- final LocalExecution thisExecutionFinal = thisExecution;
return new CacheHandle() {
@Override
public boolean hasResult() {
@@ -215,8 +167,8 @@
@Override
public void store(SpawnResult result) throws ExecException, InterruptedException {
- if (!remoteExecutionService.commitResultAndDecideWhetherToUpload(
- result, thisExecutionFinal)) {
+ boolean uploadResults = Status.SUCCESS.equals(result.status()) && result.exitCode() == 0;
+ if (!uploadResults) {
return;
}
@@ -233,14 +185,12 @@
}
}
- // As soon as the result is in the cache, actions can get the result from it instead of
- // from the first in-flight execution. Not keeping in-flight executions around
- // indefinitely is important to avoid excessive memory pressure - Spawns can be very
- // large.
- remoteExecutionService.uploadOutputs(
- action, result, () -> inFlightExecutions.remove(action.getActionKey()));
+ remoteExecutionService.uploadOutputs(action, result);
}
+ @Override
+ public void close() {}
+
private void checkForConcurrentModifications()
throws IOException, ForbiddenActionInputException {
for (ActionInput input : action.getInputMap(true).values()) {
@@ -254,13 +204,6 @@
}
}
}
-
- @Override
- public void close() {
- if (thisExecutionFinal != null) {
- thisExecutionFinal.cancel();
- }
- }
};
} else {
return SpawnCache.NO_RESULT_NO_STORE;
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 f5fa69f..934cdff 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
@@ -695,7 +695,7 @@
}
}
- remoteExecutionService.uploadOutputs(action, result, () -> {});
+ remoteExecutionService.uploadOutputs(action, result);
return result;
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
index 5819cc3a..3a2c708 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
@@ -17,6 +17,7 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.PathMapper;
import com.google.devtools.build.lib.actions.Spawn;
@@ -75,6 +76,12 @@
*/
Path outputPathToLocalPath(String outputPath);
+ /** Resolves the local {@link Path} for the {@link ActionInput}. */
+ default Path outputPathToLocalPath(ActionInput actionInput) {
+ String outputPath = localPathToOutputPath(actionInput.getExecPath());
+ return outputPathToLocalPath(outputPath);
+ }
+
/** Creates the default {@link RemotePathResolver}. */
static RemotePathResolver createDefault(Path execRoot) {
return new DefaultRemotePathResolver(execRoot);
@@ -132,6 +139,11 @@
public Path outputPathToLocalPath(String outputPath) {
return execRoot.getRelative(outputPath);
}
+
+ @Override
+ public Path outputPathToLocalPath(ActionInput actionInput) {
+ return ActionInputHelper.toInputPath(actionInput, execRoot);
+ }
}
/**
@@ -212,6 +224,10 @@
return getBase().getRelative(outputPath);
}
+ @Override
+ public Path outputPathToLocalPath(ActionInput actionInput) {
+ return ActionInputHelper.toInputPath(actionInput, execRoot);
+ }
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
index 4a46e6d..653e207 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
@@ -1646,7 +1646,7 @@
// act
UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
// assert
ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1693,7 +1693,7 @@
// act
UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
// assert
ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1767,7 +1767,7 @@
// act
UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
// assert
ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1803,7 +1803,7 @@
// act
UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
// assert
ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1855,7 +1855,7 @@
.build();
// act
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
// assert
assertThat(
@@ -1881,7 +1881,7 @@
.when(cache)
.uploadActionResult(any(), any(), any());
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
assertThat(eventHandler.getEvents()).hasSize(1);
Event evt = eventHandler.getEvents().get(0);
@@ -1906,7 +1906,7 @@
.setRunnerName("test")
.build();
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
assertThat(eventHandler.getPosts())
.containsAtLeast(
@@ -1933,7 +1933,7 @@
.setRunnerName("test")
.build();
- service.uploadOutputs(action, spawnResult, () -> {});
+ service.uploadOutputs(action, spawnResult);
// assert
assertThat(cache.getNumFindMissingDigests()).isEmpty();
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 057ea41..5f89e19 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
@@ -18,7 +18,6 @@
import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
@@ -52,7 +51,6 @@
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.SimpleSpawn;
-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.clock.JavaClock;
@@ -83,7 +81,6 @@
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.OutputService;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -100,7 +97,6 @@
import org.junit.runners.JUnit4;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
-import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
@@ -122,7 +118,7 @@
private Path execRoot;
private TempPathGenerator tempPathGenerator;
private SimpleSpawn simpleSpawn;
- private SpawnExecutionContext simplePolicy;
+ private FakeActionInputFileCache fakeFileCache;
@Mock private RemoteCache remoteCache;
private FileOutErr outErr;
@@ -131,102 +127,101 @@
private Reporter reporter;
private RemotePathResolver remotePathResolver;
- private static SpawnExecutionContext createSpawnExecutionContext(
- Spawn spawn, Path execRoot, FakeActionInputFileCache fakeFileCache, FileOutErr outErr) {
- return new SpawnExecutionContext() {
- @Nullable private com.google.devtools.build.lib.exec.Protos.Digest digest;
+ private final SpawnExecutionContext simplePolicy =
+ new SpawnExecutionContext() {
+ @Nullable private com.google.devtools.build.lib.exec.Protos.Digest digest;
- @Override
- public int getId() {
- return 0;
- }
+ @Override
+ public int getId() {
+ return 0;
+ }
- @Override
- public void setDigest(com.google.devtools.build.lib.exec.Protos.Digest digest) {
- checkState(this.digest == null);
- this.digest = digest;
- }
+ @Override
+ public void setDigest(com.google.devtools.build.lib.exec.Protos.Digest digest) {
+ checkState(this.digest == null);
+ this.digest = digest;
+ }
- @Override
- @Nullable
- public com.google.devtools.build.lib.exec.Protos.Digest getDigest() {
- return digest;
- }
+ @Override
+ @Nullable
+ public com.google.devtools.build.lib.exec.Protos.Digest getDigest() {
+ return digest;
+ }
- @Override
- public ListenableFuture<Void> prefetchInputs() {
- return immediateVoidFuture();
- }
+ @Override
+ public ListenableFuture<Void> prefetchInputs() {
+ return immediateVoidFuture();
+ }
- @Override
- public void lockOutputFiles(int exitCode, String errorMessage, FileOutErr outErr) {}
+ @Override
+ public void lockOutputFiles(int exitCode, String errorMessage, FileOutErr outErr) {}
- @Override
- public boolean speculating() {
- return false;
- }
+ @Override
+ public boolean speculating() {
+ return false;
+ }
- @Override
- public InputMetadataProvider getInputMetadataProvider() {
- return fakeFileCache;
- }
+ @Override
+ public InputMetadataProvider getInputMetadataProvider() {
+ return fakeFileCache;
+ }
- @Override
- public ArtifactPathResolver getPathResolver() {
- return ArtifactPathResolver.forExecRoot(execRoot);
- }
+ @Override
+ public ArtifactPathResolver getPathResolver() {
+ return ArtifactPathResolver.forExecRoot(execRoot);
+ }
- @Override
- public ArtifactExpander getArtifactExpander() {
- throw new UnsupportedOperationException();
- }
+ @Override
+ public ArtifactExpander getArtifactExpander() {
+ throw new UnsupportedOperationException();
+ }
- @Override
- public SpawnInputExpander getSpawnInputExpander() {
- return new SpawnInputExpander(execRoot, /*strict*/ false);
- }
+ @Override
+ public SpawnInputExpander getSpawnInputExpander() {
+ return new SpawnInputExpander(execRoot, /*strict*/ false);
+ }
- @Override
- public Duration getTimeout() {
- return Duration.ZERO;
- }
+ @Override
+ public Duration getTimeout() {
+ return Duration.ZERO;
+ }
- @Override
- public FileOutErr getFileOutErr() {
- return outErr;
- }
+ @Override
+ public FileOutErr getFileOutErr() {
+ return outErr;
+ }
- @Override
- public SortedMap<PathFragment, ActionInput> getInputMapping(
- PathFragment baseDirectory, boolean willAccessRepeatedly)
- throws IOException, ForbiddenActionInputException {
- return getSpawnInputExpander()
- .getInputMapping(spawn, SIMPLE_ARTIFACT_EXPANDER, baseDirectory, fakeFileCache);
- }
+ @Override
+ public SortedMap<PathFragment, ActionInput> getInputMapping(
+ PathFragment baseDirectory, boolean willAccessRepeatedly)
+ throws IOException, ForbiddenActionInputException {
+ return getSpawnInputExpander()
+ .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, baseDirectory, fakeFileCache);
+ }
- @Override
- public void report(ProgressStatus progress) {}
+ @Override
+ public void report(ProgressStatus progress) {
+ }
- @Override
- public boolean isRewindingEnabled() {
- return false;
- }
+ @Override
+ public boolean isRewindingEnabled() {
+ return false;
+ }
- @Override
- public void checkForLostInputs() {}
+ @Override
+ public void checkForLostInputs() {}
- @Override
- public <T extends ActionContext> T getContext(Class<T> identifyingType) {
- throw new UnsupportedOperationException();
- }
+ @Override
+ public <T extends ActionContext> T getContext(Class<T> identifyingType) {
+ throw new UnsupportedOperationException();
+ }
- @Nullable
- @Override
- public FileSystem getActionFileSystem() {
- return null;
- }
- };
- }
+ @Nullable
+ @Override
+ public FileSystem getActionFileSystem() {
+ return null;
+ }
+ };
private static SimpleSpawn simpleSpawnWithExecutionInfo(
ImmutableMap<String, String> executionInfo) {
@@ -241,27 +236,6 @@
ResourceSet.ZERO);
}
- private static SimpleSpawn simplePathMappedSpawn(String configSegment) {
- String inputPath = "bazel-bin/%s/bin/input";
- String outputPath = "bazel-bin/%s/bin/output";
- return new SimpleSpawn(
- new FakeOwner("Mnemonic", "Progress Message", "//dummy:label"),
- ImmutableList.of("cp", inputPath.formatted("cfg"), outputPath.formatted("cfg")),
- ImmutableMap.of("VARIABLE", "value"),
- ImmutableMap.of(ExecutionRequirements.SUPPORTS_PATH_MAPPING, ""),
- /* runfilesSupplier= */ null,
- /* filesetMappings= */ ImmutableMap.of(),
- /* inputs= */ NestedSetBuilder.create(
- Order.STABLE_ORDER, ActionInputHelper.fromPath(inputPath.formatted(configSegment))),
- /* tools= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
- /* outputs= */ ImmutableSet.of(
- ActionInputHelper.fromPath(outputPath.formatted(configSegment))),
- /* mandatoryOutputs= */ null,
- ResourceSet.ZERO,
- execPath ->
- execPath.subFragment(0, 1).getRelative("cfg").getRelative(execPath.subFragment(2)));
- }
-
private RemoteSpawnCache createRemoteSpawnCache() {
return remoteSpawnCacheWithOptions(Options.getDefaults(RemoteOptions.class));
}
@@ -297,7 +271,7 @@
execRoot = fs.getPath("/exec/root");
execRoot.createDirectoryAndParents();
tempPathGenerator = new TempPathGenerator(fs.getPath("/execroot/_tmp/actions/remote"));
- FakeActionInputFileCache fakeFileCache = new FakeActionInputFileCache(execRoot);
+ fakeFileCache = new FakeActionInputFileCache(execRoot);
simpleSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of());
Path stdout = fs.getPath("/tmp/stdout");
@@ -310,7 +284,6 @@
reporter.addHandler(eventHandler);
remotePathResolver = RemotePathResolver.createDefault(execRoot);
- simplePolicy = createSpawnExecutionContext(simpleSpawn, execRoot, fakeFileCache, outErr);
fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().getSingleton(), "xyz");
}
@@ -363,7 +336,7 @@
verify(service)
.downloadOutputs(
any(), eq(RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult))));
- verify(service, never()).uploadOutputs(any(), any(), any());
+ verify(service, never()).uploadOutputs(any(), any());
assertThat(result.getDigest())
.isEqualTo(digestUtil.asSpawnLogProto(actionKeyCaptor.getValue()));
assertThat(result.setupSuccess()).isTrue();
@@ -394,9 +367,9 @@
.setStatus(Status.SUCCESS)
.setRunnerName("test")
.build();
- doNothing().when(service).uploadOutputs(any(), any(), any());
+ doNothing().when(service).uploadOutputs(any(), any());
entry.store(result);
- verify(service).uploadOutputs(any(), any(), any());
+ verify(service).uploadOutputs(any(), any());
}
@Test
@@ -560,7 +533,7 @@
.setRunnerName("test")
.build();
entry.store(result);
- verify(service, never()).uploadOutputs(any(), any(), any());
+ verify(service, never()).uploadOutputs(any(), any());
}
@Test
@@ -583,9 +556,9 @@
.setRunnerName("test")
.build();
- doNothing().when(service).uploadOutputs(any(), any(), any());
+ doNothing().when(service).uploadOutputs(any(), any());
entry.store(result);
- verify(service).uploadOutputs(any(), eq(result), any());
+ verify(service).uploadOutputs(any(), eq(result));
assertThat(eventHandler.getEvents()).hasSize(1);
Event evt = eventHandler.getEvents().get(0);
@@ -631,9 +604,9 @@
.setRunnerName("test")
.build();
- doNothing().when(service).uploadOutputs(any(), any(), any());
+ doNothing().when(service).uploadOutputs(any(), any());
entry.store(result);
- verify(service).uploadOutputs(any(), eq(result), any());
+ verify(service).uploadOutputs(any(), eq(result));
assertThat(eventHandler.getEvents()).isEmpty(); // no warning is printed.
}
@@ -707,141 +680,4 @@
assertThat(evt.getKind()).isEqualTo(EventKind.WARNING);
assertThat(evt.getMessage()).contains(downloadFailure.getMessage());
}
-
- @Test
- public void pathMappedActionIsDeduplicated() throws Exception {
- // arrange
- RemoteSpawnCache cache = createRemoteSpawnCache();
-
- SimpleSpawn firstSpawn = simplePathMappedSpawn("k8-fastbuild");
- FakeActionInputFileCache firstFakeFileCache = new FakeActionInputFileCache(execRoot);
- firstFakeFileCache.createScratchInput(firstSpawn.getInputFiles().getSingleton(), "xyz");
- SpawnExecutionContext firstPolicy =
- createSpawnExecutionContext(firstSpawn, execRoot, firstFakeFileCache, outErr);
-
- SimpleSpawn secondSpawn = simplePathMappedSpawn("k8-opt");
- FakeActionInputFileCache secondFakeFileCache = new FakeActionInputFileCache(execRoot);
- secondFakeFileCache.createScratchInput(secondSpawn.getInputFiles().getSingleton(), "xyz");
- SpawnExecutionContext secondPolicy =
- createSpawnExecutionContext(secondSpawn, execRoot, secondFakeFileCache, outErr);
-
- RemoteExecutionService remoteExecutionService = cache.getRemoteExecutionService();
- Mockito.doCallRealMethod().when(remoteExecutionService).waitForAndReuseOutputs(any(), any());
- // Simulate a very slow upload to the remote cache to ensure that the second spawn is
- // deduplicated rather than a cache hit. This is a slight hack, but also avoid introducing
- // concurrency to this test.
- Mockito.doNothing().when(remoteExecutionService).uploadOutputs(any(), any(), any());
-
- // act
- try (CacheHandle firstCacheHandle = cache.lookup(firstSpawn, firstPolicy)) {
- FileSystemUtils.writeContent(
- fs.getPath("/exec/root/bazel-bin/k8-fastbuild/bin/output"), UTF_8, "hello");
- firstCacheHandle.store(
- new SpawnResult.Builder()
- .setExitCode(0)
- .setStatus(Status.SUCCESS)
- .setRunnerName("test")
- .build());
- }
- CacheHandle secondCacheHandle = cache.lookup(secondSpawn, secondPolicy);
-
- // assert
- assertThat(secondCacheHandle.hasResult()).isTrue();
- assertThat(secondCacheHandle.getResult().getRunnerName()).isEqualTo("deduplicated");
- assertThat(
- FileSystemUtils.readContent(
- fs.getPath("/exec/root/bazel-bin/k8-opt/bin/output"), UTF_8))
- .isEqualTo("hello");
- assertThat(secondCacheHandle.willStore()).isFalse();
- }
-
- @Test
- public void deduplicatedActionWithNonZeroExitCodeIsACacheMiss() throws Exception {
- // arrange
- RemoteSpawnCache cache = createRemoteSpawnCache();
-
- SimpleSpawn firstSpawn = simplePathMappedSpawn("k8-fastbuild");
- FakeActionInputFileCache firstFakeFileCache = new FakeActionInputFileCache(execRoot);
- firstFakeFileCache.createScratchInput(firstSpawn.getInputFiles().getSingleton(), "xyz");
- SpawnExecutionContext firstPolicy =
- createSpawnExecutionContext(firstSpawn, execRoot, firstFakeFileCache, outErr);
-
- SimpleSpawn secondSpawn = simplePathMappedSpawn("k8-opt");
- FakeActionInputFileCache secondFakeFileCache = new FakeActionInputFileCache(execRoot);
- secondFakeFileCache.createScratchInput(secondSpawn.getInputFiles().getSingleton(), "xyz");
- SpawnExecutionContext secondPolicy =
- createSpawnExecutionContext(secondSpawn, execRoot, secondFakeFileCache, outErr);
-
- RemoteExecutionService remoteExecutionService = cache.getRemoteExecutionService();
- Mockito.doCallRealMethod().when(remoteExecutionService).waitForAndReuseOutputs(any(), any());
- // Simulate a very slow upload to the remote cache to ensure that the second spawn is
- // deduplicated rather than a cache hit. This is a slight hack, but also avoid introducing
- // concurrency to this test.
- Mockito.doNothing().when(remoteExecutionService).uploadOutputs(any(), any(), any());
-
- // act
- try (CacheHandle firstCacheHandle = cache.lookup(firstSpawn, firstPolicy)) {
- FileSystemUtils.writeContent(
- fs.getPath("/exec/root/bazel-bin/k8-fastbuild/bin/output"), UTF_8, "hello");
- firstCacheHandle.store(
- new SpawnResult.Builder()
- .setExitCode(1)
- .setStatus(Status.NON_ZERO_EXIT)
- .setFailureDetail(
- FailureDetail.newBuilder()
- .setMessage("test spawn failed")
- .setSpawn(
- FailureDetails.Spawn.newBuilder()
- .setCode(FailureDetails.Spawn.Code.NON_ZERO_EXIT))
- .build())
- .setRunnerName("test")
- .build());
- }
- CacheHandle secondCacheHandle = cache.lookup(secondSpawn, secondPolicy);
-
- // assert
- assertThat(secondCacheHandle.hasResult()).isFalse();
- assertThat(secondCacheHandle.willStore()).isTrue();
- }
-
- @Test
- public void deduplicatedActionWithMissingOutputIsACacheMiss() throws Exception {
- // arrange
- RemoteSpawnCache cache = createRemoteSpawnCache();
-
- SimpleSpawn firstSpawn = simplePathMappedSpawn("k8-fastbuild");
- FakeActionInputFileCache firstFakeFileCache = new FakeActionInputFileCache(execRoot);
- firstFakeFileCache.createScratchInput(firstSpawn.getInputFiles().getSingleton(), "xyz");
- SpawnExecutionContext firstPolicy =
- createSpawnExecutionContext(firstSpawn, execRoot, firstFakeFileCache, outErr);
-
- SimpleSpawn secondSpawn = simplePathMappedSpawn("k8-opt");
- FakeActionInputFileCache secondFakeFileCache = new FakeActionInputFileCache(execRoot);
- secondFakeFileCache.createScratchInput(secondSpawn.getInputFiles().getSingleton(), "xyz");
- SpawnExecutionContext secondPolicy =
- createSpawnExecutionContext(secondSpawn, execRoot, secondFakeFileCache, outErr);
-
- RemoteExecutionService remoteExecutionService = cache.getRemoteExecutionService();
- Mockito.doCallRealMethod().when(remoteExecutionService).waitForAndReuseOutputs(any(), any());
- // Simulate a very slow upload to the remote cache to ensure that the second spawn is
- // deduplicated rather than a cache hit. This is a slight hack, but also avoid introducing
- // concurrency to this test.
- Mockito.doNothing().when(remoteExecutionService).uploadOutputs(any(), any(), any());
-
- // act
- try (CacheHandle firstCacheHandle = cache.lookup(firstSpawn, firstPolicy)) {
- // Do not create the output.
- firstCacheHandle.store(
- new SpawnResult.Builder()
- .setExitCode(0)
- .setStatus(Status.SUCCESS)
- .setRunnerName("test")
- .build());
- }
- CacheHandle secondCacheHandle = cache.lookup(secondSpawn, secondPolicy);
-
- // assert
- assertThat(secondCacheHandle.hasResult()).isFalse();
- assertThat(secondCacheHandle.willStore()).isTrue();
- }
}
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 616e404..ce9c71a 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
@@ -242,7 +242,7 @@
// TODO(olaola): verify that the uploaded action has the doNotCache set.
verify(service, never()).lookupCache(any());
- verify(service, never()).uploadOutputs(any(), any(), any());
+ verify(service, never()).uploadOutputs(any(), any());
verifyNoMoreInteractions(localRunner);
}
@@ -317,7 +317,7 @@
RemoteSpawnRunner runner = spy(newSpawnRunner());
RemoteExecutionService service = runner.getRemoteExecutionService();
- doNothing().when(service).uploadOutputs(any(), any(), any());
+ doNothing().when(service).uploadOutputs(any(), any());
// Throw an IOException to trigger the local fallback.
when(executor.executeRemotely(
@@ -343,7 +343,7 @@
verify(localRunner).exec(eq(spawn), eq(policy));
verify(runner)
.execLocallyAndUpload(any(), eq(spawn), eq(policy), /* uploadLocalResults= */ eq(true));
- verify(service).uploadOutputs(any(), eq(res), any());
+ verify(service).uploadOutputs(any(), eq(res));
}
@Test
@@ -376,7 +376,7 @@
verify(localRunner).exec(eq(spawn), eq(policy));
verify(runner)
.execLocallyAndUpload(any(), eq(spawn), eq(policy), /* uploadLocalResults= */ eq(true));
- verify(service, never()).uploadOutputs(any(), any(), any());
+ verify(service, never()).uploadOutputs(any(), any());
}
@Test
@@ -403,7 +403,7 @@
any(ExecuteRequest.class),
any(OperationObserver.class)))
.thenThrow(IOException.class);
- doNothing().when(service).uploadOutputs(any(), any(), any());
+ doNothing().when(service).uploadOutputs(any(), any());
Spawn spawn = newSimpleSpawn();
SpawnExecutionContext policy = getSpawnContext(spawn);
@@ -421,7 +421,7 @@
verify(localRunner).exec(eq(spawn), eq(policy));
verify(runner)
.execLocallyAndUpload(any(), eq(spawn), eq(policy), /* uploadLocalResults= */ eq(true));
- verify(service).uploadOutputs(any(), eq(result), any());
+ verify(service).uploadOutputs(any(), eq(result));
verify(service, never()).downloadOutputs(any(), any());
}
diff --git a/src/test/shell/bazel/path_mapping_test.sh b/src/test/shell/bazel/path_mapping_test.sh
index 4199c82..682c04e 100755
--- a/src/test/shell/bazel/path_mapping_test.sh
+++ b/src/test/shell/bazel/path_mapping_test.sh
@@ -46,15 +46,6 @@
source "$(rlocation "io_bazel/src/test/shell/bazel/remote/remote_utils.sh")" \
|| { echo "remote_utils.sh not found!" >&2; exit 1; }
-case "$(uname -s | tr [:upper:] [:lower:])" in
-msys*|mingw*|cygwin*)
- function is_windows() { true; }
- ;;
-*)
- function is_windows() { false; }
- ;;
-esac
-
function set_up() {
start_worker
@@ -603,269 +594,4 @@
expect_log ' 3 remote cache hit'
}
-function test_path_stripping_deduplicated_action() {
- if is_windows; then
- echo "Skipping test_path_stripping_deduplicated_action on Windows as it requires sandboxing"
- return
- fi
-
- mkdir rules
- touch rules/BUILD
- cat > rules/defs.bzl <<'EOF'
-def _slow_rule_impl(ctx):
- out_file = ctx.actions.declare_file(ctx.attr.name + "_file")
- out_dir = ctx.actions.declare_directory(ctx.attr.name + "_dir")
- out_symlink = ctx.actions.declare_symlink(ctx.attr.name + "_symlink")
- outs = [out_file, out_dir, out_symlink]
- args = ctx.actions.args().add_all(outs, expand_directories = False)
- ctx.actions.run_shell(
- outputs = outs,
- command = """
- # Sleep to ensure that two actions are scheduled in parallel.
- sleep 3
-
- echo "Hello, stdout!"
- >&2 echo "Hello, stderr!"
-
- echo 'echo "Hello, file!"' > $1
- chmod +x $1
- echo 'Hello, dir!' > $2/file
- ln -s $(basename $2)/file $3
- """,
- arguments = [args],
- execution_requirements = {"supports-path-mapping": ""},
- )
- return [
- DefaultInfo(files = depset(outs)),
- ]
-
-slow_rule = rule(_slow_rule_impl)
-EOF
-
- mkdir -p pkg
- cat > pkg/BUILD <<'EOF'
-load("//rules:defs.bzl", "slow_rule")
-
-slow_rule(name = "my_rule")
-
-COMMAND = """
-function validate() {
- # Sorted by file name.
- local -r dir=$$1
- local -r file=$$2
- local -r symlink=$$3
-
- [[ $$($$file) == "Hello, file!" ]] || exit 1
-
- [[ -d $$dir ]] || exit 1
- [[ $$(cat $$dir/file) == "Hello, dir!" ]] || exit 1
-
- [[ -L $$symlink ]] || exit 1
- [[ $$(cat $$symlink) == "Hello, dir!" ]] || exit 1
-}
-validate $(execpaths :my_rule)
-touch $@
-"""
-
-genrule(
- name = "gen_exec",
- outs = ["out_exec"],
- cmd = COMMAND,
- tools = [":my_rule"],
-)
-
-genrule(
- name = "gen_target",
- outs = ["out_target"],
- cmd = COMMAND,
- srcs = [":my_rule"],
-)
-EOF
-
- bazel build \
- --experimental_output_paths=strip \
- --remote_cache=grpc://localhost:${worker_port} \
- //pkg:all &> $TEST_log || fail "build failed unexpectedly"
- # The first slow_write action plus two genrules.
- expect_log '3 \(linux\|darwin\|processwrapper\)-sandbox'
- expect_log '1 deduplicated'
-
- # Even though the spawn is only executed once, its stdout/stderr should be
- # printed as if it wasn't deduplicated.
- expect_log_once 'INFO: From Action pkg/my_rule_file:'
- expect_log_once 'INFO: From Action pkg/my_rule_file \[for tool\]:'
- expect_log_n 'Hello, stderr!' 2
- expect_log_n 'Hello, stdout!' 2
-
- bazel clean || fail "clean failed unexpectedly"
- bazel build \
- --experimental_output_paths=strip \
- --remote_cache=grpc://localhost:${worker_port} \
- //pkg:all &> $TEST_log || fail "build failed unexpectedly"
- # The cache is checked before deduplication.
- expect_log '4 remote cache hit'
-}
-
-function test_path_stripping_deduplicated_action_output_not_created() {
- if is_windows; then
- echo "Skipping test_path_stripping_deduplicated_action_output_not_created on Windows as it requires sandboxing"
- return
- fi
-
- mkdir rules
- touch rules/BUILD
- cat > rules/defs.bzl <<'EOF'
-def _slow_rule_impl(ctx):
- out_file = ctx.actions.declare_file(ctx.attr.name + "_file")
- outs = [out_file]
- args = ctx.actions.args().add_all(outs)
- ctx.actions.run_shell(
- outputs = outs,
- command = """
- # Sleep to ensure that two actions are scheduled in parallel.
- sleep 3
-
- echo "Hello, stdout!"
- >&2 echo "Hello, stderr!"
-
- # Do not create the output file
- """,
- arguments = [args],
- execution_requirements = {"supports-path-mapping": ""},
- )
- return [
- DefaultInfo(files = depset(outs)),
- ]
-
-slow_rule = rule(_slow_rule_impl)
-EOF
-
- mkdir -p pkg
- cat > pkg/BUILD <<'EOF'
-load("//rules:defs.bzl", "slow_rule")
-
-slow_rule(name = "my_rule")
-
-COMMAND = """
-[[ $$($(execpath :my_rule)) == "Hello, file!" ]] || exit 1
-touch $@
-"""
-
-genrule(
- name = "gen_exec",
- outs = ["out_exec"],
- cmd = COMMAND,
- tools = [":my_rule"],
-)
-
-genrule(
- name = "gen_target",
- outs = ["out_target"],
- cmd = COMMAND,
- srcs = [":my_rule"],
-)
-EOF
-
- bazel build \
- --experimental_output_paths=strip \
- --remote_cache=grpc://localhost:${worker_port} \
- --keep_going \
- //pkg:all &> $TEST_log && fail "build succeeded unexpectedly"
- # The second action runs normally after discovering that the first one failed
- # to create the output file.
- expect_log '2 \(linux\|darwin\|processwrapper\)-sandbox'
- expect_not_log '[0-9] deduplicated'
-
- expect_log 'Action pkg/my_rule_file failed:'
- expect_log 'Action pkg/my_rule_file \[for tool\] failed:'
- # Remote cache warning.
- expect_log 'Expected output pkg/my_rule_file was not created locally.'
-
- expect_log_once 'INFO: From Action pkg/my_rule_file:'
- expect_log_once 'INFO: From Action pkg/my_rule_file \[for tool\]:'
- expect_log_n 'Hello, stderr!' 2
- expect_log_n 'Hello, stdout!' 2
-}
-
-function test_path_stripping_deduplicated_action_non_zero_exit_code() {
- if is_windows; then
- echo "Skipping test_path_stripping_deduplicated_action_non_zero_exit_code on Windows as it requires sandboxing"
- return
- fi
-
- mkdir rules
- touch rules/BUILD
- cat > rules/defs.bzl <<'EOF'
-def _slow_rule_impl(ctx):
- out_file = ctx.actions.declare_file(ctx.attr.name + "_file")
- outs = [out_file]
- args = ctx.actions.args().add_all(outs)
- ctx.actions.run_shell(
- outputs = outs,
- command = """
- # Sleep to ensure that two actions are scheduled in parallel.
- sleep 3
-
- echo "Hello, stdout!"
- >&2 echo "Hello, stderr!"
-
- # Create the output file, but with a non-zero exit code.
- echo 'echo "Hello, file!"' > $1
- exit 1
- """,
- arguments = [args],
- execution_requirements = {"supports-path-mapping": ""},
- )
- return [
- DefaultInfo(files = depset(outs)),
- ]
-
-slow_rule = rule(_slow_rule_impl)
-EOF
-
- mkdir -p pkg
- cat > pkg/BUILD <<'EOF'
-load("//rules:defs.bzl", "slow_rule")
-
-slow_rule(name = "my_rule")
-
-COMMAND = """
-[[ $$($(execpath :my_rule)) == "Hello, file!" ]] || exit 1
-touch $@
-"""
-
-genrule(
- name = "gen_exec",
- outs = ["out_exec"],
- cmd = COMMAND,
- tools = [":my_rule"],
-)
-
-genrule(
- name = "gen_target",
- outs = ["out_target"],
- cmd = COMMAND,
- srcs = [":my_rule"],
-)
-EOF
-
- bazel build \
- --experimental_output_paths=strip \
- --remote_cache=grpc://localhost:${worker_port} \
- --keep_going \
- //pkg:all &> $TEST_log && fail "build succeeded unexpectedly"
- # Failing actions are not deduplicated.
- expect_not_log '[0-9] deduplicated'
-
- expect_log 'Action pkg/my_rule_file failed:'
- expect_log 'Action pkg/my_rule_file \[for tool\] failed:'
-
- # The first execution emits stdout/stderr, the second doesn't.
- # stdout/stderr are emitted as part of the failing action error, not as an
- # info.
- expect_not_log 'INFO: From Action pkg/my_rule_file'
- expect_log_n 'Hello, stderr!' 2
- expect_log_n 'Hello, stdout!' 2
-}
-
run_suite "path mapping tests"
