Skip to content

Commit

Permalink
fix: add error message for github PR url in dep
Browse files Browse the repository at this point in the history
Prior to this, using a github PR URL would cause cargo to attempt to
fetch from an incorrect URL several times before failing.
Providing a github pull request url now fails with an error message
that shows how to fix the problem.

E.g.:
```toml
bar = { git = "https://github.com/foo/bar/pull/123" }
```
Now gives the following error message:
```
dependency (bar) specifies a GitHub pull request link. If you were trying to specify a specific github PR, replace the URL with the git URL (e.g. `git = "https://github.com/foo/bar.git"`) and add `rev = "refs/pull/123/head"` in the dependency declaration.
```

Fixes: rust-lang#15001
  • Loading branch information
joshka committed Jan 2, 2025
1 parent 0200aa0 commit 1a7720c
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 0 deletions.
22 changes: 22 additions & 0 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2144,6 +2144,8 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
.unwrap_or(GitReference::DefaultBranch);
let loc = git.into_url()?;

bail_if_github_pull_request(&name_in_toml, &loc)?;

if let Some(fragment) = loc.fragment() {
let msg = format!(
"URL fragment `#{fragment}` in git URL is ignored for dependency ({name_in_toml}). \
Expand Down Expand Up @@ -2182,6 +2184,26 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
}
}

/// Checks if the URL is a GitHub pull request URL.
///
/// If the URL is a GitHub pull request URL, an error is returned with a message that explains
/// how to specify a specific git revision.
fn bail_if_github_pull_request(name_in_toml: &str, url: &Url) -> CargoResult<()> {
if url.host_str() != Some("github.com") {
return Ok(());
}
let path_components = url.path().split('/').collect::<Vec<_>>();
if let ["", owner, repo, "pull", pr_number, ..] = path_components[..] {
bail!(
"dependency ({name_in_toml}) specifies a GitHub pull request link. \
If you were trying to specify a specific github PR, replace the URL with the git \
URL (e.g. `git = \"https://github.com/{owner}/{repo}.git\"`) \
and add `rev = \"refs/pull/{pr_number}/head\"` in the dependency declaration.",
);
}
Ok(())
}

pub(crate) fn lookup_path_base<'a>(
base: &PathBaseName,
gctx: &GlobalContext,
Expand Down
31 changes: 31 additions & 0 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2151,6 +2151,37 @@ Caused by:
.run();
}

#[cargo_test]
fn github_pull_request_url() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.0"
edition = "2015"
authors = []
[dependencies.bar]
git = "https://github.com/foo/bar/pull/123"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check -v")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
dependency (bar) specifies a GitHub pull request link. If you were trying to specify a specific github PR, replace the URL with the git URL (e.g. `git = "https://github.com/foo/bar.git"`) and add `rev = "refs/pull/123/head"` in the dependency declaration.
"#]])
.run();
}

#[cargo_test]
fn fragment_in_git_url() {
let p = project()
Expand Down

0 comments on commit 1a7720c

Please sign in to comment.