From 71b37f2fd15493a32674416791a6cb567cde92e9 Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 8 Jun 2023 15:27:23 +0000 Subject: [PATCH 1/7] add test case --- tests/testsuite/git.rs | 66 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 7c717e9671c..08e1abb1a36 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -3619,3 +3619,69 @@ fn cleans_temp_pack_files() { p.cargo("generate-lockfile").run(); assert!(!tmp_path.exists()); } + +#[cargo_test] +fn different_user_relative_submodules() { + let user1_git_project = git::new("user1/dep1", |project| { + project + .file("Cargo.toml", &basic_lib_manifest("dep1")) + .file("src/lib.rs", "") + }); + + let user2_git_project = git::new("user2/dep1", |project| { + project + .file("Cargo.toml", &basic_lib_manifest("dep1")) + .file("src/lib.rs", "") + }); + let user2_git_project2 = git::new("user2/dep2", |project| { + project + .file("Cargo.toml", &basic_lib_manifest("dep1")) + .file("src/lib.rs", "") + }); + + let user2_repo = git2::Repository::open(&user2_git_project.root()).unwrap(); + let url = "../dep2"; + git::add_submodule(&user2_repo, url, Path::new("dep2")); + git::commit(&user2_repo); + + let user1_repo = git2::Repository::open(&user1_git_project.root()).unwrap(); + let url = user2_git_project.url(); + git::add_submodule(&user1_repo, url.as_str(), Path::new("user2/dep1")); + git::commit(&user1_repo); + + let project = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.5.0" + + [dependencies.dep1] + git = '{}' + "#, + user1_git_project.url() + ), + ) + .file("src/main.rs", &main_file(r#""hello""#, &[])) + .build(); + + project + .cargo("build") + .with_stderr(&format!( + "[UPDATING] git repository `{}`\n\ + [UPDATING] git submodule `{}`\n\ + [UPDATING] git submodule `{}`\n\ + [COMPILING] dep1 v0.5.0 ({}#[..])\n\ + [COMPILING] foo v0.5.0 ([CWD])\n\ + [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]\n", + path2url(&user1_git_project.root()), + path2url(&user2_git_project.root()), + path2url(&user2_git_project2.root()), + path2url(&user1_git_project.root()), + )) + .run(); + + assert!(project.bin("foo").is_file()); +} From 6343e68b7f0cf1236057af66a6c7bcaad8cd426b Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 8 Jun 2023 15:27:56 +0000 Subject: [PATCH 2/7] rename `url` to `child_remote_url` --- src/cargo/sources/git/utils.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 4789c1c21af..979449b5525 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -444,7 +444,7 @@ impl<'a> GitCheckout<'a> { // See [`git submodule add`] documentation. // // [`git submodule add`]: https://git-scm.com/docs/git-submodule - let url = if child_url_str.starts_with("./") || child_url_str.starts_with("../") { + 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()); @@ -498,10 +498,10 @@ impl<'a> GitCheckout<'a> { let reference = GitReference::Rev(head.to_string()); cargo_config .shell() - .status("Updating", format!("git submodule `{}`", url))?; + .status("Updating", format!("git submodule `{}`", child_remote_url))?; fetch( &mut repo, - &url, + &child_remote_url, &reference, cargo_config, RemoteKind::GitDependency, @@ -510,7 +510,7 @@ impl<'a> GitCheckout<'a> { format!( "failed to fetch submodule `{}` from {}", child.name().unwrap_or(""), - url + child_remote_url ) })?; From 2717a34b60af17282ecfa66e8f47fa83a63ebf0d Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 8 Jun 2023 15:31:48 +0000 Subject: [PATCH 3/7] make `child_remote_url` a `Url` and pass it to `update_submodules` --- src/cargo/sources/git/utils.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 979449b5525..23e58f7760a 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -454,7 +454,7 @@ impl<'a> GitCheckout<'a> { new_parent_remote_url.set_path(&new_path); match new_parent_remote_url.join(child_url_str) { - Ok(x) => x.to_string(), + Ok(x) => x, Err(err) => Err(err).with_context(|| { format!( "failed to parse relative child submodule url `{}` using parent base url `{}`", @@ -463,7 +463,7 @@ impl<'a> GitCheckout<'a> { })?, } } else { - child_url_str.to_string() + Url::parse(child_url_str)? }; // A submodule which is listed in .gitmodules but not actually @@ -484,7 +484,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, parent_remote_url); + return update_submodules(&repo, cargo_config, &child_remote_url); } repo } @@ -501,7 +501,7 @@ impl<'a> GitCheckout<'a> { .status("Updating", format!("git submodule `{}`", child_remote_url))?; fetch( &mut repo, - &child_remote_url, + child_remote_url.as_str(), &reference, cargo_config, RemoteKind::GitDependency, @@ -516,7 +516,7 @@ impl<'a> GitCheckout<'a> { let obj = repo.find_object(head, None)?; reset(&repo, &obj, cargo_config)?; - update_submodules(&repo, cargo_config, parent_remote_url) + update_submodules(&repo, cargo_config, &child_remote_url) } } } From cf438765fa99a610e7137efd0e71d271d7ead367 Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 8 Jun 2023 15:37:29 +0000 Subject: [PATCH 4/7] remove `match` expr --- src/cargo/sources/git/utils.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 23e58f7760a..c20a9f6ac16 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -444,7 +444,9 @@ 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 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()); @@ -453,15 +455,12 @@ impl<'a> GitCheckout<'a> { } new_parent_remote_url.set_path(&new_path); - match new_parent_remote_url.join(child_url_str) { - Ok(x) => x, - Err(err) => Err(err).with_context(|| { - format!( - "failed to parse relative child submodule url `{}` using parent base url `{}`", - child_url_str, new_parent_remote_url - ) - })?, - } + new_parent_remote_url.join(child_url_str).with_context(|| { + format!( + "failed to parse relative child submodule url `{}` using parent base url `{}`", + child_url_str, new_parent_remote_url + ) + })? } else { Url::parse(child_url_str)? }; From 4f5ac2a20f5a38dcc145e323d1d38d018169e1d1 Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 8 Jun 2023 17:03:28 +0000 Subject: [PATCH 5/7] use implicit arg capture and wrap line --- src/cargo/sources/git/utils.rs | 35 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index c20a9f6ac16..b4b8e627398 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -444,26 +444,25 @@ 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 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('/'); - } - new_parent_remote_url.set_path(&new_path); + let mut new_path = Cow::from(parent_remote_url.path()); + if !new_path.ends_with('/') { + new_path.to_mut().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 `{}` using parent base url `{}`", - child_url_str, new_parent_remote_url - ) - })? - } else { - Url::parse(child_url_str)? - }; + 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}`" + ) + })? + } else { + Url::parse(child_url_str)? + }; // A submodule which is listed in .gitmodules but not actually // checked out will not have a head id, so we should ignore it. From f07753dedaa95757a9cfb5ddb797064b250d933c Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 8 Jun 2023 17:07:13 +0000 Subject: [PATCH 6/7] add `Url::parse` context --- src/cargo/sources/git/utils.rs | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index b4b8e627398..db6cbbabee3 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -444,25 +444,31 @@ 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 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('/'); - } - new_parent_remote_url.set_path(&new_path); + let mut new_path = Cow::from(parent_remote_url.path()); + if !new_path.ends_with('/') { + new_path.to_mut().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}` \ + 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}`" + ) + })? + } 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}`" ) })? - } else { - Url::parse(child_url_str)? - }; + }; // A submodule which is listed in .gitmodules but not actually // checked out will not have a head id, so we should ignore it. From 56318e0b6c13d3aff1f4ef7489b26b282a722335 Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 8 Jun 2023 17:09:21 +0000 Subject: [PATCH 7/7] remove escapes and leading whitespace from string matches --- tests/testsuite/git.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 08e1abb1a36..f60ee978a36 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -3670,12 +3670,14 @@ fn different_user_relative_submodules() { project .cargo("build") .with_stderr(&format!( - "[UPDATING] git repository `{}`\n\ - [UPDATING] git submodule `{}`\n\ - [UPDATING] git submodule `{}`\n\ - [COMPILING] dep1 v0.5.0 ({}#[..])\n\ - [COMPILING] foo v0.5.0 ([CWD])\n\ - [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]\n", + "\ +[UPDATING] git repository `{}` +[UPDATING] git submodule `{}` +[UPDATING] git submodule `{}` +[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()),