Skip to content

Commit da498c8

Browse files
borsbors
committed
Auto merge of #12614 - epage:partial, r=weihanglo
feat(pkgid): Allow incomplete versions when unambigious ### What does this PR try to resolve? This was proposed in #12425 to help sand off some of the rough edges around `cargo update` for its wider use it would be getting. Its easy to accidentally get duplicate copies packages in a repo and a pain to have to specify the full version when `cargo update -p foo@1` is sufficient to describe it. Other effects - profile overrides also supports this since we already allow a spec to match multiple items - `cargo clean -p foo@...` already ignored the version, so now we also parse and ignore the partial version - `cargo tree --prune` will now accept partial versions and will match all of them Parts not effected: - Replacements - Two of the cases were found and we treat it as if the version isn't present which will error, so I think that is correct ### How should we test and review this PR? This extracts `PartialVersion` from `RustVersion` where `RustVersion` is a more specialized variant, not allowing prerelease or build. This works by adopting `PartialVersion` into `PackageIdSpec`. For `PackageIdSpec::query`, this will "just work". ### Additional information
2 parents d5336f8 + 82f9bd3 commit da498c8

File tree

22 files changed

+420
-128
lines changed

22 files changed

+420
-128
lines changed

‎crates/resolver-tests/src/lib.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use cargo::core::Resolve;
1717
use cargo::core::{Dependency, PackageId, Registry, Summary};
1818
use cargo::core::{GitReference, SourceId};
1919
use cargo::sources::source::QueryKind;
20-
use cargo::util::{CargoResult, Config, Graph, IntoUrl, PartialVersion};
20+
use cargo::util::{CargoResult, Config, Graph, IntoUrl, RustVersion};
2121

2222
use proptest::collection::{btree_map, vec};
2323
use proptest::prelude::*;
@@ -185,7 +185,7 @@ pub fn resolve_with_config_raw(
185185
deps,
186186
&BTreeMap::new(),
187187
None::<&String>,
188-
None::<PartialVersion>,
188+
None::<RustVersion>,
189189
)
190190
.unwrap();
191191
let opts = ResolveOpts::everything();
@@ -588,7 +588,7 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
588588
dep,
589589
&BTreeMap::new(),
590590
link,
591-
None::<PartialVersion>,
591+
None::<RustVersion>,
592592
)
593593
.unwrap()
594594
}
@@ -616,7 +616,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
616616
Vec::new(),
617617
&BTreeMap::new(),
618618
link,
619-
None::<PartialVersion>,
619+
None::<RustVersion>,
620620
)
621621
.unwrap()
622622
}
@@ -630,7 +630,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
630630
deps,
631631
&BTreeMap::new(),
632632
sum.links().map(|a| a.as_str()),
633-
None::<PartialVersion>,
633+
None::<RustVersion>,
634634
)
635635
.unwrap()
636636
}

‎src/bin/cargo/commands/remove.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ fn spec_has_match(
288288
}
289289

290290
let version_matches = match (spec.version(), dep.version()) {
291-
(Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(v),
291+
(Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(&v),
292292
(Some(_), None) => false,
293293
(None, None | Some(_)) => true,
294294
};

‎src/cargo/core/manifest.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::core::{Edition, Feature, Features, WorkspaceConfig};
1919
use crate::util::errors::*;
2020
use crate::util::interning::InternedString;
2121
use crate::util::toml::{TomlManifest, TomlProfiles};
22-
use crate::util::{short_hash, Config, Filesystem, PartialVersion};
22+
use crate::util::{short_hash, Config, Filesystem, RustVersion};
2323

2424
pub enum EitherManifest {
2525
Real(Manifest),
@@ -58,7 +58,7 @@ pub struct Manifest {
5858
original: Rc<TomlManifest>,
5959
unstable_features: Features,
6060
edition: Edition,
61-
rust_version: Option<PartialVersion>,
61+
rust_version: Option<RustVersion>,
6262
im_a_teapot: Option<bool>,
6363
default_run: Option<String>,
6464
metabuild: Option<Vec<String>>,
@@ -112,7 +112,7 @@ pub struct ManifestMetadata {
112112
pub documentation: Option<String>, // URL
113113
pub badges: BTreeMap<String, BTreeMap<String, String>>,
114114
pub links: Option<String>,
115-
pub rust_version: Option<PartialVersion>,
115+
pub rust_version: Option<RustVersion>,
116116
}
117117

118118
#[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
@@ -401,7 +401,7 @@ impl Manifest {
401401
workspace: WorkspaceConfig,
402402
unstable_features: Features,
403403
edition: Edition,
404-
rust_version: Option<PartialVersion>,
404+
rust_version: Option<RustVersion>,
405405
im_a_teapot: Option<bool>,
406406
default_run: Option<String>,
407407
original: Rc<TomlManifest>,
@@ -570,8 +570,8 @@ impl Manifest {
570570
self.edition
571571
}
572572

573-
pub fn rust_version(&self) -> Option<PartialVersion> {
574-
self.rust_version
573+
pub fn rust_version(&self) -> Option<&RustVersion> {
574+
self.rust_version.as_ref()
575575
}
576576

577577
pub fn custom_metadata(&self) -> Option<&toml::Value> {

‎src/cargo/core/package.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use crate::util::network::http::http_handle_and_timeout;
3131
use crate::util::network::http::HttpTimeout;
3232
use crate::util::network::retry::{Retry, RetryResult};
3333
use crate::util::network::sleep::SleepTracker;
34-
use crate::util::PartialVersion;
34+
use crate::util::RustVersion;
3535
use crate::util::{self, internal, Config, Progress, ProgressStyle};
3636

3737
pub const MANIFEST_PREAMBLE: &str = "\
@@ -104,7 +104,7 @@ pub struct SerializedPackage {
104104
#[serde(skip_serializing_if = "Option::is_none")]
105105
metabuild: Option<Vec<String>>,
106106
default_run: Option<String>,
107-
rust_version: Option<PartialVersion>,
107+
rust_version: Option<RustVersion>,
108108
}
109109

110110
impl Package {
@@ -178,7 +178,7 @@ impl Package {
178178
self.targets().iter().any(|target| target.proc_macro())
179179
}
180180
/// Gets the package's minimum Rust version.
181-
pub fn rust_version(&self) -> Option<PartialVersion> {
181+
pub fn rust_version(&self) -> Option<&RustVersion> {
182182
self.manifest().rust_version()
183183
}
184184

@@ -263,7 +263,7 @@ impl Package {
263263
metabuild: self.manifest().metabuild().cloned(),
264264
publish: self.publish().as_ref().cloned(),
265265
default_run: self.manifest().default_run().map(|s| s.to_owned()),
266-
rust_version: self.rust_version(),
266+
rust_version: self.rust_version().cloned(),
267267
}
268268
}
269269
}

‎src/cargo/core/package_id_spec.rs

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ use crate::core::PackageId;
1010
use crate::util::edit_distance;
1111
use crate::util::errors::CargoResult;
1212
use crate::util::interning::InternedString;
13-
use crate::util::{validate_package_name, IntoUrl, ToSemver};
13+
use crate::util::PartialVersion;
14+
use crate::util::{validate_package_name, IntoUrl};
1415

1516
/// Some or all of the data required to identify a package:
1617
///
@@ -24,7 +25,7 @@ use crate::util::{validate_package_name, IntoUrl, ToSemver};
2425
#[derive(Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)]
2526
pub struct PackageIdSpec {
2627
name: InternedString,
27-
version: Option<Version>,
28+
version: Option<PartialVersion>,
2829
url: Option<Url>,
2930
}
3031

@@ -70,7 +71,7 @@ impl PackageIdSpec {
7071
let mut parts = spec.splitn(2, [':', '@']);
7172
let name = parts.next().unwrap();
7273
let version = match parts.next() {
73-
Some(version) => Some(version.to_semver()?),
74+
Some(version) => Some(version.parse::<PartialVersion>()?),
7475
None => None,
7576
};
7677
validate_package_name(name, "pkgid", "")?;
@@ -94,12 +95,12 @@ impl PackageIdSpec {
9495
spec.query(i)
9596
}
9697

97-
/// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `Version` and `Url`
98+
/// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url`
9899
/// fields filled in.
99100
pub fn from_package_id(package_id: PackageId) -> PackageIdSpec {
100101
PackageIdSpec {
101102
name: package_id.name(),
102-
version: Some(package_id.version().clone()),
103+
version: Some(package_id.version().clone().into()),
103104
url: Some(package_id.source_id().url().clone()),
104105
}
105106
}
@@ -125,14 +126,14 @@ impl PackageIdSpec {
125126
match frag {
126127
Some(fragment) => match fragment.split_once([':', '@']) {
127128
Some((name, part)) => {
128-
let version = part.to_semver()?;
129+
let version = part.parse::<PartialVersion>()?;
129130
(InternedString::new(name), Some(version))
130131
}
131132
None => {
132133
if fragment.chars().next().unwrap().is_alphabetic() {
133134
(InternedString::new(&fragment), None)
134135
} else {
135-
let version = fragment.to_semver()?;
136+
let version = fragment.parse::<PartialVersion>()?;
136137
(InternedString::new(path_name), Some(version))
137138
}
138139
}
@@ -151,7 +152,12 @@ impl PackageIdSpec {
151152
self.name
152153
}
153154

154-
pub fn version(&self) -> Option<&Version> {
155+
/// Full `semver::Version`, if present
156+
pub fn version(&self) -> Option<Version> {
157+
self.version.as_ref().and_then(|v| v.version())
158+
}
159+
160+
pub fn partial_version(&self) -> Option<&PartialVersion> {
155161
self.version.as_ref()
156162
}
157163

@@ -170,7 +176,8 @@ impl PackageIdSpec {
170176
}
171177

172178
if let Some(ref v) = self.version {
173-
if v != package_id.version() {
179+
let req = v.exact_req();
180+
if !req.matches(package_id.version()) {
174181
return false;
175182
}
176183
}
@@ -319,7 +326,6 @@ mod tests {
319326
use super::PackageIdSpec;
320327
use crate::core::{PackageId, SourceId};
321328
use crate::util::interning::InternedString;
322-
use crate::util::ToSemver;
323329
use url::Url;
324330

325331
#[test]
@@ -344,16 +350,25 @@ mod tests {
344350
"https://crates.io/foo#1.2.3",
345351
PackageIdSpec {
346352
name: InternedString::new("foo"),
347-
version: Some("1.2.3".to_semver().unwrap()),
353+
version: Some("1.2.3".parse().unwrap()),
348354
url: Some(Url::parse("https://crates.io/foo").unwrap()),
349355
},
350356
"https://crates.io/foo#1.2.3",
351357
);
358+
ok(
359+
"https://crates.io/foo#1.2",
360+
PackageIdSpec {
361+
name: InternedString::new("foo"),
362+
version: Some("1.2".parse().unwrap()),
363+
url: Some(Url::parse("https://crates.io/foo").unwrap()),
364+
},
365+
"https://crates.io/foo#1.2",
366+
);
352367
ok(
353368
"https://crates.io/foo#bar:1.2.3",
354369
PackageIdSpec {
355370
name: InternedString::new("bar"),
356-
version: Some("1.2.3".to_semver().unwrap()),
371+
version: Some("1.2.3".parse().unwrap()),
357372
url: Some(Url::parse("https://crates.io/foo").unwrap()),
358373
},
359374
"https://crates.io/foo#[email protected]",
@@ -362,11 +377,20 @@ mod tests {
362377
"https://crates.io/foo#[email protected]",
363378
PackageIdSpec {
364379
name: InternedString::new("bar"),
365-
version: Some("1.2.3".to_semver().unwrap()),
380+
version: Some("1.2.3".parse().unwrap()),
366381
url: Some(Url::parse("https://crates.io/foo").unwrap()),
367382
},
368383
"https://crates.io/foo#[email protected]",
369384
);
385+
ok(
386+
"https://crates.io/foo#[email protected]",
387+
PackageIdSpec {
388+
name: InternedString::new("bar"),
389+
version: Some("1.2".parse().unwrap()),
390+
url: Some(Url::parse("https://crates.io/foo").unwrap()),
391+
},
392+
"https://crates.io/foo#[email protected]",
393+
);
370394
ok(
371395
"foo",
372396
PackageIdSpec {
@@ -380,7 +404,7 @@ mod tests {
380404
"foo:1.2.3",
381405
PackageIdSpec {
382406
name: InternedString::new("foo"),
383-
version: Some("1.2.3".to_semver().unwrap()),
407+
version: Some("1.2.3".parse().unwrap()),
384408
url: None,
385409
},
386410
"[email protected]",
@@ -389,21 +413,29 @@ mod tests {
389413
"[email protected]",
390414
PackageIdSpec {
391415
name: InternedString::new("foo"),
392-
version: Some("1.2.3".to_semver().unwrap()),
416+
version: Some("1.2.3".parse().unwrap()),
393417
url: None,
394418
},
395419
"[email protected]",
396420
);
421+
ok(
422+
"[email protected]",
423+
PackageIdSpec {
424+
name: InternedString::new("foo"),
425+
version: Some("1.2".parse().unwrap()),
426+
url: None,
427+
},
428+
"[email protected]",
429+
);
397430
}
398431

399432
#[test]
400433
fn bad_parsing() {
401434
assert!(PackageIdSpec::parse("baz:").is_err());
402435
assert!(PackageIdSpec::parse("baz:*").is_err());
403-
assert!(PackageIdSpec::parse("baz:1.0").is_err());
404436
assert!(PackageIdSpec::parse("baz@").is_err());
405437
assert!(PackageIdSpec::parse("baz@*").is_err());
406-
assert!(PackageIdSpec::parse("[email protected]").is_err());
438+
assert!(PackageIdSpec::parse("baz@^1.0").is_err());
407439
assert!(PackageIdSpec::parse("https://baz:1.0").is_err());
408440
assert!(PackageIdSpec::parse("https://#baz:1.0").is_err());
409441
}
@@ -421,5 +453,6 @@ mod tests {
421453
assert!(!PackageIdSpec::parse("foo:1.2.2").unwrap().matches(foo));
422454
assert!(PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
423455
assert!(!PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
456+
assert!(PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
424457
}
425458
}

‎src/cargo/core/resolver/dep_cache.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry,
2020
use crate::sources::source::QueryKind;
2121
use crate::util::errors::CargoResult;
2222
use crate::util::interning::InternedString;
23-
use crate::util::PartialVersion;
23+
use crate::util::RustVersion;
2424

2525
use anyhow::Context as _;
2626
use std::collections::{BTreeSet, HashMap, HashSet};
@@ -36,7 +36,7 @@ pub struct RegistryQueryer<'a> {
3636
/// versions first. That allows `cargo update -Z minimal-versions` which will
3737
/// specify minimum dependency versions to be used.
3838
minimal_versions: bool,
39-
max_rust_version: Option<PartialVersion>,
39+
max_rust_version: Option<RustVersion>,
4040
/// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_minimal_version`)
4141
registry_cache: HashMap<(Dependency, bool), Poll<Rc<Vec<Summary>>>>,
4242
/// a cache of `Dependency`s that are required for a `Summary`
@@ -58,14 +58,14 @@ impl<'a> RegistryQueryer<'a> {
5858
replacements: &'a [(PackageIdSpec, Dependency)],
5959
version_prefs: &'a VersionPreferences,
6060
minimal_versions: bool,
61-
max_rust_version: Option<PartialVersion>,
61+
max_rust_version: Option<&RustVersion>,
6262
) -> Self {
6363
RegistryQueryer {
6464
registry,
6565
replacements,
6666
version_prefs,
6767
minimal_versions,
68-
max_rust_version,
68+
max_rust_version: max_rust_version.cloned(),
6969
registry_cache: HashMap::new(),
7070
summary_cache: HashMap::new(),
7171
used_replacements: HashMap::new(),
@@ -115,7 +115,8 @@ impl<'a> RegistryQueryer<'a> {
115115

116116
let mut ret = Vec::new();
117117
let ready = self.registry.query(dep, QueryKind::Exact, &mut |s| {
118-
if self.max_rust_version.is_none() || s.rust_version() <= self.max_rust_version {
118+
if self.max_rust_version.is_none() || s.rust_version() <= self.max_rust_version.as_ref()
119+
{
119120
ret.push(s);
120121
}
121122
})?;

‎src/cargo/core/resolver/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ use crate::util::config::Config;
7171
use crate::util::errors::CargoResult;
7272
use crate::util::network::PollExt;
7373
use crate::util::profile;
74-
use crate::util::PartialVersion;
74+
use crate::util::RustVersion;
7575

7676
use self::context::Context;
7777
use self::dep_cache::RegistryQueryer;
@@ -139,7 +139,7 @@ pub fn resolve(
139139
version_prefs: &VersionPreferences,
140140
config: Option<&Config>,
141141
check_public_visible_dependencies: bool,
142-
mut max_rust_version: Option<PartialVersion>,
142+
mut max_rust_version: Option<&RustVersion>,
143143
) -> CargoResult<Resolve> {
144144
let _p = profile::start("resolving");
145145
let minimal_versions = match config {

0 commit comments

Comments
 (0)