From e7ca58544558cb2507f591391ecfefcb0abe5ed7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Dec 2023 15:15:46 -0600 Subject: [PATCH 1/4] test(pkgid): Demonstrate ambiguos specs --- tests/testsuite/pkgid.rs | 87 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 88d991e80d6..92f258019d6 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -1,5 +1,7 @@ //! Tests for the `cargo pkgid` command. +use cargo_test_support::basic_lib_manifest; +use cargo_test_support::git; use cargo_test_support::project; use cargo_test_support::registry::Package; @@ -195,3 +197,88 @@ Did you mean one of these? ) .run(); } + +// Not for `cargo pkgid` but the `PackageIdSpec` format +#[cargo_test] +fn multiple_git_same_version() { + // Test what happens if different packages refer to the same git repo with + // different refs, and the package version is the same. + let (xyz_project, xyz_repo) = git::new_repo("xyz", |project| { + project + .file("Cargo.toml", &basic_lib_manifest("xyz")) + .file("src/lib.rs", "fn example() {}") + }); + let rev1 = xyz_repo.revparse_single("HEAD").unwrap().id(); + xyz_project.change_file("src/lib.rs", "pub fn example() {}"); + git::add(&xyz_repo); + let rev2 = git::commit(&xyz_repo); + // Both rev1 and rev2 point to version 0.1.0. + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar = {{ path = "bar" }} + xyz = {{ git = "{}", rev = "{}" }} + + "#, + xyz_project.url(), + rev1 + ), + ) + .file("src/lib.rs", "") + .file( + "bar/Cargo.toml", + &format!( + r#" + [package] + name = "bar" + version = "0.1.0" + + [dependencies] + xyz = {{ git = "{}", rev = "{}" }} + "#, + xyz_project.url(), + rev2 + ), + ) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("check").run(); + p.cargo("tree") + .with_stdout(&format!( + "\ +foo v0.1.0 ([..]/foo) +├── bar v0.1.0 ([..]/foo/bar) +│ └── xyz v0.5.0 (file://[..]/xyz?rev={}#{}) +└── xyz v0.5.0 (file://[..]/xyz?rev={}#{}) +", + rev2, + &rev2.to_string()[..8], + rev1, + &rev1.to_string()[..8] + )) + .run(); + // FIXME: This fails since xyz is ambiguous, but the + // possible pkgids are also ambiguous. + p.cargo("pkgid xyz") + .with_status(101) + .with_stderr( + "\ +error: There are multiple `xyz` packages in your project, and the specification `xyz` is ambiguous. +Please re-run this command with one of the following specifications: + file://[..]/xyz#0.5.0 + file://[..]/xyz#0.5.0 +", + ) + .run(); + // TODO, what should the `-p` value be here? + //p.cargo("update -p") +} From 005b55fdbe173b02d164f05c8783c62cf5b67f93 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 14:52:54 -0600 Subject: [PATCH 2/4] refactor(spec): Allow tracking the kind --- src/cargo/core/mod.rs | 2 +- src/cargo/core/package_id_spec.rs | 27 +++++++++++++++++++++++++++ src/cargo/core/source_id.rs | 2 +- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 9860d2617b6..eea910b6651 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -8,7 +8,7 @@ pub use self::package_id_spec::PackageIdSpec; pub use self::registry::Registry; pub use self::resolver::{Resolve, ResolveVersion}; pub use self::shell::{Shell, Verbosity}; -pub use self::source_id::{GitReference, SourceId}; +pub use self::source_id::{GitReference, SourceId, SourceKind}; pub use self::summary::{FeatureMap, FeatureValue, Summary}; pub use self::workspace::{ find_workspace_root, resolve_relative_path, MaybePackage, Workspace, WorkspaceConfig, diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index c617c1f7ab0..122c40f1976 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -7,6 +7,7 @@ use serde::{de, ser}; use url::Url; use crate::core::PackageId; +use crate::core::SourceKind; use crate::util::edit_distance; use crate::util::errors::CargoResult; use crate::util::{validate_package_name, IntoUrl}; @@ -26,6 +27,7 @@ pub struct PackageIdSpec { name: String, version: Option, url: Option, + kind: Option, } impl PackageIdSpec { @@ -78,6 +80,7 @@ impl PackageIdSpec { name: String::from(name), version, url: None, + kind: None, }) } @@ -101,6 +104,7 @@ impl PackageIdSpec { name: String::from(package_id.name().as_str()), version: Some(package_id.version().clone().into()), url: Some(package_id.source_id().url().clone()), + kind: None, } } @@ -144,6 +148,7 @@ impl PackageIdSpec { name, version, url: Some(url), + kind: None, }) } @@ -216,6 +221,7 @@ impl PackageIdSpec { name: self.name.clone(), version: self.version.clone(), url: None, + kind: None, }, &mut suggestion, ); @@ -226,6 +232,7 @@ impl PackageIdSpec { name: self.name.clone(), version: None, url: None, + kind: None, }, &mut suggestion, ); @@ -346,6 +353,7 @@ mod tests { name: String::from("foo"), version: None, url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, }, "https://crates.io/foo", ); @@ -355,6 +363,7 @@ mod tests { name: String::from("foo"), version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, }, "https://crates.io/foo#1.2.3", ); @@ -364,6 +373,7 @@ mod tests { name: String::from("foo"), version: Some("1.2".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, }, "https://crates.io/foo#1.2", ); @@ -373,6 +383,7 @@ mod tests { name: String::from("bar"), version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, }, "https://crates.io/foo#bar@1.2.3", ); @@ -382,6 +393,7 @@ mod tests { name: String::from("bar"), version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, }, "https://crates.io/foo#bar@1.2.3", ); @@ -391,6 +403,7 @@ mod tests { name: String::from("bar"), version: Some("1.2".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, }, "https://crates.io/foo#bar@1.2", ); @@ -400,6 +413,7 @@ mod tests { name: String::from("foo"), version: None, url: None, + kind: None, }, "foo", ); @@ -409,6 +423,7 @@ mod tests { name: String::from("foo"), version: Some("1.2.3".parse().unwrap()), url: None, + kind: None, }, "foo@1.2.3", ); @@ -418,6 +433,7 @@ mod tests { name: String::from("foo"), version: Some("1.2.3".parse().unwrap()), url: None, + kind: None, }, "foo@1.2.3", ); @@ -427,6 +443,7 @@ mod tests { name: String::from("foo"), version: Some("1.2".parse().unwrap()), url: None, + kind: None, }, "foo@1.2", ); @@ -438,6 +455,7 @@ mod tests { name: String::from("regex"), version: None, url: None, + kind: None, }, "regex", ); @@ -447,6 +465,7 @@ mod tests { name: String::from("regex"), version: Some("1.4".parse().unwrap()), url: None, + kind: None, }, "regex@1.4", ); @@ -456,6 +475,7 @@ mod tests { name: String::from("regex"), version: Some("1.4.3".parse().unwrap()), url: None, + kind: None, }, "regex@1.4.3", ); @@ -465,6 +485,7 @@ mod tests { name: String::from("regex"), version: None, url: Some(Url::parse("https://github.com/rust-lang/crates.io-index").unwrap()), + kind: None, }, "https://github.com/rust-lang/crates.io-index#regex", ); @@ -474,6 +495,7 @@ mod tests { name: String::from("regex"), version: Some("1.4.3".parse().unwrap()), url: Some(Url::parse("https://github.com/rust-lang/crates.io-index").unwrap()), + kind: None, }, "https://github.com/rust-lang/crates.io-index#regex@1.4.3", ); @@ -483,6 +505,7 @@ mod tests { name: String::from("cargo"), version: Some("0.52.0".parse().unwrap()), url: Some(Url::parse("https://github.com/rust-lang/cargo").unwrap()), + kind: None, }, "https://github.com/rust-lang/cargo#0.52.0", ); @@ -492,6 +515,7 @@ mod tests { name: String::from("cargo-platform"), version: Some("0.1.2".parse().unwrap()), url: Some(Url::parse("https://github.com/rust-lang/cargo").unwrap()), + kind: None, }, "https://github.com/rust-lang/cargo#cargo-platform@0.1.2", ); @@ -501,6 +525,7 @@ mod tests { name: String::from("regex"), version: Some("1.4.3".parse().unwrap()), url: Some(Url::parse("ssh://git@github.com/rust-lang/regex.git").unwrap()), + kind: None, }, "ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", ); @@ -510,6 +535,7 @@ mod tests { name: String::from("foo"), version: None, url: Some(Url::parse("file:///path/to/my/project/foo").unwrap()), + kind: None, }, "file:///path/to/my/project/foo", ); @@ -519,6 +545,7 @@ mod tests { name: String::from("foo"), version: Some("1.1.8".parse().unwrap()), url: Some(Url::parse("file:///path/to/my/project/foo").unwrap()), + kind: None, }, "file:///path/to/my/project/foo#1.1.8", ); diff --git a/src/cargo/core/source_id.rs b/src/cargo/core/source_id.rs index e53b1704db0..2d8d9711370 100644 --- a/src/cargo/core/source_id.rs +++ b/src/cargo/core/source_id.rs @@ -85,7 +85,7 @@ impl fmt::Display for Precise { /// The possible kinds of code source. /// Along with [`SourceIdInner`], this fully defines the source. #[derive(Debug, Clone, PartialEq, Eq, Hash)] -enum SourceKind { +pub enum SourceKind { /// A git repository. Git(GitReference), /// A local path. From 9b9c6838102c5e004a9725b41a6f232890498f13 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 16:26:49 -0600 Subject: [PATCH 3/4] feat(spec): Track source kind --- src/cargo/core/package_id_spec.rs | 122 +++++++++++++++++++++++++++- src/cargo/core/source_id.rs | 6 +- src/doc/src/reference/pkgid-spec.md | 40 +++++---- tests/testsuite/pkgid.rs | 15 ++-- 4 files changed, 155 insertions(+), 28 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 122c40f1976..9852b11fcc0 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -6,6 +6,7 @@ use semver::Version; use serde::{de, ser}; use url::Url; +use crate::core::GitReference; use crate::core::PackageId; use crate::core::SourceKind; use crate::util::edit_distance; @@ -104,17 +105,47 @@ impl PackageIdSpec { name: String::from(package_id.name().as_str()), version: Some(package_id.version().clone().into()), url: Some(package_id.source_id().url().clone()), - kind: None, + kind: Some(package_id.source_id().kind().clone()), } } /// Tries to convert a valid `Url` to a `PackageIdSpec`. fn from_url(mut url: Url) -> CargoResult { + let mut kind = None; + if let Some((kind_str, scheme)) = url.scheme().split_once('+') { + match kind_str { + "git" => { + let git_ref = GitReference::DefaultBranch; + kind = Some(SourceKind::Git(git_ref)); + url = strip_url_protocol(&url); + } + "registry" => { + kind = Some(SourceKind::Registry); + url = strip_url_protocol(&url); + } + "sparse" => { + kind = Some(SourceKind::SparseRegistry); + // Leave `sparse` as part of URL, see `SourceId::new` + // url = strip_url_protocol(&url); + } + "path" => { + if scheme != "file" { + anyhow::bail!("`path+{scheme}` is unsupported; `path+file` and `file` schemes are supported"); + } + kind = Some(SourceKind::Path); + url = strip_url_protocol(&url); + } + kind => anyhow::bail!("unsupported source protocol: {kind}"), + } + } + if url.query().is_some() { bail!("cannot have a query string in a pkgid: {}", url) } + let frag = url.fragment().map(|s| s.to_owned()); url.set_fragment(None); + let (name, version) = { let mut path = url .path_segments() @@ -148,7 +179,7 @@ impl PackageIdSpec { name, version, url: Some(url), - kind: None, + kind, }) } @@ -173,6 +204,14 @@ impl PackageIdSpec { self.url = Some(url); } + pub fn kind(&self) -> Option<&SourceKind> { + self.kind.as_ref() + } + + pub fn set_kind(&mut self, kind: SourceKind) { + self.kind = Some(kind); + } + /// Checks whether the given `PackageId` matches the `PackageIdSpec`. pub fn matches(&self, package_id: PackageId) -> bool { if self.name() != package_id.name().as_str() { @@ -191,6 +230,12 @@ impl PackageIdSpec { } } + if let Some(k) = &self.kind { + if k != package_id.source_id().kind() { + return false; + } + } + true } @@ -287,11 +332,20 @@ impl PackageIdSpec { } } +fn strip_url_protocol(url: &Url) -> Url { + // Ridiculous hoop because `Url::set_scheme` errors when changing to http/https + let raw = url.to_string(); + raw.split_once('+').unwrap().1.parse().unwrap() +} + impl fmt::Display for PackageIdSpec { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut printed_name = false; match self.url { Some(ref url) => { + if let Some(protocol) = self.kind.as_ref().and_then(|k| k.protocol()) { + write!(f, "{protocol}+")?; + } write!(f, "{}", url)?; if url.path_segments().unwrap().next_back().unwrap() != &*self.name { printed_name = true; @@ -332,7 +386,7 @@ impl<'de> de::Deserialize<'de> for PackageIdSpec { #[cfg(test)] mod tests { use super::PackageIdSpec; - use crate::core::{PackageId, SourceId}; + use crate::core::{GitReference, PackageId, SourceId, SourceKind}; use url::Url; #[test] @@ -407,6 +461,26 @@ mod tests { }, "https://crates.io/foo#bar@1.2", ); + ok( + "registry+https://crates.io/foo#bar@1.2", + PackageIdSpec { + name: String::from("bar"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: Some(SourceKind::Registry), + }, + "registry+https://crates.io/foo#bar@1.2", + ); + ok( + "sparse+https://crates.io/foo#bar@1.2", + PackageIdSpec { + name: String::from("bar"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("sparse+https://crates.io/foo").unwrap()), + kind: Some(SourceKind::SparseRegistry), + }, + "sparse+https://crates.io/foo#bar@1.2", + ); ok( "foo", PackageIdSpec { @@ -499,6 +573,18 @@ mod tests { }, "https://github.com/rust-lang/crates.io-index#regex@1.4.3", ); + ok( + "sparse+https://github.com/rust-lang/crates.io-index#regex@1.4.3", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4.3".parse().unwrap()), + url: Some( + Url::parse("sparse+https://github.com/rust-lang/crates.io-index").unwrap(), + ), + kind: Some(SourceKind::SparseRegistry), + }, + "sparse+https://github.com/rust-lang/crates.io-index#regex@1.4.3", + ); ok( "https://github.com/rust-lang/cargo#0.52.0", PackageIdSpec { @@ -529,6 +615,16 @@ mod tests { }, "ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", ); + ok( + "git+ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4.3".parse().unwrap()), + url: Some(Url::parse("ssh://git@github.com/rust-lang/regex.git").unwrap()), + kind: Some(SourceKind::Git(GitReference::DefaultBranch)), + }, + "git+ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", + ); ok( "file:///path/to/my/project/foo", PackageIdSpec { @@ -549,6 +645,16 @@ mod tests { }, "file:///path/to/my/project/foo#1.1.8", ); + ok( + "path+file:///path/to/my/project/foo#1.1.8", + PackageIdSpec { + name: String::from("foo"), + version: Some("1.1.8".parse().unwrap()), + url: Some(Url::parse("file:///path/to/my/project/foo").unwrap()), + kind: Some(SourceKind::Path), + }, + "path+file:///path/to/my/project/foo#1.1.8", + ); } #[test] @@ -560,6 +666,10 @@ mod tests { assert!(PackageIdSpec::parse("baz@^1.0").is_err()); assert!(PackageIdSpec::parse("https://baz:1.0").is_err()); assert!(PackageIdSpec::parse("https://#baz:1.0").is_err()); + assert!( + PackageIdSpec::parse("foobar+https://github.com/rust-lang/crates.io-index").is_err() + ); + assert!(PackageIdSpec::parse("path+https://github.com/rust-lang/crates.io-index").is_err()); } #[test] @@ -581,6 +691,12 @@ mod tests { assert!(!PackageIdSpec::parse("https://bob.com#foo@1.2") .unwrap() .matches(foo)); + assert!(PackageIdSpec::parse("registry+https://example.com#foo@1.2") + .unwrap() + .matches(foo)); + assert!(!PackageIdSpec::parse("git+https://example.com#foo@1.2") + .unwrap() + .matches(foo)); let meta = PackageId::new("meta", "1.2.3+hello", sid).unwrap(); assert!(PackageIdSpec::parse("meta").unwrap().matches(meta)); diff --git a/src/cargo/core/source_id.rs b/src/cargo/core/source_id.rs index 2d8d9711370..cde204377b0 100644 --- a/src/cargo/core/source_id.rs +++ b/src/cargo/core/source_id.rs @@ -373,6 +373,10 @@ impl SourceId { Some(self.inner.url.to_file_path().unwrap()) } + pub fn kind(&self) -> &SourceKind { + &self.inner.kind + } + /// Returns `true` if this source is from a registry (either local or not). pub fn is_registry(self) -> bool { matches!( @@ -748,7 +752,7 @@ impl SourceKind { SourceKind::Path => Some("path"), SourceKind::Git(_) => Some("git"), SourceKind::Registry => Some("registry"), - // Sparse registry URL already includes the `sparse+` prefix + // Sparse registry URL already includes the `sparse+` prefix, see `SourceId::new` SourceKind::SparseRegistry => None, SourceKind::LocalRegistry => Some("local-registry"), SourceKind::Directory => Some("directory"), diff --git a/src/doc/src/reference/pkgid-spec.md b/src/doc/src/reference/pkgid-spec.md index 7f20973b509..7c4be10b552 100644 --- a/src/doc/src/reference/pkgid-spec.md +++ b/src/doc/src/reference/pkgid-spec.md @@ -21,11 +21,12 @@ qualified with a version to make it unique, such as `regex@1.4.3`. The formal grammar for a Package Id Specification is: ```notrust -spec := pkgname - | proto "://" hostname-and-path [ "#" ( pkgname | semver ) ] +spec := pkgname | + [ kind "+" ] proto "://" hostname-and-path [ "#" ( pkgname | semver ) ] pkgname := name [ ("@" | ":" ) semver ] semver := digits [ "." digits [ "." digits [ "-" prerelease ] [ "+" build ]]] +kind = "registry" | "git" | "file" proto := "http" | "git" | ... ``` @@ -38,28 +39,31 @@ that come from different sources such as different registries. The following are references to the `regex` package on `crates.io`: -| Spec | Name | Version | -|:------------------------------------------------------------|:-------:|:-------:| -| `regex` | `regex` | `*` | -| `regex@1.4` | `regex` | `1.4.*` | -| `regex@1.4.3` | `regex` | `1.4.3` | -| `https://github.com/rust-lang/crates.io-index#regex` | `regex` | `*` | -| `https://github.com/rust-lang/crates.io-index#regex@1.4.3` | `regex` | `1.4.3` | +| Spec | Name | Version | +|:------------------------------------------------------------------|:-------:|:-------:| +| `regex` | `regex` | `*` | +| `regex@1.4` | `regex` | `1.4.*` | +| `regex@1.4.3` | `regex` | `1.4.3` | +| `https://github.com/rust-lang/crates.io-index#regex` | `regex` | `*` | +| `https://github.com/rust-lang/crates.io-index#regex@1.4.3` | `regex` | `1.4.3` | +| `registry+https://github.com/rust-lang/crates.io-index#regex@1.4.3` | `regex` | `1.4.3` | The following are some examples of specs for several different git dependencies: -| Spec | Name | Version | -|:----------------------------------------------------------|:----------------:|:--------:| -| `https://github.com/rust-lang/cargo#0.52.0` | `cargo` | `0.52.0` | -| `https://github.com/rust-lang/cargo#cargo-platform@0.1.2` | `cargo-platform` | `0.1.2` | -| `ssh://git@github.com/rust-lang/regex.git#regex@1.4.3` | `regex` | `1.4.3` | +| Spec | Name | Version | +|:-----------------------------------------------------------|:----------------:|:--------:| +| `https://github.com/rust-lang/cargo#0.52.0` | `cargo` | `0.52.0` | +| `https://github.com/rust-lang/cargo#cargo-platform@0.1.2` | `cargo-platform` | `0.1.2` | +| `ssh://git@github.com/rust-lang/regex.git#regex@1.4.3` | `regex` | `1.4.3` | +| `git+ssh://git@github.com/rust-lang/regex.git#regex@1.4.3` | `regex` | `1.4.3` | Local packages on the filesystem can use `file://` URLs to reference them: -| Spec | Name | Version | -|:---------------------------------------|:-----:|:-------:| -| `file:///path/to/my/project/foo` | `foo` | `*` | -| `file:///path/to/my/project/foo#1.1.8` | `foo` | `1.1.8` | +| Spec | Name | Version | +|:--------------------------------------------|:-----:|:-------:| +| `file:///path/to/my/project/foo` | `foo` | `*` | +| `file:///path/to/my/project/foo#1.1.8` | `foo` | `1.1.8` | +| `path+file:///path/to/my/project/foo#1.1.8` | `foo` | `1.1.8` | ### Brevity of specifications diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 92f258019d6..54c0d4751be 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -36,7 +36,10 @@ fn local() { p.cargo("generate-lockfile").run(); p.cargo("pkgid foo") - .with_stdout(format!("file://[..]{}#0.1.0", p.root().to_str().unwrap())) + .with_stdout(format!( + "path+file://[..]{}#0.1.0", + p.root().to_str().unwrap() + )) .run(); // Bad file URL. @@ -91,7 +94,7 @@ fn registry() { p.cargo("generate-lockfile").run(); p.cargo("pkgid crates-io") - .with_stdout("https://github.com/rust-lang/crates.io-index#crates-io@0.1.0") + .with_stdout("registry+https://github.com/rust-lang/crates.io-index#crates-io@0.1.0") .run(); // Bad URL. @@ -145,7 +148,7 @@ fn multiple_versions() { p.cargo("generate-lockfile").run(); p.cargo("pkgid two-ver:0.2.0") - .with_stdout("https://github.com/rust-lang/crates.io-index#two-ver@0.2.0") + .with_stdout("registry+https://github.com/rust-lang/crates.io-index#two-ver@0.2.0") .run(); // Incomplete version. @@ -165,7 +168,7 @@ Please re-run this command with one of the following specifications: p.cargo("pkgid two-ver@0.2") .with_stdout( "\ -https://github.com/rust-lang/crates.io-index#two-ver@0.2.0 +registry+https://github.com/rust-lang/crates.io-index#two-ver@0.2.0 ", ) .run(); @@ -274,8 +277,8 @@ foo v0.1.0 ([..]/foo) "\ error: There are multiple `xyz` packages in your project, and the specification `xyz` is ambiguous. Please re-run this command with one of the following specifications: - file://[..]/xyz#0.5.0 - file://[..]/xyz#0.5.0 + git+file://[..]/xyz#0.5.0 + git+file://[..]/xyz#0.5.0 ", ) .run(); From 64f5b0c8692484452aaf1581ee749d2c851d22b8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 16:38:36 -0600 Subject: [PATCH 4/4] feat(spec): Track git ref --- src/cargo/core/package_id_spec.rs | 47 ++++++++++++++++++++++++++--- src/doc/src/reference/pkgid-spec.md | 4 ++- tests/testsuite/pkgid.rs | 4 +-- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 9852b11fcc0..81f2b1dec01 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -115,20 +115,30 @@ impl PackageIdSpec { if let Some((kind_str, scheme)) = url.scheme().split_once('+') { match kind_str { "git" => { - let git_ref = GitReference::DefaultBranch; + let git_ref = GitReference::from_query(url.query_pairs()); + url.set_query(None); kind = Some(SourceKind::Git(git_ref)); url = strip_url_protocol(&url); } "registry" => { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } kind = Some(SourceKind::Registry); url = strip_url_protocol(&url); } "sparse" => { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } kind = Some(SourceKind::SparseRegistry); // Leave `sparse` as part of URL, see `SourceId::new` // url = strip_url_protocol(&url); } "path" => { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } if scheme != "file" { anyhow::bail!("`path+{scheme}` is unsupported; `path+file` and `file` schemes are supported"); } @@ -137,10 +147,10 @@ impl PackageIdSpec { } kind => anyhow::bail!("unsupported source protocol: {kind}"), } - } - - if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {}", url) + } else { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } } let frag = url.fragment().map(|s| s.to_owned()); @@ -347,6 +357,11 @@ impl fmt::Display for PackageIdSpec { write!(f, "{protocol}+")?; } write!(f, "{}", url)?; + if let Some(SourceKind::Git(git_ref)) = self.kind.as_ref() { + if let Some(pretty) = git_ref.pretty_ref(true) { + write!(f, "?{}", pretty)?; + } + } if url.path_segments().unwrap().next_back().unwrap() != &*self.name { printed_name = true; write!(f, "#{}", self.name)?; @@ -625,6 +640,16 @@ mod tests { }, "git+ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", ); + ok( + "git+ssh://git@github.com/rust-lang/regex.git?branch=dev#regex@1.4.3", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4.3".parse().unwrap()), + url: Some(Url::parse("ssh://git@github.com/rust-lang/regex.git").unwrap()), + kind: Some(SourceKind::Git(GitReference::Branch("dev".to_owned()))), + }, + "git+ssh://git@github.com/rust-lang/regex.git?branch=dev#regex@1.4.3", + ); ok( "file:///path/to/my/project/foo", PackageIdSpec { @@ -670,6 +695,18 @@ mod tests { PackageIdSpec::parse("foobar+https://github.com/rust-lang/crates.io-index").is_err() ); assert!(PackageIdSpec::parse("path+https://github.com/rust-lang/crates.io-index").is_err()); + + // Only `git+` can use `?` + assert!(PackageIdSpec::parse("file:///path/to/my/project/foo?branch=dev").is_err()); + assert!(PackageIdSpec::parse("path+file:///path/to/my/project/foo?branch=dev").is_err()); + assert!(PackageIdSpec::parse( + "registry+https://github.com/rust-lang/cargo#0.52.0?branch=dev" + ) + .is_err()); + assert!(PackageIdSpec::parse( + "sparse+https://github.com/rust-lang/cargo#0.52.0?branch=dev" + ) + .is_err()); } #[test] diff --git a/src/doc/src/reference/pkgid-spec.md b/src/doc/src/reference/pkgid-spec.md index 7c4be10b552..b2dfe827b1c 100644 --- a/src/doc/src/reference/pkgid-spec.md +++ b/src/doc/src/reference/pkgid-spec.md @@ -22,7 +22,8 @@ The formal grammar for a Package Id Specification is: ```notrust spec := pkgname | - [ kind "+" ] proto "://" hostname-and-path [ "#" ( pkgname | semver ) ] + [ kind "+" ] proto "://" hostname-and-path [ "?" query] [ "#" ( pkgname | semver ) ] +query = ( "branch" | "tag" | "rev" ) "=" ref pkgname := name [ ("@" | ":" ) semver ] semver := digits [ "." digits [ "." digits [ "-" prerelease ] [ "+" build ]]] @@ -56,6 +57,7 @@ The following are some examples of specs for several different git dependencies: | `https://github.com/rust-lang/cargo#cargo-platform@0.1.2` | `cargo-platform` | `0.1.2` | | `ssh://git@github.com/rust-lang/regex.git#regex@1.4.3` | `regex` | `1.4.3` | | `git+ssh://git@github.com/rust-lang/regex.git#regex@1.4.3` | `regex` | `1.4.3` | +| `git+ssh://git@github.com/rust-lang/regex.git?branch=dev#regex@1.4.3` | `regex` | `1.4.3` | Local packages on the filesystem can use `file://` URLs to reference them: diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 54c0d4751be..fee45b21552 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -277,8 +277,8 @@ foo v0.1.0 ([..]/foo) "\ error: There are multiple `xyz` packages in your project, and the specification `xyz` is ambiguous. Please re-run this command with one of the following specifications: - git+file://[..]/xyz#0.5.0 - git+file://[..]/xyz#0.5.0 + git+file://[..]/xyz?rev=[..]#0.5.0 + git+file://[..]/xyz?rev=[..]#0.5.0 ", ) .run();