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

Don't downgrade on prerelease VersionReq when update with --breaking. #14250

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

linyihai
Copy link
Contributor

What does this PR try to resolve?

Do nothing with prerelease when update with --breaking.

Fixes #14178

How should we test and review this PR?

Previous commit add a test, next commit to fix and update.

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Jul 15, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-manifest Area: Cargo.toml issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 15, 2024
@linyihai linyihai force-pushed the update-breaking-prerelease branch from e3aab3a to 066d0fb Compare July 15, 2024 10:31
@linyihai linyihai changed the title Update breaking prerelease Do nothing with prerelease when update with --breaking. Jul 15, 2024
src/cargo/util/toml_mut/upgrade.rs Show resolved Hide resolved
src/cargo/util/toml_mut/upgrade.rs Outdated Show resolved Hide resolved
@linyihai linyihai force-pushed the update-breaking-prerelease branch from 066d0fb to 3973d78 Compare July 17, 2024 01:48
@linyihai

This comment was marked as outdated.

@linyihai linyihai marked this pull request as draft July 17, 2024 01:53
@linyihai linyihai force-pushed the update-breaking-prerelease branch from 3973d78 to 7bbb671 Compare July 17, 2024 02:28
@linyihai linyihai marked this pull request as ready for review July 17, 2024 02:28
@linyihai

This comment was marked as outdated.

@epage
Copy link
Contributor

epage commented Jul 18, 2024

CC @torhovland

I feel like one of your PRs had touched on this already but we wanted to split it out for focusing the reviews, or am I thinking of something else?

@torhovland
Copy link
Contributor

CC @torhovland

I feel like one of your PRs had touched on this already but we wanted to split it out for focusing the reviews, or am I thinking of something else?

I think these are separate issues. This one here is about update --breaking not respecting that the user already has a higher pre-release installed, thus trying to "upgrade" to a lower version.

The one I have encountered is about a precise version req not matching its exact version if it is a pre-release. See de1a4a2

@linyihai linyihai force-pushed the update-breaking-prerelease branch 2 times, most recently from 99951d6 to 0bd64aa Compare July 20, 2024 11:09
@linyihai linyihai changed the title Do nothing with prerelease when update with --breaking. Don't downgrade on prerelease VersionReq when update with --breaking. Jul 20, 2024
@linyihai
Copy link
Contributor Author

I copied the matches_greater from semver to make sure prerelease VersionReq won't downgrade.

@linyihai linyihai force-pushed the update-breaking-prerelease branch from 0bd64aa to 1891c40 Compare July 22, 2024 03:22
src/cargo/util/toml_mut/upgrade.rs Outdated Show resolved Hide resolved
src/cargo/util/toml_mut/upgrade.rs Outdated Show resolved Hide resolved
@@ -2616,3 +2616,39 @@ fn update_breaking_mixed_pinning_renaming() {
"#]],
);
}

#[cargo_test]
fn update_breaking_pre_release() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From https://github.com/rust-lang/cargo/pull/14250/files#r1680348122:

The update --breaking had filtered the prerelease version to update to. So here equals to no upgrade to prerelease also. :/

Since that is the case, we might want to have UI tests for

  1. 2.0.0-beta.22 is available, but 2.0.0-beta.21 won't upgrade to it.
  2. 2.0.0 is available, but 2.0.0-beta.21 won't upgrade to it.
  3. 3.0.0 is available, and 2.0.0-beta.21 will upgrade to it.

We should document the first two as an unresolved issue. That is something we want to support, especially the second one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added update_breaking_pre_release_upgrade to reflect the current behavior.

2.0.0 is available, but 2.0.0-beta.21 won't upgrade to it.

This case out of my expection, it seems need to support.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Task added: #12425 (comment).

@linyihai linyihai force-pushed the update-breaking-prerelease branch from 1891c40 to bc38ae0 Compare July 23, 2024 06:21
@linyihai linyihai force-pushed the update-breaking-prerelease branch from bc38ae0 to 9e2701a Compare July 23, 2024 06:38
@weihanglo
Copy link
Member

While there are still some edge cases having to be supported, we shouldn't block this from fixing wrong downgrades. Thank you for the contribution!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 24, 2024

📌 Commit 9e2701a has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2024
@bors
Copy link
Contributor

bors commented Jul 24, 2024

⌛ Testing commit 9e2701a with merge a7917fd...

@bors
Copy link
Contributor

bors commented Jul 24, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing a7917fd to master...

@bors bors merged commit a7917fd into rust-lang:master Jul 24, 2024
22 checks passed
@linyihai linyihai deleted the update-breaking-prerelease branch July 25, 2024 01:02
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2024
Update cargo

14 commits in 5f6b9a92201d78af75dc24f14662c3e2dacbbbe1..b5d44db1daf0469b227a6211b987162a39a54730
2024-07-19 18:09:17 +0000 to 2024-07-26 21:27:12 +0000
- Package workspaces (rust-lang/cargo#13947)
- test: migrate messages to snapbox (rust-lang/cargo#14242)
- chore: Update dependencies (rust-lang/cargo#14299)
- fix: remove rustc probe for `--check-cfg` support (rust-lang/cargo#14302)
- Misc test clean up (rust-lang/cargo#14297)
- Don't downgrade on prerelease `VersionReq` when update with --breaking. (rust-lang/cargo#14250)
- test: Migrate some json tests to snapbox (rust-lang/cargo#14293)
- Revert "fix: Ensure dep/feature activates the dependency on 2024" (rust-lang/cargo#14295)
- chore: bump cargo-test-support to 0.4.0 (rust-lang/cargo#14286)
- Bump to 0.83.0; update changelog (rust-lang/cargo#14285)
- Improved error message when `update --breaking` invalid spec. (rust-lang/cargo#14279)
- docs(test): Expand documentation of cargo-test-support (rust-lang/cargo#14272)
- test: Fix some test based on rustc version (rust-lang/cargo#14282)
- Use `Rc` instead of `Arc` for storing rustflags (rust-lang/cargo#14273)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2024
Update cargo

14 commits in 5f6b9a92201d78af75dc24f14662c3e2dacbbbe1..b5d44db1daf0469b227a6211b987162a39a54730
2024-07-19 18:09:17 +0000 to 2024-07-26 21:27:12 +0000
- Package workspaces (rust-lang/cargo#13947)
- test: migrate messages to snapbox (rust-lang/cargo#14242)
- chore: Update dependencies (rust-lang/cargo#14299)
- fix: remove rustc probe for `--check-cfg` support (rust-lang/cargo#14302)
- Misc test clean up (rust-lang/cargo#14297)
- Don't downgrade on prerelease `VersionReq` when update with --breaking. (rust-lang/cargo#14250)
- test: Migrate some json tests to snapbox (rust-lang/cargo#14293)
- Revert "fix: Ensure dep/feature activates the dependency on 2024" (rust-lang/cargo#14295)
- chore: bump cargo-test-support to 0.4.0 (rust-lang/cargo#14286)
- Bump to 0.83.0; update changelog (rust-lang/cargo#14285)
- Improved error message when `update --breaking` invalid spec. (rust-lang/cargo#14279)
- docs(test): Expand documentation of cargo-test-support (rust-lang/cargo#14272)
- test: Fix some test based on rustc version (rust-lang/cargo#14282)
- Use `Rc` instead of `Arc` for storing rustflags (rust-lang/cargo#14273)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2024
Update cargo

14 commits in 5f6b9a92201d78af75dc24f14662c3e2dacbbbe1..b5d44db1daf0469b227a6211b987162a39a54730
2024-07-19 18:09:17 +0000 to 2024-07-26 21:27:12 +0000
- Package workspaces (rust-lang/cargo#13947)
- test: migrate messages to snapbox (rust-lang/cargo#14242)
- chore: Update dependencies (rust-lang/cargo#14299)
- fix: remove rustc probe for `--check-cfg` support (rust-lang/cargo#14302)
- Misc test clean up (rust-lang/cargo#14297)
- Don't downgrade on prerelease `VersionReq` when update with --breaking. (rust-lang/cargo#14250)
- test: Migrate some json tests to snapbox (rust-lang/cargo#14293)
- Revert "fix: Ensure dep/feature activates the dependency on 2024" (rust-lang/cargo#14295)
- chore: bump cargo-test-support to 0.4.0 (rust-lang/cargo#14286)
- Bump to 0.83.0; update changelog (rust-lang/cargo#14285)
- Improved error message when `update --breaking` invalid spec. (rust-lang/cargo#14279)
- docs(test): Expand documentation of cargo-test-support (rust-lang/cargo#14272)
- test: Fix some test based on rustc version (rust-lang/cargo#14282)
- Use `Rc` instead of `Arc` for storing rustflags (rust-lang/cargo#14273)

r? ghost
@rustbot rustbot added this to the 1.82.0 milestone Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-manifest Area: Cargo.toml issues S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo update --breaking doesn't respect pre release
6 participants