From baa440f8983c2decc0e3adfa7d3a157d62dbf5b2 Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Tue, 13 Apr 2021 17:45:30 +0800 Subject: [PATCH] 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. 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 https://github.com/bazelbuild/bazel/issues/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 https://github.com/bazelbuild/bazel/issues/13188. --- .../lib/analysis/platform/PlatformUtils.java | 12 +++++++++++- .../build/lib/remote/RemoteSpawnCache.java | 3 ++- .../build/lib/remote/RemoteSpawnRunner.java | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java index 621a598b25aea4..8d906d47c50195 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java @@ -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; @@ -61,7 +62,8 @@ public static Platform buildPlatformProto(Map executionPropertie } @Nullable - public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions remoteOptions) + public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions remoteOptions, + List extraProperties) throws UserExecException { SortedMap defaultExecProperties = remoteOptions != null @@ -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) 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 f198bab5dc312c..1ea0b5323fa39d 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 @@ -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( 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 547ebd8673d645..c9fb821498950d 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 @@ -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; @@ -696,6 +697,22 @@ private SpawnResult handleError( .build(); } + static List 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,