diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index 7c16c2973542f0..ae57825d11a2bb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/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> queryRemoteCache( @@ -182,7 +199,8 @@ private Single> queryRemoteCache( List filesToQuery = new ArrayList<>(); Set 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> uploadLocalFiles( path.getPath(), /*digest=*/ null, path.isDirectory(), - path.isRemote())); + path.isRemote(), + path.isOmitted())); }); }) .collect(Collectors.toList()); @@ -265,13 +284,17 @@ private Single upload(Set 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 skippedPaths; private final Set localPaths; - PathConverterImpl( - String remoteServerInstanceName, List uploads, Set localPaths) { + PathConverterImpl(String remoteServerInstanceName, List uploads) { Preconditions.checkNotNull(uploads); this.remoteServerInstanceName = remoteServerInstanceName; pathToDigest = new HashMap<>(uploads.size()); ImmutableSet.Builder skippedPaths = ImmutableSet.builder(); + ImmutableSet.Builder 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 diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index c14f7b2a6c023a..d84afa55b4c5ea 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/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 <