search

apache / arrow-rs

Official Rust implementation of Apache Arrow
https://arrow.apache.org/
Apache License 2.0
2.37k stars 698 forks source link

Adopt a MSRV policy #181

Open alamb opened 3 years ago

alamb commented 3 years ago

Note: migrated from original JIRA: https://issues.apache.org/jira/browse/ARROW-11605

With all our crates now supporting stable Rust, we can decide on a Minimum Supported Rust Version, so that we don't introduce breakage to people relying on older Rust versions.

We could:

This might mean that when there's fresh new changes landing in Stable, we'd likely hold off on them until those changes meet our MSRV.

Thoughts [~Dandandan] [~alamb] [~jorgecarleitao] [~andygrove] [~paddyhoran] [~sunchao]?

alamb commented 3 years ago

Comment from Andrew Lamb(alamb) @ 2021-02-18T10:40:55.550+0000:

[~nevi_me] I don't have any well formed thoughts here -- it sounds like a reasonable idea to me.
dbr commented 3 years ago

This would be nice to have - for example from arrow 4.2 to 4.3 the required Rust version jumped from 1.51 to 1.52 (as noted in #456), but wasn't mentioned in release notes, and was a little surprising in a point release

alamb commented 3 years ago

Thanks @dbr -- what would work best for you? Only changing the required rust version on major updates?

dbr commented 3 years ago

Practically, as long the requirement changes are noted in the CHANGELOG.md, I'd be happy!

The main inconvenience with the 4.3 release to me was just I wasn't sure what had happened - I ran cargo update and got a strange error, and the changelog didn't mention anything significant. Had I not found #456 it would have taken a bit of poking around to realize exactly what happened

I think it makes more logical sense for minimum-Rust-version changes to be major bumps (since the change makes previously compiling code start erroring, practically the same as any other breaking change), but I know there is a lot of debate around this and I don't have any especially strong opinions on it!

alamb commented 3 years ago

@dbr -- got it -- as it turns out I was somewhat surprised by the min change too (I didn't realize that something we backported used a new API in rust 1.53).

In order to properly implement such a version policy, we would probably need to pin the rust version used in the arrow-rs crate's CI system (otherwise we effectively get the latest rust whenever it is released, and we often scramble to fix new clippy lints and what not)

Feel free to submit a PR if you have a moment -- maybe using something like https://github.com/influxdata/influxdb_iox/blob/main/rust-toolchain.toml to pin to rust 1.53?

eitsupi commented 1 year ago

Hi, I saw a post that arrow-rs currently only builds with Rust 1.70 (r-universe-org/help#268) (I haven't tried to follow up).

Is there any chance that a PR configured to check MSRV on CI would be acceptable? Used here. https://github.com/PRQL/prql/blob/1276b7c36d882f819b982c4369cb71e40122b88c/.github/workflows/test-all.yaml#L130-L147

This was useful in recently lowering the MSRV of r-polars to 1.65. (pola-rs/r-polars#280)

alamb commented 1 year ago

I personally think having a CI that verified a MSRV would be very valuable -- and we can have the discussion about "what should the MSRV be" after that is in place.

cc @tustvold

eitsupi commented 1 year ago

Thanks for your answer! I checked MSRV and open an issue for setting CI #4487.

alamb commented 5 months ago

Related discussion in DataFusion: https://github.com/apache/arrow-datafusion/issues/9082

https://github.com/apache/arrow-datafusion/issues/9082#issuecomment-1937754948:

FWIW the vague policy I have been following in arrow-rs is at least 2-3 versions, but to only bump this when actually necessary. The major justification for a lower MSRV might be packaging systems that lag master, e.g. those in Linux distributions, I am not sure how common such a distribution mechanism is for Rust packages though.