Skip to content

Commit

Permalink
Remote: Use execRoot as input root and do NOT set working directory b…
Browse files Browse the repository at this point in the history
…y 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. 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 bazelbuild#9172, but is broken after cl/356735700. The reason test didn't fail is 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 bazelbuild#13188.
  • Loading branch information
coeuvre committed Apr 19, 2021
1 parent bbde729 commit 6636dc2
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import build.bazel.remote.execution.v2.Platform;
import build.bazel.remote.execution.v2.Platform.Property;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Ordering;
import com.google.devtools.build.lib.actions.Spawn;
Expand Down Expand Up @@ -61,7 +62,8 @@ public static Platform buildPlatformProto(Map<String, String> executionPropertie
}

@Nullable
public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions remoteOptions)
public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions remoteOptions,
List<Property> extraProperties)
throws UserExecException {
SortedMap<String, String> defaultExecProperties =
remoteOptions != null
Expand Down Expand Up @@ -101,10 +103,18 @@ public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions rem
}
}

platformBuilder.addAllProperties(extraProperties);

sortPlatformProperties(platformBuilder);
return platformBuilder.build();
}

@Nullable
public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions remoteOptions)
throws UserExecException {
return getPlatformProto(spawn, remoteOptions, ImmutableList.of());
}

private static FailureDetail createFailureDetail(String message, Code detailedCode) {
return FailureDetail.newBuilder()
.setMessage(message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
Digest merkleTreeRoot = merkleTree.getRootDigest();

// Get the remote platform properties.
Platform platform = PlatformUtils.getPlatformProto(spawn, options);
Platform platform = PlatformUtils.getPlatformProto(spawn, options,
RemoteSpawnRunner.getExtraPlatformProperties(spawn, execRoot));

Command command =
RemoteSpawnRunner.buildCommand(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import build.bazel.remote.execution.v2.ExecutionStage.Value;
import build.bazel.remote.execution.v2.LogFile;
import build.bazel.remote.execution.v2.Platform;
import build.bazel.remote.execution.v2.Platform.Property;
import build.bazel.remote.execution.v2.RequestMetadata;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -696,6 +697,22 @@ private SpawnResult handleError(
.build();
}

static List<Property> getExtraPlatformProperties(Spawn spawn, Path execRoot) {
// Hack on Windows. The included headers dumped by cl.exe in stdout contain absolute paths.
// When compiling the file from different workspace, the shared cache will cause header
// dependency checking to fail. This was initially fixed by a hack (see
// https://github.com/bazelbuild/bazel/issues/9172 for more details), but is broken again due
// to cl/356735700. We include workspace name here so action results from different workspaces
// on Windows won't be shared.
boolean isWindows = System.getProperty("os.name").startsWith("Windows");
if (isWindows) {
return ImmutableList
.of(Property.newBuilder().setName("workspace").setValue(execRoot.getBaseName()).build());
}

return ImmutableList.of();
}

static Action buildAction(
Digest command,
Digest inputRoot,
Expand Down

0 comments on commit 6636dc2

Please sign in to comment.