search

rust-lang / rust-installer

The Bourne shell installer used by Rust and Cargo
Apache License 2.0
61 stars 68 forks source link

Support zstd compression #109

Open joshtriplett opened 3 years ago

joshtriplett commented 3 years ago

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 commented 3 years ago

cc @pietroalbini @kinnison

pietroalbini commented 3 years ago

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 commented 3 years ago

@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.

Mark-Simulacrum commented 2 years ago

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 commented 2 years ago

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.