Auto merge of #12553 - epage:version, r=weihanglo

refactor: Pull out cargo-add MSRV code for reuse

### What does this PR try to resolve?

#12078 added MSRV code in `cargo add`. Our assumption when writing it is that we'd need to generalize the code before reusing it in other places, like `cargo install`.  This PR focused purely on that refactor because I'm hopeful it will be useful for other work I'm doing.  Despite not having a user for this yet, I think the `cargo install` case is inevitable and I feel this does a bit to clean up MSRV related code by using a more specific type everywhere.

### How should we test and review this PR?

Each commit gradually progresses things along

Author: bors

crates/resolver-tests/src/lib.rs

@@ -15,7 +15,7 @@ use cargo::core::resolver::{self, ResolveOpts, VersionPreferences};
 use cargo::core::source::{GitReference, QueryKind, SourceId};
 use cargo::core::Resolve;
 use cargo::core::{Dependency, PackageId, Registry, Summary};
-use cargo::util::{CargoResult, Config, Graph, IntoUrl};
+use cargo::util::{CargoResult, Config, Graph, IntoUrl, PartialVersion};
 
 use proptest::collection::{btree_map, vec};
 use proptest::prelude::*;
@@ -183,7 +183,7 @@ pub fn resolve_with_config_raw(
         deps,
         &BTreeMap::new(),
         None::<&String>,
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap();
     let opts = ResolveOpts::everything();
@@ -584,7 +584,7 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
         dep,
         &BTreeMap::new(),
         link,
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap()
 }
@@ -612,7 +612,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
         Vec::new(),
         &BTreeMap::new(),
         link,
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap()
 }
@@ -626,7 +626,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
         deps,
         &BTreeMap::new(),
         sum.links().map(|a| a.as_str()),
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap()
 }

src/cargo/core/compiler/compilation.rs

@@ -312,6 +312,7 @@ impl<'cfg> Compilation<'cfg> {
         // crate properties which might require rebuild upon change
         // consider adding the corresponding properties to the hash
         // in BuildContext::target_metadata()
+        let rust_version = pkg.rust_version().as_ref().map(ToString::to_string);
         cmd.env("CARGO_MANIFEST_DIR", pkg.root())
             .env("CARGO_PKG_VERSION_MAJOR", &pkg.version().major.to_string())
             .env("CARGO_PKG_VERSION_MINOR", &pkg.version().minor.to_string())
@@ -342,7 +343,7 @@ impl<'cfg> Compilation<'cfg> {
             .env("CARGO_PKG_AUTHORS", &pkg.authors().join(":"))
             .env(
                 "CARGO_PKG_RUST_VERSION",
-                &pkg.rust_version().unwrap_or(&String::new()),
+                &rust_version.as_deref().unwrap_or_default(),
             )
             .env(
                 "CARGO_PKG_README",

src/cargo/core/manifest.rs

@@ -19,7 +19,7 @@ use crate::core::{Edition, Feature, Features, WorkspaceConfig};
 use crate::util::errors::*;
 use crate::util::interning::InternedString;
 use crate::util::toml::{TomlManifest, TomlProfiles};
-use crate::util::{short_hash, Config, Filesystem};
+use crate::util::{short_hash, Config, Filesystem, PartialVersion};
 
 pub enum EitherManifest {
     Real(Manifest),
@@ -58,7 +58,7 @@ pub struct Manifest {
     original: Rc<TomlManifest>,
     unstable_features: Features,
     edition: Edition,
-    rust_version: Option<String>,
+    rust_version: Option<PartialVersion>,
     im_a_teapot: Option<bool>,
     default_run: Option<String>,
     metabuild: Option<Vec<String>>,
@@ -112,7 +112,7 @@ pub struct ManifestMetadata {
     pub documentation: Option<String>, // URL
     pub badges: BTreeMap<String, BTreeMap<String, String>>,
     pub links: Option<String>,
-    pub rust_version: Option<String>,
+    pub rust_version: Option<PartialVersion>,
 }
 
 #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
@@ -401,7 +401,7 @@ impl Manifest {
         workspace: WorkspaceConfig,
         unstable_features: Features,
         edition: Edition,
-        rust_version: Option<String>,
+        rust_version: Option<PartialVersion>,
         im_a_teapot: Option<bool>,
         default_run: Option<String>,
         original: Rc<TomlManifest>,
@@ -570,8 +570,8 @@ impl Manifest {
         self.edition
     }
 
-    pub fn rust_version(&self) -> Option<&str> {
-        self.rust_version.as_deref()
+    pub fn rust_version(&self) -> Option<PartialVersion> {
+        self.rust_version
     }
 
     pub fn custom_metadata(&self) -> Option<&toml::Value> {

src/cargo/core/package.rs

@@ -31,6 +31,7 @@ use crate::util::network::http::http_handle_and_timeout;
 use crate::util::network::http::HttpTimeout;
 use crate::util::network::retry::{Retry, RetryResult};
 use crate::util::network::sleep::SleepTracker;
+use crate::util::PartialVersion;
 use crate::util::{self, internal, Config, Progress, ProgressStyle};
 
 pub const MANIFEST_PREAMBLE: &str = "\
@@ -103,7 +104,7 @@ pub struct SerializedPackage {
     #[serde(skip_serializing_if = "Option::is_none")]
     metabuild: Option<Vec<String>>,
     default_run: Option<String>,
-    rust_version: Option<String>,
+    rust_version: Option<PartialVersion>,
 }
 
 impl Package {
@@ -177,7 +178,7 @@ impl Package {
         self.targets().iter().any(|target| target.proc_macro())
     }
     /// Gets the package's minimum Rust version.
-    pub fn rust_version(&self) -> Option<&str> {
+    pub fn rust_version(&self) -> Option<PartialVersion> {
         self.manifest().rust_version()
     }
 
@@ -262,7 +263,7 @@ impl Package {
             metabuild: self.manifest().metabuild().cloned(),
             publish: self.publish().as_ref().cloned(),
             default_run: self.manifest().default_run().map(|s| s.to_owned()),
-            rust_version: self.rust_version().map(|s| s.to_owned()),
+            rust_version: self.rust_version(),
         }
     }
 }

src/cargo/core/resolver/version_prefs.rs

@@ -81,6 +81,7 @@ impl VersionPreferences {
 mod test {
     use super::*;
     use crate::core::SourceId;
+    use crate::util::PartialVersion;
     use std::collections::BTreeMap;
 
     fn pkgid(name: &str, version: &str) -> PackageId {
@@ -103,7 +104,7 @@ mod test {
             Vec::new(),
             &features,
             None::<&String>,
-            None::<&String>,
+            None::<PartialVersion>,
         )
         .unwrap()
     }

src/cargo/core/summary.rs

@@ -1,6 +1,7 @@
 use crate::core::{Dependency, PackageId, SourceId};
 use crate::util::interning::InternedString;
 use crate::util::CargoResult;
+use crate::util::PartialVersion;
 use anyhow::bail;
 use semver::Version;
 use std::collections::{BTreeMap, HashMap, HashSet};
@@ -25,7 +26,7 @@ struct Inner {
     features: Rc<FeatureMap>,
     checksum: Option<String>,
     links: Option<InternedString>,
-    rust_version: Option<InternedString>,
+    rust_version: Option<PartialVersion>,
 }
 
 impl Summary {
@@ -34,7 +35,7 @@ impl Summary {
         dependencies: Vec<Dependency>,
         features: &BTreeMap<InternedString, Vec<InternedString>>,
         links: Option<impl Into<InternedString>>,
-        rust_version: Option<impl Into<InternedString>>,
+        rust_version: Option<PartialVersion>,
     ) -> CargoResult<Summary> {
         // ****CAUTION**** If you change anything here that may raise a new
         // error, be sure to coordinate that change with either the index
@@ -56,7 +57,7 @@ impl Summary {
                 features: Rc::new(feature_map),
                 checksum: None,
                 links: links.map(|l| l.into()),
-                rust_version: rust_version.map(|l| l.into()),
+                rust_version,
             }),
         })
     }
@@ -87,7 +88,7 @@ impl Summary {
         self.inner.links
     }
 
-    pub fn rust_version(&self) -> Option<InternedString> {
+    pub fn rust_version(&self) -> Option<PartialVersion> {
         self.inner.rust_version
     }
 

src/cargo/ops/cargo_add/mod.rs

@@ -34,6 +34,7 @@ use crate::util::toml_mut::dependency::Source;
 use crate::util::toml_mut::dependency::WorkspaceSource;
 use crate::util::toml_mut::manifest::DepTable;
 use crate::util::toml_mut::manifest::LocalManifest;
+use crate::util::PartialVersion;
 use crate::CargoResult;
 use crate::Config;
 use crate_spec::CrateSpec;
@@ -567,16 +568,7 @@ fn get_latest_dependency(
             })?;
 
             if config.cli_unstable().msrv_policy && honor_rust_version {
-                fn parse_msrv(rust_version: impl AsRef<str>) -> (u64, u64, u64) {
-                    // HACK: `rust-version` is a subset of the `VersionReq` syntax that only ever
-                    // has one comparator with a required minor and optional patch, and uses no
-                    // other features. If in the future this syntax is expanded, this code will need
-                    // to be updated.
-                    let version_req = semver::VersionReq::parse(rust_version.as_ref()).unwrap();
-                    assert!(version_req.comparators.len() == 1);
-                    let comp = &version_req.comparators[0];
-                    assert_eq!(comp.op, semver::Op::Caret);
-                    assert_eq!(comp.pre, semver::Prerelease::EMPTY);
+                fn parse_msrv(comp: PartialVersion) -> (u64, u64, u64) {
                     (comp.major, comp.minor.unwrap_or(0), comp.patch.unwrap_or(0))
                 }
 
@@ -636,7 +628,7 @@ fn get_latest_dependency(
 
 fn rust_version_incompat_error(
     dep: &str,
-    rust_version: &str,
+    rust_version: PartialVersion,
     lowest_rust_version: Option<&Summary>,
 ) -> anyhow::Error {
     let mut error_msg = format!(

src/cargo/ops/cargo_compile/mod.rs

@@ -492,7 +492,7 @@ pub fn create_bcx<'a, 'cfg>(
                 None => continue,
             };
 
-            let req = semver::VersionReq::parse(version).unwrap();
+            let req = version.caret_req();
             if req.matches(&untagged_version) {
                 continue;
             }

src/cargo/ops/registry/publish.rs

@@ -383,6 +383,7 @@ fn transmit(
         ref links,
         ref rust_version,
     } = *manifest.metadata();
+    let rust_version = rust_version.as_ref().map(ToString::to_string);
     let readme_content = readme
         .as_ref()
         .map(|readme| {
@@ -435,7 +436,7 @@ fn transmit(
                 license_file: license_file.clone(),
                 badges: badges.clone(),
                 links: links.clone(),
-                rust_version: rust_version.clone(),
+                rust_version,
             },
             tarball,
         )

src/cargo/sources/registry/index.rs

@@ -91,7 +91,9 @@ use crate::core::{PackageId, SourceId, Summary};
 use crate::sources::registry::{LoadResponse, RegistryData};
 use crate::util::interning::InternedString;
 use crate::util::IntoUrl;
-use crate::util::{internal, CargoResult, Config, Filesystem, OptVersionReq, ToSemver};
+use crate::util::{
+    internal, CargoResult, Config, Filesystem, OptVersionReq, PartialVersion, ToSemver,
+};
 use anyhow::bail;
 use cargo_util::{paths, registry::make_dep_path};
 use semver::Version;
@@ -305,7 +307,7 @@ pub struct IndexPackage<'a> {
     ///
     /// Added in 2023 (see <https://github.com/rust-lang/crates.io/pull/6267>),
     /// can be `None` if published before then or if not set in the manifest.
-    rust_version: Option<InternedString>,
+    rust_version: Option<PartialVersion>,
     /// The schema version for this entry.
     ///
     /// If this is None, it defaults to version `1`. Entries with unknown

src/cargo/util/mod.rs

@@ -22,7 +22,7 @@ pub use self::progress::{Progress, ProgressStyle};
 pub use self::queue::Queue;
 pub use self::restricted_names::validate_package_name;
 pub use self::rustc::Rustc;
-pub use self::semver_ext::{OptVersionReq, VersionExt, VersionReqExt};
+pub use self::semver_ext::{OptVersionReq, PartialVersion, VersionExt, VersionReqExt};
 pub use self::to_semver::ToSemver;
 pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
 pub use self::workspace::{