From 7ee0e720a7cbcea29d96c4fa1866315175c20402 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Feb 2023 21:23:48 +0000 Subject: [PATCH] Introduce a new error type to avoid having to downcast to various foreign 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. --- src/cargo/sources/git/mod.rs | 5 ++++ src/cargo/sources/git/oxide.rs | 32 +++++++++++------------ src/cargo/sources/git/utils.rs | 46 +++++++--------------------------- src/cargo/util/network.rs | 12 +-------- 4 files changed, 31 insertions(+), 64 deletions(-) diff --git a/src/cargo/sources/git/mod.rs b/src/cargo/sources/git/mod.rs index bd5ccd65175e..7d8b8fb19219 100644 --- a/src/cargo/sources/git/mod.rs +++ b/src/cargo/sources/git/mod.rs @@ -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; +} diff --git a/src/cargo/sources/git/oxide.rs b/src/cargo/sources/git/oxide.rs index 39c9ab081401..639e7199415e 100644 --- a/src/cargo/sources/git/oxide.rs +++ b/src/cargo/sources/git/oxide.rs @@ -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<()> { @@ -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, ) -> CargoResult<()> { match res { Ok(()) => Ok(()), - Err(mut err) => { - if let Some(e) = err - .downcast_ref::() - .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 \ @@ -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, }; @@ -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()) } } } @@ -211,7 +212,7 @@ pub fn open_repo( repo_path: &std::path::Path, config_overrides: Vec, purpose: OpenMode, -) -> CargoResult { +) -> Result { git::open_opts(repo_path, { let mut opts = git::open::Options::default(); opts.permissions.config = git::permissions::Config::all(); @@ -219,7 +220,6 @@ pub fn open_repo( 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 diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index a168ca1cb6e1..f35eb2f2df35 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -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") @@ -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::().map_or(false, |err| { - use git::open::Error; - matches!( - err, - Error::NotARepository { .. }| Error::Config(_) - ) - }) - || err - .downcast_ref::() - .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::() - .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!( diff --git a/src/cargo/util/network.rs b/src/cargo/util/network.rs index dfa878767542..80a5a1cbf1f0 100644 --- a/src/cargo/util/network.rs +++ b/src/cargo/util/network.rs @@ -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::() { - if err.is_spurious() { - return true; - } - } - if let Some(err) = err.downcast_ref::() { - if err.is_spurious() { - return true; - } - } - if let Some(err) = err.downcast_ref::() { + if let Some(err) = err.downcast_ref::() { if err.is_spurious() { return true; }