tkaitchuck / aHash

aHash is a non-cryptographic hashing algorithm that uses the AES hardware instruction
https://crates.io/crates/ahash
Apache License 2.0
986 stars 94 forks source link

Significant bump in MSRV from 0.8.7 to 0.8.8 #207

Closed schungx closed 5 months ago

schungx commented 5 months ago

Bumping Rust compiler version requirement from 1.60 to 1.72 seems too big a jump and without warning...

I believe a lot of software that depends on ahash must now move their MSRV upwards to 1.72 or limit to 0.8.7...

Is there a way to avoid moving to Rust 1.72?

tnull commented 5 months ago

+1, bumping MSRV in a patch release is really unfortunate. I'm aware of at least 5 projects now that depend on 1.63 MSRV and have to deal with failing CI builds and finding workarounds.

TomAFrench commented 5 months ago

+1, doing this in a patch release has completely broken our release pipeline.

tkaitchuck commented 5 months ago

@schungx this was done to deal with https://github.com/tkaitchuck/aHash/issues/195 which was caused by https://github.com/tkaitchuck/aHash/issues/200 dispite https://github.com/tkaitchuck/aHash/pull/201 which was introduced in https://github.com/tkaitchuck/aHash/pull/183 which was added because it was a blocking issue. As a result the build was successful on X86 with all rust versions but on ARM was working on 1.60 and 1.72 and later but was broken on 1.71.

@tnull @TomAFrench The code should still work on 1.63, you should be able to use it as-is. Is there a way to simply override the check?

tnull commented 5 months ago

@tnull @TomAFrench The code should still work on 1.63, you should be able to use it as-is. Is there a way to simply override the check?

I'm not quite sure what you're suggesting? Since this bump happened in a patch release cargo will gladly select the newest 0.8.8 version, leading to a build failure when trying to build any of the affected project with a (previously perfectly fine) 1.63 MSRV:

error: package `ahash v0.8.8` cannot be built because it requires rustc 1.72.0 or newer, while the currently active rustc version is 1.63.0

The only workaround is to pin ahash back to 0.8.6 (since 0.8.7 suffered from above discussed stdsimd issue), which of course is not a good option longer term and might lead to other issues down the line. This is really painful as a downstream user and it would be great if you could find a solution that would allow ahash to return to its prior MSRV.

bkstein commented 5 months ago

Our pipeline also broke and it is not easy to update rustc in the build containers. ahash is not directly specified in our Cargo.toml. It's a dependency of actix-web, which unfortunately specifies ahab = "0.8" without the patch version. Best for us would be renaming 0.8.8 to 0.9, but not sure, if this is somehow possible.

chyyran commented 5 months ago

We’re unable to support Debian Testing at the moment which has a rustc of 1.70.0.

tkaitchuck commented 5 months ago

@chyyran @bkstein @tnull @TomAFrench Can one of you confirm that this: https://github.com/tkaitchuck/aHash/pull/208 fixes the issue for you?

schungx commented 5 months ago

Maybe add an MSRV CI?

bkstein commented 5 months ago

@tkaitchuck Thanks for the quick fix. And sorry for not checking it earlier. But it can confirm, that it works for us. :+1: