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

Cargo does not normalize relative urls for submodules in ssh git dependencies #12404

Closed
liss-h opened this issue Jul 27, 2023 · 1 comment · Fixed by #12411
Closed

Cargo does not normalize relative urls for submodules in ssh git dependencies #12404

liss-h opened this issue Jul 27, 2023 · 1 comment · Fixed by #12411
Labels
A-git Area: anything dealing with git C-bug Category: bug regression-from-stable-to-beta Regression in beta that previously worked in stable. S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@liss-h
Copy link

liss-h commented Jul 27, 2023

Problem

Github does not like relative repository names if you use ssh to download repositories.
I.e. according to github the following is not a valid url to fetch a repo: ssh://git@github.com/owner/repo/../sub
Cargo does not normalize such relative paths for submodules and therefore fails to fetch the submodule if using ssh.

Steps

If you have a crate that depends on a git repo via ssh. E.g.

Cargo.toml

crate = { git = "ssh://git@github.com/owner/repo" }

And that repo has a submodule specified with a relative path. E.g.

.gitmodules

[submodule "sub"]
    path = sub
    url = ../sub

Cargo is not able to fetch the submodule because it doesn't normalize the path; github doesn't like that.
Cargo tries the following:

Updating git repository `ssh://git@github.com/owner/repo`
Updating git submodule `ssh://git@github.com/owner/repo/../sub`

Github then throws an error because owner/repo/../sub is not a valid repository name.

Btw, if you enter this manually in the commandline, i.e. git clone ssh://git@github.com/owner/repo/../sub it doesn't work either, but it does work with http, seems to be a github server-side thing, although I don't know if this is indended or not. Git itself seems to do this normalization so relative submodules using ssh work fine there.

Possible Solution(s)

I guess normalize the repo path if using ssh and hope nothing breaks 😅

Notes

No response

Version

cargo 1.73.0-nightly (7ac9416d8 2023-07-24)
release: 1.73.0-nightly
commit-hash: 7ac9416d82cd4fc5e707c9ec3574d22dff6466e5
commit-date: 2023-07-24
host: x86_64-unknown-linux-musl
libgit2: 1.6.4 (sys:0.17.2 vendored)
libcurl: 8.1.2-DEV (sys:0.4.63+curl-8.1.2 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Alpine Linux 3.17.4 [64-bit]
@liss-h liss-h added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jul 27, 2023
@weihanglo weihanglo added A-git Area: anything dealing with git S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review regression-from-stable-to-beta Regression in beta that previously worked in stable. and removed S-triage Status: This issue is waiting on initial triage. labels Jul 27, 2023
@weihanglo
Copy link
Member

I believe the root cause is #12359. Feel like the possible solution would be:

  • Using url when possible (for standard ssh:// protocol).
  • Go using strings and non-normalized urls otherwise for scp-like URLs).

Since we never support submodule URL relative to scp-like URL, I think it is fine letting the case fail. Or if there is any scp-like URL parsing we can leverage?

weihanglo added a commit to weihanglo/cargo that referenced this issue Jul 30, 2023
Git only assumes a submodule URL is a relative path if it starts with `./`
or `../` [^1]. To fetch the correct repo, we need to construct an aboslute
submodule URL.

At this moment it comes with some limitations:

* GitHub doesn't accept non-normalized URLs wth relative paths.
  (`ssh://git@github.com/rust-lang/cargo.git/relative/..` is invalid)
* `url` crate cannot parse SCP-like URLs.
  (`git@github.com:rust-lang/cargo.git` is not a valid WHATWG URL)

To overcome these, this patch always tries `Url::parse` first to normalize
the path. If it couldn't, append the relative path as the last resort and
pray the remote git service supports non-normalized URLs.

See also rust-lang#12404 and rust-lang#12295.

[^1]: <https://git-scm.com/docs/git-submodule>
ehuss pushed a commit to weihanglo/cargo that referenced this issue Jul 30, 2023
Git only assumes a submodule URL is a relative path if it starts with `./`
or `../` [^1]. To fetch the correct repo, we need to construct an aboslute
submodule URL.

At this moment it comes with some limitations:

* GitHub doesn't accept non-normalized URLs wth relative paths.
  (`ssh://git@github.com/rust-lang/cargo.git/relative/..` is invalid)
* `url` crate cannot parse SCP-like URLs.
  (`git@github.com:rust-lang/cargo.git` is not a valid WHATWG URL)

To overcome these, this patch always tries `Url::parse` first to normalize
the path. If it couldn't, append the relative path as the last resort and
pray the remote git service supports non-normalized URLs.

See also rust-lang#12404 and rust-lang#12295.

[^1]: <https://git-scm.com/docs/git-submodule>
weihanglo added a commit to weihanglo/cargo that referenced this issue Jul 30, 2023
Git only assumes a submodule URL is a relative path if it starts with `./`
or `../` [^1]. To fetch the correct repo, we need to construct an aboslute
submodule URL.

At this moment it comes with some limitations:

* GitHub doesn't accept non-normalized URLs wth relative paths.
  (`ssh://git@github.com/rust-lang/cargo.git/relative/..` is invalid)
* `url` crate cannot parse SCP-like URLs.
  (`git@github.com:rust-lang/cargo.git` is not a valid WHATWG URL)

To overcome these, this patch always tries `Url::parse` first to normalize
the path. If it couldn't, append the relative path as the last resort and
pray the remote git service supports non-normalized URLs.

See also rust-lang#12404 and rust-lang#12295.

[^1]: <https://git-scm.com/docs/git-submodule>
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 regression-from-stable-to-beta Regression in beta that previously worked in stable. 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