feat: optional zstd dictionary binary-diff backend

[asafbennatan]

Summary

• Add an optional per-file binary diff backend using zstd dictionary compression (--patch-from style), selectable via --binary-diff zstd (default remains bsdiff).
• Reuse --compression-level for the outer delta stream and per-file patches; add --zstd-diff-level, --zstd-diff-window, and --zstd-diff-fallback-raw knobs for tuning dictionary patch quality and size.
• Extend tar-patch apply path and document the new binary diff format in file-format.md.

Test plan

• go test ./pkg/tar-diff/... ./pkg/tar-patch/...
• Run tar-diff with --binary-diff zstd on representative image layers and confirm smaller deltas vs bsdiff where expected
• Apply resulting .tardiff with tar-patch and verify output matches the new tar
• Confirm default bsdiff behavior is unchanged

follows the discussion here:
containers/oci-delta#65

[asafbennatan]

cc: @alexlarsson

[djach7]

I think this code is merge ready. Is this waiting on the testing plan in the description to be completed?

[asafbennatan]

no, this is all done

[alexlarsson]

Other than the code comments above, there are some highlevel issues. First of all, I'm not sure that the options are set up the right way, as it implies you can only have bsdiff or only zstd dict chunks. bsdiff is more costly, but it does allow fully streaming apply, which means it is more realistic to use for larger files. I can imagine maybe using zstd for smaller files and bsdiff where needed for larger files. So, these should maybe use different max sizes?

Also, I don't think we can just add a new delta op type without making this versioned in the file format. Existing patch apply codebases (like old tar-diff, or the reimplementations in composefs-rs and flatpak) will just die with some unknown delta op error. I think we need to add a version field to the format. I think we should do:

var DeltaHeader = [...]byte{'t', 'a', 'r', 'd', 'f', '1', '\n', 0}
var DeltaHeaderv2 = [...]byte{'t', 'a', 'r', 'd', 'f', '2', '\n', 0}

And emit the second when there is any zstd op in the file.

[alexlarsson]

I also tried re-implementing the apply path in C, and there is no stable api support for WithDecoderDictRaw, instead we have to rely on ZSTD_dct_auto, which checks the header of of the dict, and unless it starts with the zstd dictionary magic (0xEC30A437) word then it assumes raw. I think it is unlikely that we hit this, but can we maybe refuse to use zstd ops for files that happens to start with this prefix?

[asafbennatan]

Addressed the review in 49b2d52:

Format. v2 magic tardf2 is emitted only if the delta actually contains a DeltaOpZstdDict. Pure copy/bsdiff/rollsums deltas stay tardf1. Apply accepts both; ZstdDict on a v1 file is rejected.

Options. --max-bsdiff-size is bsdiff-only (default 192 MiB, 0 = no limit). New --max-zstd-diff-size (default 128 MiB, 0 = no extra cap, still clamped by zstd.MaxWindowSize). --binary-diff auto uses both: zstd while under the zstd cap, then bsdiff while under the bsdiff cap. Default method stays bsdiff. --binary-diff zstd stays zstd-only (no bsdiff tail).

Apply/encode. Dict still has to be fully in memory (WithDecoderDictRaw). Dropped the apply-side source-size cap and the 100MB payload cap so we do not refuse a delta we just generated. New file is streamed into the encoder; decompressed output is streamed to the dest. Always emit raw DATA when a zstd patch is not smaller (--zstd-diff-fallback-raw is gone). Removed the SpeedBestCompression downgrade; it does use the dict on klauspost 1.18.6.

[alexlarsson]

I also tried re-implementing the apply path in C, and there is no stable api support for WithDecoderDictRaw, instead we have to rely on ZSTD_dct_auto, which checks the header of of the dict, and unless it starts with the zstd dictionary magic (0xEC30A437) word then it assumes raw. I think it is unlikely that we hit this, but can we maybe refuse to use zstd ops for files that happens to start with this prefix?

Ignore this, i found a way to avoid the issue in C.

[asafbennatan]

That case is what --binary-diff auto is for: zstd while the file fits --max-zstd-diff-size / the zstd window, then bsdiff for anything still under --max-bsdiff-size. --binary-diff zstd does not fall through to bsdiff, so those multi-GB game blobs / OCI models would only get rollsums or raw copy.

For a generator that wants mostly zstd but still a streaming delta on the huge files:

tar-diff --binary-diff auto --max-zstd-diff-size 128 --max-bsdiff-size 0 ...

--max-bsdiff-size 0 means no bsdiff cap, so the GB-scale files still get bsdiff. Default --max-bsdiff-size stays 192 so a plain bsdiff run does not try to bsdiff multi-GB files unless the caller asks.

[asafbennatan]

Squashed the branch to a single commit.

Zstd window is now min(512MiB, zstd.MaxWindowSize) for both auto and --zstd-diff-window, so we never emit a frame larger than current klauspost or older tar-diff apply. C zstd can go higher; we stay at 512 so older Go apply stays compatible if klauspost raises MaxWindowSize later.

[alexlarsson]

Locally, I'm getting this:

pkg/tar-diff/zstd_diff.go:63:28: redefines-builtin-id: redefinition of the built-in function max (revive)
func sizeWithinLimit(size, max int64) bool {

Also, we're getting weird CI issues:

GOFLAGS=-buildvcs=false /home/runner/go/bin/golangci-lint run
Error: pkg/protocol/common.go:4:8: could not import path/filepath (/opt/hostedtoolcache/go/1.27.0/x64/src/path/filepath/match.go:8:2: could not import errors (/opt/hostedtoolcache/go/1.27.0/x64/src/errors/wrap.go:8:2: could not import internal/reflectlite (/opt/hostedtoolcache/go/1.27.0/x64/src/internal/reflectlite/swapper.go:8:2: could not import internal/goarch (-: could not load export data: internal error in importing "internal/goarch" (cannot decode "internal/goarch", export data version 4 is greater than maximum supported version 2); please report an issue)))) (typecheck)

[alexlarsson]

I think there is a remaining issue with the fallback in generateForFileWithZstd(). It just copies the raw data to the stream, but it needs to use WriteContent() or equivalent, because otherwise we're not getting a DeltaOpData header for the data. Also, it think there is a problem with WriteContent() in that it doesn't correctly handle buffers larger than deltaDataChunkSize(). It needs to loop and potentially generate multiple chunks in that case.

[semanticreleasebot-rmiki]

🎉 This PR is included in version 0.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀