fix(): error context for git_fetch refspec not found (#14806)

### What does this PR try to resolve?

Fixes Issue : #14621

Adds more error context for fetching a commit that doesn't exists.

### How should we test and review this PR?

I've created two tests, one for fast_path and one for libgit2.

r? @weihanglo
This commit is contained in:
Weihang Lo 2024-11-17 22:35:29 +00:00 committed by GitHub
commit 010f171ca4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 264 additions and 149 deletions

View File

@ -968,6 +968,9 @@ pub fn fetch(
let shallow = remote_kind.to_shallow_setting(repo.is_shallow(), gctx);
// Flag to keep track if the rev is a full commit hash
let mut fast_path_rev: bool = false;
let oid_to_fetch = match github_fast_path(repo, remote_url, reference, gctx) {
Ok(FastPathRev::UpToDate) => return Ok(()),
Ok(FastPathRev::NeedsFetch(rev)) => Some(rev),
@ -1008,6 +1011,7 @@ pub fn fetch(
if rev.starts_with("refs/") {
refspecs.push(format!("+{0}:{0}", rev));
} else if let Some(oid_to_fetch) = oid_to_fetch {
fast_path_rev = true;
refspecs.push(format!("+{0}:refs/commit/{0}", oid_to_fetch));
} else if !matches!(shallow, gix::remote::fetch::Shallow::NoChange)
&& rev.parse::<Oid>().is_ok()
@ -1030,158 +1034,20 @@ pub fn fetch(
}
}
if let Some(true) = gctx.net_config()?.git_fetch_with_cli {
return fetch_with_cli(repo, remote_url, &refspecs, tags, gctx);
}
if gctx.cli_unstable().gitoxide.map_or(false, |git| git.fetch) {
let git2_repo = repo;
let config_overrides = cargo_config_to_gitoxide_overrides(gctx)?;
let repo_reinitialized = AtomicBool::default();
let res = oxide::with_retry_and_progress(
&git2_repo.path().to_owned(),
gctx,
&|repo_path,
should_interrupt,
mut progress,
url_for_authentication: &mut dyn FnMut(&gix::bstr::BStr)| {
// The `fetch` operation here may fail spuriously due to a corrupt
// repository. It could also fail, however, for a whole slew of other
// reasons (aka network related reasons). We want Cargo to automatically
// recover from corrupt repositories, but we don't want Cargo to stomp
// over other legitimate errors.
//
// Consequently we save off the error of the `fetch` operation and if it
// looks like a "corrupt repo" error then we blow away the repo and try
// again. If it looks like any other kind of error, or if we've already
// blown away the repository, then we want to return the error as-is.
loop {
let res = oxide::open_repo(
repo_path,
config_overrides.clone(),
oxide::OpenMode::ForFetch,
)
.map_err(crate::sources::git::fetch::Error::from)
.and_then(|repo| {
debug!("initiating fetch of {refspecs:?} from {remote_url}");
let url_for_authentication = &mut *url_for_authentication;
let remote = repo
.remote_at(remote_url)?
.with_fetch_tags(if tags {
gix::remote::fetch::Tags::All
} else {
gix::remote::fetch::Tags::Included
})
.with_refspecs(
refspecs.iter().map(|s| s.as_str()),
gix::remote::Direction::Fetch,
)
.map_err(crate::sources::git::fetch::Error::Other)?;
let url = remote
.url(gix::remote::Direction::Fetch)
.expect("set at init")
.to_owned();
let connection = remote.connect(gix::remote::Direction::Fetch)?;
let mut authenticate = connection.configured_credentials(url)?;
let connection = connection.with_credentials(
move |action: gix::protocol::credentials::helper::Action| {
if let Some(url) = action.context().and_then(|gctx| {
gctx.url.as_ref().filter(|url| *url != remote_url)
}) {
url_for_authentication(url.as_ref());
}
authenticate(action)
},
);
let outcome = connection
.prepare_fetch(&mut progress, gix::remote::ref_map::Options::default())?
.with_shallow(shallow.clone().into())
.receive(&mut progress, should_interrupt)?;
Ok(outcome)
});
let err = match res {
Ok(_) => break,
Err(e) => e,
};
debug!("fetch failed: {}", err);
if !repo_reinitialized.load(Ordering::Relaxed)
// We check for errors that could occur if the configuration, refs or odb files are corrupted.
// We don't check for errors related to writing as `gitoxide` is expected to create missing leading
// folder before writing files into it, or else not even open a directory as git repository (which is
// also handled here).
&& err.is_corrupted()
|| has_shallow_lock_file(&err)
{
repo_reinitialized.store(true, Ordering::Relaxed);
debug!(
"looks like this is a corrupt repository, reinitializing \
and trying again"
);
if oxide::reinitialize(repo_path).is_ok() {
continue;
}
}
return Err(err.into());
}
Ok(())
},
);
if repo_reinitialized.load(Ordering::Relaxed) {
*git2_repo = git2::Repository::open(git2_repo.path())?;
}
res
let result = if let Some(true) = gctx.net_config()?.git_fetch_with_cli {
fetch_with_cli(repo, remote_url, &refspecs, tags, gctx)
} else if gctx.cli_unstable().gitoxide.map_or(false, |git| git.fetch) {
fetch_with_gitoxide(repo, remote_url, refspecs, tags, shallow, gctx)
} else {
debug!("doing a fetch for {remote_url}");
let git_config = git2::Config::open_default()?;
with_fetch_options(&git_config, remote_url, gctx, &mut |mut opts| {
if tags {
opts.download_tags(git2::AutotagOption::All);
}
if let gix::remote::fetch::Shallow::DepthAtRemote(depth) = shallow {
opts.depth(0i32.saturating_add_unsigned(depth.get()));
}
// The `fetch` operation here may fail spuriously due to a corrupt
// repository. It could also fail, however, for a whole slew of other
// reasons (aka network related reasons). We want Cargo to automatically
// recover from corrupt repositories, but we don't want Cargo to stomp
// over other legitimate errors.
//
// Consequently we save off the error of the `fetch` operation and if it
// looks like a "corrupt repo" error then we blow away the repo and try
// again. If it looks like any other kind of error, or if we've already
// blown away the repository, then we want to return the error as-is.
let mut repo_reinitialized = false;
loop {
debug!("initiating fetch of {refspecs:?} from {remote_url}");
let res =
repo.remote_anonymous(remote_url)?
.fetch(&refspecs, Some(&mut opts), None);
let err = match res {
Ok(()) => break,
Err(e) => e,
};
debug!("fetch failed: {}", err);
fetch_with_libgit2(repo, remote_url, refspecs, tags, shallow, gctx)
};
if !repo_reinitialized
&& matches!(err.class(), ErrorClass::Reference | ErrorClass::Odb)
{
repo_reinitialized = true;
debug!(
"looks like this is a corrupt repository, reinitializing \
and trying again"
);
if reinitialize(repo).is_ok() {
continue;
}
}
return Err(err.into());
}
Ok(())
})
if fast_path_rev {
if let Some(oid) = oid_to_fetch {
return result.with_context(|| format!("revision {} not found", oid));
}
}
result
}
/// `gitoxide` uses shallow locks to assure consistency when fetching to and to avoid races, and to write
@ -1251,6 +1117,171 @@ fn fetch_with_cli(
Ok(())
}
fn fetch_with_gitoxide(
repo: &mut git2::Repository,
remote_url: &str,
refspecs: Vec<String>,
tags: bool,
shallow: gix::remote::fetch::Shallow,
gctx: &GlobalContext,
) -> CargoResult<()> {
let git2_repo = repo;
let config_overrides = cargo_config_to_gitoxide_overrides(gctx)?;
let repo_reinitialized = AtomicBool::default();
let res = oxide::with_retry_and_progress(
&git2_repo.path().to_owned(),
gctx,
&|repo_path,
should_interrupt,
mut progress,
url_for_authentication: &mut dyn FnMut(&gix::bstr::BStr)| {
// The `fetch` operation here may fail spuriously due to a corrupt
// repository. It could also fail, however, for a whole slew of other
// reasons (aka network related reasons). We want Cargo to automatically
// recover from corrupt repositories, but we don't want Cargo to stomp
// over other legitimate errors.
//
// Consequently we save off the error of the `fetch` operation and if it
// looks like a "corrupt repo" error then we blow away the repo and try
// again. If it looks like any other kind of error, or if we've already
// blown away the repository, then we want to return the error as-is.
loop {
let res = oxide::open_repo(
repo_path,
config_overrides.clone(),
oxide::OpenMode::ForFetch,
)
.map_err(crate::sources::git::fetch::Error::from)
.and_then(|repo| {
debug!("initiating fetch of {refspecs:?} from {remote_url}");
let url_for_authentication = &mut *url_for_authentication;
let remote = repo
.remote_at(remote_url)?
.with_fetch_tags(if tags {
gix::remote::fetch::Tags::All
} else {
gix::remote::fetch::Tags::Included
})
.with_refspecs(
refspecs.iter().map(|s| s.as_str()),
gix::remote::Direction::Fetch,
)
.map_err(crate::sources::git::fetch::Error::Other)?;
let url = remote
.url(gix::remote::Direction::Fetch)
.expect("set at init")
.to_owned();
let connection = remote.connect(gix::remote::Direction::Fetch)?;
let mut authenticate = connection.configured_credentials(url)?;
let connection = connection.with_credentials(
move |action: gix::protocol::credentials::helper::Action| {
if let Some(url) = action
.context()
.and_then(|gctx| gctx.url.as_ref().filter(|url| *url != remote_url))
{
url_for_authentication(url.as_ref());
}
authenticate(action)
},
);
let outcome = connection
.prepare_fetch(&mut progress, gix::remote::ref_map::Options::default())?
.with_shallow(shallow.clone().into())
.receive(&mut progress, should_interrupt)?;
Ok(outcome)
});
let err = match res {
Ok(_) => break,
Err(e) => e,
};
debug!("fetch failed: {}", err);
if !repo_reinitialized.load(Ordering::Relaxed)
// We check for errors that could occur if the configuration, refs or odb files are corrupted.
// We don't check for errors related to writing as `gitoxide` is expected to create missing leading
// folder before writing files into it, or else not even open a directory as git repository (which is
// also handled here).
&& err.is_corrupted()
|| has_shallow_lock_file(&err)
{
repo_reinitialized.store(true, Ordering::Relaxed);
debug!(
"looks like this is a corrupt repository, reinitializing \
and trying again"
);
if oxide::reinitialize(repo_path).is_ok() {
continue;
}
}
return Err(err.into());
}
Ok(())
},
);
if repo_reinitialized.load(Ordering::Relaxed) {
*git2_repo = git2::Repository::open(git2_repo.path())?;
}
res
}
fn fetch_with_libgit2(
repo: &mut git2::Repository,
remote_url: &str,
refspecs: Vec<String>,
tags: bool,
shallow: gix::remote::fetch::Shallow,
gctx: &GlobalContext,
) -> CargoResult<()> {
debug!("doing a fetch for {remote_url}");
let git_config = git2::Config::open_default()?;
with_fetch_options(&git_config, remote_url, gctx, &mut |mut opts| {
if tags {
opts.download_tags(git2::AutotagOption::All);
}
if let gix::remote::fetch::Shallow::DepthAtRemote(depth) = shallow {
opts.depth(0i32.saturating_add_unsigned(depth.get()));
}
// The `fetch` operation here may fail spuriously due to a corrupt
// repository. It could also fail, however, for a whole slew of other
// reasons (aka network related reasons). We want Cargo to automatically
// recover from corrupt repositories, but we don't want Cargo to stomp
// over other legitimate errors.
//
// Consequently we save off the error of the `fetch` operation and if it
// looks like a "corrupt repo" error then we blow away the repo and try
// again. If it looks like any other kind of error, or if we've already
// blown away the repository, then we want to return the error as-is.
let mut repo_reinitialized = false;
loop {
debug!("initiating fetch of {refspecs:?} from {remote_url}");
let res = repo
.remote_anonymous(remote_url)?
.fetch(&refspecs, Some(&mut opts), None);
let err = match res {
Ok(()) => break,
Err(e) => e,
};
debug!("fetch failed: {}", err);
if !repo_reinitialized && matches!(err.class(), ErrorClass::Reference | ErrorClass::Odb)
{
repo_reinitialized = true;
debug!(
"looks like this is a corrupt repository, reinitializing \
and trying again"
);
if reinitialize(repo).is_ok() {
continue;
}
}
return Err(err.into());
}
Ok(())
})
}
/// Attempts to `git gc` a repository.
///
/// Cargo has a bunch of long-lived git repositories in its global cache and

View File

@ -4069,6 +4069,90 @@ src/lib.rs
.run();
}
#[cargo_test(public_network_test, requires_git)]
fn github_fastpath_error_message() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[dependencies]
bitflags = { git = "https://github.com/rust-lang/bitflags.git", rev="11111b376b93484341c68fbca3ca110ae5cd2790" }
"#,
)
.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 `https://github.com/rust-lang/bitflags.git`
fatal: remote [ERROR] upload-pack: not our ref 11111b376b93484341c68fbca3ca110ae5cd2790
[ERROR] failed to get `bitflags` as a dependency of package `foo v0.1.0 ([ROOT]/foo)`
Caused by:
failed to load source for dependency `bitflags`
Caused by:
Unable to update https://github.com/rust-lang/bitflags.git?rev=11111b376b93484341c68fbca3ca110ae5cd2790
Caused by:
failed to clone into: [ROOT]/home/.cargo/git/db/bitflags-[HASH]
Caused by:
revision 11111b376b93484341c68fbca3ca110ae5cd2790 not found
Caused by:
process didn't exit successfully: `git fetch --no-tags --force --update-head-ok [..]
"#]])
.run();
}
#[cargo_test(public_network_test)]
fn git_fetch_libgit2_error_message() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
[dependencies]
bitflags = { git = "https://github.com/rust-lang/bitflags.git", rev="11111b376b93484341c68fbca3ca110ae5cd2790" }
"#,
)
.file("src/lib.rs", "")
.build();
p.cargo("fetch")
.with_status(101)
.with_stderr_data(str![[r#"
[UPDATING] git repository `https://github.com/rust-lang/bitflags.git`
...
[ERROR] failed to get `bitflags` as a dependency of package `foo v0.1.0 ([ROOT]/foo)`
Caused by:
failed to load source for dependency `bitflags`
Caused by:
Unable to update https://github.com/rust-lang/bitflags.git?rev=11111b376b93484341c68fbca3ca110ae5cd2790
Caused by:
failed to clone into: [ROOT]/home/.cargo/git/db/bitflags-[HASH]
Caused by:
revision 11111b376b93484341c68fbca3ca110ae5cd2790 not found
...
"#]])
.run();
}
#[cargo_test]
fn git_worktree_with_bare_original_repo() {
let project = project().build();