Switch to zlib-rs by default and drop other zlib backends

[joshtriplett]

As of zlib-rs 0.5.0 (depended on by flate2 1.1.1), zlib-rs no longer
exports C symbols by default, so it doesn't conflict with any other zlib
that might be loaded into the address space.

This removed the primary issue that made zlib selection a challenge
that needed to be exposed to users: users may already have some other
particular preference on zlib implementations, or want to use a system
library, or want to ensure C dependencies use a particular zlib. Since
zlib-rs 0.5.0 no longer conflicts with other zlib implementations, gix
can make this choice independently and not create issues for the user.

Given that, use zlib-rs by default, for performance out of the box with
no C compiler requirement, and deprecate all the feature flags for other
variations. A future major version bump can drop all of these features.

This also means that (for instance) max-performance becomes the same as
max-performance-safe, so such features are deprecated as well.

Depend on flate2 with default-features = false, which brings us closer
to eliminating the miniz_oxide dependency, which will improve build
time for everyone. Currently, the gix-archive dependency on zip
still activates the zip/deflate feature which enables
flate2/rust_backend; this can be fixed to use deflate-flate2 as soon
as zip-rs/zip2#340 is fixed upstream.

Fixes: #1961

[joshtriplett]

OK, CI is passing now.

[Byron]

Thanks a lot for this wonderful PR! I am very much looking forward to massively simplify the features configuration in a follow-up step. It also feels like gix-features doesn't have much purpose anymore as consumers of gix might not have to use it for anything anymore. Thus it would be better to split it up into individual crates that are mostly used internally.

It's also great to see that zlib-rs seems to be responsible for a 1% performance improvement in a real-world workload.

❯ hyperfine -w1 -N "gix free pack verify $PWD/tests/fixtures/repos/linux.git/objects/pack/pack-b73c0dd365b8ab881a163a1d342dac7416f52591.idx" "./target/release/gix free pack verify $PWD/tests/fixtures/repos/linux.git/objects/pack/pack-b73c0dd365b8ab881a163a1d342dac7416f52591.idx"
Benchmark 1: gix free pack verify /Users/byron/dev/github.com/GitoxideLabs/gitoxide/tests/fixtures/repos/linux.git/objects/pack/pack-b73c0dd365b8ab881a163a1d342dac7416f52591.idx
  Time (mean ± σ):     12.144 s ±  0.125 s    [User: 163.061 s, System: 2.224 s]
  Range (min … max):   11.977 s … 12.297 s    10 runs

Benchmark 2: ./target/release/gix free pack verify /Users/byron/dev/github.com/GitoxideLabs/gitoxide/tests/fixtures/repos/linux.git/objects/pack/pack-b73c0dd365b8ab881a163a1d342dac7416f52591.idx
  Time (mean ± σ):     12.073 s ±  0.070 s    [User: 162.446 s, System: 2.216 s]
  Range (min … max):   11.938 s … 12.202 s    10 runs

Summary
  ./target/release/gix free pack verify /Users/byron/dev/github.com/GitoxideLabs/gitoxide/tests/fixtures/repos/linux.git/objects/pack/pack-b73c0dd365b8ab881a163a1d342dac7416f52591.idx ran
    1.01 ± 0.01 times faster than gix free pack verify /Users/byron/dev/github.com/GitoxideLabs/gitoxide/tests/fixtures/repos/linux.git/objects/pack/pack-b73c0dd365b8ab881a163a1d342dac7416f52591.idx

It's a bit unfortunate that it recently became more than 2x slower (#1915), but this probably means the 1% is even more of an accomplishment now.

[joshtriplett]

@Byron Thanks for merging this!

Regarding the test fix for cargo test -r, I think it'd be fine to use cfg(test) to make the API not exist when not testing. cfg(test) should work any time you're doing cargo test, whether you're using -r or not.

[Byron]

I am thanking you!

Regarding the test fix for cargo test -r, I think it'd be fine to use cfg(test) to make the API not exist when not testing. cfg(test) should work any time you're doing cargo test, whether you're using -r or not.

That didn't work, unfortunately, as integration tests don't build the crate under test with test, just the test-binary itself if built with test. I tried that first and settled with leaving it in, but making it hard to discover while discouraging its usage.

[joshtriplett]

On Mon, Apr 21, 2025 at 05:06:08AM -0700, Sebastian Thiel wrote: > Regarding the test fix for `cargo test -r`, I _think_ it'd be fine to use `cfg(test)` to make the API not exist when not testing. `cfg(test)` should work any time you're doing `cargo test`, whether you're using `-r` or not. That didn't work, unfortunately, as integration tests don't build the crate under test with `test`, just the test-binary itself if built with `test`. I tried that first and settled with leaving it in, but making it hard to discover while discouraging its usage.

Oh, right: that does work for unit tests *in* the crate, but not for *integration* tests in `tests/`. So, if you want to keep that API private, you could move the tests you have into the crate as integration tests.

[Byron]

Indeed, that would work and it's nothing I even considered, admittedly. Moving tests from integration to unit-level can still happen, of course, but I think it's fine to do it once there is a trigger, like code in the wild that somehow abuses the presence of this test-trait.