@thaJeztah PTAL

Can we exec C implementation? I assume it is faster than Go.

Can we exec C implementation? I assume it is faster than Go.

wouldn't that be a problem to require the zstd compression tool to be installed? Should the Go version be used as fallback?

Thanks for opening this PR! So, I definitely would like to see support for zstd (it can be a huge space-saver!), I still have my concerns about the current approach in the image-spec. I realise this is not something "new", and not your fault, but something that should be looked at, to prevent being painted into a corner.

Also see the more detailed discussion on opencontainers/image-spec#803. I did discuss this (and related) PRs internally (also with our Hub team), and I see that @justincormack has added this topic to Today's OCI call agenda. I may not be able to join that call myself, but looking forward to the outcome of that, hoping that we can move support for zstd (and other compressions) forward.

I see that @justincormack has added this topic to Today's OCI call agenda

Same here, I may not be able to join the meeting. Is it possible to keep the discussion asynchronous first?

I think we should move on with this. I (mostly) read through opencontainers/image-spec#803. It doesn't seem to be going anywhere and we shouldn't let it block us.

We already support non-gzip blobs, so this is nothing new or breaking in that perspective. It follows the same rules used previously.

The only real current issue in opencontainers/image-spec#803 is that mediatype is not in OCI release. But given that Moby implementation doesn't validate mediatypes at all and many other tools already use this, I don't see a chance for any breaking changes there.

All the discussion about possible better ways to migrate between compression formats are separate from this and optional. If it involves a schema change, then everything will be broken anyway. If some kind of manifest list extension, then no client implement that currently, and that is a different component.

One thing to note is that this same function is used in Dockerfile ADD command extraction. I don't mind supporting zstd there as well, but we need to make sure the change is consistent when we do a release(eg. that this is vendored to buildkit).

Can we exec C implementation? I assume it is faster than Go.

I created issue for the same thing in buildkit moby/buildkit#2345 . It looks like there isn't a good package with cgo/no-cgo switch atm(or exec), and the existing cgo packages are not ideal either. I agree with providing the fastest implementation but it doesn't need to be in the first PR.

LGTM from me after rebase

I think we should move on with this. I (mostly) read through opencontainers/image-spec#803. It doesn't seem to be going anywhere and we shouldn't let it block us.

thanks for your review. I've just rebased it and pushed a new version

Can we have an integration test to run a zstd image? Just “echo hello” would be fine.

For the follow-up test I pushed tonistiigi/hello-world:zstd that could be added to the frozen images list. It is a modified version of library/hello-world built from https://gist.github.com/tonistiigi/239ed89a2cc70c086141def545890f44

thanks everyone for the review.

Is there anything more left before this can be merged?