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

MSRV policy going forward #2736

Closed
MabezDev opened this issue Dec 11, 2024 · 6 comments · Fixed by #2951
Closed

MSRV policy going forward #2736

MabezDev opened this issue Dec 11, 2024 · 6 comments · Fixed by #2951
Assignees

Comments

@MabezDev
Copy link
Member

With #2615 we ourselves in a situation where we have to bump the MSRV even though we're not using any new compiler features on the RISCV side. This is likely to happen again in the future.

The simplest proposal is to maintain two MSRVs, essentially a "arch-specific" MSRV. This will allow us to keep the RISCV one lower, and allow us to bump the Xtensa MSRV when we run into compiler issues.

@MabezDev MabezDev added RFC Request for comment 1.0-blocker labels Dec 11, 2024
@bugadani
Copy link
Contributor

We can also specify "latest stable" globally and accept that while a patch release may keep API compatibility, an update may break a build that uses an older compiler. It's semver-compatible, though maybe not the most polite way to approach the issue.

@jessebraham
Copy link
Member

jessebraham commented Dec 11, 2024

The simplest proposal is to maintain two MSRVs, essentially a "arch-specific" MSRV. This will allow us to keep the RISCV one lower, and allow us to bump the Xtensa MSRV when we run into compiler issues.

Not sure I agree with this, this will complicate our MSRV checks in CI and we will not be able to specify rust-version in the Cargo manifest anymore (which maybe is not the end of the world, but does provide a nicer experience when using unsupported toolchains IMO). I also think this will lead to confusion for users.

@jessebraham
Copy link
Member

We can also specify "latest stable" globally and accept that while a patch release may keep API compatibility, an update may break a build that uses an older compiler. It's semver-compatible, though maybe not the most polite way to approach the issue.

This is probably a reasonable approach, but will have to think about it.

@bjoernQ
Copy link
Contributor

bjoernQ commented Dec 11, 2024

One advantage of bumping both MSRVs this time would be that we get core::error::Error - implementing it on our error types would be a good thing

Other than that, it's probably not a problem to have "arch specific" MSRVs in general

@jessebraham
Copy link
Member

I'm sort of leaning towards stable or stable - 1 at this point, just to make things a bit easier on ourselves with regards to potential future compiler issues. We can always extend the MSRV in the future, and I don't think this is an unreasonable limitation. There is of course always the caveat that things may work with older compiler versions, we will just choose not to officially support them. Curious to hear what others think about this, though.

@tom-borcin tom-borcin added the investigation Not needed for 1.0, but don't know if we can support without breaking the driver. label Dec 18, 2024
@MabezDev
Copy link
Member Author

Let's go with STABLE as our MSRV policy. Now that cargo is MSRV aware, this becomes a much more reasonable policy because it avoids random breakage, and clearly tells the user that they need to update their compiler to enjoy the new release.

To complete this work we should:

  • Update the various READMEs on this policy
  • (Optional) Add a checklist the PR template that checks if the MSRV has been bumped, in which case the README should be updated

@MabezDev MabezDev removed RFC Request for comment investigation Not needed for 1.0, but don't know if we can support without breaking the driver. labels Jan 13, 2025
@MabezDev MabezDev changed the title [RFC] MSRV policy going forward MSRV policy going forward Jan 13, 2025
@jessebraham jessebraham self-assigned this Jan 13, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants