Remote: Use execRoot as input root and do NOT set working directory by default.
When --experimental_sibling_external_layout is set, use the parent directory of execRoot as input root and set working directory to the base name of execRoot. Paths of output files are relative to working directory.
When --incompatible_remote_paths_relative_to_input_root is set, paths of output files are relative to input root.
Introduce RemotePathResolver which is used to convert local paths to remote paths and vice versa. We should prefer this class instead of using execRoot directly whenever possible in remote module. execRoot usages in RemoteCache are all replaced.
On Windows, shared action results cache for cl.exe across different workspaces causing header dependency checking to fail. This was initially fixed by #9172, but is broken after https://github.com/bazelbuild/bazel/commit/24c980b6d4adef456cf4b4f84ac3b8ec4580b172. The reason test didn't fail before this change is that two builds from different workspaces do not share the cache since input files are relative to the parent of exec root which contains workspace name. This change fixes that by adding workspace name as platform property for action running on Windows.
Fixes https://github.com/bazelbuild/bazel/issues/13188.
Closes #13339.
PiperOrigin-RevId: 369168230
diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD
index 741a246..ad5159d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD
@@ -70,6 +70,7 @@
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/remote/common",
"//src/main/java/com/google/devtools/build/lib/remote/disk",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index 35f7afb..b3ff7de 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -27,7 +27,11 @@
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.SpawnCache;
import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
+import com.google.devtools.build.lib.remote.common.RemotePathResolver;
+import com.google.devtools.build.lib.remote.common.RemotePathResolver.DefaultRemotePathResolver;
+import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingRepositoryLayoutResolver;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -80,6 +84,22 @@
env, cache, executor, retryScheduler, digestUtil, logDir);
}
+ RemotePathResolver createRemotePathResolver() {
+ Path execRoot = env.getExecRoot();
+ BuildLanguageOptions buildLanguageOptions =
+ env.getOptions().getOptions(BuildLanguageOptions.class);
+ RemotePathResolver remotePathResolver;
+ if (buildLanguageOptions != null && buildLanguageOptions.experimentalSiblingRepositoryLayout) {
+ RemoteOptions remoteOptions = checkNotNull(env.getOptions().getOptions(RemoteOptions.class));
+ remotePathResolver =
+ new SiblingRepositoryLayoutResolver(
+ execRoot, remoteOptions.incompatibleRemoteOutputPathsRelativeToInputRoot);
+ } else {
+ remotePathResolver = new DefaultRemotePathResolver(execRoot);
+ }
+ return remotePathResolver;
+ }
+
/**
* Registers a remote spawn strategy if this instance was created with an executor, otherwise does
* nothing.
@@ -108,7 +128,8 @@
retryScheduler,
digestUtil,
logDir,
- filesToDownload);
+ filesToDownload,
+ createRemotePathResolver());
registryBuilder.registerStrategy(
new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner, verboseFailures), "remote");
}
@@ -129,7 +150,8 @@
env.getCommandId().toString(),
env.getReporter(),
digestUtil,
- filesToDownload);
+ filesToDownload,
+ createRemotePathResolver());
registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache");
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
index 1406218..ce008c1 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -59,6 +59,7 @@
import com.google.devtools.build.lib.remote.common.RemoteActionFileArtifactValue;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemotePathResolver;
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.InMemoryOutput;
@@ -130,16 +131,17 @@
*/
public ActionResult upload(
RemoteActionExecutionContext context,
+ RemotePathResolver remotePathResolver,
ActionKey actionKey,
Action action,
Command command,
- Path execRoot,
Collection<Path> outputs,
FileOutErr outErr,
int exitCode)
throws ExecException, IOException, InterruptedException {
ActionResult.Builder resultBuilder = ActionResult.newBuilder();
- uploadOutputs(context, execRoot, actionKey, action, command, outputs, outErr, resultBuilder);
+ uploadOutputs(
+ context, remotePathResolver, actionKey, action, command, outputs, outErr, resultBuilder);
resultBuilder.setExitCode(exitCode);
ActionResult result = resultBuilder.build();
if (exitCode == 0 && !action.getDoNotCache()) {
@@ -150,20 +152,27 @@
public ActionResult upload(
RemoteActionExecutionContext context,
+ RemotePathResolver remotePathResolver,
ActionKey actionKey,
Action action,
Command command,
- Path execRoot,
Collection<Path> outputs,
FileOutErr outErr)
throws ExecException, IOException, InterruptedException {
return upload(
- context, actionKey, action, command, execRoot, outputs, outErr, /* exitCode= */ 0);
+ context,
+ remotePathResolver,
+ actionKey,
+ action,
+ command,
+ outputs,
+ outErr,
+ /* exitCode= */ 0);
}
private void uploadOutputs(
RemoteActionExecutionContext context,
- Path execRoot,
+ RemotePathResolver remotePathResolver,
ActionKey actionKey,
Action action,
Command command,
@@ -174,8 +183,8 @@
UploadManifest manifest =
new UploadManifest(
digestUtil,
+ remotePathResolver,
result,
- execRoot,
options.incompatibleRemoteSymlinks,
options.allowSymlinkUpload);
manifest.addFiles(files);
@@ -309,15 +318,12 @@
*/
public void download(
RemoteActionExecutionContext context,
+ RemotePathResolver remotePathResolver,
ActionResult result,
- Path execRoot,
FileOutErr origOutErr,
OutputFilesLocker outputFilesLocker)
throws ExecException, IOException, InterruptedException {
- // The input root for RBE is the parent directory of the exec root so that paths to files in
- // external repositories don't start with an uplevel reference
- Path inputRoot = execRoot.getParentDirectory();
- ActionResultMetadata metadata = parseActionResultMetadata(context, result, inputRoot);
+ ActionResultMetadata metadata = parseActionResultMetadata(context, remotePathResolver, result);
List<ListenableFuture<FileMetadata>> downloads =
Stream.concat(
@@ -352,12 +358,12 @@
try {
// Delete any (partially) downloaded output files.
for (OutputFile file : result.getOutputFilesList()) {
- toTmpDownloadPath(inputRoot.getRelative(file.getPath())).delete();
+ toTmpDownloadPath(remotePathResolver.outputPathToLocalPath(file.getPath())).delete();
}
for (OutputDirectory directory : result.getOutputDirectoriesList()) {
// Only delete the directories below the output directories because the output
// directories will not be re-created
- inputRoot.getRelative(directory.getPath()).deleteTreesBelow();
+ remotePathResolver.outputPathToLocalPath(directory.getPath()).deleteTreesBelow();
}
if (tmpOutErr != null) {
tmpOutErr.clearOut();
@@ -566,7 +572,6 @@
* @param inMemoryOutputPath the path of an output file whose contents should be returned in
* memory by this method.
* @param outErr stdout and stderr of this action
- * @param execRoot the execution root
* @param metadataInjector the action's metadata injector that allows this method to inject
* metadata about an action output instead of downloading the output
* @param outputFilesLocker ensures that we are the only ones writing to the output files when
@@ -577,12 +582,11 @@
@Nullable
public InMemoryOutput downloadMinimal(
RemoteActionExecutionContext context,
- String actionId,
+ RemotePathResolver remotePathResolver,
ActionResult result,
Collection<? extends ActionInput> outputs,
@Nullable PathFragment inMemoryOutputPath,
OutErr outErr,
- Path execRoot,
MetadataInjector metadataInjector,
OutputFilesLocker outputFilesLocker)
throws IOException, InterruptedException {
@@ -592,11 +596,7 @@
ActionResultMetadata metadata;
try (SilentCloseable c = Profiler.instance().profile("Remote.parseActionResultMetadata")) {
- // We tell RBE that the input root of the action is the parent directory of what is locally
- // the execroot. This is so that paths of artifacts in external repositories don't start with
- // an uplevel reference.
- Path inputRoot = execRoot.getParentDirectory();
- metadata = parseActionResultMetadata(context, result, inputRoot);
+ metadata = parseActionResultMetadata(context, remotePathResolver, result);
}
if (!metadata.symlinks().isEmpty()) {
@@ -613,8 +613,8 @@
Digest inMemoryOutputDigest = null;
for (ActionInput output : outputs) {
if (inMemoryOutputPath != null && output.getExecPath().equals(inMemoryOutputPath)) {
- Path p = execRoot.getRelative(output.getExecPath());
- FileMetadata m = metadata.file(p);
+ Path localPath = remotePathResolver.outputPathToLocalPath(output);
+ FileMetadata m = metadata.file(localPath);
if (m == null) {
// A declared output wasn't created. Ignore it here. SkyFrame will fail if not all
// outputs were created.
@@ -624,7 +624,8 @@
inMemoryOutput = output;
}
if (output instanceof Artifact) {
- injectRemoteArtifact((Artifact) output, metadata, execRoot, metadataInjector, actionId);
+ injectRemoteArtifact(
+ context, remotePathResolver, (Artifact) output, metadata, metadataInjector);
}
}
@@ -646,15 +647,15 @@
}
private void injectRemoteArtifact(
+ RemoteActionExecutionContext context,
+ RemotePathResolver remotePathResolver,
Artifact output,
ActionResultMetadata metadata,
- Path execRoot,
- MetadataInjector metadataInjector,
- String actionId)
+ MetadataInjector metadataInjector)
throws IOException {
+ Path path = remotePathResolver.outputPathToLocalPath(output);
if (output.isTreeArtifact()) {
- DirectoryMetadata directory =
- metadata.directory(execRoot.getRelative(output.getExecPathString()));
+ DirectoryMetadata directory = metadata.directory(path);
if (directory == null) {
// A declared output wasn't created. It might have been an optional output and if not
// SkyFrame will make sure to fail.
@@ -675,13 +676,13 @@
DigestUtil.toBinaryDigest(file.digest()),
file.digest().getSizeBytes(),
/*locationIndex=*/ 1,
- actionId,
+ context.getRequestMetadata().getActionId(),
file.isExecutable());
tree.putChild(child, value);
}
metadataInjector.injectTree(parent, tree.build());
} else {
- FileMetadata outputMetadata = metadata.file(execRoot.getRelative(output.getExecPathString()));
+ FileMetadata outputMetadata = metadata.file(path);
if (outputMetadata == null) {
// A declared output wasn't created. It might have been an optional output and if not
// SkyFrame will make sure to fail.
@@ -693,7 +694,7 @@
DigestUtil.toBinaryDigest(outputMetadata.digest()),
outputMetadata.digest().getSizeBytes(),
/*locationIndex=*/ 1,
- actionId,
+ context.getRequestMetadata().getActionId(),
outputMetadata.isExecutable()));
}
}
@@ -727,14 +728,16 @@
}
private ActionResultMetadata parseActionResultMetadata(
- RemoteActionExecutionContext context, ActionResult actionResult, Path inputRoot)
+ RemoteActionExecutionContext context,
+ RemotePathResolver remotePathResolver,
+ ActionResult actionResult)
throws IOException, InterruptedException {
Preconditions.checkNotNull(actionResult, "actionResult");
Map<Path, ListenableFuture<Tree>> dirMetadataDownloads =
Maps.newHashMapWithExpectedSize(actionResult.getOutputDirectoriesCount());
for (OutputDirectory dir : actionResult.getOutputDirectoriesList()) {
dirMetadataDownloads.put(
- inputRoot.getRelative(dir.getPath()),
+ remotePathResolver.outputPathToLocalPath(dir.getPath()),
Futures.transform(
downloadBlob(context, dir.getTreeDigest()),
(treeBytes) -> {
@@ -764,12 +767,10 @@
ImmutableMap.Builder<Path, FileMetadata> files = ImmutableMap.builder();
for (OutputFile outputFile : actionResult.getOutputFilesList()) {
+ Path localPath = remotePathResolver.outputPathToLocalPath(outputFile.getPath());
files.put(
- inputRoot.getRelative(outputFile.getPath()),
- new FileMetadata(
- inputRoot.getRelative(outputFile.getPath()),
- outputFile.getDigest(),
- outputFile.getIsExecutable()));
+ localPath,
+ new FileMetadata(localPath, outputFile.getDigest(), outputFile.getIsExecutable()));
}
ImmutableMap.Builder<Path, SymlinkMetadata> symlinks = ImmutableMap.builder();
@@ -778,10 +779,9 @@
actionResult.getOutputFileSymlinksList(),
actionResult.getOutputDirectorySymlinksList());
for (OutputSymlink symlink : outputSymlinks) {
+ Path localPath = remotePathResolver.outputPathToLocalPath(symlink.getPath());
symlinks.put(
- inputRoot.getRelative(symlink.getPath()),
- new SymlinkMetadata(
- inputRoot.getRelative(symlink.getPath()), PathFragment.create(symlink.getTarget())));
+ localPath, new SymlinkMetadata(localPath, PathFragment.create(symlink.getTarget())));
}
return new ActionResultMetadata(files.build(), symlinks.build(), directories.build());
@@ -790,8 +790,8 @@
/** UploadManifest adds output metadata to a {@link ActionResult}. */
static class UploadManifest {
private final DigestUtil digestUtil;
+ private final RemotePathResolver remotePathResolver;
private final ActionResult.Builder result;
- private final Path execRoot;
private final boolean allowSymlinks;
private final boolean uploadSymlinks;
private final Map<Digest, Path> digestToFile = new HashMap<>();
@@ -805,13 +805,13 @@
*/
public UploadManifest(
DigestUtil digestUtil,
+ RemotePathResolver remotePathResolver,
ActionResult.Builder result,
- Path execRoot,
boolean uploadSymlinks,
boolean allowSymlinks) {
this.digestUtil = digestUtil;
+ this.remotePathResolver = remotePathResolver;
this.result = result;
- this.execRoot = execRoot;
this.uploadSymlinks = uploadSymlinks;
this.allowSymlinks = allowSymlinks;
}
@@ -917,21 +917,21 @@
private void addFileSymbolicLink(Path file, PathFragment target) throws IOException {
result
.addOutputFileSymlinksBuilder()
- .setPath(file.relativeTo(execRoot).getPathString())
+ .setPath(remotePathResolver.localPathToOutputPath(file))
.setTarget(target.toString());
}
private void addDirectorySymbolicLink(Path file, PathFragment target) throws IOException {
result
.addOutputDirectorySymlinksBuilder()
- .setPath(file.relativeTo(execRoot).getPathString())
+ .setPath(remotePathResolver.localPathToOutputPath(file))
.setTarget(target.toString());
}
private void addFile(Digest digest, Path file) throws IOException {
result
.addOutputFilesBuilder()
- .setPath(file.relativeTo(execRoot).getPathString())
+ .setPath(remotePathResolver.localPathToOutputPath(file))
.setDigest(digest)
.setIsExecutable(file.isExecutable());
@@ -949,7 +949,7 @@
if (result != null) {
result
.addOutputDirectoriesBuilder()
- .setPath(dir.relativeTo(execRoot).getPathString())
+ .setPath(remotePathResolver.localPathToOutputPath(dir))
.setTreeDigest(digest);
}
@@ -1017,7 +1017,7 @@
"Output %s is a %s. Only regular files and directories may be "
+ "uploaded to a remote cache. "
+ "Change the file type or use --remote_allow_symlink_upload.",
- what.relativeTo(execRoot), kind);
+ remotePathResolver.localPathToOutputPath(what), kind);
throw new UserExecException(createFailureDetail(message, Code.ILLEGAL_OUTPUT));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java
index 54e200b..f239fe3 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java
@@ -44,6 +44,7 @@
import java.io.IOException;
import java.time.Duration;
import java.util.Map;
+import java.util.TreeSet;
/** The remote package's implementation of {@link RepositoryRemoteExecutor}. */
public class RemoteRepositoryRemoteExecutor implements RepositoryRemoteExecutor {
@@ -110,9 +111,21 @@
RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata);
Platform platform = PlatformUtils.buildPlatformProto(executionProperties);
- Command command =
- RemoteSpawnRunner.buildCommand(
- /* outputs= */ ImmutableList.of(), arguments, environment, platform, workingDirectory);
+
+ Command.Builder commandBuilder = Command.newBuilder().addAllArguments(arguments);
+ // Sorting the environment pairs by variable name.
+ TreeSet<String> variables = new TreeSet<>(environment.keySet());
+ for (String var : variables) {
+ commandBuilder.addEnvironmentVariablesBuilder().setName(var).setValue(environment.get(var));
+ }
+ if (platform != null) {
+ commandBuilder.setPlatform(platform);
+ }
+ if (workingDirectory != null) {
+ commandBuilder.setWorkingDirectory(workingDirectory);
+ }
+
+ Command command = commandBuilder.build();
Digest commandHash = digestUtil.compute(command);
MerkleTree merkleTree = MerkleTree.build(inputFiles, digestUtil);
Action action =
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 488105b..322321e 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
@@ -51,6 +51,7 @@
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.merkletree.MerkleTree;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
@@ -85,6 +86,7 @@
private final Set<String> reportedErrors = new HashSet<>();
private final DigestUtil digestUtil;
+ private final RemotePathResolver remotePathResolver;
/**
* If {@link RemoteOutputsMode#TOPLEVEL} is specified it contains the artifacts that should be
@@ -101,7 +103,8 @@
String commandId,
@Nullable Reporter cmdlineReporter,
DigestUtil digestUtil,
- ImmutableSet<ActionInput> filesToDownload) {
+ ImmutableSet<ActionInput> filesToDownload,
+ RemotePathResolver remotePathResolver) {
this.execRoot = execRoot;
this.options = options;
this.verboseFailures = verboseFailures;
@@ -111,6 +114,7 @@
this.commandId = commandId;
this.digestUtil = digestUtil;
this.filesToDownload = Preconditions.checkNotNull(filesToDownload, "filesToDownload");
+ this.remotePathResolver = remotePathResolver;
}
@Override
@@ -125,8 +129,7 @@
Stopwatch totalTime = Stopwatch.createStarted();
- SortedMap<PathFragment, ActionInput> inputMap =
- context.getInputMapping(PathFragment.create(execRoot.getBaseName()));
+ SortedMap<PathFragment, ActionInput> inputMap = remotePathResolver.getInputMapping(context);
MerkleTree merkleTree =
MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil);
SpawnMetrics.Builder spawnMetrics =
@@ -144,7 +147,7 @@
spawn.getArguments(),
spawn.getEnvironment(),
platform,
- execRoot.getBaseName());
+ remotePathResolver);
RemoteOutputsMode remoteOutputsMode = options.remoteOutputsMode;
Action action =
RemoteSpawnRunner.buildAction(
@@ -186,8 +189,8 @@
prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs")) {
remoteCache.download(
remoteActionExecutionContext,
+ remotePathResolver,
result,
- execRoot,
context.getFileOutErr(),
context::lockOutputFiles);
}
@@ -199,12 +202,11 @@
inMemoryOutput =
remoteCache.downloadMinimal(
remoteActionExecutionContext,
- actionKey.getDigest().getHash(),
+ remotePathResolver,
result,
spawn.getOutputFiles(),
inMemoryOutputPath,
context.getFileOutErr(),
- execRoot,
context.getMetadataInjector(),
context::lockOutputFiles);
}
@@ -288,10 +290,10 @@
try (SilentCloseable c = prof.profile(ProfilerTask.UPLOAD_TIME, "upload outputs")) {
remoteCache.upload(
remoteActionExecutionContext,
+ remotePathResolver,
actionKey,
action,
command,
- execRoot.getParentDirectory(),
files,
context.getFileOutErr());
} catch (IOException e) {
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 92027b8..a5a0e56 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
@@ -71,6 +71,7 @@
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
+import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.merkletree.MerkleTree;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
@@ -122,6 +123,7 @@
private final String commandId;
private final DigestUtil digestUtil;
private final Path logDir;
+ private final RemotePathResolver remotePathResolver;
/**
* If {@link RemoteOutputsMode#TOPLEVEL} is specified it contains the artifacts that should be
@@ -145,7 +147,8 @@
ListeningScheduledExecutorService retryService,
DigestUtil digestUtil,
Path logDir,
- ImmutableSet<ActionInput> filesToDownload) {
+ ImmutableSet<ActionInput> filesToDownload,
+ RemotePathResolver remotePathResolver) {
this.execRoot = execRoot;
this.remoteOptions = remoteOptions;
this.executionOptions = executionOptions;
@@ -159,6 +162,7 @@
this.digestUtil = digestUtil;
this.logDir = logDir;
this.filesToDownload = Preconditions.checkNotNull(filesToDownload, "filesToDownload");
+ this.remotePathResolver = remotePathResolver;
}
@Override
@@ -213,14 +217,8 @@
context.report(ProgressStatus.SCHEDULING, getName());
RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode;
- // The "root directory" of the action from the point of view of RBE is the parent directory of
- // the execroot locally. This is so that paths of artifacts in external repositories don't
- // start with an uplevel reference...
- SortedMap<PathFragment, ActionInput> inputMap =
- context.getInputMapping(PathFragment.create(execRoot.getBaseName()));
- // ...however, MerkleTree.build() uses its execRoot parameter to resolve artifacts based on
- // ActionInput.getExecPath(), so it needs the execroot and not its parent directory.
+ SortedMap<PathFragment, ActionInput> inputMap = remotePathResolver.getInputMapping(context);
final MerkleTree merkleTree =
MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil);
SpawnMetrics.Builder spawnMetrics =
@@ -238,7 +236,7 @@
spawn.getArguments(),
spawn.getEnvironment(),
platform,
- execRoot.getBaseName());
+ remotePathResolver);
Digest commandHash = digestUtil.compute(command);
Action action =
buildAction(
@@ -281,7 +279,6 @@
try {
return downloadAndFinalizeSpawnResult(
remoteActionExecutionContext,
- actionKey.getDigest().getHash(),
cachedResult,
/* cacheHit= */ true,
spawn,
@@ -382,7 +379,6 @@
try {
return downloadAndFinalizeSpawnResult(
remoteActionExecutionContext,
- actionKey.getDigest().getHash(),
actionResult,
reply.getCachedResult(),
spawn,
@@ -461,7 +457,6 @@
private SpawnResult downloadAndFinalizeSpawnResult(
RemoteActionExecutionContext remoteActionExecutionContext,
- String actionId,
ActionResult actionResult,
boolean cacheHit,
Spawn spawn,
@@ -483,8 +478,8 @@
try (SilentCloseable c = Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs")) {
remoteCache.download(
remoteActionExecutionContext,
+ remotePathResolver,
actionResult,
- execRoot,
context.getFileOutErr(),
context::lockOutputFiles);
}
@@ -495,12 +490,11 @@
inMemoryOutput =
remoteCache.downloadMinimal(
remoteActionExecutionContext,
- actionId,
+ remotePathResolver,
actionResult,
spawn.getOutputFiles(),
inMemoryOutputPath,
context.getFileOutErr(),
- execRoot,
context.getMetadataInjector(),
context::lockOutputFiles);
}
@@ -636,8 +630,8 @@
// We try to download all (partial) results even on server error, for debuggability.
remoteCache.download(
remoteActionExecutionContext,
+ remotePathResolver,
resp.getResult(),
- execRoot,
outErr,
context::lockOutputFiles);
} catch (BulkTransferException bulkTransferEx) {
@@ -726,17 +720,12 @@
List<String> arguments,
ImmutableMap<String, String> env,
@Nullable Platform platform,
- @Nullable String workingDirectoryString) {
+ RemotePathResolver remotePathResolver) {
Command.Builder command = Command.newBuilder();
ArrayList<String> outputFiles = new ArrayList<>();
ArrayList<String> outputDirectories = new ArrayList<>();
- PathFragment workingDirectoryPathFragment =
- workingDirectoryString == null
- ? PathFragment.EMPTY_FRAGMENT
- : PathFragment.create(workingDirectoryString);
for (ActionInput output : outputs) {
- String pathString =
- workingDirectoryPathFragment.getRelative(output.getExecPath()).getPathString();
+ String pathString = remotePathResolver.localPathToOutputPath(output);
if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
outputDirectories.add(pathString);
} else {
@@ -758,8 +747,9 @@
command.addEnvironmentVariablesBuilder().setName(var).setValue(env.get(var));
}
- if (!Strings.isNullOrEmpty(workingDirectoryString)) {
- command.setWorkingDirectory(workingDirectoryString);
+ String workingDirectory = remotePathResolver.getWorkingDirectory();
+ if (!Strings.isNullOrEmpty(workingDirectory)) {
+ command.setWorkingDirectory(workingDirectory);
}
return command.build();
}
@@ -815,10 +805,10 @@
try (SilentCloseable c = Profiler.instance().profile(UPLOAD_TIME, "upload outputs")) {
remoteCache.upload(
remoteActionExecutionContext,
+ remotePathResolver,
actionKey,
action,
command,
- execRoot,
outputFiles,
context.getFileOutErr());
} catch (IOException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD
index 515eb63..36853a5 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD
@@ -14,9 +14,13 @@
name = "common",
srcs = glob(["*.java"]),
deps = [
+ "//src/main/java/com/google/devtools/build/lib/actions",
+ "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/concurrent",
+ "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/vfs",
+ "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:guava",
"//third_party/protobuf:protobuf_java",
"@googleapis//:google_longrunning_operations_java_proto",
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
new file mode 100644
index 0000000..d067820
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
@@ -0,0 +1,189 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.remote.common;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputHelper;
+import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.IOException;
+import java.util.SortedMap;
+
+/**
+ * A {@link RemotePathResolver} is used to resolve input/output paths for remote execution from
+ * Bazel's internal path, or vice versa.
+ */
+public interface RemotePathResolver {
+
+ /**
+ * Returns the {@code workingDirectory} for a remote action. Empty string if working directory is
+ * the input root.
+ */
+ String getWorkingDirectory();
+
+ /**
+ * Returns a {@link SortedMap} which maps from input paths for remote action to {@link
+ * ActionInput}.
+ */
+ SortedMap<PathFragment, ActionInput> getInputMapping(SpawnExecutionContext context)
+ throws IOException;
+
+ /** Resolves the output path relative to input root for the given {@link Path}. */
+ String localPathToOutputPath(Path path);
+
+ /**
+ * Resolves the output path relative to input root for the given {@link PathFragment}.
+ *
+ * @param execPath a path fragment relative to {@code execRoot}.
+ */
+ String localPathToOutputPath(PathFragment execPath);
+
+ /** Resolves the output path relative to input root for the {@link ActionInput}. */
+ default String localPathToOutputPath(ActionInput actionInput) {
+ return localPathToOutputPath(actionInput.getExecPath());
+ }
+
+ /**
+ * Resolves the local {@link Path} of an output file.
+ *
+ * @param outputPath the return value of {@link #localPathToOutputPath(PathFragment)}.
+ */
+ 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);
+ }
+
+ /**
+ * The default {@link RemotePathResolver} which use {@code execRoot} as input root and do NOT set
+ * {@code workingDirectory} for remote actions.
+ */
+ class DefaultRemotePathResolver implements RemotePathResolver {
+
+ private final Path execRoot;
+
+ public DefaultRemotePathResolver(Path execRoot) {
+ this.execRoot = execRoot;
+ }
+
+ @Override
+ public String getWorkingDirectory() {
+ return "";
+ }
+
+ @Override
+ public SortedMap<PathFragment, ActionInput> getInputMapping(SpawnExecutionContext context)
+ throws IOException {
+ return context.getInputMapping(PathFragment.EMPTY_FRAGMENT);
+ }
+
+ @Override
+ public String localPathToOutputPath(Path path) {
+ return path.relativeTo(execRoot).getPathString();
+ }
+
+ @Override
+ public String localPathToOutputPath(PathFragment execPath) {
+ return execPath.getPathString();
+ }
+
+ @Override
+ public Path outputPathToLocalPath(String outputPath) {
+ return execRoot.getRelative(outputPath);
+ }
+
+ @Override
+ public Path outputPathToLocalPath(ActionInput actionInput) {
+ return ActionInputHelper.toInputPath(actionInput, execRoot);
+ }
+ }
+
+ /**
+ * A {@link RemotePathResolver} used when {@code --experimental_sibling_repository_layout} is set.
+ * Use parent directory of {@code execRoot} and set {@code workingDirectory} to the base name of
+ * {@code execRoot}.
+ *
+ * <p>The paths of outputs are relative to {@code workingDirectory} if {@code
+ * --incompatible_remote_output_paths_relative_to_input_root} is not set, otherwise, relative to
+ * input root.
+ */
+ class SiblingRepositoryLayoutResolver implements RemotePathResolver {
+
+ private final Path execRoot;
+ private final boolean incompatibleRemoteOutputPathsRelativeToInputRoot;
+
+ public SiblingRepositoryLayoutResolver(Path execRoot) {
+ this(execRoot, /* incompatibleRemoteOutputPathsRelativeToInputRoot= */ false);
+ }
+
+ public SiblingRepositoryLayoutResolver(
+ Path execRoot, boolean incompatibleRemoteOutputPathsRelativeToInputRoot) {
+ this.execRoot = execRoot;
+ this.incompatibleRemoteOutputPathsRelativeToInputRoot =
+ incompatibleRemoteOutputPathsRelativeToInputRoot;
+ }
+
+ @Override
+ public String getWorkingDirectory() {
+ return execRoot.getBaseName();
+ }
+
+ @Override
+ public SortedMap<PathFragment, ActionInput> getInputMapping(SpawnExecutionContext context)
+ throws IOException {
+ // The "root directory" of the action from the point of view of RBE is the parent directory of
+ // the execroot locally. This is so that paths of artifacts in external repositories don't
+ // start with an uplevel reference.
+ return context.getInputMapping(PathFragment.create(checkNotNull(getWorkingDirectory())));
+ }
+
+ private Path getBase() {
+ if (incompatibleRemoteOutputPathsRelativeToInputRoot) {
+ return execRoot.getParentDirectory();
+ } else {
+ return execRoot;
+ }
+ }
+
+ @Override
+ public String localPathToOutputPath(Path path) {
+ return path.relativeTo(getBase()).getPathString();
+ }
+
+ @Override
+ public String localPathToOutputPath(PathFragment execPath) {
+ return localPathToOutputPath(execRoot.getRelative(execPath));
+ }
+
+ @Override
+ public Path outputPathToLocalPath(String outputPath) {
+ return getBase().getRelative(outputPath);
+ }
+
+ @Override
+ public Path outputPathToLocalPath(ActionInput actionInput) {
+ return ActionInputHelper.toInputPath(actionInput, execRoot);
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java
index 7f7725a..872fe3a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java
@@ -267,6 +267,19 @@
public boolean incompatibleRemoteResultsIgnoreDisk;
@Option(
+ name = "incompatible_remote_output_paths_relative_to_input_root",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.REMOTE,
+ effectTags = {OptionEffectTag.UNKNOWN},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "If set to true, output paths are relative to input root instead of working directory.")
+ public boolean incompatibleRemoteOutputPathsRelativeToInputRoot;
+
+ @Option(
name = "remote_instance_name",
defaultValue = "",
documentationCategory = OptionDocumentationCategory.REMOTE,
