cue-labs / oci

Go modules related to OCI (Open Container Initiative) registries
Apache License 2.0
22 stars 3 forks source link

ocimem.NewBytesReader claims to fill in digest and size, but does not do so #25

Closed tianon closed 4 months ago

tianon commented 4 months ago

The godoc for https://pkg.go.dev/cuelabs.dev/go/oci/ociregistry@v0.0.0-20240206113203-1f693b7466af/ocimem#NewBytesReader claims that it will supplement/fill in size and digest on the provided descriptor, but it does not.

It's trivial to calculate size (int64(len(b))), and in my case I already have digest, so I don't mind either way, but this should probably either be fixed to do what it says on the tin, or the godoc updated to say that it does not actually do that. :smile:

(without adding them to the provided descriptor myself, I was getting invalid checksum digest format thanks to the empty digest value)

rogpeppe commented 4 months ago

I changed the code to act as the docs suggest and this ended up actually breaking some tests, because some code use NewBytesReader to create a reader for a subrange of the total body. In that case, the descriptor size and digest still needs to reflect the digest of the entire data, not that of the subrange. So I think it's better just to document that the responsibility for filling out the descriptor fields is up to the caller.

tianon commented 4 months ago

Sounds good to me, thanks for taking a look! 😀