Skip to content

Commit f2836a7

Browse files
motorailgunmotorailgun
committed
fix: make messages to be shown within git operation
1 parent d771595 commit f2836a7

File tree

5 files changed

+64
-47
lines changed

5 files changed

+64
-47
lines changed

‎src/cargo/sources/git/oxide.rs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use tracing::debug;
1616

1717
/// For the time being, `repo_path` makes it easy to instantiate a gitoxide repo just for fetching.
1818
/// In future this may change to be the gitoxide repository itself.
19-
pub fn with_retry_and_progress(
19+
pub fn with_retry_and_progress<'a>(
2020
repo_path: &std::path::Path,
2121
gctx: &GlobalContext,
2222
cb: &(
@@ -25,7 +25,7 @@ pub fn with_retry_and_progress(
2525
&AtomicBool,
2626
&mut gix::progress::tree::Item,
2727
&mut dyn FnMut(&gix::bstr::BStr),
28-
) -> Result<(), crate::sources::git::fetch::Error>
28+
) -> (Result<(), crate::sources::git::fetch::Error>, &'a str)
2929
+ Send
3030
+ Sync
3131
),
@@ -46,15 +46,15 @@ pub fn with_retry_and_progress(
4646
let thread = s.spawn(move || {
4747
let mut progress = progress_root.add_child("operation");
4848
let mut urls = RefCell::new(Default::default());
49-
let res = cb(
49+
let (res, remote_url) = cb(
5050
&repo_path,
5151
&AtomicBool::default(),
5252
&mut progress,
5353
&mut |url| {
5454
*urls.borrow_mut() = Some(url.to_owned());
5555
},
5656
);
57-
amend_authentication_hints(res, urls.get_mut().take())
57+
amend_authentication_hints(res, remote_url, urls.get_mut().take())
5858
});
5959
translate_progress_to_bar(&mut progress_bar, root, is_shallow)?;
6060
thread.join().expect("no panic in scoped thread")
@@ -180,6 +180,7 @@ fn translate_progress_to_bar(
180180

181181
fn amend_authentication_hints(
182182
res: Result<(), crate::sources::git::fetch::Error>,
183+
remote_url: &str,
183184
last_url_for_authentication: Option<gix::bstr::BString>,
184185
) -> CargoResult<()> {
185186
let Err(err) = res else { return Ok(()) };
@@ -189,6 +190,7 @@ fn amend_authentication_hints(
189190
) => Some(err),
190191
_ => None,
191192
};
193+
192194
if let Some(e) = e {
193195
let auth_message = match e {
194196
gix::protocol::handshake::Error::Credentials(_) => {
@@ -203,10 +205,14 @@ fn amend_authentication_hints(
203205
.into()
204206
}
205207
gix::protocol::handshake::Error::Transport(_) => {
206-
let msg = concat!(
207-
"network failure seems to have happened\n",
208-
"if a proxy or similar is necessary `net.git-fetch-with-cli` may help here\n",
209-
"https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli"
208+
let msg = format!(
209+
concat!(
210+
"network failure seems to have happened\n",
211+
"if a proxy or similar is necessary `net.git-fetch-with-cli` may help here\n",
212+
"https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli",
213+
"{}"
214+
),
215+
super::utils::note_github_pull_request(remote_url)
210216
);
211217
return Err(anyhow::Error::from(err).context(msg));
212218
}

‎src/cargo/sources/git/utils.rs‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -798,12 +798,14 @@ where
798798
| ErrorClass::FetchHead
799799
| ErrorClass::Ssh
800800
| ErrorClass::Http => {
801-
let mut msg = "network failure seems to have happened\n".to_string();
802-
msg.push_str(
803-
"if a proxy or similar is necessary `net.git-fetch-with-cli` may help here\n",
804-
);
805-
msg.push_str(
806-
"https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli",
801+
let msg = format!(
802+
concat!(
803+
"network failure seems to have happened\n",
804+
"if a proxy or similar is necessary `net.git-fetch-with-cli` may help here\n",
805+
"https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli",
806+
"{}"
807+
),
808+
note_github_pull_request(url)
807809
);
808810
err = err.context(msg);
809811
}
@@ -1225,9 +1227,9 @@ fn fetch_with_gitoxide(
12251227
}
12261228
}
12271229

1228-
return Err(err.into());
1230+
return (Err(err.into()), &remote_url);
12291231
}
1230-
Ok(())
1232+
(Ok(()), remote_url)
12311233
},
12321234
);
12331235
if repo_reinitialized.load(Ordering::Relaxed) {
@@ -1589,6 +1591,33 @@ fn is_github(url: &Url) -> bool {
15891591
url.host_str() == Some("github.com")
15901592
}
15911593

1594+
// Give some messages on GitHub PR URL given as is
1595+
pub(crate) fn note_github_pull_request(url: &str) -> String {
1596+
if let Ok(url) = url.parse::<Url>()
1597+
&& is_github(&url)
1598+
{
1599+
let path_segments = url
1600+
.path_segments()
1601+
.map(|p| p.into_iter().collect::<Vec<_>>())
1602+
.unwrap_or_default();
1603+
if let [owner, repo, "pull", pr_number, ..] = path_segments[..] {
1604+
let repo_url = format!("https://github.com/{owner}/{repo}.git");
1605+
let rev = format!("refs/pull/{pr_number}/head");
1606+
return format!(
1607+
concat!(
1608+
"\n\nnote: GitHub url {} is not a repository. \n",
1609+
" Replace the dependency with \n",
1610+
" `git = \"{}\" rev = \"{}\"` \n",
1611+
" to specify pull requests as dependencies' revision."
1612+
),
1613+
url, repo_url, rev
1614+
);
1615+
}
1616+
}
1617+
1618+
"".into()
1619+
}
1620+
15921621
/// Whether a `rev` looks like a commit hash (ASCII hex digits).
15931622
fn looks_like_commit_hash(rev: &str) -> bool {
15941623
rev.len() >= 7 && rev.chars().all(|ch| ch.is_ascii_hexdigit())

‎src/cargo/util/toml/mod.rs‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2389,8 +2389,6 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
23892389
.unwrap_or(GitReference::DefaultBranch);
23902390
let loc = git.into_url()?;
23912391

2392-
warn_if_github_pull_request(&name_in_toml, &loc, manifest_ctx.warnings);
2393-
23942392
if let Some(fragment) = loc.fragment() {
23952393
let msg = format!(
23962394
"URL fragment `#{fragment}` in git URL is ignored for dependency ({name_in_toml}). \
@@ -2429,30 +2427,6 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
24292427
}
24302428
}
24312429

2432-
/// Checks if the URL is a GitHub pull request URL.
2433-
///
2434-
/// If the URL is a GitHub pull request URL, an warning is emitted with a message that explains how
2435-
/// to specify a specific git revision.
2436-
///
2437-
/// At some point in the future it might be worth considering making this a hard error, but for now
2438-
/// it's just a warning. See <https://github.com/rust-lang/cargo/pull/15003#discussion_r1908005924>.
2439-
fn warn_if_github_pull_request(name_in_toml: &str, url: &Url, warnings: &mut Vec<String>) {
2440-
if url.host_str() != Some("github.com") {
2441-
return;
2442-
}
2443-
let path_components = url.path().split('/').collect::<Vec<_>>();
2444-
if let ["", owner, repo, "pull", pr_number, ..] = path_components[..] {
2445-
let repo_url = format!("https://github.com/{owner}/{repo}.git");
2446-
let rev = format!("refs/pull/{pr_number}/head");
2447-
let warning = format!(
2448-
"dependency ({name_in_toml}) git url {url} is not a repository. \
2449-
The path looks like a pull request. Try replacing the dependency with: \
2450-
`git = \"{repo_url}\" rev = \"{rev}\"` in the dependency declaration.",
2451-
);
2452-
warnings.push(warning);
2453-
}
2454-
}
2455-
24562430
pub(crate) fn lookup_path_base<'a>(
24572431
base: &PathBaseName,
24582432
gctx: &GlobalContext,

‎tests/testsuite/bad_config.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2281,7 +2281,6 @@ fn github_pull_request_url() {
22812281
p.cargo("check -v")
22822282
.with_status(101)
22832283
.with_stderr_data(str![[r#"
2284-
[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.
22852284
[UPDATING] git repository `https://github.com/foo/bar/pull/123`
22862285
...
22872286
[ERROR] failed to get `bar` as a dependency of package `foo v0.0.0 ([ROOT]/foo)`
@@ -2292,7 +2291,11 @@ Caused by:
22922291
Caused by:
22932292
Unable to update https://github.com/foo/bar/pull/123
22942293
...
2295-
2294+
[NOTE] GitHub url https://github.com/foo/bar/pull/123 is not a repository.
2295+
Replace the dependency with
2296+
`git = "https://github.com/foo/bar.git" rev = "refs/pull/123/head"`
2297+
to specify pull requests as dependencies' revision.
2298+
...
22962299
"#]])
22972300
.run();
22982301
}

‎tests/testsuite/patch.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,8 +395,7 @@ Caused by:
395395
p.cargo("check -v")
396396
.with_status(101)
397397
.with_stderr_data(format!(
398-
r#"[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.
399-
[UPDATING] git repository `https://github.com/foo/bar/pull/123`
398+
r#"[UPDATING] git repository `https://github.com/foo/bar/pull/123`
400399
...
401400
[ERROR] failed to load source for dependency `bar`
402401
@@ -411,9 +410,15 @@ Caused by:
411410
if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
412411
https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli
413412
413+
[NOTE] GitHub url https://github.com/foo/bar/pull/123 is not a repository.
414+
Replace the dependency with
415+
`git = "https://github.com/foo/bar.git" rev = "refs/pull/123/head"`
416+
to specify pull requests as dependencies' revision.
417+
414418
Caused by:
415419
{traiter}
416-
"#))
420+
"#
421+
))
417422
.run();
418423
}
419424

0 commit comments

Comments
 (0)