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 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 bazelbuild#9172, but is broken after bazelbuild@24c980b. 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 bazelbuild#13188.

Closes bazelbuild#13339.

PiperOrigin-RevId: 369168230
  • Loading branch information
coeuvre committed Jul 16, 2021
1 parent c901827 commit b56a2aa
Show file tree
Hide file tree
Showing 23 changed files with 845 additions and 245 deletions.
Expand Up @@ -246,4 +246,8 @@ public enum WorkerProtocolFormat {
* followed by a {@code SIGKILL} after a grace period).
*/
public static final String GRACEFUL_TERMINATION = "supports-graceful-termination";

/** Requires the execution service do NOT share caches across different workspace. */
public static final String DIFFERENTIATE_WORKSPACE_CACHE =
"internal-differentiate-workspace-cache";
}
Expand Up @@ -43,6 +43,7 @@ java_library(
srcs = ["PlatformUtils.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
Expand Down
Expand Up @@ -19,6 +19,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Ordering;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
Expand Down Expand Up @@ -101,6 +102,16 @@ public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions rem
}
}

String workspace =
spawn.getExecutionInfo().get(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE);
if (workspace != null) {
platformBuilder.addProperties(
Property.newBuilder()
.setName("bazel-differentiate-workspace-cache")
.setValue(workspace)
.build());
}

sortPlatformProperties(platformBuilder);
return platformBuilder.build();
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/remote/BUILD
Expand Up @@ -70,6 +70,7 @@ java_library(
"//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",
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -80,6 +84,22 @@ public static RemoteActionContextProvider createForRemoteExecution(
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.
Expand Down Expand Up @@ -108,7 +128,8 @@ public void registerRemoteSpawnStrategyIfApplicable(
retryScheduler,
digestUtil,
logDir,
filesToDownload);
filesToDownload,
createRemotePathResolver());
registryBuilder.registerStrategy(
new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner, verboseFailures), "remote");
}
Expand All @@ -129,7 +150,8 @@ public void registerSpawnCache(ModuleActionContextRegistry.Builder registryBuild
env.getCommandId().toString(),
env.getReporter(),
digestUtil,
filesToDownload);
filesToDownload,
createRemotePathResolver());
registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache");
}

Expand Down

0 comments on commit b56a2aa

Please sign in to comment.