Skip to content

Commit

Permalink
Replace error with warning
Browse files Browse the repository at this point in the history
  • Loading branch information
joshka committed Jan 9, 2025
1 parent d743987 commit 85c9387
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 17 deletions.
18 changes: 10 additions & 8 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::str::{self, FromStr};
use tracing_subscriber::fmt::format;

use crate::core::summary::MissingDependencyError;
use crate::AlreadyPrintedError;
Expand Down Expand Up @@ -2145,7 +2144,7 @@ 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)?;
warn_if_github_pull_request(&name_in_toml, &loc, manifest_ctx.warnings);

if let Some(fragment) = loc.fragment() {
let msg = format!(
Expand Down Expand Up @@ -2187,23 +2186,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 the URL is a GitHub pull request URL, an warning is emitted with a message that explains how
/// to specify a specific git revision.
///
/// At some point in the future it might be worth considering making this a hard error, but for now
/// it's just a warning. See <https://github.com/rust-lang/cargo/pull/15003#discussion_r1908005924>.
fn warn_if_github_pull_request(name_in_toml: &str, url: &Url, warnings: &mut Vec<String>) {
if url.host_str() != Some("github.com") {
return Ok(());
return;
}
let path_components = url.path().split('/').collect::<Vec<_>>();
if let ["", owner, repo, "pull", pr_number, ..] = path_components[..] {
let repo_url = format!("https://github.com/{owner}/{repo}.git");
let rev = format!("refs/pull/{pr_number}/head");
bail!(
let warning = format!(
"dependency ({name_in_toml}) git url {url} is not a repository. \
The path looks like a pull request. Try replacing the dependency with: \
`git = \"{repo_url}\" rev = \"{rev}\"` in the dependency declaration.",
);
warnings.push(warning);
}
Ok(())
}

pub(crate) fn lookup_path_base<'a>(
Expand Down
23 changes: 21 additions & 2 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2173,10 +2173,29 @@ fn github_pull_request_url() {
p.cargo("check -v")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
[WARNING] dependency (bar) git url https://github.com/foo/bar/pull/123 is not a repository. The path looks like a pull request. Try replacing the dependency with: `git = "https://github.com/foo/bar.git" rev = "refs/pull/123/head"` in the dependency declaration.
[UPDATING] git repository `https://github.com/foo/bar/pull/123`
[WARNING] spurious network error (3 tries remaining): unexpected http status code: 404; class=Http (34)
[WARNING] spurious network error (2 tries remaining): unexpected http status code: 404; class=Http (34)
[WARNING] spurious network error (1 tries remaining): unexpected http status code: 404; class=Http (34)
[ERROR] failed to get `bar` as a dependency of package `foo v0.0.0 ([ROOT]/foo)`
Caused by:
failed to load source for dependency `bar`
Caused by:
Unable to update https://github.com/foo/bar/pull/123
Caused by:
failed to clone into: [ROOT]/home/.cargo/git/db/123-[HASH]
Caused by:
network failure seems to have happened
if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli
Caused by:
dependency (bar) git url https://github.com/foo/bar/pull/123 is not a repository. The path looks like a pull request. Try replacing the dependency with: `git = "https://github.com/foo/bar.git" rev = "refs/pull/123/head"` in the dependency declaration.
unexpected http status code: 404; class=Http (34)
"#]])
.run();
Expand Down
25 changes: 18 additions & 7 deletions tests/testsuite/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,6 @@ fn patch_to_git() {

#[cargo_test]
fn patch_to_git_pull_request() {
let bar = git::repo(&paths::root().join("override"))
.file("Cargo.toml", &basic_manifest("bar", "0.1.0"))
.file("src/lib.rs", "pub fn bar() {}")
.build();

Package::new("bar", "0.1.0").publish();

let p = project()
Expand Down Expand Up @@ -392,10 +387,26 @@ fn patch_to_git_pull_request() {
p.cargo("check -v")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
[WARNING] dependency (bar) git url https://github.com/foo/bar/pull/123 is not a repository. The path looks like a pull request. Try replacing the dependency with: `git = "https://github.com/foo/bar.git" rev = "refs/pull/123/head"` in the dependency declaration.
[UPDATING] git repository `https://github.com/foo/bar/pull/123`
[WARNING] spurious network error (3 tries remaining): unexpected http status code: 404; class=Http (34)
[WARNING] spurious network error (2 tries remaining): unexpected http status code: 404; class=Http (34)
[WARNING] spurious network error (1 tries remaining): unexpected http status code: 404; class=Http (34)
[ERROR] failed to load source for dependency `bar`
Caused by:
Unable to update https://github.com/foo/bar/pull/123
Caused by:
failed to clone into: [ROOT]/home/.cargo/git/db/123-[HASH]
Caused by:
network failure seems to have happened
if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli
Caused by:
dependency (bar) git url https://github.com/foo/bar/pull/123 is not a repository. The path looks like a pull request. Try replacing the dependency with: `git = "https://github.com/foo/bar.git" rev = "refs/pull/123/head"` in the dependency declaration.
unexpected http status code: 404; class=Http (34)
"#]])
.run();
Expand Down

0 comments on commit 85c9387

Please sign in to comment.