diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index d6dbe03e4c0..d63e6dca283 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -100,9 +100,29 @@ pub struct GitSource<'gctx> { impl<'gctx> GitSource<'gctx> { /// Creates a git source for the given [`SourceId`]. pub fn new(source_id: SourceId, gctx: &'gctx GlobalContext) -> CargoResult> { + let remote = GitRemote::new(source_id.url()); + Self::new_with_remote(source_id, remote, gctx) + } + + /// Creates a git source for a submodule with an URL that may not be a valid WHATWG URL. + /// + /// This is needed because [`SourceId`] hasn't yet supported SCP-like URLs. + pub(super) fn new_for_submodule( + source_id: SourceId, + fetch_url: String, + gctx: &'gctx GlobalContext, + ) -> CargoResult> { + let remote = GitRemote::new_from_str(fetch_url); + Self::new_with_remote(source_id, remote, gctx) + } + + fn new_with_remote( + source_id: SourceId, + remote: GitRemote, + gctx: &'gctx GlobalContext, + ) -> CargoResult> { assert!(source_id.is_git(), "id is not git, id={}", source_id); - let remote = GitRemote::new(source_id.url()); // Fallback to git ref from manifest if there is no locked revision. let locked_rev = source_id .precise_git_fragment() @@ -132,7 +152,7 @@ impl<'gctx> GitSource<'gctx> { /// Gets the remote repository URL. pub fn url(&self) -> &Url { - self.remote.url() + self.source_id.url() } /// Returns the packages discovered by this source. It may fetch the Git @@ -291,7 +311,7 @@ fn ident_shallow(id: &SourceId, is_shallow: bool) -> String { impl<'gctx> Debug for GitSource<'gctx> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "git repo at {}", self.remote.url())?; + write!(f, "git repo at {}", self.source_id.url())?; match &self.locked_rev { Revision::Deferred(git_ref) => match git_ref.pretty_ref(true) { Some(s) => write!(f, " ({})", s), diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index ac856134cd9..12d52e3aa95 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -45,7 +45,11 @@ impl GitShortID { #[derive(PartialEq, Clone, Debug)] pub struct GitRemote { /// URL to a remote repository. - url: Url, + /// + /// This may differ from the [`SourceId`] URL when the original URL + /// can't be represented as a WHATWG [`Url`], for example SCP-like URLs. + /// See . + url: String, } /// A local clone of a remote repository's database. Multiple [`GitCheckout`]s @@ -74,11 +78,20 @@ pub struct GitCheckout<'a> { impl GitRemote { /// Creates an instance for a remote repository URL. pub fn new(url: &Url) -> GitRemote { - GitRemote { url: url.clone() } + GitRemote { + url: url.as_str().to_owned(), + } + } + + /// Creates an instance with an URL that may not be a valid WHATWG URL. + /// + /// This is needed because [`SourceId`] hasn't yet supported SCP-like URLs. + pub(super) fn new_from_str(url: String) -> GitRemote { + GitRemote { url } } /// Gets the remote repository URL. - pub fn url(&self) -> &Url { + pub fn url(&self) -> &str { &self.url } @@ -102,7 +115,7 @@ impl GitRemote { if let Some(mut db) = db { fetch( &mut db.repo, - self.url.as_str(), + self.url(), reference, gctx, RemoteKind::GitDependency, @@ -124,7 +137,7 @@ impl GitRemote { let mut repo = init(into, true)?; fetch( &mut repo, - self.url.as_str(), + self.url(), reference, gctx, RemoteKind::GitDependency, @@ -266,8 +279,8 @@ impl<'a> GitCheckout<'a> { } /// Gets the remote repository URL. - fn remote_url(&self) -> &Url { - &self.database.remote.url() + fn remote_url(&self) -> &str { + self.database.remote.url() } /// Clone a repo for a `revision` into a local path from a `database`. @@ -378,7 +391,7 @@ impl<'a> GitCheckout<'a> { /// /// [^1]: fn update_submodules(&self, gctx: &GlobalContext, quiet: bool) -> CargoResult<()> { - return update_submodules(&self.repo, gctx, quiet, self.remote_url().as_str()); + return update_submodules(&self.repo, gctx, quiet, self.remote_url()); /// Recursive helper for [`GitCheckout::update_submodules`]. fn update_submodules( @@ -460,17 +473,37 @@ impl<'a> GitCheckout<'a> { // Fetch submodule database and checkout to target revision let reference = GitReference::Rev(head.to_string()); + // SCP-like URL is not a WHATWG Standard URL. + // `url` crate can't parse SCP-like URLs. + // We convert to `ssh://` for SourceId, + // but preserve the original URL for fetch to maintain correct semantics + // See + let (source_url, fetch_url) = match child_remote_url.as_ref().into_url() { + Ok(url) => (url, None), + Err(_) => { + let ssh_url = scp_to_ssh(&child_remote_url) + .ok_or_else(|| anyhow::format_err!("invalid url `{child_remote_url}`"))? + .as_str() + .into_url()?; + (ssh_url, Some(child_remote_url.into_owned())) + } + }; + // GitSource created from SourceId without git precise will result to // locked_rev being Deferred and fetch_db always try to fetch if online - let source_id = SourceId::for_git(&child_remote_url.into_url()?, reference)? - .with_git_precise(Some(head.to_string())); + let source_id = + SourceId::for_git(&source_url, reference)?.with_git_precise(Some(head.to_string())); - let mut source = GitSource::new(source_id, gctx)?; + let mut source = match &fetch_url { + Some(url) => GitSource::new_for_submodule(source_id, url.to_owned(), gctx)?, + None => GitSource::new(source_id, gctx)?, + }; source.set_quiet(quiet); let (db, actual_rev) = source.fetch_db(true).with_context(|| { let name = child.name().unwrap_or(""); - format!("failed to fetch submodule `{name}` from {child_remote_url}",) + let url = fetch_url.unwrap_or_else(|| source_url.to_string()); + format!("failed to fetch submodule `{name}` from {url}") })?; db.copy_to(actual_rev, repo.path(), gctx, quiet)?; Ok(()) @@ -547,18 +580,20 @@ fn absolute_submodule_url<'s>(base_url: &str, submodule_url: &'s str) -> CargoRe Cow::from(submodule_url) }; - let absolute_url = match gix::url::parse(gix::bstr::BStr::new(absolute_url.as_ref().as_bytes())) - { - Ok(mut url) if url.serialize_alternative_form && url.scheme == gix::url::Scheme::Ssh => { - url.serialize_alternative_form = false; - Cow::from(url.to_bstring().to_string()) - } - _ => absolute_url, - }; - Ok(absolute_url) } +/// Converts an SCP-like URL to `ssh://` format. +fn scp_to_ssh(url: &str) -> Option { + let mut gix_url = gix::url::parse(gix::bstr::BStr::new(url.as_bytes())).ok()?; + if gix_url.serialize_alternative_form && gix_url.scheme == gix::url::Scheme::Ssh { + gix_url.serialize_alternative_form = false; + Some(gix_url.to_bstring().to_string()) + } else { + None + } +} + /// Prepare the authentication callbacks for cloning a git repository. /// /// The main purpose of this function is to construct the "authentication @@ -1633,7 +1668,7 @@ mod tests { ( "ssh://git@gitub.com/rust-lang/cargo", "git@github.com:rust-lang/cargo.git", - "ssh://git@github.com/rust-lang/cargo.git", + "git@github.com:rust-lang/cargo.git", ), ( "ssh://git@gitub.com/rust-lang/cargo", @@ -1678,37 +1713,37 @@ mod tests { ( "git@github.com:rust-lang/cargo.git", "./", - "ssh://git@github.com/rust-lang/cargo.git/./", + "git@github.com:rust-lang/cargo.git/./", ), ( "git@github.com:rust-lang/cargo.git", "../", - "ssh://git@github.com/rust-lang/cargo.git/../", + "git@github.com:rust-lang/cargo.git/../", ), ( "git@github.com:rust-lang/cargo.git", "./foo", - "ssh://git@github.com/rust-lang/cargo.git/./foo", + "git@github.com:rust-lang/cargo.git/./foo", ), ( "git@github.com:rust-lang/cargo.git/", "./foo", - "ssh://git@github.com/rust-lang/cargo.git/./foo", + "git@github.com:rust-lang/cargo.git/./foo", ), ( "git@github.com:rust-lang/cargo.git", "../foo", - "ssh://git@github.com/rust-lang/cargo.git/../foo", + "git@github.com:rust-lang/cargo.git/../foo", ), ( "git@github.com:rust-lang/cargo.git/", "../foo", - "ssh://git@github.com/rust-lang/cargo.git/../foo", + "git@github.com:rust-lang/cargo.git/../foo", ), ( "git@github.com:rust-lang/cargo.git", "../foo/bar/../baz", - "ssh://git@github.com/rust-lang/cargo.git/../foo/bar/../baz", + "git@github.com:rust-lang/cargo.git/../foo/bar/../baz", ), ]; diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 9dc1a4a141a..4f2e3feeb7a 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -4421,9 +4421,10 @@ fn dep_with_scp_like_submodule_url() { .file("src/lib.rs", "extern crate dep1;") .build(); - // With the SCP-like URL fix, Cargo converts `git@github.com:foo/bar.git` - // to `ssh://git@github.com/foo/bar.git` and tries to fetch, which fails - // with other errors like authentication failure or SSH server not reachable. + // With the SCP-like URL fix, Cargo preserves the original SCP-like URL + // for the actual fetch, while using the ssh:// form internally for caching. + // The fetch fails because the SSH server is not reachable, but the URL + // shown in messages is the original SCP-like form. p.cargo("fetch") .env( "GIT_SSH_COMMAND", @@ -4432,7 +4433,7 @@ fn dep_with_scp_like_submodule_url() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] git repository `[ROOTURL]/dep1` -[UPDATING] git submodule `ssh://git@github.com/foo/bar.git` +[UPDATING] git submodule `git@github.com:foo/bar.git` [ERROR] failed to get `dep1` as a dependency of package `foo v0.5.0 ([ROOT]/foo)` Caused by: @@ -4445,7 +4446,7 @@ Caused by: failed to update submodule `submod` Caused by: - failed to fetch submodule `submod` from ssh://git@github.com/foo/bar.git + failed to fetch submodule `submod` from git@github.com:foo/bar.git ... "#]]) .run();