Frommi / miniz_oxide

Rust replacement for miniz
MIT License
168 stars 48 forks source link

v0.6.4 doesn't build in rust 1.59 #135

Closed Pascal-So closed 1 year ago

Pascal-So commented 1 year ago

Hi, I only just came across this library as a transitive dependency because I was trying to build a project using Rust 1.59. The readme states that this project has MSRV 1.40 for v0.5 and v0.6, is this information maybe out of date?

I tested the following setup:

$ cargo init
$ cargo add miniz_oxide
$ cat > rust-toolchain.toml
[toolchain]
channel = "1.59.0"
$ cargo b
   Compiling adler v1.0.2
   Compiling miniz_oxide v0.6.4
error[E0658]: use of unstable library feature 'int_abs_diff'
   --> /home/pascal/.cargo/registry/src/github.com-1ecc6299db9ec823/miniz_oxide-0.6.4/src/inflate/core.rs:824:54
    |
824 |     if out_buf_size_mask == usize::MAX && source_pos.abs_diff(out_pos) == 1 && out_pos > source_pos {
    |                                                      ^^^^^^^^
    |
    = note: see issue #89492 <https://github.com/rust-lang/rust/issues/89492> for more information

error[E0658]: use of unstable library feature 'int_abs_diff'
   --> /home/pascal/.cargo/registry/src/github.com-1ecc6299db9ec823/miniz_oxide-0.6.4/src/inflate/core.rs:833:61
    |
833 |     } else if out_buf_size_mask == usize::MAX && source_pos.abs_diff(out_pos) >= 4 && out_pos > source_pos {
    |                                                             ^^^^^^^^
    |
    = note: see issue #89492 <https://github.com/rust-lang/rust/issues/89492> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `miniz_oxide` due to 2 previous errors

Let me know if I misunderstood anything.

Pascal-So commented 1 year ago

looks like abs_diff is only stable since 1.60: https://doc.rust-lang.org/std/primitive.usize.html#method.abs_diff

jasonish commented 1 year ago

We just came across this as well, our MSRV is 1.58. I do have a patch but might be an hour or 2 before I can submit it.

oyvindln commented 1 year ago

Oh, I thought the CI built it on 1.40 but apparently the "1.40" CI build was using nightly....

oyvindln commented 1 year ago

So, 0.6.x will stay at requiring 1.40, and I'll add 0.7 if the latest performance stuff that need a version bump.

Pascal-So commented 1 year ago

Thanks everyone for the quick fix and merge!

oyvindln commented 1 year ago

Yeah sorry about any problems caused by this, it's all on me screwing up the CI workflow file at one point making me think it was building on 1.40.0 when it wasn't. 🤦

jasonish commented 1 year ago

Yeah sorry about any problems caused by this, it's all on me screwing up the CI workflow file at one point making me think it was building on 1.40.0 when it wasn't. facepalm

Thanks for addressing this quickly and actually caring about MSRV.,. Much appreciated!