Skip to content

Commit

Permalink
Support multiple remote execution digest functions
Browse files Browse the repository at this point in the history
The following PR on the remote-apis side adds support for remote
execution services to announce support for multiple digest functions:

bazelbuild/remote-apis#236

This makes migrating from one digest function to the other more
graceful. This change extends Bazel's server capabilities checking code
to take the new field in the execution capabilities into account.
  • Loading branch information
EdSchouten committed Nov 18, 2022
1 parent 44918c5 commit 8b62a48
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,23 @@ public static ClientServerCompatibilityStatus checkClientServerCompatibility(
return result.build(); // No point checking other execution fields.
}

// Check execution digest function.
if (execCap.getDigestFunction() == DigestFunction.Value.UNKNOWN) {
// Server side error -- this is not supposed to happen.
result.addError("Remote server error: UNKNOWN execution digest function.");
}
if (execCap.getDigestFunction() != digestFunction) {
// Check execution digest function. The protocol only later added
// support for multiple digest functions for remote execution, so
// check both the singular and repeated field.
if (execCap.getDigestFunctionsList().isEmpty() &&
execCap.getDigestFunction() != DigestFunction.Value.UNKNOWN &&
execCap.getDigestFunction() != digestFunction) {
result.addError(
String.format(
"Cannot use hash function %s with remote execution. "
+ "Server supported function is %s",
digestFunction, execCap.getDigestFunction()));
} else if (!execCap.getDigestFunctionsList().contains(digestFunction)) {
result.addError(
String.format(
"Cannot use hash function %s with remote execution. "
+ "Server supported functions are: %s",
digestFunction, execCap.getDigestFunctionsList()));
}

// Check execution priority is in the supported range.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,30 @@ public void testCheckClientServerCompatibility_executionCapsOnly() throws Except
assertThat(st.isOk()).isTrue();
}

@Test
public void testCheckClientServerCompatibility_executionCapsDigestFunctionsList() throws Exception {
ServerCapabilities caps =
ServerCapabilities.newBuilder()
.setLowApiVersion(ApiVersion.current.toSemVer())
.setHighApiVersion(ApiVersion.current.toSemVer())
.setExecutionCapabilities(
ExecutionCapabilities.newBuilder()
.addDigestFunctions(DigestFunction.Value.MD5)
.addDigestFunctions(DigestFunction.Value.SHA256)
.setExecEnabled(true)
.build())
.build();
RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
remoteOptions.remoteExecutor = "server:port";
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
caps,
remoteOptions,
DigestFunction.Value.SHA256,
ServerCapabilitiesRequirement.EXECUTION);
assertThat(st.isOk()).isTrue();
}

@Test
public void testCheckClientServerCompatibility_cacheCapsOnly() throws Exception {
ServerCapabilities caps =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1843,7 +1843,10 @@ message CacheCapabilities {

// Capabilities of the remote execution system.
message ExecutionCapabilities {
// Remote execution may only support a single digest function.
// Legacy field for indicating which digest function is supported by
// the remote execution system. Implementations should consider the
// repeated digest_functions field first, falling back to this
// singular field if unset.
DigestFunction.Value digest_function = 1;

// Whether remote execution is enabled for the particular server/instance.
Expand All @@ -1854,6 +1857,10 @@ message ExecutionCapabilities {

// Supported node properties.
repeated string supported_node_properties = 4;

// All the digest functions supported by the remote execution system.
// If this field is set, it MUST also contain digest_function.
repeated DigestFunction.Value digest_functions = 5;
}

// Details for the tool used to call the API.
Expand Down

0 comments on commit 8b62a48

Please sign in to comment.