search

ejmahler / RustFFT

RustFFT is a high-performance FFT library written in pure Rust.
Apache License 2.0
678 stars 47 forks source link

Neon feature is a breaking change #107

Closed WalterSmuts closed 1 year ago

WalterSmuts commented 1 year ago

I think you may have unintentionally made a breaking change when adding neon support. It requires a higher rustc version and was added as a default feature. So any arm folks who were using rustfft on a pre 1.61 rustc compiler will encounter issues.

I haven't tested since I don't have an arm machine handy...

So I believe publishing 6.1.0 breaks semver. You could publish a new major version 7.0.0 or yank 6.1.0 and publish another 6.1.1 that doesn't have neon as a default feature I believe. Not too sure about non-default features and semver but I suspect it'll be fine.

Or I guess it can be left as it hasn't really made much of an impact. Just thought I'd raise the issue...

HEnquist commented 1 year ago

It was done as s a compromise. The idea was that making this change would give arm users a nice "free" speed increase, at the cost of risking build errors on old compilers. That was based on the theory that people building on arm likely were using recent compilers. Bit of a gamble but so far nobody has complained :) See the discussion here: https://github.com/ejmahler/RustFFT/pull/84

ejmahler commented 1 year ago

From the readme:

Supported Rust Version RustFFT requires rustc 1.37 or newer. Minor releases of RustFFT may upgrade the MSRV(minimum supported Rust version) to a newer version of rustc. However, if we need to increase the MSRV, the new Rust version must have been released at least six months ago.

So this change is consistent with our promises around stability.

More generally, last I heard, there wasn't a rust community consensus on whether msrv increases counted as breaking changes, with some compelling arguments for no. And for some reference to other projects, IIRC i copied this "six month" policy directly from tokio's 1.0 release. If you're aware of any changes in community consensus on that issue I'd love to know.