From f9e1aeb1fb92815576abd263638a96bfaaf6b2ee Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 11 Jul 2023 13:58:35 +0100 Subject: [PATCH] fix(git): respect scp-like URL for nested submodules `parent_remote_url` used to be `&str` before #12244. However, we changed the type to `Url` and it started failing to parse scp-like URLs since they are not compliant with WHATWG URL spec. In this commit, we change it back to `&str` and construct the URL manually. This should be safe since Cargo already checks if it is a relative URL for that if branch. --- src/cargo/sources/git/utils.rs | 46 +++++++++++----------------------- tests/testsuite/git.rs | 6 ++--- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index db6cbbabee3..b763ba61757 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -392,13 +392,13 @@ impl<'a> GitCheckout<'a> { /// /// [^1]: fn update_submodules(&self, cargo_config: &Config) -> CargoResult<()> { - return update_submodules(&self.repo, cargo_config, self.remote_url()); + return update_submodules(&self.repo, cargo_config, self.remote_url().as_str()); /// Recusive helper for [`GitCheckout::update_submodules`]. fn update_submodules( repo: &git2::Repository, cargo_config: &Config, - parent_remote_url: &Url, + parent_remote_url: &str, ) -> CargoResult<()> { debug!("update submodules for: {:?}", repo.workdir().unwrap()); @@ -420,7 +420,7 @@ impl<'a> GitCheckout<'a> { parent: &git2::Repository, child: &mut git2::Submodule<'_>, cargo_config: &Config, - parent_remote_url: &Url, + parent_remote_url: &str, ) -> CargoResult<()> { child.init(false)?; @@ -444,30 +444,15 @@ impl<'a> GitCheckout<'a> { // See [`git submodule add`] documentation. // // [`git submodule add`]: https://git-scm.com/docs/git-submodule - let child_remote_url = if child_url_str.starts_with("./") - || child_url_str.starts_with("../") - { - let mut new_parent_remote_url = parent_remote_url.clone(); - - let mut new_path = Cow::from(parent_remote_url.path()); - if !new_path.ends_with('/') { - new_path.to_mut().push('/'); + let child_remote_url = if ["./", "../"].iter().any(|p| child_url_str.starts_with(p)) { + let mut new_remote_url = parent_remote_url.to_string(); + if !new_remote_url.ends_with('/') { + new_remote_url.push('/'); } - new_parent_remote_url.set_path(&new_path); - - new_parent_remote_url.join(child_url_str).with_context(|| { - format!( - "failed to parse relative child submodule url `{child_url_str}` \ - using parent base url `{new_parent_remote_url}`" - ) - })? + new_remote_url.push_str(child_url_str); + Cow::from(new_remote_url) } else { - Url::parse(child_url_str).with_context(|| { - let child_module_name = child.name().unwrap_or(""); - format!( - "failed to parse url for submodule `{child_module_name}`: `{child_url_str}`" - ) - })? + Cow::from(child_url_str) }; // A submodule which is listed in .gitmodules but not actually @@ -502,20 +487,17 @@ impl<'a> GitCheckout<'a> { let reference = GitReference::Rev(head.to_string()); cargo_config .shell() - .status("Updating", format!("git submodule `{}`", child_remote_url))?; + .status("Updating", format!("git submodule `{child_remote_url}`"))?; fetch( &mut repo, - child_remote_url.as_str(), + &child_remote_url, &reference, cargo_config, RemoteKind::GitDependency, ) .with_context(|| { - format!( - "failed to fetch submodule `{}` from {}", - child.name().unwrap_or(""), - child_remote_url - ) + let name = child.name().unwrap_or(""); + format!("failed to fetch submodule `{name}` from {child_remote_url}",) })?; let obj = repo.find_object(head, None)?; diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index f60ee978a36..09250e7ce11 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -3633,7 +3633,7 @@ fn different_user_relative_submodules() { .file("Cargo.toml", &basic_lib_manifest("dep1")) .file("src/lib.rs", "") }); - let user2_git_project2 = git::new("user2/dep2", |project| { + let _user2_git_project2 = git::new("user2/dep2", |project| { project .file("Cargo.toml", &basic_lib_manifest("dep1")) .file("src/lib.rs", "") @@ -3673,14 +3673,14 @@ fn different_user_relative_submodules() { "\ [UPDATING] git repository `{}` [UPDATING] git submodule `{}` -[UPDATING] git submodule `{}` +[UPDATING] git submodule `{}/../dep2` [COMPILING] dep1 v0.5.0 ({}#[..]) [COMPILING] foo v0.5.0 ([CWD]) [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", path2url(&user1_git_project.root()), path2url(&user2_git_project.root()), - path2url(&user2_git_project2.root()), + path2url(&user2_git_project.root()), path2url(&user1_git_project.root()), )) .run();