Retry git fetch operations

[Veetaha]

Describe the problem you are trying to solve
The problem here is that since git fetch operation is a network operation it may suddenly fail due to temporary connectivity issues.
Cargo already has CARGO_NET_RETRY configuration for network retries, however, it seems that this one is not applied to git fetch operations.

Describe the solution you'd like
I suggest that CARGO_NET_RETRY is taken into account when running git fetch and git fetch does get retried with the maximum retries specified in CARGO_NET_RETRY config value

[UPD]
The cargo config we are using does have net.git-fetch-with-cli parameter set to true

Notes
The issue with git fetch that cargo does regularly arises in our CI system with different errors like Failed to connect to github.com port 443: Timed out.

[Veetaha]

Hi, @alexcrichton, I was looking at the related code in this repo here:

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

Lines 812 to 820 in c227565

	// Unfortunately `libgit2` is notably lacking in the realm of authentication
	// when compared to the `git` command line. As a result, allow an escape
	// hatch for users that would prefer to use `git`-the-CLI for fetching
	// repositories instead of `libgit2`-the-library. This should make more
	// flavors of authentication possible while also still giving us all the
	// speed and portability of using `libgit2`.
	if let Some(true) = config.net_config()?.git_fetch_with_cli {
	return fetch_with_cli(repo, url, &refspecs, tags, config);
	}

So we've been using the git CLI all the time for git fetch operations in cargo. And reading this obvious implementation code there I can conclude that git fetch CLI operations are not retried, only git2 library fetch operations are.

I am going to override our net. git-fetch-with-cli to false and see if it resolves the problem for us, however, I'd like cargo to be resistant and get this fixed.

I am looking at cargo code here for the first time, I may make a contribution to wrap the CLI call there with retries. This seems like a trivial task, however, if you have any immediate hints on how to do this I'd be grateful to hear these (e.g. some existing code in cargo we could reuse), or I may leave this to you or anyone else interested.

[alexcrichton]

The test for whether an error is a spurious network error is located at

cargo/src/cargo/util/network.rs

Lines 36 to 67 in c227565

	fn maybe_spurious(err: &Error) -> bool {
	if let Some(git_err) = err.downcast_ref::<git2::Error>() {
	match git_err.class() {
	git2::ErrorClass::Net
	| git2::ErrorClass::Os
	| git2::ErrorClass::Zlib
	| git2::ErrorClass::Http => return true,
	_ => (),
	}
	}
	if let Some(curl_err) = err.downcast_ref::<curl::Error>() {
	if curl_err.is_couldnt_connect()
	|| curl_err.is_couldnt_resolve_proxy()
	|| curl_err.is_couldnt_resolve_host()
	|| curl_err.is_operation_timedout()
	|| curl_err.is_recv_error()
	|| curl_err.is_send_error()
	|| curl_err.is_http2_error()
	|| curl_err.is_http2_stream_error()
	|| curl_err.is_ssl_connect_error()
	|| curl_err.is_partial_file()
	{
	return true;
	}
	}
	if let Some(not_200) = err.downcast_ref::<HttpNot200>() {
	if 500 <= not_200.code && not_200.code < 600 {
	return true;
	}
	}
	false
	}

. We'll probably need to soup up the error coming out of fetch_with_cli or something like that to know what to inspect, and otherwise detection of whether or not it's a spurious error also needs to be implemented.