Add support for relative git submodule paths

src/cargo/sources/git/source.rs

@@ -173,7 +173,8 @@ impl<'cfg> Source for GitSource<'cfg> {
             .join("checkouts")
             .join(&self.ident)
             .join(short_id.as_str());
-        db.copy_to(actual_rev, &checkout_path, self.config)?;
+        let parent_remote_url = self.url();
+        db.copy_to(actual_rev, &checkout_path, self.config, parent_remote_url)?;
 
         let source_id = self.source_id.with_precise(Some(actual_rev.to_string()));
         let path_source = PathSource::new_recursive(&checkout_path, source_id, self.config);

src/cargo/sources/git/utils.rs

@@ -1,22 +1,26 @@
 //! Utilities for handling git repositories, mainly around
 //! authentication/cloning.
 
-use crate::core::GitReference;
-use crate::util::errors::CargoResult;
-use crate::util::{network, Config, IntoUrl, MetricsCounter, Progress};
+use std::env;
+use std::fmt;
+use std::fs::canonicalize;
+use std::path::{Path, PathBuf};
+use std::process::Command;
+use std::time::{Duration, Instant};
+
 use anyhow::{anyhow, Context as _};
+use cargo_util::paths::normalize_path;
 use cargo_util::{paths, ProcessBuilder};
 use curl::easy::List;
 use git2::{self, ErrorClass, ObjectType};
 use log::{debug, info};
 use serde::ser;
 use serde::Serialize;
-use std::env;
-use std::fmt;
-use std::path::{Path, PathBuf};
-use std::process::Command;
-use std::time::{Duration, Instant};
-use url::Url;
+use url::{ParseError, Url};
+
+use crate::core::GitReference;
+use crate::util::errors::CargoResult;
+use crate::util::{network, Config, IntoUrl, MetricsCounter, Progress};
 
 fn serialize_str<T, S>(t: &T, s: S) -> Result<S::Ok, S::Error>
 where
@@ -150,6 +154,7 @@ impl GitDatabase {
         rev: git2::Oid,
         dest: &Path,
         cargo_config: &Config,
+        parent_remote_url: &Url,
     ) -> CargoResult<GitCheckout<'_>> {
         let mut checkout = None;
         if let Ok(repo) = git2::Repository::open(dest) {
@@ -177,7 +182,7 @@ impl GitDatabase {
             Some(c) => c,
             None => GitCheckout::clone_into(dest, self, rev, cargo_config)?,
         };
-        checkout.update_submodules(cargo_config)?;
+        checkout.update_submodules(cargo_config, parent_remote_url)?;
         Ok(checkout)
     }
 
@@ -343,19 +348,25 @@ impl<'a> GitCheckout<'a> {
         Ok(())
     }
 
-    fn update_submodules(&self, cargo_config: &Config) -> CargoResult<()> {
-        return update_submodules(&self.repo, cargo_config);
+    fn update_submodules(&self, cargo_config: &Config, parent_remote_url: &Url) -> CargoResult<()> {
+        return update_submodules(&self.repo, cargo_config, parent_remote_url);
 
-        fn update_submodules(repo: &git2::Repository, cargo_config: &Config) -> CargoResult<()> {
+        fn update_submodules(
+            repo: &git2::Repository,
+            cargo_config: &Config,
+            parent_remote_url: &Url,
+        ) -> CargoResult<()> {
             info!("update submodules for: {:?}", repo.workdir().unwrap());
 
             for mut child in repo.submodules()? {
-                update_submodule(repo, &mut child, cargo_config).with_context(|| {
-                    format!(
-                        "failed to update submodule `{}`",
-                        child.name().unwrap_or("")
-                    )
-                })?;
+                update_submodule(repo, &mut child, cargo_config, parent_remote_url).with_context(
+                    || {
+                        format!(
+                            "failed to update submodule `{}`",
+                            child.name().unwrap_or("")
+                        )
+                    },
+                )?;
             }
             Ok(())
         }
@@ -364,12 +375,60 @@ impl<'a> GitCheckout<'a> {
             parent: &git2::Repository,
             child: &mut git2::Submodule<'_>,
             cargo_config: &Config,
+            parent_remote_url: &Url,
         ) -> CargoResult<()> {
             child.init(false)?;
-            let url = child.url().ok_or_else(|| {
+
+            let child_url_str = child.url().ok_or_else(|| {
                 anyhow::format_err!("non-utf8 url for submodule {:?}?", child.path())
             })?;
 
+            //There are a few possible cases here:
+            // 1. Submodule URL is not relative.
+            //    Happy path, Url is just the submodule url.
+            // 2. Submodule URL is relative and does specify ssh/https/file/etc.
+            //     We combine the parent path and relative path.
+            // 3. Submodule URL is relative and does not specify ssh/https/file/etc.
+            //     In which case we fail to parse with the error ParseError::RelativeUrlWithoutBase.
+            //     We then combine the relative url with the host/protocol from the parent url.
+            // 4. We fail to parse submodule url for some reason.
+            //     Error. Gets passed up.
+            let url = match Url::parse(child_url_str) {
+                Ok(child_url) => {
+                    if Path::new(child_url.path()).is_relative() {
+                        let parent_remote_path = Path::new(parent_remote_url.path());
+                        let child_remote_path = Path::new(child_url.path());
+                        let canonical = canonicalize(normalize_path(
+                            &parent_remote_path.join(child_remote_path),
+                        ))?;
+                        let mut final_path = parent_remote_url.clone();
+                        final_path.set_path(canonical.to_string_lossy().as_ref());
+                        final_path.to_string()
+                    } else {
+                        child_url.to_string()
+                    }
+                }
+                Err(ParseError::RelativeUrlWithoutBase) => {
+                    let path = parent_remote_url.path();
+                    let mut new_parent_remote_url = parent_remote_url.clone();
+                    new_parent_remote_url.set_path(format!("{}/", path).as_str());
+                    new_parent_remote_url = match new_parent_remote_url.join(child_url_str) {
+                        Ok(x) => x,
+                        Err(err) => Err(err)
+                            .with_context(|| format!("Failed to parse child submodule url"))?,
+                    };
+                    let final_path = canonicalize(Path::new(new_parent_remote_url.path()))?;
+                    new_parent_remote_url.set_path(final_path.to_string_lossy().as_ref());
+                    new_parent_remote_url.to_string()
+                }
+                Err(err) => {
+                    return Err(anyhow::format_err!(
+                        "Error parsing submodule url: {:?}?",
+                        err
+                    ));
+                }
+            };
+
             // A submodule which is listed in .gitmodules but not actually
             // checked out will not have a head id, so we should ignore it.
             let head = match child.head_id() {
@@ -388,7 +447,7 @@ impl<'a> GitCheckout<'a> {
             let mut repo = match head_and_repo {
                 Ok((head, repo)) => {
                     if child.head_id() == head {
-                        return update_submodules(&repo, cargo_config);
+                        return update_submodules(&repo, cargo_config, parent_remote_url);
                     }
                     repo
                 }
@@ -402,18 +461,18 @@ impl<'a> GitCheckout<'a> {
             let reference = GitReference::Rev(head.to_string());
             cargo_config
                 .shell()
-                .status("Updating", format!("git submodule `{}`", url))?;
-            fetch(&mut repo, url, &reference, cargo_config).with_context(|| {
+                .status("Updating", format!("git submodule `{}`", url.as_str()))?;
+            fetch(&mut repo, &url, &reference, cargo_config).with_context(|| {
                 format!(
                     "failed to fetch submodule `{}` from {}",
                     child.name().unwrap_or(""),
-                    url
+                    url.as_str()
                 )
             })?;
 
             let obj = repo.find_object(head, None)?;
             reset(&repo, &obj, cargo_config)?;
-            update_submodules(&repo, cargo_config)
+            update_submodules(&repo, cargo_config, parent_remote_url)
         }
     }
 }
@@ -646,9 +705,9 @@ where
         msg.push_str("https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli");
         err = err.context(msg);
 
-    // Otherwise if we didn't even get to the authentication phase them we may
-    // have failed to set up a connection, in these cases hint on the
-    // `net.git-fetch-with-cli` configuration option.
+        // Otherwise if we didn't even get to the authentication phase them we may
+        // have failed to set up a connection, in these cases hint on the
+        // `net.git-fetch-with-cli` configuration option.
     } else if let Some(e) = err.downcast_ref::<git2::Error>() {
         match e.class() {
             ErrorClass::Net

tests/testsuite/config.rs

@@ -655,6 +655,7 @@ Caused by:
   invalid value: integer `123456789`, expected i8",
     );
 
+    #[allow(dead_code)]
     #[derive(Debug, Deserialize)]
     struct S {
         f1: i64,
@@ -1154,6 +1155,7 @@ fn table_merge_failure() {
         ",
     );
 
+    #[allow(dead_code)]
     #[derive(Debug, Deserialize)]
     struct Table {
         key: StringList,

tests/testsuite/git.rs

@@ -943,6 +943,73 @@ fn dep_with_submodule() {
         .run();
 }
 
+#[cargo_test]
+fn dep_with_relative_submodule() {
+    let foo = project();
+    let base = git::new("base", |project| {
+        project
+            .file(
+                "Cargo.toml",
+                r#"
+            [package]
+            name = "base"
+            version = "0.5.0"
+
+            [dependencies]
+            deployment.path = "deployment"
+        "#,
+            )
+            .file(
+                "src/lib.rs",
+                r#"
+            pub fn dep() {
+                deployment::deployment_func();
+            }
+            "#,
+            )
+    });
+    let _deployment = git::new("deployment", |project| {
+        project
+            .file("src/lib.rs", "pub fn deployment_func() {}")
+            .file("Cargo.toml", &basic_lib_manifest("deployment"))
+    });
+
+    let base_repo = git2::Repository::open(&base.root()).unwrap();
+    git::add_submodule(&base_repo, "../deployment", Path::new("deployment"));
+    git::commit(&base_repo);
+
+    let project = foo
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                    [project]
+                    name = "foo"
+                    version = "0.5.0"
+
+                    [dependencies.base]
+                    git = '{}'
+                "#,
+                base.url()
+            ),
+        )
+        .file("src/lib.rs", "pub fn foo() {  }")
+        .build();
+
+    project
+        .cargo("build")
+        .with_stderr(
+            "\
+[UPDATING] git repository [..]
+[UPDATING] git submodule `file://[..]/deployment`
+[COMPILING] deployment [..]
+[COMPILING] base [..]
+[COMPILING] foo [..]
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]\n",
+        )
+        .run();
+}
+
 #[cargo_test]
 fn dep_with_bad_submodule() {
     let project = project();