Repository navigation
Update reported max API version to latest - #363
Conversation
| ExecEnabled: true, | ||
| }, | ||
| LowApiVersion: &semver.SemVer{Major: 2, Minor: 0}, | ||
| HighApiVersion: &semver.SemVer{Major: 2, Minor: 1}, // optimistic |
There was a problem hiding this comment.
our optimism here eventually paid off and it did get tagged, quite some time after this comment.
| @@ -0,0 +1,254 @@ | |||
| # AGENTS.md | |||
There was a problem hiding this comment.
Adding this while I'm here
peterebden
changed the title
| rootDigest, err := digest.NewFromMessage(tree.Root) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, rootDigest.ToProto(), outDir.RootDirectoryDigest) | ||
| assert.NotEqual(t, digest.Digest{}, rexclient.PackDigest(tree.Root)) |
There was a problem hiding this comment.
Why do we only assert that it is non-zero, but not assert what it actually is?
There was a problem hiding this comment.
Because it's a hash digest, we don't care exactly what it is and we don't want to pin to the specific implementation of how the tarball gets built.
I'll add a check that it's in entries which seems relevant.
| OutputDirectorySymlinks: []*pb.OutputSymlink{dir}, | ||
| })) | ||
|
|
||
| // output_symlinks is already populated, which takes priority |
There was a problem hiding this comment.
This test doesn't actually assert that output_symlinks takes priority; allOutputSymlinks could still read the deprecated fields and pass this test.
There was a problem hiding this comment.
That's true; it reverses the order so it wouldn't just work as it was before, but I'll change it to be unambiguous.
| @@ -0,0 +1,254 @@ | |||
| # AGENTS.md | |||
Handle newer symlink field for a client that sends that, and populate the new
root_directory_digestfield for a client that wants it.Not much to do otherwise, we already supported most of the features of earlier versions (like compression) before they got (retroactively) tagged. There are other changes, mostly around multiple digest functions and split-and-splice but they're opt-in and we're a SHA-256 only server so we don't have to support any of them.
Update to a more recent Go version while I'm here.
I have not upgraded
remote-apis-sdks. There are various changes in it which are IMO highly questionable (silently ignoring unreadable outputs and aggressively opening 25 parallel connections to the CAS server) and relatively little that looks particularly useful for us.