git_fetch_with_cli does not fetch submodules with cli

[jdswensen]

Problem

I’m having an issue fetching private ssh submodules with git. I dug around and found this commit which adds the option to fetch urls with the system git. However, it appears to only handle parents and any submodules are handled by this line which effectively still uses libgit2.

~/projects/proj1 (feature) $ cargo build -v
    Updating git repository `https://github.com/name/proj2.git`
     Running `git fetch --tags --force --update-head-ok 'https://github.com/name/proj2.git' 'refs/heads/*:refs/heads/*'`
error: failed to load source for a dependency on `proj2`

Caused by:
  Unable to update https://github.com/name/proj2.git

Caused by:
  failed to update submodule `proj3`

Caused by:
  invalid url `[email protected]:name/proj3.git`: relative URL without a base

Steps

1. Create a rust project with an ssh submodule
2. Add that rust project as a dependency to a new rust project via proj = { git = https://path }
3. Attempt to run cargo build

Possible Solution(s)

My options to fix this appear to be:

1. Make proj2 a submodule in proj1. This will work for this particular library but not some of the others my team is working on due to workspaces.
2. Switch every project over to HTTPS submodules. This is doable, but is currently the most disruptive option for the team as a whole.
3. Make the fix and push it upstream.

I’m willing to work on this if no one has time, but I need some guidance as to how to best approach it.

Notes

Output of cargo version: cargo 1.36.0 (c4fcfb7 2019-05-15)

[alexcrichton]

I'm not quite sure if that's the bug here, it looks like we're attempting to parse submodule urls as a Url, which isn't working here. It may be the case that we shouldn't be using a Url intermediate data structure at some point. All git fetches I believe should run through the CLI, even submodule ones, if that option is enabled.

[jdswensen]

That's a good point, its not necessarily the fetch command failing but the flag that enables it should also cause update_submodules to utilize fetch_with_cli. However, update_submodule matches to an error when parsing the Url structure at this line so the fetch function never actually gets called.

[alexcrichton]

Yeah I think we probably just need to pass a str there instead of Url and that should do the trick?