Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 17, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3edba9
test(git): added tests for issue rust-lang#14621
dacianpascu06 Nov 17, 2024
309b2ce
refactor(git): extracted fetch logic into different functions
dacianpascu06 Nov 17, 2024
43a5dee
fix(git): added extra error context for a revision that doesn't exist
dacianpascu06 Nov 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
329 changes: 180 additions & 149 deletions src/cargo/sources/git/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,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),
Expand Down Expand Up @@ -984,6 +987,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()
Expand All @@ -1006,158 +1010,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);

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;
}
}
fetch_with_libgit2(repo, remote_url, refspecs, tags, shallow, gctx)
};

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
Expand Down Expand Up @@ -1227,6 +1093,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
Expand Down
Loading