Skip to content

Commit

Permalink
Ensure namedSetOfFiles URIs specify blob type
Browse files Browse the repository at this point in the history
  • Loading branch information
@tylerwilliams
tylerwilliams committed Jul 25, 2023
1 parent f48c0f6 commit 2b83de5
Showing 1 changed file with 66 additions and 21 deletions.
87 changes: 66 additions & 21 deletions src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import static com.google.devtools.build.lib.remote.util.Utils.grpcAwareErrorMessage;

import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.DigestFunction;
import build.bazel.remote.execution.v2.RequestMetadata;
import com.google.common.base.Ascii;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
Expand Down Expand Up @@ -135,20 +137,23 @@ private static final class PathMetadata {
private final boolean symlink;
private final boolean remote;
private final boolean omitted;
private final DigestFunction.Value digestFunction;

PathMetadata(
Path path,
Digest digest,
boolean directory,
boolean symlink,
boolean remote,
boolean omitted) {
boolean omitted,
DigestFunction.Value digestFunction) {
this.path = path;
this.digest = digest;
this.directory = directory;
this.symlink = symlink;
this.remote = remote;
this.omitted = omitted;
this.digestFunction = digestFunction;
}

public Path getPath() {
Expand All @@ -174,13 +179,19 @@ public boolean isRemote() {
public boolean isOmitted() {
return omitted;
}

public DigestFunction.Value getDigestFunction() {
return digestFunction;
}
}

/**
* Collects metadata for {@code file}. Depending on the underlying filesystem used this method
* might do I/O.
*/
private PathMetadata readPathMetadata(Path path, LocalFile file) throws IOException {
DigestUtil digestUtil = new DigestUtil(xattrProvider, path.getFileSystem().getDigestFunction());

if (file.type == LocalFileType.OUTPUT_DIRECTORY
|| (file.type == LocalFileType.OUTPUT && path.isDirectory())) {
return new PathMetadata(
Expand All @@ -189,7 +200,8 @@ private PathMetadata readPathMetadata(Path path, LocalFile file) throws IOExcept
/* directory= */ true,
/* symlink= */ false,
/* remote= */ false,
/* omitted= */ false);
/* omitted= */ false,
/* digestFunction= */ digestUtil.getDigestFunction());
}
if (file.type == LocalFileType.OUTPUT_SYMLINK) {
return new PathMetadata(
Expand All @@ -198,7 +210,8 @@ private PathMetadata readPathMetadata(Path path, LocalFile file) throws IOExcept
/* directory= */ false,
/* symlink= */ true,
/* remote= */ false,
/* omitted= */ false);
/* omitted= */ false,
/* digestFunction= */ digestUtil.getDigestFunction());
}

PathFragment filePathFragment = path.asFragment();
Expand All @@ -214,10 +227,15 @@ private PathMetadata readPathMetadata(Path path, LocalFile file) throws IOExcept
}
}

DigestUtil digestUtil = new DigestUtil(xattrProvider, path.getFileSystem().getDigestFunction());
Digest digest = digestUtil.compute(path);
return new PathMetadata(
path, digest, /* directory= */ false, /* symlink= */ false, isRemoteFile(path), omitted);
path,
digest,
/* directory= */ false,
/* symlink= */ false,
isRemoteFile(path),
omitted,
digestUtil.getDigestFunction());
}

private static void processQueryResult(
Expand All @@ -235,7 +253,8 @@ private static void processQueryResult(
file.isDirectory(),
file.isSymlink(),
/* remote= */ true,
file.isOmitted());
file.isOmitted(),
file.getDigestFunction());
knownRemotePaths.add(remotePathMetadata);
}
}
Expand Down Expand Up @@ -336,7 +355,8 @@ private Single<List<PathMetadata>> uploadLocalFiles(
// set remote to true so the PathConverter will use bytestream://
// scheme to convert the URI for this file
/* remote= */ true,
path.isOmitted()))
path.isOmitted(),
path.getDigestFunction()))
.onErrorResumeNext(
error -> {
reportUploadError(error, path.getPath(), path.getDigest());
Expand Down Expand Up @@ -375,7 +395,8 @@ private Single<PathConverter> doUpload(Map<Path, LocalFile> files) {
/* directory= */ false,
/* symlink= */ false,
/* remote= */ false,
/* omitted= */ false);
/* omitted= */ false,
DigestFunction.Value.SHA256);
}
})
.collect(Collectors.toList())
Expand Down Expand Up @@ -419,7 +440,7 @@ public ReferenceCounted touch(Object o) {
private static class PathConverterImpl implements PathConverter {

private final String remoteServerInstanceName;
private final Map<Path, Digest> pathToDigest;
private final Map<Path, PathMetadata> pathToMetadata;
private final Set<Path> skippedPaths;
private final Set<Path> localPaths;

Expand All @@ -429,18 +450,18 @@ private static class PathConverterImpl implements PathConverter {
RemoteBuildEventUploadMode remoteBuildEventUploadMode) {
Preconditions.checkNotNull(uploads);
this.remoteServerInstanceName = remoteServerInstanceName;
pathToDigest = Maps.newHashMapWithExpectedSize(uploads.size());
pathToMetadata = Maps.newHashMapWithExpectedSize(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();
for (PathMetadata metadata : uploads) {
Path path = metadata.getPath();
Digest digest = metadata.getDigest();
if (digest != null) {
// Always use bytestream:// in MINIMAL mode
if (remoteBuildEventUploadMode == RemoteBuildEventUploadMode.MINIMAL) {
pathToDigest.put(path, digest);
} else if (pair.isRemote()) {
pathToDigest.put(path, digest);
pathToMetadata.put(path, metadata);
} else if (metadata.isRemote()) {
pathToMetadata.put(path, metadata);
} else {
localPaths.add(path);
}
Expand All @@ -452,6 +473,14 @@ private static class PathConverterImpl implements PathConverter {
this.localPaths = localPaths.build();
}

private static boolean isOldStyleDigestFunction(DigestFunction.Value digestFunction) {
// Old-style digest functions (SHA256, etc) are distinguishable by the length
// of their hash alone and do not require extra specification, but newer
// digest functions (which may have the same length hashes as the older
// functions!) must be explicitly specified in the upload resource name.
return digestFunction.getNumber() <= 7;
}

@Override
@Nullable
public String apply(Path path) {
Expand All @@ -461,18 +490,34 @@ public String apply(Path path) {
return String.format("file://%s", path.getPathString());
}

Digest digest = pathToDigest.get(path);
if (digest == null) {
PathMetadata metadata = pathToMetadata.get(path);
if (metadata == null) {
if (skippedPaths.contains(path)) {
return null;
}
// It's a programming error to reference a file that has not been uploaded.
throw new IllegalStateException(
String.format("Illegal file reference: '%s'", path.getPathString()));
}
return String.format(
"bytestream://%s/blobs/%s/%d",
remoteServerInstanceName, digest.getHash(), digest.getSizeBytes());

Digest digest = metadata.getDigest();
DigestFunction.Value digestFunction = metadata.getDigestFunction();
String out;
if (isOldStyleDigestFunction(digestFunction)) {
out =
String.format(
"bytestream://%s/blobs/%s/%d",
remoteServerInstanceName, digest.getHash(), digest.getSizeBytes());
} else {
out =
String.format(
"bytestream://%s/blobs/%s/%s/%d",
remoteServerInstanceName,
Ascii.toLowerCase(digestFunction.getValueDescriptor().getName()),
digest.getHash(),
digest.getSizeBytes());
}
return out;
}
}
}

0 comments on commit 2b83de5

Please sign in to comment.