From 81778aa8f2a6aafdbffd1ce3299bdf739de8d80d Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Sat, 10 Jul 2021 13:49:52 +0200 Subject: [PATCH 1/2] error if force push was rejected --- asyncgit/src/sync/remotes/callbacks.rs | 199 +++++++++++++++++++++++++ asyncgit/src/sync/remotes/mod.rs | 9 +- asyncgit/src/sync/remotes/push.rs | 141 ++---------------- asyncgit/src/sync/remotes/tags.rs | 20 ++- 4 files changed, 227 insertions(+), 142 deletions(-) create mode 100644 asyncgit/src/sync/remotes/callbacks.rs diff --git a/asyncgit/src/sync/remotes/callbacks.rs b/asyncgit/src/sync/remotes/callbacks.rs new file mode 100644 index 0000000000..54c0655637 --- /dev/null +++ b/asyncgit/src/sync/remotes/callbacks.rs @@ -0,0 +1,199 @@ +#![allow(dead_code)] + +use super::push::ProgressNotification; +use crate::{error::Result, sync::cred::BasicAuthCredential}; +use crossbeam_channel::Sender; +use git2::{Cred, Error as GitError, RemoteCallbacks}; +use std::sync::{Arc, Mutex}; + +/// +#[derive(Default, Clone)] +pub struct CallbackStats { + pub push_rejected_msg: Option<(String, String)>, +} + +/// +#[derive(Clone)] +pub struct Callbacks { + sender: Option>, + basic_credential: Option, + stats: Arc>, +} + +impl Callbacks { + /// + pub fn new( + sender: Option>, + basic_credential: Option, + ) -> Self { + let stats = Arc::new(Mutex::new(CallbackStats::default())); + + Self { + sender, + basic_credential, + stats, + } + } + + /// + pub fn get_stats(&self) -> Result { + let stats = self.stats.lock()?; + Ok(stats.clone()) + } + + /// + pub fn callbacks<'a>(&self) -> RemoteCallbacks<'a> { + let mut callbacks = RemoteCallbacks::new(); + let sender_clone = self.sender.clone(); + callbacks.push_transfer_progress( + move |current, total, bytes| { + log::debug!( + "progress: {}/{} ({} B)", + current, + total, + bytes, + ); + + sender_clone.clone().map(|sender| { + sender.send(ProgressNotification::PushTransfer { + current, + total, + bytes, + }) + }); + }, + ); + + let sender_clone = self.sender.clone(); + callbacks.update_tips(move |name, a, b| { + log::debug!("update tips: '{}' [{}] [{}]", name, a, b); + + sender_clone.clone().map(|sender| { + sender.send(ProgressNotification::UpdateTips { + name: name.to_string(), + a: a.into(), + b: b.into(), + }) + }); + true + }); + + let this = self.clone(); + callbacks.transfer_progress(move |p| { + this.transfer_progress(&p); + true + }); + + let this = self.clone(); + callbacks.pack_progress(move |stage, current, total| { + this.pack_progress(stage, total, current); + }); + + let this = self.clone(); + callbacks.push_update_reference(move |reference, msg| { + this.push_update_reference(reference, msg); + Ok(()) + }); + + let mut first_call_to_credentials = true; + let creds = self.basic_credential.clone(); + // This boolean is used to avoid multiple calls to credentials callback. + // If credentials are bad, we don't ask the user to re-fill their creds. We push an error and they will be able to restart their action (for example a push) and retype their creds. + // This behavior is explained in a issue on git2-rs project : https://github.com/rust-lang/git2-rs/issues/347 + // An implementation reference is done in cargo : https://github.com/rust-lang/cargo/blob/9fb208dddb12a3081230a5fd8f470e01df8faa25/src/cargo/sources/git/utils.rs#L588 + // There is also a guide about libgit2 authentication : https://libgit2.org/docs/guides/authentication/ + callbacks.credentials( + move |url, username_from_url, allowed_types| { + log::debug!( + "creds: '{}' {:?} ({:?})", + url, + username_from_url, + allowed_types + ); + if first_call_to_credentials { + first_call_to_credentials = false; + } else { + return Err(GitError::from_str("Bad credentials.")); + } + + match &creds { + _ if allowed_types.is_ssh_key() => { + match username_from_url { + Some(username) => { + Cred::ssh_key_from_agent(username) + } + None => Err(GitError::from_str( + " Couldn't extract username from url.", + )), + } + } + Some(BasicAuthCredential { + username: Some(user), + password: Some(pwd), + }) if allowed_types.is_user_pass_plaintext() => { + Cred::userpass_plaintext(user, pwd) + } + Some(BasicAuthCredential { + username: Some(user), + password: _, + }) if allowed_types.is_username() => { + Cred::username(user) + } + _ if allowed_types.is_default() => Cred::default(), + _ => Err(GitError::from_str( + "Couldn't find credentials", + )), + } + }, + ); + + callbacks + } + + fn push_update_reference( + &self, + reference: &str, + msg: Option<&str>, + ) { + log::debug!( + "push_update_reference: '{}' {:?}", + reference, + msg + ); + + if let Ok(mut stats) = self.stats.lock() { + stats.push_rejected_msg = msg + .map(|msg| (reference.to_string(), msg.to_string())); + } + } + + fn pack_progress( + &self, + stage: git2::PackBuilderStage, + total: usize, + current: usize, + ) { + log::debug!("packing: {:?} - {}/{}", stage, current, total); + self.sender.clone().map(|sender| { + sender.send(ProgressNotification::Packing { + stage, + total, + current, + }) + }); + } + + fn transfer_progress(&self, p: &git2::Progress) { + log::debug!( + "transfer: {}/{}", + p.received_objects(), + p.total_objects() + ); + self.sender.clone().map(|sender| { + sender.send(ProgressNotification::Transfer { + objects: p.received_objects(), + total_objects: p.total_objects(), + }) + }); + } +} diff --git a/asyncgit/src/sync/remotes/mod.rs b/asyncgit/src/sync/remotes/mod.rs index d82905c28d..1366ad83c8 100644 --- a/asyncgit/src/sync/remotes/mod.rs +++ b/asyncgit/src/sync/remotes/mod.rs @@ -1,5 +1,6 @@ //! +mod callbacks; pub(crate) mod push; pub(crate) mod tags; @@ -12,10 +13,10 @@ use crate::{ }; use crossbeam_channel::Sender; use git2::{BranchType, FetchOptions, Repository}; -use push::remote_callbacks; use scopetime::scope_time; use utils::bytes2string; +pub use callbacks::Callbacks; pub use tags::tags_missing_remote; /// origin @@ -93,10 +94,8 @@ pub(crate) fn fetch( let mut remote = repo.find_remote(&remote_name)?; let mut options = FetchOptions::new(); - options.remote_callbacks(remote_callbacks( - progress_sender, - basic_credential, - )); + let callbacks = Callbacks::new(progress_sender, basic_credential); + options.remote_callbacks(callbacks.callbacks()); remote.fetch(&[branch], Some(&mut options), None)?; diff --git a/asyncgit/src/sync/remotes/push.rs b/asyncgit/src/sync/remotes/push.rs index 0f7887a0f5..ea4b303579 100644 --- a/asyncgit/src/sync/remotes/push.rs +++ b/asyncgit/src/sync/remotes/push.rs @@ -1,17 +1,14 @@ use super::utils; use crate::{ - error::Result, + error::{Error, Result}, progress::ProgressPercent, sync::{ branch::branch_set_upstream, cred::BasicAuthCredential, - CommitId, + remotes::Callbacks, CommitId, }, }; use crossbeam_channel::Sender; -use git2::{ - Cred, Error as GitError, PackBuilderStage, PushOptions, - RemoteCallbacks, -}; +use git2::{PackBuilderStage, PushOptions}; use scopetime::scope_time; /// @@ -108,10 +105,8 @@ pub(crate) fn push( let mut options = PushOptions::new(); - options.remote_callbacks(remote_callbacks( - progress_sender, - basic_credential, - )); + let callbacks = Callbacks::new(progress_sender, basic_credential); + options.remote_callbacks(callbacks.callbacks()); options.packbuilder_parallelism(0); let branch_name = format!("refs/heads/{}", branch); @@ -123,125 +118,19 @@ pub(crate) fn push( } else { remote.push(&[branch_name.as_str()], Some(&mut options))?; } - branch_set_upstream(&repo, branch)?; - - Ok(()) -} - -#[allow(clippy::redundant_pub_crate)] -pub(crate) fn remote_callbacks<'a>( - sender: Option>, - basic_credential: Option, -) -> RemoteCallbacks<'a> { - let mut callbacks = RemoteCallbacks::new(); - let sender_clone = sender.clone(); - callbacks.push_transfer_progress(move |current, total, bytes| { - log::debug!("progress: {}/{} ({} B)", current, total, bytes,); - - sender_clone.clone().map(|sender| { - sender.send(ProgressNotification::PushTransfer { - current, - total, - bytes, - }) - }); - }); - - let sender_clone = sender.clone(); - callbacks.update_tips(move |name, a, b| { - log::debug!("update tips: '{}' [{}] [{}]", name, a, b); - - sender_clone.clone().map(|sender| { - sender.send(ProgressNotification::UpdateTips { - name: name.to_string(), - a: a.into(), - b: b.into(), - }) - }); - true - }); - - let sender_clone = sender.clone(); - callbacks.transfer_progress(move |p| { - log::debug!( - "transfer: {}/{}", - p.received_objects(), - p.total_objects() - ); - - sender_clone.clone().map(|sender| { - sender.send(ProgressNotification::Transfer { - objects: p.received_objects(), - total_objects: p.total_objects(), - }) - }); - true - }); - callbacks.pack_progress(move |stage, current, total| { - log::debug!("packing: {:?} - {}/{}", stage, current, total); - - sender.clone().map(|sender| { - sender.send(ProgressNotification::Packing { - stage, - total, - current, - }) - }); - }); - - let mut first_call_to_credentials = true; - // This boolean is used to avoid multiple calls to credentials callback. - // If credentials are bad, we don't ask the user to re-fill their creds. We push an error and they will be able to restart their action (for example a push) and retype their creds. - // This behavior is explained in a issue on git2-rs project : https://github.com/rust-lang/git2-rs/issues/347 - // An implementation reference is done in cargo : https://github.com/rust-lang/cargo/blob/9fb208dddb12a3081230a5fd8f470e01df8faa25/src/cargo/sources/git/utils.rs#L588 - // There is also a guide about libgit2 authentication : https://libgit2.org/docs/guides/authentication/ - callbacks.credentials( - move |url, username_from_url, allowed_types| { - log::debug!( - "creds: '{}' {:?} ({:?})", - url, - username_from_url, - allowed_types - ); - if first_call_to_credentials { - first_call_to_credentials = false; - } else { - return Err(GitError::from_str("Bad credentials.")); - } + if let Some((reference, msg)) = + callbacks.get_stats()?.push_rejected_msg + { + return Err(Error::Generic(format!( + "push to '{}' rejected: {}", + reference, msg + ))); + } - match &basic_credential { - _ if allowed_types.is_ssh_key() => { - match username_from_url { - Some(username) => { - Cred::ssh_key_from_agent(username) - } - None => Err(GitError::from_str( - " Couldn't extract username from url.", - )), - } - } - Some(BasicAuthCredential { - username: Some(user), - password: Some(pwd), - }) if allowed_types.is_user_pass_plaintext() => { - Cred::userpass_plaintext(user, pwd) - } - Some(BasicAuthCredential { - username: Some(user), - password: _, - }) if allowed_types.is_username() => { - Cred::username(user) - } - _ if allowed_types.is_default() => Cred::default(), - _ => Err(GitError::from_str( - "Couldn't find credentials", - )), - } - }, - ); + branch_set_upstream(&repo, branch)?; - callbacks + Ok(()) } #[cfg(test)] diff --git a/asyncgit/src/sync/remotes/tags.rs b/asyncgit/src/sync/remotes/tags.rs index de819ed22b..5b4cb30fd4 100644 --- a/asyncgit/src/sync/remotes/tags.rs +++ b/asyncgit/src/sync/remotes/tags.rs @@ -1,12 +1,10 @@ //! -use super::{ - push::{remote_callbacks, AsyncProgress}, - utils, -}; +use super::{push::AsyncProgress, utils}; use crate::{ - error::Result, progress::ProgressPercent, - sync::cred::BasicAuthCredential, + error::Result, + progress::ProgressPercent, + sync::{cred::BasicAuthCredential, remotes::Callbacks}, }; use crossbeam_channel::Sender; use git2::{Direction, PushOptions}; @@ -54,9 +52,10 @@ fn remote_tag_refs( let repo = utils::repo(repo_path)?; let mut remote = repo.find_remote(remote)?; + let callbacks = Callbacks::new(None, basic_credential); let conn = remote.connect_auth( Direction::Fetch, - Some(remote_callbacks(None, basic_credential)), + Some(callbacks.callbacks()), None, )?; @@ -127,10 +126,9 @@ pub fn push_tags( for (idx, tag) in tags_missing.into_iter().enumerate() { let mut options = PushOptions::new(); - options.remote_callbacks(remote_callbacks( - None, - basic_credential.clone(), - )); + let callbacks = + Callbacks::new(None, basic_credential.clone()); + options.remote_callbacks(callbacks.callbacks()); options.packbuilder_parallelism(0); remote.push(&[tag.as_str()], Some(&mut options))?; From 93d733ebb2aaa7867cdc69ca6bfb99e362a91e57 Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Sat, 10 Jul 2021 14:10:42 +0200 Subject: [PATCH 2/2] more cleanup --- CHANGELOG.md | 1 + asyncgit/src/sync/remotes/callbacks.rs | 166 ++++++++++++++----------- 2 files changed, 96 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87110608f0..b05e5df08d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - new `undo-last-commit` command [[@remique](https://github.com/remique)] ([#758](https://github.com/extrawurst/gitui/issues/758)) - taglist: show arrow-symbol on tags not present on origin [[@cruessler](https://github.com/cruessler)] ([#776](https://github.com/extrawurst/gitui/issues/776)) - new quit key `[q]` ([#771](https://github.com/extrawurst/gitui/issues/771)) +- proper error message if remote rejects force push ([#801](https://github.com/extrawurst/gitui/issues/801)) ## Fixed - openssl vendoring broken on macos ([#772](https://github.com/extrawurst/gitui/issues/772)) diff --git a/asyncgit/src/sync/remotes/callbacks.rs b/asyncgit/src/sync/remotes/callbacks.rs index 54c0655637..4f8b82c532 100644 --- a/asyncgit/src/sync/remotes/callbacks.rs +++ b/asyncgit/src/sync/remotes/callbacks.rs @@ -4,7 +4,10 @@ use super::push::ProgressNotification; use crate::{error::Result, sync::cred::BasicAuthCredential}; use crossbeam_channel::Sender; use git2::{Cred, Error as GitError, RemoteCallbacks}; -use std::sync::{Arc, Mutex}; +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, +}; /// #[derive(Default, Clone)] @@ -18,6 +21,7 @@ pub struct Callbacks { sender: Option>, basic_credential: Option, stats: Arc>, + first_call_to_credentials: Arc, } impl Callbacks { @@ -32,6 +36,9 @@ impl Callbacks { sender, basic_credential, stats, + first_call_to_credentials: Arc::new(AtomicBool::new( + true, + )), } } @@ -44,37 +51,17 @@ impl Callbacks { /// pub fn callbacks<'a>(&self) -> RemoteCallbacks<'a> { let mut callbacks = RemoteCallbacks::new(); - let sender_clone = self.sender.clone(); + + let this = self.clone(); callbacks.push_transfer_progress( move |current, total, bytes| { - log::debug!( - "progress: {}/{} ({} B)", - current, - total, - bytes, - ); - - sender_clone.clone().map(|sender| { - sender.send(ProgressNotification::PushTransfer { - current, - total, - bytes, - }) - }); + this.push_transfer_progress(current, total, bytes); }, ); - let sender_clone = self.sender.clone(); + let this = self.clone(); callbacks.update_tips(move |name, a, b| { - log::debug!("update tips: '{}' [{}] [{}]", name, a, b); - - sender_clone.clone().map(|sender| { - sender.send(ProgressNotification::UpdateTips { - name: name.to_string(), - a: a.into(), - b: b.into(), - }) - }); + this.update_tips(name, a, b); true }); @@ -95,55 +82,14 @@ impl Callbacks { Ok(()) }); - let mut first_call_to_credentials = true; - let creds = self.basic_credential.clone(); - // This boolean is used to avoid multiple calls to credentials callback. - // If credentials are bad, we don't ask the user to re-fill their creds. We push an error and they will be able to restart their action (for example a push) and retype their creds. - // This behavior is explained in a issue on git2-rs project : https://github.com/rust-lang/git2-rs/issues/347 - // An implementation reference is done in cargo : https://github.com/rust-lang/cargo/blob/9fb208dddb12a3081230a5fd8f470e01df8faa25/src/cargo/sources/git/utils.rs#L588 - // There is also a guide about libgit2 authentication : https://libgit2.org/docs/guides/authentication/ + let this = self.clone(); callbacks.credentials( move |url, username_from_url, allowed_types| { - log::debug!( - "creds: '{}' {:?} ({:?})", + this.credentials( url, username_from_url, - allowed_types - ); - if first_call_to_credentials { - first_call_to_credentials = false; - } else { - return Err(GitError::from_str("Bad credentials.")); - } - - match &creds { - _ if allowed_types.is_ssh_key() => { - match username_from_url { - Some(username) => { - Cred::ssh_key_from_agent(username) - } - None => Err(GitError::from_str( - " Couldn't extract username from url.", - )), - } - } - Some(BasicAuthCredential { - username: Some(user), - password: Some(pwd), - }) if allowed_types.is_user_pass_plaintext() => { - Cred::userpass_plaintext(user, pwd) - } - Some(BasicAuthCredential { - username: Some(user), - password: _, - }) if allowed_types.is_username() => { - Cred::username(user) - } - _ if allowed_types.is_default() => Cred::default(), - _ => Err(GitError::from_str( - "Couldn't find credentials", - )), - } + allowed_types, + ) }, ); @@ -196,4 +142,82 @@ impl Callbacks { }) }); } + + fn update_tips(&self, name: &str, a: git2::Oid, b: git2::Oid) { + log::debug!("update tips: '{}' [{}] [{}]", name, a, b); + self.sender.clone().map(|sender| { + sender.send(ProgressNotification::UpdateTips { + name: name.to_string(), + a: a.into(), + b: b.into(), + }) + }); + } + + fn push_transfer_progress( + &self, + current: usize, + total: usize, + bytes: usize, + ) { + log::debug!("progress: {}/{} ({} B)", current, total, bytes,); + self.sender.clone().map(|sender| { + sender.send(ProgressNotification::PushTransfer { + current, + total, + bytes, + }) + }); + } + + // If credentials are bad, we don't ask the user to re-fill their creds. We push an error and they will be able to restart their action (for example a push) and retype their creds. + // This behavior is explained in a issue on git2-rs project : https://github.com/rust-lang/git2-rs/issues/347 + // An implementation reference is done in cargo : https://github.com/rust-lang/cargo/blob/9fb208dddb12a3081230a5fd8f470e01df8faa25/src/cargo/sources/git/utils.rs#L588 + // There is also a guide about libgit2 authentication : https://libgit2.org/docs/guides/authentication/ + fn credentials( + &self, + url: &str, + username_from_url: Option<&str>, + allowed_types: git2::CredentialType, + ) -> std::result::Result { + log::debug!( + "creds: '{}' {:?} ({:?})", + url, + username_from_url, + allowed_types + ); + + // This boolean is used to avoid multiple calls to credentials callback. + if self.first_call_to_credentials.load(Ordering::Relaxed) { + self.first_call_to_credentials + .store(false, Ordering::Relaxed); + } else { + return Err(GitError::from_str("Bad credentials.")); + } + + match &self.basic_credential { + _ if allowed_types.is_ssh_key() => { + match username_from_url { + Some(username) => { + Cred::ssh_key_from_agent(username) + } + None => Err(GitError::from_str( + " Couldn't extract username from url.", + )), + } + } + Some(BasicAuthCredential { + username: Some(user), + password: Some(pwd), + }) if allowed_types.is_user_pass_plaintext() => { + Cred::userpass_plaintext(user, pwd) + } + Some(BasicAuthCredential { + username: Some(user), + password: _, + }) if allowed_types.is_username() => Cred::username(user), + _ if allowed_types.is_default() => Cred::default(), + _ => Err(GitError::from_str("Couldn't find credentials")), + } + } }