Skip to content

Commit e3db2dc

Browse files
authored
Add retry for git fetch failures in CARGO_NET_GIT_FETCH_WITH_CLI path (#16016)
### What does this PR try to resolve? #15856 This PR addresses transient failures when using the `CARGO_NET_GIT_FETCH_WITH_CLI` path by adding support for marking process failures as *spurious* and enabling retries for `git fetch`. #### Changes - Introduce a new error wrap `GitCliError` with `is_spurious` meta information. - Update the `CARGO_NET_GIT_FETCH_WITH_CLI` code path to mark `git fetch` as spurious and retry on failure ### How to test and review this PR? A test project: ```toml # ... [dependencies] my-dep = { git = "http://localhost:9999" } ``` ```bash CARGO_NET_GIT_FETCH_WITH_CLI=true cargo build ```
2 parents ad14280 + fd69670 commit e3db2dc

File tree

4 files changed

+68
-3
lines changed

4 files changed

+68
-3
lines changed

src/cargo/sources/git/utils.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::sources::git::fetch::RemoteKind;
66
use crate::sources::git::oxide;
77
use crate::sources::git::oxide::cargo_config_to_gitoxide_overrides;
88
use crate::util::HumanBytes;
9-
use crate::util::errors::CargoResult;
9+
use crate::util::errors::{CargoResult, GitCliError};
1010
use crate::util::{GlobalContext, IntoUrl, MetricsCounter, Progress, network};
1111
use anyhow::{Context as _, anyhow};
1212
use cargo_util::{ProcessBuilder, paths};
@@ -1110,7 +1110,11 @@ fn fetch_with_cli(
11101110
.cwd(repo.path());
11111111
gctx.shell()
11121112
.verbose(|s| s.status("Running", &cmd.to_string()))?;
1113-
cmd.exec()?;
1113+
network::retry::with_retry(gctx, || {
1114+
cmd.exec()
1115+
.map_err(|error| GitCliError::new(error, true).into())
1116+
})?;
1117+
11141118
Ok(())
11151119
}
11161120

src/cargo/util/errors.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,46 @@ impl From<std::io::Error> for CliError {
338338
}
339339
}
340340

341+
// =============================================================================
342+
// Git CLI errors
343+
344+
pub type GitCliResult = Result<(), GitCliError>;
345+
346+
/// An error that occurred while invoking the `git` command-line tool.
347+
///
348+
/// This is used for errors from Git operations performed via CLI.
349+
/// It wraps a lower-level error and indicates whether the failure
350+
/// should be considered *spurious*.
351+
///
352+
/// Spurious failures might involve retry mechanism.
353+
#[derive(Debug)]
354+
pub struct GitCliError {
355+
inner: Error,
356+
is_spurious: bool,
357+
}
358+
359+
impl GitCliError {
360+
pub fn new(inner: Error, is_spurious: bool) -> GitCliError {
361+
GitCliError { inner, is_spurious }
362+
}
363+
364+
pub fn is_spurious(&self) -> bool {
365+
self.is_spurious
366+
}
367+
}
368+
369+
impl std::error::Error for GitCliError {
370+
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
371+
self.inner.source()
372+
}
373+
}
374+
375+
impl fmt::Display for GitCliError {
376+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
377+
self.inner.fmt(f)
378+
}
379+
}
380+
341381
// =============================================================================
342382
// Construction helpers
343383

src/cargo/util/network/retry.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
//! - <https://en.wikipedia.org/wiki/Exponential_backoff>
4242
//! - <https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After>
4343
44-
use crate::util::errors::HttpNotSuccessful;
44+
use crate::util::errors::{GitCliError, HttpNotSuccessful};
4545
use crate::{CargoResult, GlobalContext};
4646
use anyhow::Error;
4747
use rand::Rng;
@@ -223,6 +223,12 @@ fn maybe_spurious(err: &Error) -> bool {
223223
}
224224
}
225225

226+
if let Some(err) = err.downcast_ref::<GitCliError>() {
227+
if err.is_spurious() {
228+
return true;
229+
}
230+
}
231+
226232
false
227233
}
228234

@@ -402,3 +408,12 @@ fn retry_after_parsing() {
402408
_ => panic!("unexpected non-retry"),
403409
}
404410
}
411+
412+
#[test]
413+
fn git_cli_error_spurious() {
414+
let error = GitCliError::new(Error::msg("test-git-cli-error"), false);
415+
assert!(!maybe_spurious(&error.into()));
416+
417+
let error = GitCliError::new(Error::msg("test-git-cli-error"), true);
418+
assert!(maybe_spurious(&error.into()));
419+
}

tests/testsuite/git.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4092,6 +4092,12 @@ fn github_fastpath_error_message() {
40924092
.with_stderr_data(str![[r#"
40934093
[UPDATING] git repository `https://github.com/rust-lang/bitflags.git`
40944094
fatal: remote [ERROR] upload-pack: not our ref 11111b376b93484341c68fbca3ca110ae5cd2790
4095+
[WARNING] spurious network error (3 tries remaining): process didn't exit successfully: `git fetch --no-tags --force --update-head-ok [..]
4096+
fatal: remote [ERROR] upload-pack: not our ref 11111b376b93484341c68fbca3ca110ae5cd2790
4097+
[WARNING] spurious network error (2 tries remaining): process didn't exit successfully: `git fetch --no-tags --force --update-head-ok [..]
4098+
fatal: remote [ERROR] upload-pack: not our ref 11111b376b93484341c68fbca3ca110ae5cd2790
4099+
[WARNING] spurious network error (1 try remaining): process didn't exit successfully: `git fetch --no-tags --force --update-head-ok [..]
4100+
fatal: remote [ERROR] upload-pack: not our ref 11111b376b93484341c68fbca3ca110ae5cd2790
40954101
[ERROR] failed to get `bitflags` as a dependency of package `foo v0.1.0 ([ROOT]/foo)`
40964102
40974103
Caused by:

0 commit comments

Comments
 (0)