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

Prerelease candidates error message #12659

Merged
merged 4 commits into from
Sep 14, 2023
Merged

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Sep 11, 2023

What does this PR try to resolve?

Error messages reporting on versions that do not match the request incorrectly ignore pre-release versions. This is because the version requirement "*" cannot match prerelease versions. #12315

How should we test and review this PR?

Sorry for the large amount of white space changes, fmt got to fmt. 🤷‍♂️

The process was:

Additional information

The old "did you mean to specify a pre-release" #7191 check only occurred when version requirement does not match any versions and you depended on a package that did not have any non-prerelease versions. Making it rarely useful.
The new one will appear any time your version requirement does not match any versions and the package does have pre-release versions. Which may be too common.
I'm open to suggestions for better heuristic.

It's also not clear that the new message make sense in the case of patched versions.

@rustbot
Copy link
Collaborator

rustbot commented Sep 11, 2023

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-crate-dependencies Area: [dependencies] of any kind A-dependency-resolution Area: dependency resolution and the resolver A-manifest Area: Cargo.toml issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2023
tests/testsuite/patch.rs Outdated Show resolved Hide resolved
tests/testsuite/patch.rs Outdated Show resolved Hide resolved
Comment on lines 2695 to 2699
required by package `foo v0.1.0 [..]`
if you are looking for the prerelease package it needs to be specified explicitly
prerelease-deps = { version = "0.1.1-pre1" }
perhaps a crate was updated and forgotten to be re-vendored?"#,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should we indent the suggested version req both for "quoting" it and for setting it apart from the suggestion that may come after it?

@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 12, 2023

I'm gonna go ahead and edit history. @loloicci, I am sorry if this removes the attribution on your work.

@Eh2406 Eh2406 force-pushed the Prerelease_candidates branch from 6642373 to e8f3e02 Compare September 12, 2023 15:05
@Eh2406 Eh2406 force-pushed the Prerelease_candidates branch from e8f3e02 to 50f1455 Compare September 12, 2023 16:13
@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 12, 2023

Edited history so that the test was in a separate commit, and so that all tests pass on every commit. This allows you to track the incremental progress on the error messages.

@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 13, 2023

@epage you approved but did not tell bors, what did you intend to convey by that response?

@epage
Copy link
Contributor

epage commented Sep 14, 2023

Sorry, thought I said to r= when ready

@bors r+

@bors
Copy link
Contributor

bors commented Sep 14, 2023

📌 Commit 50f1455 has been approved by epage

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 Sep 14, 2023
@bors
Copy link
Contributor

bors commented Sep 14, 2023

⌛ Testing commit 50f1455 with merge 8d43632...

@bors
Copy link
Contributor

bors commented Sep 14, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 8d43632 to master...

@bors bors merged commit 8d43632 into rust-lang:master Sep 14, 2023
19 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2023
Update cargo

11 commits in 2fc85d15a542bfb610aff7682073412cf635352f..d5336f813df39d476e61fc46daabb1446350660a
2023-09-09 01:49:46 +0000 to 2023-09-14 19:55:49 +0000
- fix: emit a warning for `credential-alias` shadowing (rust-lang/cargo#12671)
- refactor: fix lint errors in preparation of `[lints]` table integration (rust-lang/cargo#12669)
- Limit cargo add feature print (rust-lang/cargo#12662)
- Clippy (rust-lang/cargo#12667)
- Prerelease candidates error message (rust-lang/cargo#12659)
- Fix typos: `informations` -> `information` (rust-lang/cargo#12666)
- chore: update globset to 0.4.13 (rust-lang/cargo#12665)
- refactor: Consolidate clap/shell styles (rust-lang/cargo#12655)
- libgit2 fixed upstream (rust-lang/cargo#12657)
- Index summary enum (rust-lang/cargo#12643)
- feat(help): Add styling to help output (rust-lang/cargo#12578)

r? ghost
@Eh2406 Eh2406 deleted the Prerelease_candidates branch September 27, 2023 17:51
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-crate-dependencies Area: [dependencies] of any kind A-dependency-resolution Area: dependency resolution and the resolver 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.

6 participants