Fix checking remote cache for omitted files in buildevent file
Enabling both `--noremote_upload_local_results` and
`--incompatible_remote_build_event_upload_respect_no_cache` caused
ByteStreamBuildEventArtifactuploader not to check if some files already
existed remotely because local files were not to be uploaded. When using
remote execution some of these files might exist remotely, so we want to
check remote cache for those files even if we would not upload them if
missing. The test case included depicts this failure case.
Closes #15280.
PiperOrigin-RevId: 442798312
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 693aca6..5d6d58b 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
@@ -112,12 +112,14 @@
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() {
@@ -135,6 +137,10 @@
public boolean isRemote() {
return remote;
}
+
+ public boolean isOmitted() {
+ return omitted;
+ }
}
/**
@@ -143,22 +149,29 @@
*/
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(xattrProvider, 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(
@@ -171,14 +184,18 @@
} 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(
@@ -187,7 +204,8 @@
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 {
@@ -244,7 +262,8 @@
path.getPath(),
/*digest=*/ null,
path.isDirectory(),
- path.isRemote()));
+ path.isRemote(),
+ path.isOmitted()));
});
})
.collect(Collectors.toList());
@@ -271,13 +290,17 @@
} 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);
}
@@ -311,23 +334,25 @@
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
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index de06eb5..b4813d0 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -3606,6 +3606,35 @@
[[ "$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
