Remote: Use Action's salt field to differentiate cache across workspa…

…ces.

Resolves #13339 (comment).

Closes #14184.

PiperOrigin-RevId: 410407819

src/main/java/com/google/devtools/build/lib/analysis/platform/BUILD

@@ -44,7 +44,6 @@ 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",

src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java

@@ -19,7 +19,6 @@
 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;
@@ -115,16 +114,6 @@ 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();
  }

src/main/java/com/google/devtools/build/lib/remote/BUILD

@@ -48,6 +48,7 @@ java_library(
  "//src/main/java/com/google/devtools/build/lib/actions",
  "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
  "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+ "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
  "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
  "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink",
  "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

@@ -73,6 +73,7 @@
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
 import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
 import com.google.devtools.build.lib.actions.MetadataProvider;
@@ -307,6 +308,11 @@ public String getActionId() {
  return actionKey.getDigest().getHash();
  }
 
+ /** Returns underlying {@link Action} of this remote action. */
+ public Action getAction() {
+ return action;
+ }
+
  /**
  * Returns a {@link SortedMap} which maps from input paths for remote action to {@link
  * ActionInput}.
@@ -420,6 +426,22 @@ public MerkleTree uncachedBuildMerkleTreeVisitor(
  return MerkleTree.merge(subMerkleTrees, digestUtil);
  }
 
+ @Nullable
+ private static ByteString buildSalt(Spawn spawn) {
+ String workspace =
+ spawn.getExecutionInfo().get(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE);
+ if (workspace != null) {
+ Platform platform =
+ Platform.newBuilder()
+ .addProperties(
+ Platform.Property.newBuilder().setName("workspace").setValue(workspace).build())
+ .build();
+ return platform.toByteString();
+ }
+
+ return null;
+ }
+
  /** Creates a new {@link RemoteAction} instance from spawn. */
  public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context)
  throws IOException, UserExecException, ForbiddenActionInputException {
@@ -442,7 +464,8 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
  merkleTree.getRootDigest(),
  platform,
  context.getTimeout(),
- Spawns.mayBeCachedRemotely(spawn));
+ Spawns.mayBeCachedRemotely(spawn),
+ buildSalt(spawn));
 
  ActionKey actionKey = digestUtil.computeActionKey(action);
 

src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java

@@ -132,7 +132,13 @@ public ExecutionResult execute(
  Digest commandHash = digestUtil.compute(command);
  MerkleTree merkleTree = MerkleTree.build(inputFiles, digestUtil);
  Action action =
- buildAction(commandHash, merkleTree.getRootDigest(), platform, timeout, acceptCached);
+ buildAction(
+ commandHash,
+ merkleTree.getRootDigest(),
+ platform,
+ timeout,
+ acceptCached,
+ /*salt=*/ null);
  Digest actionDigest = digestUtil.compute(action);
  ActionKey actionKey = new ActionKey(actionDigest);
  CachedActionResult cachedActionResult;

src/main/java/com/google/devtools/build/lib/remote/util/Utils.java

@@ -433,7 +433,8 @@ public static Action buildAction(
  Digest inputRoot,
  @Nullable Platform platform,
  java.time.Duration timeout,
- boolean cacheable) {
+ boolean cacheable,
+ @Nullable ByteString salt) {
  Action.Builder action = Action.newBuilder();
  action.setCommandDigest(command);
  action.setInputRootDigest(inputRoot);
@@ -446,6 +447,9 @@ public static Action buildAction(
  if (platform != null) {
  action.setPlatform(platform);
  }
+ if (salt != null) {
+ action.setSalt(salt);
+ }
  return action.build();
  }
 

src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformUtilsTest.java

@@ -19,7 +19,6 @@
 import build.bazel.remote.execution.v2.Platform;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.exec.util.SpawnBuilder;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
@@ -99,26 +98,6 @@ public void testParsePlatformSortsProperties_execProperties() throws Exception {
  assertThat(PlatformUtils.getPlatformProto(s, null)).isEqualTo(expected);
  }
 
- @Test
- public void testGetPlatformProto_differentiateWorkspace() throws Exception {
- Spawn s =
- new SpawnBuilder("dummy")
- .withExecutionInfo(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "aa")
- .build();
-
- Platform expected =
- Platform.newBuilder()
- .addProperties(Platform.Property.newBuilder().setName("a").setValue("1"))
- .addProperties(Platform.Property.newBuilder().setName("b").setValue("2"))
- .addProperties(
- Platform.Property.newBuilder()
- .setName("bazel-differentiate-workspace-cache")
- .setValue("aa"))
- .build();
- // execProperties are sorted by key
- assertThat(PlatformUtils.getPlatformProto(s, remoteOptions())).isEqualTo(expected);
- }
-
  @Test
  public void getPlatformProto_mergeTargetExecPropertiesWithPlatform() throws Exception {
  Spawn spawn = new SpawnBuilder("dummy").withExecProperties(ImmutableMap.of("c", "3")).build();

src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java

@@ -40,6 +40,7 @@
 import build.bazel.remote.execution.v2.OutputDirectory;
 import build.bazel.remote.execution.v2.OutputFile;
 import build.bazel.remote.execution.v2.OutputSymlink;
+import build.bazel.remote.execution.v2.Platform;
 import build.bazel.remote.execution.v2.RequestMetadata;
 import build.bazel.remote.execution.v2.SymlinkNode;
 import build.bazel.remote.execution.v2.Tree;
@@ -59,6 +60,7 @@
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.SimpleSpawn;
@@ -76,6 +78,7 @@
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.exec.util.FakeOwner;
+import com.google.devtools.build.lib.exec.util.SpawnBuilder;
 import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction;
 import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult;
 import com.google.devtools.build.lib.remote.common.BulkTransferException;
@@ -164,6 +167,24 @@ public final void setUp() throws Exception {
  remoteActionExecutionContext = RemoteActionExecutionContext.create(metadata);
  }
 
+ @Test
+ public void buildRemoteAction_differentiateWorkspace_generateActionSalt() throws Exception {
+ Spawn spawn =
+ new SpawnBuilder("dummy")
+ .withExecutionInfo(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "aa")
+ .build();
+ FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
+ RemoteExecutionService service = newRemoteExecutionService();
+
+ RemoteAction remoteAction = service.buildRemoteAction(spawn, context);
+
+ Platform expected =
+ Platform.newBuilder()
+ .addProperties(Platform.Property.newBuilder().setName("workspace").setValue("aa"))
+ .build();
+ assertThat(remoteAction.getAction().getSalt()).isEqualTo(expected.toByteString());
+ }
+
  @Test
  public void downloadOutputs_outputFiles_executableBitIgnored() throws Exception {
  // Test that executable bit of downloaded output files are ignored since it will be chmod 555