-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat: Stabilize Edition 2024 #14828
Merged
+189
−312
Merged
feat: Stabilize Edition 2024 #14828
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,7 +76,7 @@ resolver = "2" | |
[package] | ||
name = "hello_world" # the name of the package | ||
version = "0.1.0" # the current version, obeying semver | ||
edition = "2021" # the edition, will have no effect on a resolver used in the workspace | ||
edition = "2024" # the edition, will have no effect on a resolver used in the workspace | ||
authors = ["Alice <[email protected]>", "Bob <[email protected]>"] | ||
``` | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to point again to RFC-2052 especially to the hard constraints section there:
Given that this changes the resolver behavior you can end up in situations that result in broken builds if you specify a minimal supported rust version and a too broad minimal dependency version. That can then result in a changed version resolution to an dependency version which does not contain a public element that is used by the actual crate.
No warning about the possible broken build is emitted with the 2021 edition!
Ed Page pointed out before that this only happens if there is no
Cargo.lock
file as otherwise dependency resolution is not rerun on edition updates. Also he pointed out that this code is already faulty.I argue again that neither arguments are valid here as:
Cargo.lock
file checked in git repositories where the default at the introduction of the 2021 edition, so cargo must expect that such projects still exists. After all these compile without warning on the latest cargo version-Z minimal-version
) in the past, not with any updates (at least as long as the dependency follows semver, but that's assumed as default). So this is a new built-error mode. (Or worded differently the rust compiler itself does also introduce warning periods for similar "fixes")EDIT: To be clear here, I'm not calling for not stabilizing this feature, I'm just asking for adding the warning required by the RFC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was discussed in #14754 and the Cargo team still checked their boxes and didn't raise a concern to block FCP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know the cargo team is responsible for managing cargo. The edition is a language feature, which means the cargo team is not the only responsible team to decide whether something violates the relevant RFC or not.
In addition to that above: Completing the FCP does not change fact that this violates that hard constraint as no such warning is emitted today. I would even argue that this might indicate that the cargo team knowingly decides to violate this RFC and therefore break the given stability guarantees (again).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moderator note: both of you have stated everything there is to it from your perspectives. Further discussion between you two is not fruitful in my opinion. So other cargo team members can chime in, but I would like it if you two refrained from further commentary on this topic unless asked to by cargo team members.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Cargo team discussed these concerns, fully understands these concerns, and does not feel that a change is needed here. We do not think a warning could or should be provided here, due among other reasons to the high likelihood of false positives. We have been carefully considering the ecosystem impact of many different possible approaches to integrating MSRV into the dependency resolver, and we want to highlight that the experience of crate maintainers in many different circumstances has always been our foremost consideration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ehuss I'm sorry to write that but your response does again not address the core of my concern. So let it me boil down to a simple question:
Which RCF or similar language level decision does exempt the cargo team from the following the rules outlined in RFC-2052?
Can you or a different team member answer this simple question instead of bringing up yet another iteration of excuses why this feature is important or why the given example is already broken.