[6.4.0] Take the no-remote-exec tag into account when computing the action salt

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

@@ -117,6 +117,7 @@ java_library(
  "//src/main/java/com/google/devtools/build/skyframe",
  "//src/main/java/com/google/devtools/common/options",
  "//src/main/protobuf:failure_details_java_proto",
+ "//src/main/protobuf:spawn_java_proto",
  "//third_party:auth",
  "//third_party:caffeine",
  "//third_party:flogger",

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

@@ -77,6 +77,7 @@
 import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.exec.Protos.CacheSalt;
 import com.google.devtools.build.lib.exec.SpawnInputExpander.InputWalker;
 import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
 import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
@@ -475,18 +476,16 @@ public MerkleTree uncachedBuildMerkleTreeVisitor(
 
  @Nullable
  private static ByteString buildSalt(Spawn spawn) {
+ CacheSalt.Builder saltBuilder =
+ CacheSalt.newBuilder().setMayBeExecutedRemotely(Spawns.mayBeExecutedRemotely(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();
+ saltBuilder.setWorkspace(workspace);
  }
 
- return null;
+ return saltBuilder.build().toByteString();
  }
 
  /**

src/main/protobuf/spawn.proto

@@ -185,3 +185,17 @@ message SpawnExec {
  // Timing, size and memory statistics.
  SpawnMetrics metrics = 20;
 }
+
+// Additional information that should be taken into account when
+// computing the key of an action, thereby ensuring that actions remain
+// distinct.
+message CacheSalt {
+ // Whether or not the action may be executed remotely, if remote
+ // execution were to be enabled. This ensures that adding/removing the
+ // "no-remote-exec" tag from a target forces a local/remote rebuild.
+ bool may_be_executed_remotely = 1;
+
+ // Requires the execution service do NOT share caches across different
+ // workspace.
+ string workspace = 2;
+}

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

@@ -98,6 +98,7 @@ java_test(
  "//src/main/java/com/google/devtools/common/options",
  "//src/main/protobuf:failure_details_java_proto",
  "//src/main/protobuf:remote_execution_log_java_proto",
+ "//src/main/protobuf:spawn_java_proto",
  "//src/test/java/com/google/devtools/build/lib:test_runner",
  "//src/test/java/com/google/devtools/build/lib/actions/util",
  "//src/test/java/com/google/devtools/build/lib/analysis/util",

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

@@ -80,6 +80,7 @@
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.exec.Protos.CacheSalt;
 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.RemoteActionResult;
@@ -261,7 +262,7 @@ public void buildRemoteAction_withActionInputDirectoryAsOutput() throws Exceptio
  }
 
  @Test
- public void buildRemoteAction_differentiateWorkspace_generateActionSalt() throws Exception {
+ public void buildRemoteAction_generateActionSalt_differentiateWorkspaceCache() throws Exception {
  Spawn spawn =
  new SpawnBuilder("dummy")
  .withExecutionInfo(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "aa")
@@ -271,10 +272,23 @@ public void buildRemoteAction_differentiateWorkspace_generateActionSalt() throws
 
  RemoteAction remoteAction = service.buildRemoteAction(spawn, context);
 
- Platform expected =
- Platform.newBuilder()
- .addProperties(Platform.Property.newBuilder().setName("workspace").setValue("aa"))
+ CacheSalt expected =
+ CacheSalt.newBuilder().setMayBeExecutedRemotely(true).setWorkspace("aa").build();
+ assertThat(remoteAction.getAction().getSalt()).isEqualTo(expected.toByteString());
+ }
+
+ @Test
+ public void buildRemoteAction_generateActionSalt_noRemoteExec() throws Exception {
+ Spawn spawn =
+ new SpawnBuilder("dummy")
+ .withExecutionInfo(ExecutionRequirements.NO_REMOTE_EXEC, "")
  .build();
+ FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
+ RemoteExecutionService service = newRemoteExecutionService();
+
+ RemoteAction remoteAction = service.buildRemoteAction(spawn, context);
+
+ CacheSalt expected = CacheSalt.newBuilder().setMayBeExecutedRemotely(false).build();
  assertThat(remoteAction.getAction().getSalt()).isEqualTo(expected.toByteString());
  }
 

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

@@ -152,7 +152,7 @@ public class RemoteSpawnRunnerTest {
 
  // The action key of the Spawn returned by newSimpleSpawn().
  private final String simpleActionId =
- "eb45b20cc979d504f96b9efc9a08c48103c6f017afa09c0df5c70a5f92a98ea8";
+ "31aea267dc597b047a9b6993100415b6406f82822318dc8988e4164a535b51ee";
 
  @Before
  public final void setUp() throws Exception {
@@ -557,7 +557,7 @@ public void testHumanReadableServerLogsSavedForFailingActionWithSiblingRepositor
  Digest logDigest = digestUtil.computeAsUtf8("bla");
  Path logPath =
  logDir
- .getRelative("b9a727771337fd8ce54821f4805e2d451c4739e92fec6f8ecdb18ff9d1983b27")
+ .getRelative("e0a5a3561464123504c1240b3587779cdfd6adee20f72aa136e388ecfd570c12")
  .getRelative("logname");
  ExecuteResponse resp =
  ExecuteResponse.newBuilder()