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..81f2b1dec01 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -6,7 +6,9 @@ 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; use crate::util::errors::CargoResult; use crate::util::{validate_package_name, IntoUrl}; @@ -26,6 +28,7 @@ pub struct PackageIdSpec { name: String, version: Option, url: Option, + kind: Option, } impl PackageIdSpec { @@ -78,6 +81,7 @@ impl PackageIdSpec { name: String::from(name), version, url: None, + kind: None, }) } @@ -101,16 +105,57 @@ 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: Some(package_id.source_id().kind().clone()), } } /// Tries to convert a valid `Url` to a `PackageIdSpec`. fn from_url(mut url: Url) -> CargoResult { - if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {}", url) + let mut kind = None; + if let Some((kind_str, scheme)) = url.scheme().split_once('+') { + match kind_str { + "git" => { + 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"); + } + kind = Some(SourceKind::Path); + url = strip_url_protocol(&url); + } + kind => anyhow::bail!("unsupported source protocol: {kind}"), + } + } 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()); url.set_fragment(None); + let (name, version) = { let mut path = url .path_segments() @@ -144,6 +189,7 @@ impl PackageIdSpec { name, version, url: Some(url), + kind, }) } @@ -168,6 +214,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() { @@ -186,6 +240,12 @@ impl PackageIdSpec { } } + if let Some(k) = &self.kind { + if k != package_id.source_id().kind() { + return false; + } + } + true } @@ -216,6 +276,7 @@ impl PackageIdSpec { name: self.name.clone(), version: self.version.clone(), url: None, + kind: None, }, &mut suggestion, ); @@ -226,6 +287,7 @@ impl PackageIdSpec { name: self.name.clone(), version: None, url: None, + kind: None, }, &mut suggestion, ); @@ -280,12 +342,26 @@ 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 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)?; @@ -325,7 +401,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] @@ -346,6 +422,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 +432,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 +442,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 +452,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 +462,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,15 +472,37 @@ 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", ); + 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 { name: String::from("foo"), version: None, url: None, + kind: None, }, "foo", ); @@ -409,6 +512,7 @@ mod tests { name: String::from("foo"), version: Some("1.2.3".parse().unwrap()), url: None, + kind: None, }, "foo@1.2.3", ); @@ -418,6 +522,7 @@ mod tests { name: String::from("foo"), version: Some("1.2.3".parse().unwrap()), url: None, + kind: None, }, "foo@1.2.3", ); @@ -427,6 +532,7 @@ mod tests { name: String::from("foo"), version: Some("1.2".parse().unwrap()), url: None, + kind: None, }, "foo@1.2", ); @@ -438,6 +544,7 @@ mod tests { name: String::from("regex"), version: None, url: None, + kind: None, }, "regex", ); @@ -447,6 +554,7 @@ mod tests { name: String::from("regex"), version: Some("1.4".parse().unwrap()), url: None, + kind: None, }, "regex@1.4", ); @@ -456,6 +564,7 @@ mod tests { name: String::from("regex"), version: Some("1.4.3".parse().unwrap()), url: None, + kind: None, }, "regex@1.4.3", ); @@ -465,6 +574,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,15 +584,29 @@ 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", ); + 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 { 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 +616,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,15 +626,37 @@ 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", ); + 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( + "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 { 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,9 +666,20 @@ 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", ); + 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] @@ -533,6 +691,22 @@ 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()); + + // 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] @@ -554,6 +728,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 e53b1704db0..cde204377b0 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. @@ -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..b2dfe827b1c 100644 --- a/src/doc/src/reference/pkgid-spec.md +++ b/src/doc/src/reference/pkgid-spec.md @@ -21,11 +21,13 @@ 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 [ "?" query] [ "#" ( pkgname | semver ) ] +query = ( "branch" | "tag" | "rev" ) "=" ref pkgname := name [ ("@" | ":" ) semver ] semver := digits [ "." digits [ "." digits [ "-" prerelease ] [ "+" build ]]] +kind = "registry" | "git" | "file" proto := "http" | "git" | ... ``` @@ -38,28 +40,32 @@ 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` | +| `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: -| 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 88d991e80d6..fee45b21552 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; @@ -34,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. @@ -89,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. @@ -143,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. @@ -163,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(); @@ -195,3 +200,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: + git+file://[..]/xyz?rev=[..]#0.5.0 + git+file://[..]/xyz?rev=[..]#0.5.0 +", + ) + .run(); + // TODO, what should the `-p` value be here? + //p.cargo("update -p") +}