Skip to content

Commit

Permalink
util: provide a better error message for invalid SSH URLs (#15185)
Browse files Browse the repository at this point in the history
### What does this PR try to resolve?

It's very common for users to attempt to use the pseudo-URLs that GitHub
or other providers provide in the form
`[email protected]:rust-lang/rust.git` as a source in Cargo.toml, which are
the default format accepted by OpenSSH. Unfortunately, these are not
actually URLs, and unsurprisingly, the `url` crate doesn't accept them.

However, our error message is unhelpful and looks like this:

invalid url `[email protected]:rust-lang/rust.git`: relative URL without a
base

This is technically true, but we can do better. The user actually wants
to write a real SSH URL, so if the non-URL starts with `git@`, let's
rewrite it into a real URL for them to help them and include that in the
error message.

`git@` is the prefix used by all major forges, as well as the default
configuration for do-it-yourself implementations like Gitolite. While
other options are possible, they are much less common, and this is an
extremely easy and cheap heuristic that does not necessitate complicated
parsing, but which we can change in the future should that be necessary.

This also avoids the problem where users try to turn the pseudo-URL into
a real URL by just prepending `ssh://`, which causes an error message
about the invalid port number due to the colon which they have not
changed. Since they can just copy and paste the proposed answer, they'll
be less likely to attempt this invalid approach.

Fixes #13549

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

1. `cargo init pkg1`
2. `cd pkg1`
3. Edit `Cargo.toml` to add the dependency line `bytes = { git =
"[email protected]:tokio-rs/bytes.git", tag = "v1.10.0" }`.
4. Run `cargo build`
5. Notice that the error suggests a URL to try.
6. Replace the Git URL with the suggested URL.
7. Run `cargo build`.
8. Notice that the package compiles cleanly.

### Additional information

N/A
  • Loading branch information
weihanglo authored Feb 14, 2025
2 parents 26f5f3f + 7cd8890 commit ce948f4
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
13 changes: 12 additions & 1 deletion src/cargo/util/into_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,18 @@ pub trait IntoUrl {

impl<'a> IntoUrl for &'a str {
fn into_url(self) -> CargoResult<Url> {
Url::parse(self).map_err(|s| anyhow::format_err!("invalid url `{}`: {}", self, s))
Url::parse(self).map_err(|s| {
if self.starts_with("git@") {
anyhow::format_err!(
"invalid url `{}`: {}; try using `{}` instead",
self,
s,
format_args!("ssh://{}", self.replacen(':', "/", 1))
)
} else {
anyhow::format_err!("invalid url `{}`: {}", self, s)
}
})
}
}

Expand Down
5 changes: 3 additions & 2 deletions tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ this is dep1 this is dep2
#[cargo_test]
fn cargo_compile_with_short_ssh_git() {
let url = "[email protected]:a/dep";
let well_formed_url = "ssh://[email protected]/a/dep";

let p = project()
.file(
Expand Down Expand Up @@ -565,9 +566,9 @@ fn cargo_compile_with_short_ssh_git() {
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
invalid url `{}`: relative URL without a base
invalid url `{}`: relative URL without a base; try using `{}` instead
",
url
url, well_formed_url
))
.run();
}
Expand Down

0 comments on commit ce948f4

Please sign in to comment.