From 70e7868ddbe78a56e6dcc4201c4abdffc7304045 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 20 Jan 2024 23:53:57 -0500 Subject: [PATCH 1/7] refactor(registry): merge `called` flags into one --- src/cargo/sources/registry/mod.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 487ca633306..01cd1803279 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -751,20 +751,24 @@ impl<'cfg> Source for RegistrySource<'cfg> { req.update_precise(&requested); } + let mut called = false; + let callback = &mut |s| { + called = true; + f(s); + }; + // If this is a locked dependency, then it came from a lock file and in // theory the registry is known to contain this version. If, however, we // come back with no summaries, then our registry may need to be // updated, so we fall back to performing a lazy update. if kind == QueryKind::Exact && req.is_locked() && !self.ops.is_updated() { debug!("attempting query without update"); - let mut called = false; ready!(self .index .query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| { if dep.matches(s.as_summary()) { // We are looking for a package from a lock file so we do not care about yank - called = true; - f(s); + callback(s) } },))?; if called { @@ -775,7 +779,6 @@ impl<'cfg> Source for RegistrySource<'cfg> { Poll::Pending } } else { - let mut called = false; ready!(self .index .query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| { @@ -789,8 +792,7 @@ impl<'cfg> Source for RegistrySource<'cfg> { if matched && (!s.is_yanked() || self.yanked_whitelist.contains(&s.package_id())) { - f(s); - called = true; + callback(s); } }))?; if called { From 515f6e3f9dd0e36ec22ef3dacd5f1c6ea5814f9c Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 21 Jan 2024 14:26:10 -0500 Subject: [PATCH 2/7] refactor: rename items in `OptVersionReq` * `OptVersionReq::update_precise` to `precise_to` * `OptVersionReq::UpdatePrecise` to `Precise` --- src/cargo/sources/registry/mod.rs | 2 +- src/cargo/util/semver_ext.rs | 25 +++++++++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 01cd1803279..477e7ce6c77 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -748,7 +748,7 @@ impl<'cfg> Source for RegistrySource<'cfg> { .precise_registry_version(dep.package_name().as_str()) .filter(|(c, _)| req.matches(c)) { - req.update_precise(&requested); + req.precise_to(&requested); } let mut called = false; diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index fed957ef7d9..dcf680a509b 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -33,7 +33,11 @@ pub enum OptVersionReq { /// The exact locked version and the original version requirement. Locked(Version, VersionReq), /// The exact requested version and the original version requirement. - UpdatePrecise(Version, VersionReq), + /// + /// This looks identical to [`OptVersionReq::Locked`] but has a different + /// meaning, and is used for the `--precise` field of `cargo update`. + /// See comments in [`OptVersionReq::matches`] for more. + Precise(Version, VersionReq), } impl OptVersionReq { @@ -51,7 +55,7 @@ impl OptVersionReq { pub fn is_exact(&self) -> bool { match self { OptVersionReq::Any => false, - OptVersionReq::Req(req) | OptVersionReq::UpdatePrecise(_, req) => { + OptVersionReq::Req(req) | OptVersionReq::Precise(_, req) => { req.comparators.len() == 1 && { let cmp = &req.comparators[0]; cmp.op == Op::Exact && cmp.minor.is_some() && cmp.patch.is_some() @@ -67,18 +71,19 @@ impl OptVersionReq { let version = version.clone(); *self = match self { Any => Locked(version, VersionReq::STAR), - Req(req) | Locked(_, req) | UpdatePrecise(_, req) => Locked(version, req.clone()), + Req(req) | Locked(_, req) | Precise(_, req) => Locked(version, req.clone()), }; } - pub fn update_precise(&mut self, version: &Version) { + /// Makes the requirement precise to the requested version. + /// + /// This is used for the `--precise` field of `cargo update`. + pub fn precise_to(&mut self, version: &Version) { use OptVersionReq::*; let version = version.clone(); *self = match self { - Any => UpdatePrecise(version, VersionReq::STAR), - Req(req) | Locked(_, req) | UpdatePrecise(_, req) => { - UpdatePrecise(version, req.clone()) - } + Any => Precise(version, VersionReq::STAR), + Req(req) | Locked(_, req) | Precise(_, req) => Precise(version, req.clone()), }; } @@ -108,7 +113,7 @@ impl OptVersionReq { // we should not silently use `1.0.0+foo` even though they have the same version. v == version } - OptVersionReq::UpdatePrecise(v, _) => { + OptVersionReq::Precise(v, _) => { // This is used for the `--precise` field of cargo update. // // Unfortunately crates.io allowed versions to differ only @@ -135,7 +140,7 @@ impl Display for OptVersionReq { OptVersionReq::Any => f.write_str("*"), OptVersionReq::Req(req) | OptVersionReq::Locked(_, req) - | OptVersionReq::UpdatePrecise(_, req) => Display::fmt(req, f), + | OptVersionReq::Precise(_, req) => Display::fmt(req, f), } } } From 4f1e41ebe3c1a034e24c5d633a13384ab379f40a Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 21 Jan 2024 13:08:34 -0500 Subject: [PATCH 3/7] test(update): demonstrate not able to `update --precise ` --- tests/testsuite/update.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs index a36f187089f..fe2df05b955 100644 --- a/tests/testsuite/update.rs +++ b/tests/testsuite/update.rs @@ -1370,3 +1370,40 @@ fn update_precise_git_revisions() { assert!(p.read_lockfile().contains(&head_id)); assert!(!p.read_lockfile().contains(&tag_commit_id)); } + +#[cargo_test] +fn precise_yanked() { + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.1.1").yanked(true).publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + + [dependencies] + bar = "0.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile").run(); + + // Use non-yanked version. + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("\nname = \"bar\"\nversion = \"0.1.0\"")); + + p.cargo("update --precise 0.1.1 bar") + .with_status(101) + .with_stderr( + "\ +[UPDATING] `dummy-registry` index +[ERROR] no matching package named `bar` found +location searched: registry `crates-io` +required by package `foo v0.0.0 ([CWD])` +", + ) + .run() +} From 3f2f4ed29bcc245868efbcb41ace98228aed0375 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 21 Jan 2024 14:36:16 -0500 Subject: [PATCH 4/7] feat(cargo-update): `--precise` to allow yanked versions --- src/cargo/sources/registry/mod.rs | 31 +++++++++++++++++++++++++++--- src/cargo/util/semver_ext.rs | 12 ++++++++++++ tests/testsuite/update.rs | 32 +++++++++++++++++++++++++++---- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 477e7ce6c77..c363d609970 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -779,6 +779,7 @@ impl<'cfg> Source for RegistrySource<'cfg> { Poll::Pending } } else { + let mut precise_yanked_in_use = false; ready!(self .index .query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| { @@ -786,15 +787,39 @@ impl<'cfg> Source for RegistrySource<'cfg> { QueryKind::Exact => dep.matches(s.as_summary()), QueryKind::Fuzzy => true, }; + if !matched { + return; + } // Next filter out all yanked packages. Some yanked packages may // leak through if they're in a whitelist (aka if they were // previously in `Cargo.lock` - if matched - && (!s.is_yanked() || self.yanked_whitelist.contains(&s.package_id())) - { + if !s.is_yanked() { + callback(s); + } else if self.yanked_whitelist.contains(&s.package_id()) { callback(s); + } else if req.is_precise() { + precise_yanked_in_use = true; + if self.config.cli_unstable().unstable_options { + callback(s); + } } }))?; + if precise_yanked_in_use { + self.config + .cli_unstable() + .fail_if_stable_opt("--precise ", 4225)?; + let name = dep.package_name(); + let version = req + .precise_version() + .expect("--precise in use"); + let source = self.source_id(); + let mut shell = self.config.shell(); + shell.warn(format_args!( + "yanked package `{name}@{version}` is selected by the `--precise` flag from {source}", + ))?; + shell.note("it is not recommended to depend on a yanked version")?; + shell.note("if possible, try other SemVer-compatbile versions")?; + } if called { return Poll::Ready(Ok(())); } diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index dcf680a509b..ae32483f3f3 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -87,6 +87,18 @@ impl OptVersionReq { }; } + pub fn is_precise(&self) -> bool { + matches!(self, OptVersionReq::Precise(..)) + } + + /// Gets the version to which this req is precise to, if any. + pub fn precise_version(&self) -> Option<&Version> { + match self { + OptVersionReq::Precise(version, _) => Some(version), + _ => None, + } + } + pub fn is_locked(&self) -> bool { matches!(self, OptVersionReq::Locked(..)) } diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs index fe2df05b955..a3864772ed8 100644 --- a/tests/testsuite/update.rs +++ b/tests/testsuite/update.rs @@ -1400,10 +1400,34 @@ fn precise_yanked() { .with_stderr( "\ [UPDATING] `dummy-registry` index -[ERROR] no matching package named `bar` found -location searched: registry `crates-io` -required by package `foo v0.0.0 ([CWD])` +[ERROR] failed to get `bar` as a dependency of package `foo v0.0.0 ([CWD])` + +Caused by: + failed to query replaced source registry `crates-io` + +Caused by: + the `--precise ` flag is unstable[..] + See [..] + See [..] +", + ) + .run(); + + p.cargo("update --precise 0.1.1 bar") + .masquerade_as_nightly_cargo(&["--precise "]) + .arg("-Zunstable-options") + .with_stderr( + "\ +[UPDATING] `dummy-registry` index +[WARNING] yanked package `bar@0.1.1` is selected by the `--precise` flag from registry `dummy-registry` +[NOTE] it is not recommended to depend on a yanked version +[NOTE] if possible, try other SemVer-compatbile versions +[UPDATING] bar v0.1.0 -> v0.1.1 ", ) - .run() + .run(); + + // Use yanked version. + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("\nname = \"bar\"\nversion = \"0.1.1\"")); } From 8c392fd7e525efde4316ab8b768b963c0bfd6c00 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 21 Jan 2024 16:00:19 -0500 Subject: [PATCH 5/7] doc(cargo-update): `--precise` to allow yanked versions --- src/doc/man/cargo-update.md | 4 ++++ src/doc/man/generated_txt/cargo-update.txt | 5 +++++ src/doc/src/commands/cargo-update.md | 5 ++++- src/doc/src/reference/resolver.md | 4 +++- src/etc/man/cargo-update.1 | 4 ++++ 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/doc/man/cargo-update.md b/src/doc/man/cargo-update.md index b392b83145f..9c2503407ca 100644 --- a/src/doc/man/cargo-update.md +++ b/src/doc/man/cargo-update.md @@ -42,6 +42,10 @@ Cannot be used with `--precise`. When used with _spec_, allows you to specify a specific version number to set the package to. If the package comes from a git repository, this can be a git revision (such as a SHA hash or tag). + +While not recommended, you can specify a yanked version of a package (nightly only). +When possible, try other non-yanked SemVer-compatible versions or seek help +from the maintainers of the package. {{/option}} {{#option "`-w`" "`--workspace`" }} diff --git a/src/doc/man/generated_txt/cargo-update.txt b/src/doc/man/generated_txt/cargo-update.txt index a96a34c962e..14023dd4ada 100644 --- a/src/doc/man/generated_txt/cargo-update.txt +++ b/src/doc/man/generated_txt/cargo-update.txt @@ -35,6 +35,11 @@ OPTIONS to set the package to. If the package comes from a git repository, this can be a git revision (such as a SHA hash or tag). + While not recommended, you can specify a yanked version of a package + (nightly only). When possible, try other non-yanked + SemVer-compatible versions or seek help from the maintainers of the + package. + -w, --workspace Attempt to update only packages defined in the workspace. Other packages are updated only if they don’t already exist in the diff --git a/src/doc/src/commands/cargo-update.md b/src/doc/src/commands/cargo-update.md index ef44e487bb0..be38dd4d2f1 100644 --- a/src/doc/src/commands/cargo-update.md +++ b/src/doc/src/commands/cargo-update.md @@ -39,7 +39,10 @@ Cannot be used with --precise.
--precise precise
When used with spec, allows you to specify a specific version number to set the package to. If the package comes from a git repository, this can be a git -revision (such as a SHA hash or tag).
+revision (such as a SHA hash or tag).

+

While not recommended, you can specify a yanked version of a package (nightly only). +When possible, try other non-yanked SemVer-compatible versions or seek help +from the maintainers of the package.

-w
diff --git a/src/doc/src/reference/resolver.md b/src/doc/src/reference/resolver.md index a7fea8a071f..6a284b2ed7d 100644 --- a/src/doc/src/reference/resolver.md +++ b/src/doc/src/reference/resolver.md @@ -325,9 +325,11 @@ the `links` field if your library is in common use. [Yanked releases][yank] are those that are marked that they should not be used. When the resolver is building the graph, it will ignore all yanked -releases unless they already exist in the `Cargo.lock` file. +releases unless they already exist in the `Cargo.lock` file or are explicitly +requested by the [`--precise`] flag of `cargo update` (nightly only). [yank]: publishing.md#cargo-yank +[`--precise`]: ../commands/cargo-update.md#option-cargo-update---precise ## Dependency updates diff --git a/src/etc/man/cargo-update.1 b/src/etc/man/cargo-update.1 index c9fe881c8fc..943c87b46fd 100644 --- a/src/etc/man/cargo-update.1 +++ b/src/etc/man/cargo-update.1 @@ -39,6 +39,10 @@ Cannot be used with \fB\-\-precise\fR\&. When used with \fIspec\fR, allows you to specify a specific version number to set the package to. If the package comes from a git repository, this can be a git revision (such as a SHA hash or tag). +.sp +While not recommended, you can specify a yanked version of a package (nightly only). +When possible, try other non\-yanked SemVer\-compatible versions or seek help +from the maintainers of the package. .RE .sp \fB\-w\fR, From bc5da432ef4b68a0afb10e24f791d237fa3b8bec Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 21 Jan 2024 20:18:17 -0500 Subject: [PATCH 6/7] test(cargo-update): verify `--precise warns twice This is not ideal and we need to track selected yanked versions. --- tests/testsuite/update.rs | 47 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs index a3864772ed8..fc93e84a0d7 100644 --- a/tests/testsuite/update.rs +++ b/tests/testsuite/update.rs @@ -1431,3 +1431,50 @@ Caused by: let lockfile = p.read_lockfile(); assert!(lockfile.contains("\nname = \"bar\"\nversion = \"0.1.1\"")); } + +#[cargo_test] +fn precise_yanked_multiple_presence() { + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.1.1").yanked(true).publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + + [dependencies] + bar = "0.1" + baz = { package = "bar", version = "0.1" } + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile").run(); + + // Use non-yanked version. + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("\nname = \"bar\"\nversion = \"0.1.0\"")); + + p.cargo("update --precise 0.1.1 bar") + .masquerade_as_nightly_cargo(&["--precise "]) + .arg("-Zunstable-options") + .with_stderr( + "\ +[UPDATING] `dummy-registry` index +[WARNING] yanked package `bar@0.1.1` is selected by the `--precise` flag from registry `dummy-registry` +[NOTE] it is not recommended to depend on a yanked version +[NOTE] if possible, try other SemVer-compatbile versions +[WARNING] yanked package `bar@0.1.1` is selected by the `--precise` flag from registry `dummy-registry` +[NOTE] it is not recommended to depend on a yanked version +[NOTE] if possible, try other SemVer-compatbile versions +[UPDATING] bar v0.1.0 -> v0.1.1 +", + ) + .run(); + + // Use yanked version. + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("\nname = \"bar\"\nversion = \"0.1.1\"")); +} From caeaaa529c39ebad8df18d9a0112dddb2cf57b7d Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 21 Jan 2024 20:21:14 -0500 Subject: [PATCH 7/7] fix(cargo-update): once warn once for `--precise ` This also tweaks the error message a bit. --- src/cargo/sources/registry/mod.rs | 21 ++++++++++++++------- tests/testsuite/update.rs | 13 ++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index c363d609970..f2f2bb037fa 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -261,6 +261,12 @@ pub struct RegistrySource<'cfg> { /// Otherwise, the resolver would think that those entries no longer /// exist, and it would trigger updates to unrelated packages. yanked_whitelist: HashSet, + /// Yanked versions that have already been selected during queries. + /// + /// As of this writing, this is for not emitting the `--precise ` + /// warning twice, with the assumption of (`dep.package_name()` + `--precise` + /// version) being sufficient to uniquely identify the same query result. + selected_precise_yanked: HashSet<(InternedString, semver::Version)>, } /// The [`config.json`] file stored in the index. @@ -531,6 +537,7 @@ impl<'cfg> RegistrySource<'cfg> { index: index::RegistryIndex::new(source_id, ops.index_path(), config), yanked_whitelist: yanked_whitelist.clone(), ops, + selected_precise_yanked: HashSet::new(), } } @@ -812,13 +819,13 @@ impl<'cfg> Source for RegistrySource<'cfg> { let version = req .precise_version() .expect("--precise in use"); - let source = self.source_id(); - let mut shell = self.config.shell(); - shell.warn(format_args!( - "yanked package `{name}@{version}` is selected by the `--precise` flag from {source}", - ))?; - shell.note("it is not recommended to depend on a yanked version")?; - shell.note("if possible, try other SemVer-compatbile versions")?; + if self.selected_precise_yanked.insert((name, version.clone())) { + let mut shell = self.config.shell(); + shell.warn(format_args!( + "selected package `{name}@{version}` was yanked by the author" + ))?; + shell.note("if possible, try a compatible non-yanked version")?; + } } if called { return Poll::Ready(Ok(())); diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs index fc93e84a0d7..0d283d58079 100644 --- a/tests/testsuite/update.rs +++ b/tests/testsuite/update.rs @@ -1419,9 +1419,8 @@ Caused by: .with_stderr( "\ [UPDATING] `dummy-registry` index -[WARNING] yanked package `bar@0.1.1` is selected by the `--precise` flag from registry `dummy-registry` -[NOTE] it is not recommended to depend on a yanked version -[NOTE] if possible, try other SemVer-compatbile versions +[WARNING] selected package `bar@0.1.1` was yanked by the author +[NOTE] if possible, try a compatible non-yanked version [UPDATING] bar v0.1.0 -> v0.1.1 ", ) @@ -1463,12 +1462,8 @@ fn precise_yanked_multiple_presence() { .with_stderr( "\ [UPDATING] `dummy-registry` index -[WARNING] yanked package `bar@0.1.1` is selected by the `--precise` flag from registry `dummy-registry` -[NOTE] it is not recommended to depend on a yanked version -[NOTE] if possible, try other SemVer-compatbile versions -[WARNING] yanked package `bar@0.1.1` is selected by the `--precise` flag from registry `dummy-registry` -[NOTE] it is not recommended to depend on a yanked version -[NOTE] if possible, try other SemVer-compatbile versions +[WARNING] selected package `bar@0.1.1` was yanked by the author +[NOTE] if possible, try a compatible non-yanked version [UPDATING] bar v0.1.0 -> v0.1.1 ", )