From 5ed17b7a8a1d83b82c37d3e33e8af5d6fafea9c2 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Thu, 7 Mar 2024 18:45:43 +0800 Subject: [PATCH] with_remote_git_callbacks: Add new option to print sideband progress messages --- cli/src/commands/git.rs | 11 +++-------- cli/src/git_util.rs | 25 +++++++++++++++++++++---- lib/src/git.rs | 29 +++++++++++++---------------- lib/tests/test_git.rs | 10 +++++----- 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 84ccfce2576..2f63fe4957f 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -536,7 +536,7 @@ fn cmd_git_fetch( }; let mut tx = workspace_command.start_transaction(); for remote in &remotes { - let stats = with_remote_git_callbacks(ui, |cb| { + let stats = with_remote_git_callbacks(ui, false, |cb| { git::fetch( tx.mut_repo(), &git_repo, @@ -759,7 +759,7 @@ fn do_git_clone( git_repo.remote(remote_name, source).unwrap(); let mut fetch_tx = workspace_command.start_transaction(); - let stats = with_remote_git_callbacks(ui, |cb| { + let stats = with_remote_git_callbacks(ui, false, |cb| { git::fetch( fetch_tx.mut_repo(), &git_repo, @@ -1089,7 +1089,7 @@ fn cmd_git_push( branch_updates, force_pushed_branches, }; - let remote_message = with_remote_git_callbacks(ui, |cb| { + with_remote_git_callbacks(ui, true, |cb| { git::push_branches(tx.mut_repo(), &git_repo, &remote, &targets, cb) }) .map_err(|err| match err { @@ -1101,11 +1101,6 @@ fn cmd_git_push( ), _ => user_error(err), })?; - if let Some(remote_message) = remote_message { - for line in remote_message.lines() { - writeln!(ui.stderr(), "remote: {line}")?; - } - } tx.finish(ui, tx_description)?; Ok(()) } diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 5b4f0b834e0..41c703a1735 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -140,20 +140,37 @@ fn get_ssh_keys(_username: &str) -> Vec { pub fn with_remote_git_callbacks( ui: &mut Ui, + print_sideband_messages: bool, f: impl FnOnce(git::RemoteCallbacks<'_>) -> T, ) -> T { + let mut callbacks = git::RemoteCallbacks::default(); let mut ui = Mutex::new(ui); - let mut callback = None; + let mut progress_callback = None; if let Some(mut output) = ui.get_mut().unwrap().progress_output() { let mut progress = Progress::new(Instant::now()); - callback = Some(move |x: &git::Progress| { + progress_callback = Some(move |x: &git::Progress| { _ = progress.update(Instant::now(), x, &mut output); }); } - let mut callbacks = git::RemoteCallbacks::default(); - callbacks.progress = callback + callbacks.progress = progress_callback .as_mut() .map(|x| x as &mut dyn FnMut(&git::Progress)); + let mut sideband_progress_callback = None; + if print_sideband_messages { + sideband_progress_callback = Some(|progress: &[u8]| { + let message = String::from_utf8_lossy(progress); + if message.contains("\n") { + for line in message.lines() { + _ = writeln!(ui.lock().unwrap().stderr(), "remote: {line}"); + } + } else { + _ = write!(ui.lock().unwrap().stderr(), "remote: {message}"); + } + }); + } + callbacks.sideband_progress = sideband_progress_callback + .as_mut() + .map(|x| x as &mut dyn FnMut(&[u8])); let mut get_ssh_keys = get_ssh_keys; // Coerce to unit fn type callbacks.get_ssh_keys = Some(&mut get_ssh_keys); let mut get_pw = |url: &str, _username: &str| { diff --git a/lib/src/git.rs b/lib/src/git.rs index a05f4d2b320..ee3748c18bf 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -1223,7 +1223,7 @@ pub fn push_branches( remote_name: &str, targets: &GitBranchPushTargets, callbacks: RemoteCallbacks<'_>, -) -> Result, GitPushError> { +) -> Result<(), GitPushError> { let ref_updates = targets .branch_updates .iter() @@ -1233,7 +1233,7 @@ pub fn push_branches( new_target: update.new_target.clone(), }) .collect_vec(); - let remote_message = push_updates(git_repo, remote_name, &ref_updates, callbacks)?; + push_updates(git_repo, remote_name, &ref_updates, callbacks)?; // TODO: add support for partially pushed refs? we could update the view // excluding rejected refs, but the transaction would be aborted anyway @@ -1248,7 +1248,7 @@ pub fn push_branches( mut_repo.set_remote_branch(branch_name, remote_name, new_remote_ref); } - Ok(remote_message) + Ok(()) } /// Pushes the specified Git refs without updating the repo view. @@ -1257,7 +1257,7 @@ pub fn push_updates( remote_name: &str, updates: &[GitRefUpdate], callbacks: RemoteCallbacks<'_>, -) -> Result, GitPushError> { +) -> Result<(), GitPushError> { let mut temp_refs = vec![]; let mut qualified_remote_refs = vec![]; let mut refspecs = vec![]; @@ -1310,7 +1310,7 @@ fn push_refs( qualified_remote_refs: &[&str], refspecs: &[String], callbacks: RemoteCallbacks<'_>, -) -> Result, GitPushError> { +) -> Result<(), GitPushError> { if remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO { return Err(GitPushError::RemoteReservedForLocalGitRepo); } @@ -1322,21 +1322,11 @@ fn push_refs( } })?; let mut remaining_remote_refs: HashSet<_> = qualified_remote_refs.iter().copied().collect(); - let mut remote_message: Option = None; let mut push_options = git2::PushOptions::new(); let mut proxy_options = git2::ProxyOptions::new(); proxy_options.auto(); push_options.proxy_options(proxy_options); let mut callbacks = callbacks.into_git(); - callbacks.sideband_progress(|progress| { - let message = String::from_utf8_lossy(progress); - if let Some(remote_message) = &mut remote_message { - remote_message.push_str(&message); - } else { - remote_message = Some(message.to_string()); - } - true - }); callbacks.push_update_reference(|refname, status| { // The status is Some if the ref update was rejected if status.is_none() { @@ -1355,7 +1345,7 @@ fn push_refs( })?; drop(push_options); if remaining_remote_refs.is_empty() { - Ok(remote_message) + Ok(()) } else { Err(GitPushError::RefUpdateRejected( remaining_remote_refs @@ -1372,6 +1362,7 @@ fn push_refs( #[allow(clippy::type_complexity)] pub struct RemoteCallbacks<'a> { pub progress: Option<&'a mut dyn FnMut(&Progress)>, + pub sideband_progress: Option<&'a mut dyn FnMut(&[u8])>, pub get_ssh_keys: Option<&'a mut dyn FnMut(&str) -> Vec>, pub get_password: Option<&'a mut dyn FnMut(&str, &str) -> Option>, pub get_username_password: Option<&'a mut dyn FnMut(&str) -> Option<(String, String)>>, @@ -1391,6 +1382,12 @@ impl<'a> RemoteCallbacks<'a> { true }); } + if let Some(sideband_progress_cb) = self.sideband_progress { + callbacks.sideband_progress(move |data| { + sideband_progress_cb(data); + true + }); + } // TODO: We should expose the callbacks to the caller instead -- the library // crate shouldn't read environment variables. let mut tried_ssh_agent = false; diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 458ac3e700f..5bdf2c09707 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -2383,7 +2383,7 @@ fn test_push_branches_success() { &targets, git::RemoteCallbacks::default(), ); - assert_eq!(result, Ok(None)); + assert_eq!(result, Ok(())); // Check that the ref got updated in the source repo let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap(); @@ -2453,7 +2453,7 @@ fn test_push_branches_deletion() { &targets, git::RemoteCallbacks::default(), ); - assert_eq!(result, Ok(None)); + assert_eq!(result, Ok(())); // Check that the ref got deleted in the source repo assert!(source_repo.find_reference("refs/heads/main").is_err()); @@ -2511,7 +2511,7 @@ fn test_push_branches_mixed_deletion_and_addition() { &targets, git::RemoteCallbacks::default(), ); - assert_eq!(result, Ok(None)); + assert_eq!(result, Ok(())); // Check that the topic ref got updated in the source repo let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap(); @@ -2606,7 +2606,7 @@ fn test_push_branches_not_fast_forward_with_force() { &targets, git::RemoteCallbacks::default(), ); - assert_eq!(result, Ok(None)); + assert_eq!(result, Ok(())); // Check that the ref got updated in the source repo let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap(); @@ -2633,7 +2633,7 @@ fn test_push_updates_success() { }], git::RemoteCallbacks::default(), ); - assert_eq!(result, Ok(None)); + assert_eq!(result, Ok(())); // Check that the ref got updated in the source repo let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap();