Skip to content

Commit

Permalink
Introduce a new error type to avoid having to downcast to various for…
Browse files Browse the repository at this point in the history
…eign error types.

Thanks to the collated error type it's possible to provide an
API over a set of errors to determine if the repository is likely
corrupted or the fetch operation failed spuriously.
  • Loading branch information
Byron committed Feb 9, 2023
1 parent 03bf94b commit 7ee0e72
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 64 deletions.
5 changes: 5 additions & 0 deletions src/cargo/sources/git/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ mod known_hosts;
mod oxide;
mod source;
mod utils;

pub mod fetch {
use git_repository as git;
pub type Error = git::env::collate::fetch::Error<git::refspec::parse::Error>;
}
32 changes: 16 additions & 16 deletions src/cargo/sources/git/oxide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn with_retry_and_progress(
&AtomicBool,
&mut git::progress::tree::Item,
&mut dyn FnMut(&git::bstr::BStr),
) -> CargoResult<()>
) -> Result<(), crate::sources::git::fetch::Error>
+ Send
+ Sync),
) -> CargoResult<()> {
Expand Down Expand Up @@ -132,21 +132,22 @@ fn translate_progress_to_bar(
}

fn amend_authentication_hints(
res: CargoResult<()>,
res: Result<(), crate::sources::git::fetch::Error>,
last_url_for_authentication: Option<git::bstr::BString>,
) -> CargoResult<()> {
match res {
Ok(()) => Ok(()),
Err(mut err) => {
if let Some(e) = err
.downcast_ref::<git::remote::fetch::prepare::Error>()
.and_then(|e| match e {
Err(err) => {
let e = match &err {
crate::sources::git::fetch::Error::PrepareFetch(
git::remote::fetch::prepare::Error::RefMap(
git::remote::ref_map::Error::Handshake(e),
) => Some(e),
_ => None,
})
{
git::remote::ref_map::Error::Handshake(err),
),
) => Some(err),
_ => None,
};
if let Some(e) = e {
use anyhow::Context;
let auth_message = match e {
git::protocol::handshake::Error::Credentials(_) => {
"\n* attempted to find username/password via \
Expand All @@ -164,7 +165,7 @@ fn amend_authentication_hints(
"if a proxy or similar is necessary `net.git-fetch-with-cli` may help here\n",
"https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli"
);
return Err(err.context(msg));
return Err(anyhow::Error::from(err)).context(msg);
}
_ => None,
};
Expand All @@ -185,10 +186,10 @@ fn amend_authentication_hints(
msg.push_str(
"https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli",
);
err = err.context(msg);
return Err(anyhow::Error::from(err)).context(msg);
}
}
Err(err)
Err(err.into())
}
}
}
Expand All @@ -211,15 +212,14 @@ pub fn open_repo(
repo_path: &std::path::Path,
config_overrides: Vec<BString>,
purpose: OpenMode,
) -> CargoResult<git::Repository> {
) -> Result<git::Repository, git::open::Error> {
git::open_opts(repo_path, {
let mut opts = git::open::Options::default();
opts.permissions.config = git::permissions::Config::all();
opts.permissions.config.git_binary = matches!(purpose, OpenMode::ForFetch);
opts.with(git::sec::Trust::Full)
.config_overrides(config_overrides)
})
.map_err(Into::into)
}

/// Convert `git` related cargo configuration into the respective `git` configuration which can be
Expand Down
46 changes: 9 additions & 37 deletions src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,14 +909,17 @@ pub fn fetch(
config_overrides.clone(),
oxide::OpenMode::ForFetch,
)
.map_err(anyhow::Error::from)
.map_err(crate::sources::git::fetch::Error::from)
.and_then(|repo| {
debug!("initiating fetch of {:?} from {}", refspecs, orig_url);
let url_for_authentication = &mut *url_for_authentication;
let remote = repo.remote_at(orig_url)?.with_refspecs(
refspecs.iter().map(|s| s.as_str()),
git::remote::Direction::Fetch,
)?;
let remote = repo
.remote_at(orig_url)?
.with_refspecs(
refspecs.iter().map(|s| s.as_str()),
git::remote::Direction::Fetch,
)
.map_err(crate::sources::git::fetch::Error::Other)?;
let url = remote
.url(git::remote::Direction::Fetch)
.expect("set at init")
Expand Down Expand Up @@ -951,38 +954,7 @@ pub fn fetch(
// 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.downcast_ref::<git::open::Error>().map_or(false, |err| {
use git::open::Error;
matches!(
err,
Error::NotARepository { .. }| Error::Config(_)
)
})
|| err
.downcast_ref::<git::remote::fetch::prepare::Error>()
.map_or(false, |err| match err {
git::remote::fetch::prepare::Error::RefMap(err) => {
use git::remote::ref_map::Error;
matches!(
err,
Error::ListRefs(_)
| Error::GatherTransportConfig { .. }
| Error::ConfigureCredentials(_)
)
}
_ => false,
})
|| err
.downcast_ref::<git::remote::fetch::Error>()
.map_or(false, |err| {
use git::remote::fetch::Error;
matches!(
err,
Error::PackThreads(_)
| Error::PackIndexVersion(_)
| Error::RemovePackKeepFile { .. }
)
})
&& err.is_corrupted()
{
repo_reinitialized.store(true, Ordering::Relaxed);
debug!(
Expand Down
12 changes: 1 addition & 11 deletions src/cargo/util/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,7 @@ fn maybe_spurious(err: &Error) -> bool {
use git::protocol::transport::IsSpuriousError;
use git_repository as git;

if let Some(err) = err.downcast_ref::<git::remote::connect::Error>() {
if err.is_spurious() {
return true;
}
}
if let Some(err) = err.downcast_ref::<git::remote::fetch::prepare::Error>() {
if err.is_spurious() {
return true;
}
}
if let Some(err) = err.downcast_ref::<git::remote::fetch::Error>() {
if let Some(err) = err.downcast_ref::<crate::sources::git::fetch::Error>() {
if err.is_spurious() {
return true;
}
Expand Down

0 comments on commit 7ee0e72

Please sign in to comment.