From 5c02b92c45b5422971b3164685d9edb771367a1b Mon Sep 17 00:00:00 2001
From: Jakob Buchgraber <buchgr@google.com>
Date: Thu, 29 Aug 2019 05:31:04 -0700
Subject: [PATCH] Make --workspace_status_command work with "Builds without the
 Bytes".

Fixes #8385

Due to a typo in delete() stable-status.txt would not get deleted
between builds. More details in #8385.

Thanks @emfree for the debugging work and pointing out the root cause!

Closes #9280.

PiperOrigin-RevId: 266118848
---
 .../lib/remote/RemoteActionFileSystem.java    |  2 +-
 .../remote/RemoteActionFileSystemTest.java    | 29 +++++++++++++++
 .../bazel/remote/remote_execution_test.sh     | 37 +++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
index fbdcee74f2ee4b..5afe207e86fb87 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
@@ -74,7 +74,7 @@ public String getFileSystemType(Path path) {
   @Override
   public boolean delete(Path path) throws IOException {
     RemoteFileArtifactValue m = getRemoteInputMetadata(path);
-    if (m != null) {
+    if (m == null) {
       return super.delete(path);
     }
     return true;
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
index 400276b5aa6a65..2e66e630ea2e24 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
@@ -126,6 +126,35 @@ public void testCreateSymbolicLink() throws InterruptedException, IOException {
     verifyNoMoreInteractions(inputFetcher);
   }
 
+  @Test
+  public void testDeleteRemoteFile() throws Exception {
+    // arrange
+    ActionInputMap inputs = new ActionInputMap(1);
+    Artifact remoteArtifact = createRemoteArtifact("remote-file", "remote contents", inputs);
+    FileSystem actionFs = newRemoteActionFileSystem(inputs);
+
+    // act
+    boolean success = actionFs.delete(actionFs.getPath(remoteArtifact.getPath().getPathString()));
+
+    // assert
+    assertThat(success).isTrue();
+  }
+
+  @Test
+  public void testDeleteLocalFile() throws Exception {
+    // arrange
+    ActionInputMap inputs = new ActionInputMap(0);
+    FileSystem actionFs = newRemoteActionFileSystem(inputs);
+    Path filePath = actionFs.getPath(execRoot.getPathString()).getChild("local-file");
+    FileSystemUtils.writeContent(filePath, StandardCharsets.UTF_8, "local contents");
+
+    // act
+    boolean success = actionFs.delete(actionFs.getPath(filePath.getPathString()));
+
+    // assert
+    assertThat(success).isTrue();
+  }
+
   private FileSystem newRemoteActionFileSystem(ActionInputMap inputs) {
     return new RemoteActionFileSystem(
         fs,
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index cca4f8273ab141..af6a303831e685 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -1158,6 +1158,43 @@ EOF
   expect_not_log "remote cache hit"
 }
 
+function test_downloads_minimal_stable_status() {
+  # Regression test for #8385
+
+  mkdir -p a
+  cat > a/BUILD <<'EOF'
+genrule(
+  name = "foo",
+  srcs = [],
+  outs = ["foo.txt"],
+  cmd = "echo \"foo\" > \"$@\"",
+)
+EOF
+
+cat > status.sh << 'EOF'
+#!/bin/sh
+echo "STABLE_FOO 1"
+EOF
+chmod +x status.sh
+
+  bazel build \
+    --remote_executor=grpc://localhost:${worker_port} \
+    --experimental_remote_download_minimal \
+    --workspace_status_command=status.sh \
+    //a:foo || "Failed to build //a:foo"
+
+cat > status.sh << 'EOF'
+#!/bin/sh
+echo "STABLE_FOO 2"
+EOF
+
+  bazel build \
+    --remote_executor=grpc://localhost:${worker_port} \
+    --experimental_remote_download_minimal \
+    --workspace_status_command=status.sh \
+    //a:foo || "Failed to build //a:foo"
+}
+
 # TODO(alpha): Add a test that fails remote execution when remote worker
 # supports sandbox.