Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DISCUSSION] Define a MSRV (Minimum Supported Rust Version) policy #9082

Closed
alamb opened this issue Jan 31, 2024 · 11 comments · Fixed by #9681
Closed

[DISCUSSION] Define a MSRV (Minimum Supported Rust Version) policy #9082

alamb opened this issue Jan 31, 2024 · 11 comments · Fixed by #9681
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Jan 31, 2024

Is your feature request related to a problem or challenge?

As crates that DataFusion depends on release new versions, sometimes they require newer versions of Rust than the minimum supported by DataFusion. This means we can't update to those newer libraries. Recent examples:

However, we have no "MSRV" policy that says when we should update the MSRV so it isn't clear when we should increase the version, in issues like #8997

We currently have a MSRV of 1.70 which seems to have been released about 6 months ago: https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1700-2023-06-01 and we are starting to hit dependencies that require new versions

Background

DataFusion now has a minimum supported rust version that we test with a CI run: https://github.com/apache/arrow-datafusion/blob/15f59d9861082a4d5d39bddce63d81cc7b9fb299/.github/workflows/rust.yml#L496

And the minimum supported rust version is defined in Cargo.toml https://github.com/apache/arrow-datafusion/blob/15f59d9861082a4d5d39bddce63d81cc7b9fb299/Cargo.toml#L31

And we document what we test: https://github.com/apache/arrow-datafusion?tab=readme-ov-file#rust-version-compatibility

🎉

Describe the solution you'd like

I propose we adopt a policy of "last two stable version of Rust" which will result in DataFusion being close to but not quite bleeding edge and compatible with most of the rest of the crate ecosystem

This would result in DataFusion upgrading the MSRV roughly every 6 weeks, given the most recent release history of Rust
https://github.com/rust-lang/rust/blob/master/RELEASES.md

Describe alternatives you've considered

We could also adopt a policy like "Stable version of rust that is at most 3 months old" (or 6 months) (edited: previously this said "at least 3 months")

So for example, a 6 month policy at the time of writing, 2024-01-31, would mean supporting:

Additional context

I think our previous convention (due to testing constraints) was simply to support the most recent version of rust

@alamb alamb added the enhancement New feature or request label Jan 31, 2024
@alamb alamb changed the title Define a MSRV (Minimum Supported Rust Version) policy [DISCUSSION] Define a MSRV (Minimum Supported Rust Version) policy Jan 31, 2024
@alamb
Copy link
Contributor Author

alamb commented Jan 31, 2024

Once we get some consensus here we should document the MSRV policy on the readme page

@Omega359
Copy link
Contributor

How would this tie into the testing done via github? Would that need to be updated to run across multiple versions - say minimum + latest?

@westonpace
Copy link
Member

Minor nit:

We could also adopt a policy like "Stable version of rust that is at least 3 months old" (or 6 months)

I think you mean "at most 3 months old"?

Otherwise, no strong opinions here. lance requires 1.75 already thanks to a compiler bug but I don't think we have an official MSRV policy yet and I personally don't have enough background to really know how much work it is to support older rust versions.

@alamb
Copy link
Contributor Author

alamb commented Jan 31, 2024

I think you mean "at most 3 months old"?

Yes, that is correct -- I have updated the the description to say so

@alamb
Copy link
Contributor Author

alamb commented Jan 31, 2024

How would this tie into the testing done via github? Would that need to be updated to run across multiple versions - say minimum + latest?

The current tests run against latest, and we run the MSRV check to make sure DataFusion compiles on the MSRV. It is a good point we don't currently run tests with MSRV 🤔

@eitsupi
Copy link

eitsupi commented Jan 31, 2024

Given that DataFusion depends on arrow-rs, isn't it necessary for arrow-rs' policy to be determined first?
Since DataFusion's MSRV should always be a newer version than arrow-rs's MSRV, right?

@alamb
Copy link
Contributor Author

alamb commented Jan 31, 2024

Given that DataFusion depends on arrow-rs, isn't it necessary for arrow-rs' policy to be determined first? Since DataFusion's MSRV should always be a newer version than arrow-rs's MSRV, right?

Sorting out arrow-rs MSRV would be good for sure, but I don't think it required (for example, we can delay updating arrow-rs in DataFusion if its MSRV is too recent)

@comphead
Copy link
Contributor

comphead commented Jan 31, 2024

Reg to https://github.com/rust-lang/rust/blob/master/RELEASES.md the major Rust versions gets released every 6 weeks or so as @alamb already mentioned. This is considered stable branch and means safe but we know it is now always the case that is why minor Rust versions released with unknown cadence.

Rust is backwards compatible so it can compile binaries for external crates that have been compiled by much older compilers, that means we shouldn't go much into the past and let actively developing libraries to get benefit from newest compiler.

So choosing between most recent release(which stable but still might be unstable) and some minimum rust release version, probably it is good to be within 2-3 last versions.

So if 1.76 released the MSRV we can consider is 1.73/1.74

@tustvold
Copy link
Contributor

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.

@alamb
Copy link
Contributor Author

alamb commented Feb 12, 2024

I found apache/arrow-rs#181 which tracks the arrow MSRV policy discussion

@alamb
Copy link
Contributor Author

alamb commented Mar 18, 2024

Here is a PR with a proposed documented MSRV policy (6 months) #9681

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants