Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.4.0] Add support for the BLAKE3 digest function #19191

Merged
merged 9 commits into from
Aug 8, 2023
Merged
1 change: 1 addition & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ genrule(
pkg_tar(
name = "bootstrap-jars",
srcs = [
"@blake3",
"@com_google_protobuf//:protobuf_java",
"@com_google_protobuf//:protobuf_java_util",
"@com_google_protobuf//:protobuf_javalite",
Expand Down
1 change: 1 addition & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ bazel_dep(name = "platforms", version = "0.0.5")
bazel_dep(name = "rules_pkg", version = "0.7.0")
bazel_dep(name = "stardoc", version = "0.5.0", repo_name = "io_bazel_skydoc")
bazel_dep(name = "zstd-jni", version = "1.5.2-3")
bazel_dep(name = "blake3", version = "1.3.3")
bazel_dep(name = "zlib", version = "1.2.13")

# The following are required when building without WORKSPACE SUFFIX
Expand Down
8 changes: 8 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,14 @@ dist_http_archive(
strip_prefix = "zstd-jni-1.5.2-3",
)

dist_http_archive(
name = "blake3",
build_file = "//third_party:blake3/blake3.BUILD",
patch_cmds = EXPORT_WORKSPACE_IN_BUILD_BAZEL_FILE,
patch_cmds_win = EXPORT_WORKSPACE_IN_BUILD_BAZEL_FILE_WIN,
strip_prefix = "BLAKE3-1.3.3",
)

http_archive(
name = "org_snakeyaml",
build_file_content = """
Expand Down
15 changes: 15 additions & 0 deletions distdir_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,21 @@ DIST_DEPS = {
"additional_distfiles",
],
},
"blake3": {
"archive": "1.3.3.zip",
"sha256": "bb529ba133c0256df49139bd403c17835edbf60d2ecd6463549c6a5fe279364d",
"urls": [
"https://github.com/BLAKE3-team/BLAKE3/archive/refs/tags/1.3.3.zip",
],
"used_in": [
"additional_distfiles",
],
"license_kinds": [
"@rules_license//licenses/spdx:Apache-2.0",
],
"license_text": "LICENSE",
"package_version": "1.3.3",
},
###################################################
#
# Build time dependencies for testing and packaging
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ filegroup(
"//src/main/java/com/google/devtools/build/lib/versioning:srcs",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs:srcs",
"//src/main/java/com/google/devtools/build/lib/vfs:srcs",
"//src/main/java/com/google/devtools/build/lib/vfs/bazel:srcs",
"//src/main/java/com/google/devtools/build/lib/windows:srcs",
"//src/main/java/com/google/devtools/build/lib/worker:srcs",
"//src/main/java/com/google/devtools/build/skyframe:srcs",
Expand Down Expand Up @@ -403,6 +404,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:output_service",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/lib/vfs/bazel",
"//src/main/java/com/google/devtools/build/lib/windows",
"//src/main/java/com/google/devtools/build/lib/worker:worker_metric",
"//src/main/java/com/google/devtools/build/skyframe",
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/lib/vfs/bazel",
"//src/main/java/com/google/devtools/build/lib/windows",
"//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:failure_details_java_proto",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.bazel.BazelHashFunctions;
import com.google.devtools.build.lib.windows.WindowsFileSystem;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsParsingResult;
Expand All @@ -44,6 +45,10 @@
* com.google.devtools.build.lib.vfs.FileSystem} class use {@code SHA256} by default.
*/
public class BazelFileSystemModule extends BlazeModule {
static {
BazelHashFunctions.ensureRegistered();
}

@Override
public ModuleFileSystem getFileSystem(
OptionsParsingResult startupOptions, PathFragment realExecRootBase)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@
// limitations under the License.
package com.google.devtools.build.lib.remote;

import static com.google.devtools.build.lib.remote.util.DigestUtil.isOldStyleDigestFunction;
import static com.google.devtools.build.lib.remote.util.RxFutures.toCompletable;
import static com.google.devtools.build.lib.remote.util.RxFutures.toListenableFuture;
import static com.google.devtools.build.lib.remote.util.RxFutures.toSingle;
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 @@ -133,13 +136,21 @@ private static final class PathMetadata {
private final boolean directory;
private final boolean remote;
private final boolean omitted;

PathMetadata(Path path, Digest digest, boolean directory, boolean remote, boolean omitted) {
private final DigestFunction.Value digestFunction;

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

public Path getPath() {
Expand All @@ -161,20 +172,27 @@ 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 file) throws IOException {
DigestUtil digestUtil = new DigestUtil(xattrProvider, file.getFileSystem().getDigestFunction());

if (file.isDirectory()) {
return new PathMetadata(
file,
/* digest= */ null,
/* directory= */ true,
/* remote= */ false,
/* omitted= */ false);
/* omitted= */ false,
/* digestFunction= */ digestUtil.getDigestFunction());
}

PathFragment filePathFragment = file.asFragment();
Expand All @@ -190,9 +208,14 @@ private PathMetadata readPathMetadata(Path file) throws IOException {
}
}

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

private static void processQueryResult(
Expand All @@ -209,7 +232,8 @@ private static void processQueryResult(
file.getDigest(),
file.isDirectory(),
/* remote= */ true,
file.isOmitted());
file.isOmitted(),
file.getDigestFunction());
knownRemotePaths.add(remotePathMetadata);
}
}
Expand Down Expand Up @@ -305,7 +329,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 @@ -341,7 +366,8 @@ private Single<PathConverter> upload(Set<Path> files) {
/* digest= */ null,
/* directory= */ false,
/* remote= */ false,
/* omitted= */ false);
/* omitted= */ false,
DigestFunction.Value.SHA256);
}
})
.collect(Collectors.toList())
Expand Down Expand Up @@ -385,7 +411,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 @@ -395,18 +421,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 @@ -427,18 +453,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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
import static com.google.devtools.build.lib.remote.util.DigestUtil.isOldStyleDigestFunction;
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
import static com.google.devtools.build.lib.remote.util.Utils.waitForBulkTransfer;
import static java.lang.String.format;
import static java.util.concurrent.TimeUnit.SECONDS;

import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.DigestFunction;
import com.google.bytestream.ByteStreamGrpc;
import com.google.bytestream.ByteStreamGrpc.ByteStreamFutureStub;
import com.google.bytestream.ByteStreamGrpc.ByteStreamStub;
Expand Down Expand Up @@ -69,6 +71,7 @@ class ByteStreamUploader {
private final CallCredentialsProvider callCredentialsProvider;
private final long callTimeoutSecs;
private final RemoteRetrier retrier;
private final DigestFunction.Value digestFunction;

@Nullable private final Semaphore openedFilePermits;

Expand All @@ -89,14 +92,16 @@ class ByteStreamUploader {
CallCredentialsProvider callCredentialsProvider,
long callTimeoutSecs,
RemoteRetrier retrier,
int maximumOpenFiles) {
int maximumOpenFiles,
DigestFunction.Value digestFunction) {
checkArgument(callTimeoutSecs > 0, "callTimeoutSecs must be gt 0.");
this.instanceName = instanceName;
this.channel = channel;
this.callCredentialsProvider = callCredentialsProvider;
this.callTimeoutSecs = callTimeoutSecs;
this.retrier = retrier;
this.openedFilePermits = maximumOpenFiles != -1 ? new Semaphore(maximumOpenFiles) : null;
this.digestFunction = digestFunction;
}

@VisibleForTesting
Expand Down Expand Up @@ -175,11 +180,26 @@ public ListenableFuture<Void> uploadBlobAsync(
MoreExecutors.directExecutor());
}

private static String buildUploadResourceName(
private String buildUploadResourceName(
String instanceName, UUID uuid, Digest digest, boolean compressed) {
String template =
compressed ? "uploads/%s/compressed-blobs/zstd/%s/%d" : "uploads/%s/blobs/%s/%d";
String resourceName = format(template, uuid, digest.getHash(), digest.getSizeBytes());

String resourceName;

if (isOldStyleDigestFunction(digestFunction)) {
String template =
compressed ? "uploads/%s/compressed-blobs/zstd/%s/%d" : "uploads/%s/blobs/%s/%d";
resourceName = format(template, uuid, digest.getHash(), digest.getSizeBytes());
} else {
String template =
compressed ? "uploads/%s/compressed-blobs/zstd/%s/%s/%d" : "uploads/%s/blobs/%s/%s/%d";
resourceName =
format(
template,
uuid,
Ascii.toLowerCase(digestFunction.getValueDescriptor().getName()),
digest.getHash(),
digest.getSizeBytes());
}
if (!Strings.isNullOrEmpty(instanceName)) {
resourceName = instanceName + "/" + resourceName;
}
Expand Down
Loading