Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support zstd compression #109

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

joshtriplett
Copy link
Member

Add initial support for compressing tarballs with zstd compression. This
provides comparable or in some cases better compression, while
substantially improving decompression performance.

This doesn't change any of the defaults (which still use gz and xz),
just introduces initial off-by-default support for zst to allow us to
experiment.

Add initial support for compressing tarballs with zstd compression. This
provides comparable or in some cases better compression, while
substantially improving decompression performance.

This doesn't change any of the defaults (which still use gz and xz),
just introduces initial off-by-default support for zst to allow us to
experiment.
@joshtriplett
Copy link
Member Author

cc @pietroalbini @kinnison

@pietroalbini
Copy link
Member

I'm wondering, what's your plan for experimenting?

If you want to do test releases with zstd in addition to gz and xz the best place would be promote-release, not here. We recently changed CI to only produce one format instead of all the supported ones to save storage space in our artifacts bucket, delegating the recompression to promote-release at release time.

If you want to have test releases with ztd I'd add recompression to promote-release behind an environment variable.

Still, I didn't do a full review but from a cursory glance this looks correct! We'll definitely want to land this once we start producing zstd tarballs consistently.

@joshtriplett
Copy link
Member Author

@pietroalbini My original intention was to add support here, then experiment with doing some CI builds that added zstd. But if promote-release can do this instead, that seems like a great plan.

Also, if compression happens in promote-release rather than here, we could experiment with higher compression levels there as well.

aswild added a commit to aswild/rust-installer that referenced this pull request Feb 22, 2022
Based on [1], but using compression level 3 for speed rather than 19 for
competitiveness with xz.

[1] rust-lang#109
@Mark-Simulacrum
Copy link
Member

Yeah, I think we should push to avoid further expansion of compression format support in rust-installer, which is harder to update (needs a submodule bump in rust-lang/rust). (In fact, I'd be OK dropping xz support too, but that's potentially a harder conversation).

We can probably do this in promote-release pretty easily though, if someone wanted to file a similarish PR there.

@kinnison
Copy link

Rustup supports zstd as of 1.24.0 though with only local test cases in the codebase I can't be certain it'll work exactly as you hope, so perhaps making it happen in nightly only to begin with makes sense.

aswild added a commit to aswild/rust-installer that referenced this pull request Jul 27, 2022
Based on [1], but using compression level 3 for speed rather than 19 for
competitiveness with xz.

[1] rust-lang#109
aswild added a commit to aswild/rust-installer that referenced this pull request Jan 27, 2023
Based on [1], but using compression level 3 for speed rather than 19 for
competitiveness with xz.

[1] rust-lang#109
aswild added a commit to aswild/rust-installer that referenced this pull request Mar 4, 2023
Based on [1], but using compression level 3 for speed rather than 19 for
competitiveness with xz.

[1] rust-lang#109
aswild added a commit to aswild/rust-installer that referenced this pull request Mar 25, 2023
Based on [1], but using compression level 3 for speed rather than 19 for
competitiveness with xz.

[1] rust-lang#109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants