Skip to content

Conversation

BiagioFesta
Copy link
Contributor

@BiagioFesta BiagioFesta commented Sep 28, 2025

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:

# ...
[dependencies]
my-dep = { git = "http://localhost:9999" }
CARGO_NET_GIT_FETCH_WITH_CLI=true cargo build

@rustbot rustbot added A-cache-messages Area: caching of compiler messages A-git Area: anything dealing with git A-networking Area: networking issues, curl, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 28, 2025

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@BiagioFesta BiagioFesta force-pushed the wip/bfesta/retry-git-fetch-cli branch from 32bfdb2 to 1596c35 Compare September 28, 2025 14:32
@BiagioFesta BiagioFesta force-pushed the wip/bfesta/retry-git-fetch-cli branch from 1596c35 to 2abb59a Compare September 28, 2025 14:49
Copy link
Member

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I guess it would be great if we could introduce a cargo test for it.

For example:

#[cargo_test(public_network_test, requires = "git")]
fn git_spurious_error_message() {
    let p = project()
        .file(
            "Cargo.toml",
            r#"
                [package]
                name = "foo"
                version = "0.1.0"
                edition = "2015"

                [dependencies]
                my-dep = { git = "http://localhost:9999" }
            "#,
        )
        .file("src/lib.rs", "")
        .build();
    p.cargo("fetch")
        .env("CARGO_NET_GIT_FETCH_WITH_CLI", "true")
        .with_status(101)
        .with_stderr_data(str![[r#"
[UPDATING] git repository `http://localhost:9999/`
fatal: unable to access 'http://localhost:9999/': Failed to connect to localhost port 9999 after 0 ms: Couldn't connect to server
[WARNING] spurious network error (3 tries remaining): process didn't exit successfully: `git fetch --no-tags --force --update-head-ok 'http://localhost:9999/' '+HEAD:refs/remotes/origin/HEAD'` ([EXIT_STATUS]: 128)
fatal: unable to access 'http://localhost:9999/': Failed to connect to localhost port 9999 after 0 ms: Couldn't connect to server
[WARNING] spurious network error (2 tries remaining): process didn't exit successfully: `git fetch --no-tags --force --update-head-ok 'http://localhost:9999/' '+HEAD:refs/remotes/origin/HEAD'` ([EXIT_STATUS]: 128)
fatal: unable to access 'http://localhost:9999/': Failed to connect to localhost port 9999 after 0 ms: Couldn't connect to server
[WARNING] spurious network error (1 try remaining): process didn't exit successfully: `git fetch --no-tags --force --update-head-ok 'http://localhost:9999/' '+HEAD:refs/remotes/origin/HEAD'` ([EXIT_STATUS]: 128)
fatal: unable to access 'http://localhost:9999/': Failed to connect to localhost port 9999 after 0 ms: Couldn't connect to server
[ERROR] failed to get `my-dep` as a dependency of package `foo v0.1.0 ([ROOT]/foo)`

Caused by:
  failed to load source for dependency `my-dep`

Caused by:
  Unable to update http://localhost:9999/

Caused by:
  failed to clone into: [ROOT]/home/.cargo/git/db/_empty-[HASH]

Caused by:
  process didn't exit successfully: `git fetch --no-tags --force --update-head-ok 'http://localhost:9999/' '+HEAD:refs/remotes/origin/HEAD'` ([EXIT_STATUS]: 128)

"#]])
        .run();
}

@BiagioFesta
Copy link
Contributor Author

github_fastpath_error_message (test) already contains CARGO_NET_GIT_FETCH_WITH_CLI. Do you feel adding a new test or just changing the matching string?

@weihanglo
Copy link
Member

weihanglo commented Sep 28, 2025

Either works, but don't over match GitHub specific error messages

@BiagioFesta BiagioFesta force-pushed the wip/bfesta/retry-git-fetch-cli branch from 2abb59a to 2e5c6c2 Compare September 29, 2025 18:17
@BiagioFesta BiagioFesta requested a review from 0xPoe September 29, 2025 18:18
Mark `git fetch` (in the `CARGO_NET_GIT_FETCH_WITH_CLI` path) as a
command whose failure may be considered spurious, and enable retry logic
when such a failure occurs. This helps reduce intermittent errors in
situations where `git fetch` fails due to transient network or server
issues.
@BiagioFesta BiagioFesta force-pushed the wip/bfesta/retry-git-fetch-cli branch from 2e5c6c2 to fd69670 Compare September 29, 2025 18:18
Copy link
Member

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :shipit:

@0xPoe
Copy link
Member

0xPoe commented Sep 29, 2025

Thank you for working on this! I will let Weihang take another look before we merge it! Looks great for me.

@weihanglo weihanglo added this pull request to the merge queue Sep 30, 2025
Merged via the queue into rust-lang:master with commit e3db2dc Sep 30, 2025
25 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 30, 2025
@BiagioFesta BiagioFesta deleted the wip/bfesta/retry-git-fetch-cli branch September 30, 2025 08:10
bors added a commit to rust-lang/rust that referenced this pull request Oct 3, 2025
Update cargo submodule

24 commits in f2932725b045d361ff5f18ba02b1409dd1f44e71..2394ea6cea8b26d717aa67eb1663a2dbf2d26078
2025-09-24 11:31:26 +0000 to 2025-10-03 14:13:01 +0000
- Recommend `package.rust-version` in the Rust version section of `reference/semver.md`. (rust-lang/cargo#15806)
- fix(toml): Prevent non-script fields in Cargo scripts (rust-lang/cargo#16026)
- chore(ci): unpin libc (rust-lang/cargo#16044)
- chore: Update dependencies (rust-lang/cargo#16034)
- Fix FileLock path tracking after rename in package operation (rust-lang/cargo#16036)
- Lockfile schemas error cleanup (rust-lang/cargo#16039)
- Convert a multi-part diagnostic to a report (rust-lang/cargo#16035)
- fix(run): Override arg0 for cargo scripts  (rust-lang/cargo#16027)
- Public in private manifest errors (rust-lang/cargo#16002)
- chore(deps): update actions/checkout action to v5 (rust-lang/cargo#16031)
- fix: remove FIXME comment that's no longer a problem (rust-lang/cargo#16025)
- Add retry for `git fetch` failures in `CARGO_NET_GIT_FETCH_WITH_CLI` path (rust-lang/cargo#16016)
- Added better filesystem layout testing harness (rust-lang/cargo#15874)
- Small cleanup to normalize_dependencies (rust-lang/cargo#16022)
- fix: better error message for rust version incompatibility (rust-lang/cargo#16021)
- fix(shell): Use a distinct style for transient status (rust-lang/cargo#16019)
- chore(deps): Depend on `serde_core` in `cargo-platform` (rust-lang/cargo#15992)
- Remove package-workspace from unstable doc index (rust-lang/cargo#16014)
- fix(shell): Switch to annotate snippets for notes (rust-lang/cargo#15945)
- docs: update changelog (rust-lang/cargo#15986)
- chore(ci): add rustfmt for docs job (rust-lang/cargo#16013)
- chore: bump to 0.93.0 (rust-lang/cargo#16009)
- fix(config): combine key error context into one (rust-lang/cargo#16004)
- test(docker): openssh requires a newer libcrypto3 (rust-lang/cargo#16010)

r? ghost
@rustbot rustbot added this to the 1.92.0 milestone Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cache-messages Area: caching of compiler messages A-git Area: anything dealing with git A-networking Area: networking issues, curl, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants