Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relative git submodules did not resolve correctly #12151

Closed
LazyGeniusMan opened this issue May 18, 2023 · 1 comment · Fixed by #12244
Closed

Relative git submodules did not resolve correctly #12151

LazyGeniusMan opened this issue May 18, 2023 · 1 comment · Fixed by #12244
Labels
A-git Area: anything dealing with git C-bug Category: bug E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@LazyGeniusMan
Copy link

LazyGeniusMan commented May 18, 2023

Problem

I'm trying to add dependency from git repo that have submodule from different git org/user. That submodule also have submodule as dependency. It looks something like this:

https://github.com/messense/mupdf-rs.git
└── https://github.com/ArtifexSoftware/mupdf.git
    ├── https://github.com/ArtifexSoftware/jbig2dec.git
    ├── ...
    ├── https://github.com/ArtifexSoftware/thirdparty-curl.git
    └── ...

When cargo fetch the submodule, it's using the wrong url, instead of fetching from https://github.com/ArtifexSoftware/thirdparty-curl.git, it's fetching from https://github.com/messense/thirdparty-curl.git. When I check the .gitmodules in the repo, it has relative url.

the .gitmodules looks like this:

[submodule "thirdparty/jbig2dec"]
	path = thirdparty/jbig2dec
	url = ../jbig2dec.git
...
[submodule "thirdparty/curl"]
	path = thirdparty/curl
	url = ../thirdparty-curl.git
...

Steps

  1. Run cargo new test-submodule
  2. Run cd test-submodule
  3. Run cargo add mupdf --git "https://github.com/messense/mupdf-rs.git" --rev "77f17e2b52beeeb108927f65471f122dc572740e"
  4. Run cargo build

Possible Solution(s)

No response

Notes

log:

PS C:\Users\LazyGeniusMan\Downloads\mupdf\test-toc> cargo run
    Updating git repository `https://github.com/messense/mupdf-rs.git`
    Updating git submodule `https://github.com/messense/thirdparty-curl.git`
error: failed to get `mupdf` as a dependency of package `test-toc v0.1.0 (C:\Users\LazyGeniusMan\Downloads\mupdf\test-toc)`

Caused by:
  failed to load source for dependency `mupdf`

Caused by:
  Unable to update https://github.com/messense/mupdf-rs.git?rev=77f17e2b52beeeb108927f65471f122dc572740e

Caused by:
  failed to update submodule `mupdf-sys/mupdf`

Caused by:
  failed to update submodule `thirdparty/curl`

Caused by:
  failed to fetch submodule `thirdparty/curl` from https://github.com/messense/thirdparty-curl.git

Caused by:
  failed to authenticate when downloading repository

  * attempted to find username/password via git's `credential.helper` support, but failed

  if the git CLI succeeds then `net.git-fetch-with-cli` may help here
  https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

Caused by:
  failed to acquire username/password from local configuration

Version

cargo 1.68.2 (6feb7c9cf 2023-03-26)
release: 1.68.2
commit-hash: 6feb7c9cfc0c5604732dba75e4c3b2dbea38e8d8
commit-date: 2023-03-26
host: x86_64-pc-windows-msvc
libgit2: 1.5.0 (sys:0.16.0 vendored)
libcurl: 7.86.0-DEV (sys:0.4.59+curl-7.86.0 vendored ssl:Schannel)
os: Windows 10.0.19044 (Windows 10 Pro) [64-bit]
@LazyGeniusMan LazyGeniusMan added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels May 18, 2023
@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. S-triage Status: This issue is waiting on initial triage. and removed S-triage Status: This issue is waiting on initial triage. S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. labels May 23, 2023
@weihanglo weihanglo added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review A-git Area: anything dealing with git and removed S-triage Status: This issue is waiting on initial triage. labels Jun 7, 2023
@weihanglo
Copy link
Member

Thanks for the report!

The fix shouldn't be difficult I guess. Here and here since Cargo recurses into child submodules, we should pass the url to the child submodule in.

I'd also suggest renaming this url to something like child_remote_url or else. And perhaps make it a Url type so that it can be passed to update_submodules().

let 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);
match new_parent_remote_url.join(child_url_str) {
Ok(x) => x.to_string(),
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
)
})?,
}
} else {
child_url_str.to_string()
};

@weihanglo weihanglo added the E-easy Experience: Easy label Jun 7, 2023
bors added a commit that referenced this issue Jun 8, 2023
fix: git submodules with relative urls

### What does this PR try to resolve?

Fixes #12151.

When recursing submodules, the url of the parent remote was being passed to `update_submodules` instead of the child remote url. This caused Cargo to try to add the wrong submodule.

### How should we test and review this PR?

A test case is added in the first commit. The second one renames the `url` variable as suggested in the issue. The third includes the changes to fix the issue. The last one includes a minor refactor where a redundant `match` expr is removed.
@bors bors closed this as completed in 173b88b Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-git Area: anything dealing with git C-bug Category: bug E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants