[5.2] Fix checking remote cache for omitted files in buildevent file

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

@@ -107,12 +107,14 @@ private static final class PathMetadata {
     private final Digest digest;
     private final boolean directory;
     private final boolean remote;
+    private final boolean omitted;
 
-    PathMetadata(Path path, Digest digest, boolean directory, boolean remote) {
+    PathMetadata(Path path, Digest digest, boolean directory, boolean remote, boolean omitted) {
       this.path = path;
       this.digest = digest;
       this.directory = directory;
       this.remote = remote;
+      this.omitted = omitted;
     }
 
     public Path getPath() {
@@ -130,6 +132,10 @@ public boolean isDirectory() {
     public boolean isRemote() {
       return remote;
     }
+
+    public boolean isOmitted() {
+      return omitted;
+    }
   }
 
   /**
@@ -138,22 +144,29 @@ public boolean isRemote() {
    */
   private PathMetadata readPathMetadata(Path file) throws IOException {
     if (file.isDirectory()) {
-      return new PathMetadata(file, /* digest= */ null, /* directory= */ true, /* remote= */ false);
-    }
-    if (omittedFiles.contains(file)) {
-      return new PathMetadata(file, /*digest=*/ null, /*directory=*/ false, /*remote=*/ false);
+      return new PathMetadata(
+          file,
+          /* digest= */ null,
+          /* directory= */ true,
+          /* remote= */ false,
+          /* omitted= */ false);
     }
 
-    for (Path treeRoot : omittedTreeRoots) {
-      if (file.startsWith(treeRoot)) {
-        omittedFiles.add(file);
-        return new PathMetadata(file, /*digest=*/ null, /*directory=*/ false, /*remote=*/ false);
+    boolean omitted = false;
+    if (omittedFiles.contains(file)) {
+      omitted = true;
+    } else {
+      for (Path treeRoot : omittedTreeRoots) {
+        if (file.startsWith(treeRoot)) {
+          omittedFiles.add(file);
+          omitted = true;
+        }
       }
     }
 
     DigestUtil digestUtil = new DigestUtil(file.getFileSystem().getDigestFunction());
     Digest digest = digestUtil.compute(file);
-    return new PathMetadata(file, digest, /* directory= */ false, isRemoteFile(file));
+    return new PathMetadata(file, digest, /* directory= */ false, isRemoteFile(file), omitted);
   }
 
   private static void processQueryResult(
@@ -166,14 +179,18 @@ private static void processQueryResult(
       } else {
         PathMetadata remotePathMetadata =
             new PathMetadata(
-                file.getPath(), file.getDigest(), file.isDirectory(), /* remote= */ true);
+                file.getPath(),
+                file.getDigest(),
+                file.isDirectory(),
+                /* remote= */ true,
+                file.isOmitted());
         knownRemotePaths.add(remotePathMetadata);
       }
     }
   }
 
   private static boolean shouldUpload(PathMetadata path) {
-    return path.getDigest() != null && !path.isRemote() && !path.isDirectory();
+    return path.getDigest() != null && !path.isRemote() && !path.isDirectory() && !path.isOmitted();
   }
 
   private Single<List<PathMetadata>> queryRemoteCache(
@@ -182,7 +199,8 @@ private Single<List<PathMetadata>> queryRemoteCache(
     List<PathMetadata> filesToQuery = new ArrayList<>();
     Set<Digest> digestsToQuery = new HashSet<>();
     for (PathMetadata path : paths) {
-      if (shouldUpload(path)) {
+      // Query remote cache for files even if omitted from uploading
+      if (shouldUpload(path) || path.isOmitted()) {
         filesToQuery.add(path);
         digestsToQuery.add(path.getDigest());
       } else {
@@ -239,7 +257,8 @@ private Single<List<PathMetadata>> uploadLocalFiles(
                                 path.getPath(),
                                 /*digest=*/ null,
                                 path.isDirectory(),
-                                path.isRemote()));
+                                path.isRemote(),
+                                path.isOmitted()));
                       });
             })
         .collect(Collectors.toList());
@@ -265,13 +284,17 @@ private Single<PathConverter> upload(Set<Path> files) {
                       } catch (IOException e) {
                         reporterUploadError(e);
                         return new PathMetadata(
-                            file, /*digest=*/ null, /*directory=*/ false, /*remote=*/ false);
+                            file,
+                            /*digest=*/ null,
+                            /*directory=*/ false,
+                            /*remote=*/ false,
+                            /*omitted=*/ false);
                       }
                     })
                 .collect(Collectors.toList())
                 .flatMap(paths -> queryRemoteCache(remoteCache, context, paths))
                 .flatMap(paths -> uploadLocalFiles(remoteCache, context, paths))
-                .map(paths -> new PathConverterImpl(remoteServerInstanceName, paths, omittedFiles)),
+                .map(paths -> new PathConverterImpl(remoteServerInstanceName, paths)),
         RemoteCache::release);
   }
 
@@ -305,23 +328,25 @@ private static class PathConverterImpl implements PathConverter {
     private final Set<Path> skippedPaths;
     private final Set<Path> localPaths;
 
-    PathConverterImpl(
-        String remoteServerInstanceName, List<PathMetadata> uploads, Set<Path> localPaths) {
+    PathConverterImpl(String remoteServerInstanceName, List<PathMetadata> uploads) {
       Preconditions.checkNotNull(uploads);
       this.remoteServerInstanceName = remoteServerInstanceName;
       pathToDigest = new HashMap<>(uploads.size());
       ImmutableSet.Builder<Path> skippedPaths = ImmutableSet.builder();
+      ImmutableSet.Builder<Path> localPaths = ImmutableSet.builder();
       for (PathMetadata pair : uploads) {
         Path path = pair.getPath();
         Digest digest = pair.getDigest();
-        if (digest != null) {
+        if (!pair.isRemote() && pair.isOmitted()) {
+          localPaths.add(path);
+        } else if (digest != null) {
           pathToDigest.put(path, digest);
         } else {
           skippedPaths.add(path);
         }
       }
       this.skippedPaths = skippedPaths.build();
-      this.localPaths = localPaths;
+      this.localPaths = localPaths.build();
     }
 
     @Override

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -3498,6 +3498,35 @@ EOF
   [[ "$remote_cas_files" == 1 ]] || fail "Expected 1 remote cas entries, not $remote_cas_files"
 }
 
+function test_remote_exec_convert_remote_file() {
+  mkdir -p a
+  cat > a/BUILD <<'EOF'
+sh_test(
+  name = "test",
+  srcs = ["test.sh"],
+)
+EOF
+  cat > a/test.sh <<'EOF'
+#!/bin/bash
+set -e
+echo \"foo\"
+exit 0
+EOF
+  chmod 755 a/test.sh
+
+  bazel test \
+    --remote_executor=grpc://localhost:${worker_port} \
+    --build_event_json_file=bep.json \
+    --noremote_upload_local_results \
+    --incompatible_remote_build_event_upload_respect_no_cache \
+    //a:test >& $TEST_log || "Failed to test //a:test"
+
+  cat bep.json > $TEST_log
+  expect_not_log "test\.log.*file://" || fail "remote files generated in remote execution are not converted"
+  expect_log "test\.log.*bytestream://" || fail "remote files generated in remote execution are not converted"
+}
+
+
 function test_uploader_ignore_disk_cache_of_combined_cache() {
   mkdir -p a
   cat > a/BUILD <<EOF