Skip to content

Commit 6790a51

Browse files
borsbors
committed
Auto merge of #12938 - epage:source-refactor, r=weihanglo
refactor(source): Prepare for new PackageIDSpec syntax ### What does this PR try to resolve? This adds tests and refactors to prepare for #12933 ### How should we test and review this PR? ### Additional information
2 parents 746802c + 9f511b6 commit 6790a51

File tree

2 files changed

+156
-59
lines changed

2 files changed

+156
-59
lines changed

‎src/cargo/core/package_id_spec.rs‎

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,10 +180,13 @@ impl PackageIdSpec {
180180
}
181181
}
182182

183-
match self.url {
184-
Some(ref u) => u == package_id.source_id().url(),
185-
None => true,
183+
if let Some(u) = &self.url {
184+
if u != package_id.source_id().url() {
185+
return false;
186+
}
186187
}
188+
189+
true
187190
}
188191

189192
/// Checks a list of `PackageId`s to find 1 that matches this `PackageIdSpec`. If 0, 2, or
@@ -331,7 +334,10 @@ mod tests {
331334
fn ok(spec: &str, expected: PackageIdSpec, expected_rendered: &str) {
332335
let parsed = PackageIdSpec::parse(spec).unwrap();
333336
assert_eq!(parsed, expected);
334-
assert_eq!(parsed.to_string(), expected_rendered);
337+
let rendered = parsed.to_string();
338+
assert_eq!(rendered, expected_rendered);
339+
let reparsed = PackageIdSpec::parse(&rendered).unwrap();
340+
assert_eq!(reparsed, expected);
335341
}
336342

337343
ok(
@@ -424,6 +430,98 @@ mod tests {
424430
},
425431
"[email protected]",
426432
);
433+
434+
// pkgid-spec.md
435+
ok(
436+
"regex",
437+
PackageIdSpec {
438+
name: String::from("regex"),
439+
version: None,
440+
url: None,
441+
},
442+
"regex",
443+
);
444+
ok(
445+
"[email protected]",
446+
PackageIdSpec {
447+
name: String::from("regex"),
448+
version: Some("1.4".parse().unwrap()),
449+
url: None,
450+
},
451+
"[email protected]",
452+
);
453+
ok(
454+
"[email protected]",
455+
PackageIdSpec {
456+
name: String::from("regex"),
457+
version: Some("1.4.3".parse().unwrap()),
458+
url: None,
459+
},
460+
"[email protected]",
461+
);
462+
ok(
463+
"https://github.com/rust-lang/crates.io-index#regex",
464+
PackageIdSpec {
465+
name: String::from("regex"),
466+
version: None,
467+
url: Some(Url::parse("https://github.com/rust-lang/crates.io-index").unwrap()),
468+
},
469+
"https://github.com/rust-lang/crates.io-index#regex",
470+
);
471+
ok(
472+
"https://github.com/rust-lang/crates.io-index#[email protected]",
473+
PackageIdSpec {
474+
name: String::from("regex"),
475+
version: Some("1.4.3".parse().unwrap()),
476+
url: Some(Url::parse("https://github.com/rust-lang/crates.io-index").unwrap()),
477+
},
478+
"https://github.com/rust-lang/crates.io-index#[email protected]",
479+
);
480+
ok(
481+
"https://github.com/rust-lang/cargo#0.52.0",
482+
PackageIdSpec {
483+
name: String::from("cargo"),
484+
version: Some("0.52.0".parse().unwrap()),
485+
url: Some(Url::parse("https://github.com/rust-lang/cargo").unwrap()),
486+
},
487+
"https://github.com/rust-lang/cargo#0.52.0",
488+
);
489+
ok(
490+
"https://github.com/rust-lang/cargo#[email protected]",
491+
PackageIdSpec {
492+
name: String::from("cargo-platform"),
493+
version: Some("0.1.2".parse().unwrap()),
494+
url: Some(Url::parse("https://github.com/rust-lang/cargo").unwrap()),
495+
},
496+
"https://github.com/rust-lang/cargo#[email protected]",
497+
);
498+
ok(
499+
"ssh://[email protected]/rust-lang/regex.git#[email protected]",
500+
PackageIdSpec {
501+
name: String::from("regex"),
502+
version: Some("1.4.3".parse().unwrap()),
503+
url: Some(Url::parse("ssh://[email protected]/rust-lang/regex.git").unwrap()),
504+
},
505+
"ssh://[email protected]/rust-lang/regex.git#[email protected]",
506+
);
507+
ok(
508+
"file:///path/to/my/project/foo",
509+
PackageIdSpec {
510+
name: String::from("foo"),
511+
version: None,
512+
url: Some(Url::parse("file:///path/to/my/project/foo").unwrap()),
513+
},
514+
"file:///path/to/my/project/foo",
515+
);
516+
ok(
517+
"file:///path/to/my/project/foo#1.1.8",
518+
PackageIdSpec {
519+
name: String::from("foo"),
520+
version: Some("1.1.8".parse().unwrap()),
521+
url: Some(Url::parse("file:///path/to/my/project/foo").unwrap()),
522+
},
523+
"file:///path/to/my/project/foo#1.1.8",
524+
);
427525
}
428526

429527
#[test]
@@ -450,6 +548,12 @@ mod tests {
450548
assert!(PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
451549
assert!(!PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
452550
assert!(PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
551+
assert!(PackageIdSpec::parse("https://example.com#[email protected]")
552+
.unwrap()
553+
.matches(foo));
554+
assert!(!PackageIdSpec::parse("https://bob.com#[email protected]")
555+
.unwrap()
556+
.matches(foo));
453557

454558
let meta = PackageId::new("meta", "1.2.3+hello", sid).unwrap();
455559
assert!(PackageIdSpec::parse("meta").unwrap().matches(meta));

‎src/cargo/core/source_id.rs‎

Lines changed: 48 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -188,17 +188,7 @@ impl SourceId {
188188
match kind {
189189
"git" => {
190190
let mut url = url.into_url()?;
191-
let mut reference = GitReference::DefaultBranch;
192-
for (k, v) in url.query_pairs() {
193-
match &k[..] {
194-
// Map older 'ref' to branch.
195-
"branch" | "ref" => reference = GitReference::Branch(v.into_owned()),
196-
197-
"rev" => reference = GitReference::Rev(v.into_owned()),
198-
"tag" => reference = GitReference::Tag(v.into_owned()),
199-
_ => {}
200-
}
201-
}
191+
let reference = GitReference::from_query(url.query_pairs());
202192
let precise = url.fragment().map(|s| s.to_owned());
203193
url.set_fragment(None);
204194
url.set_query(None);
@@ -752,6 +742,20 @@ impl PartialEq for SourceIdInner {
752742
}
753743
}
754744

745+
impl SourceKind {
746+
pub(crate) fn protocol(&self) -> Option<&str> {
747+
match self {
748+
SourceKind::Path => Some("path"),
749+
SourceKind::Git(_) => Some("git"),
750+
SourceKind::Registry => Some("registry"),
751+
// Sparse registry URL already includes the `sparse+` prefix
752+
SourceKind::SparseRegistry => None,
753+
SourceKind::LocalRegistry => Some("local-registry"),
754+
SourceKind::Directory => Some("directory"),
755+
}
756+
}
757+
}
758+
755759
/// Forwards to `Ord`
756760
impl PartialOrd for SourceKind {
757761
fn partial_cmp(&self, other: &SourceKind) -> Option<Ordering> {
@@ -848,57 +852,46 @@ pub struct SourceIdAsUrl<'a> {
848852

849853
impl<'a> fmt::Display for SourceIdAsUrl<'a> {
850854
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
851-
match *self.inner {
852-
SourceIdInner {
853-
kind: SourceKind::Path,
854-
ref url,
855-
..
856-
} => write!(f, "path+{}", url),
857-
SourceIdInner {
858-
kind: SourceKind::Git(ref reference),
859-
ref url,
860-
ref precise,
861-
..
862-
} => {
863-
write!(f, "git+{}", url)?;
864-
if let Some(pretty) = reference.pretty_ref(self.encoded) {
865-
write!(f, "?{}", pretty)?;
866-
}
867-
if let Some(precise) = precise.as_ref() {
868-
write!(f, "#{}", precise)?;
869-
}
870-
Ok(())
871-
}
872-
SourceIdInner {
873-
kind: SourceKind::Registry,
874-
ref url,
875-
..
876-
} => {
877-
write!(f, "registry+{url}")
855+
if let Some(protocol) = self.inner.kind.protocol() {
856+
write!(f, "{protocol}+")?;
857+
}
858+
write!(f, "{}", self.inner.url)?;
859+
if let SourceIdInner {
860+
kind: SourceKind::Git(ref reference),
861+
ref precise,
862+
..
863+
} = *self.inner
864+
{
865+
if let Some(pretty) = reference.pretty_ref(self.encoded) {
866+
write!(f, "?{}", pretty)?;
878867
}
879-
SourceIdInner {
880-
kind: SourceKind::SparseRegistry,
881-
ref url,
882-
..
883-
} => {
884-
// Sparse registry URL already includes the `sparse+` prefix
885-
write!(f, "{url}")
868+
if let Some(precise) = precise.as_ref() {
869+
write!(f, "#{}", precise)?;
886870
}
887-
SourceIdInner {
888-
kind: SourceKind::LocalRegistry,
889-
ref url,
890-
..
891-
} => write!(f, "local-registry+{}", url),
892-
SourceIdInner {
893-
kind: SourceKind::Directory,
894-
ref url,
895-
..
896-
} => write!(f, "directory+{}", url),
897871
}
872+
Ok(())
898873
}
899874
}
900875

901876
impl GitReference {
877+
pub fn from_query(
878+
query_pairs: impl Iterator<Item = (impl AsRef<str>, impl AsRef<str>)>,
879+
) -> Self {
880+
let mut reference = GitReference::DefaultBranch;
881+
for (k, v) in query_pairs {
882+
let v = v.as_ref();
883+
match k.as_ref() {
884+
// Map older 'ref' to branch.
885+
"branch" | "ref" => reference = GitReference::Branch(v.to_owned()),
886+
887+
"rev" => reference = GitReference::Rev(v.to_owned()),
888+
"tag" => reference = GitReference::Tag(v.to_owned()),
889+
_ => {}
890+
}
891+
}
892+
reference
893+
}
894+
902895
/// Returns a `Display`able view of this git reference, or None if using
903896
/// the head of the default branch
904897
pub fn pretty_ref(&self, url_encoded: bool) -> Option<PrettyRef<'_>> {

0 commit comments

Comments
 (0)