From a7247babf60c7cb7969b764c6046d6dd391f7871 Mon Sep 17 00:00:00 2001
From: Weihang Lo <me@weihanglo.tw>
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]: <https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-none>
     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();