with_remote_git_callbacks: Add new option to print sideband progress …

…messages

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(())
 }

cli/src/git_util.rs

@@ -140,20 +140,37 @@ fn get_ssh_keys(_username: &str) -> Vec<PathBuf> {
 
 pub fn with_remote_git_callbacks<T>(
     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| {

lib/src/git.rs

@@ -1223,7 +1223,7 @@ pub fn push_branches(
     remote_name: &str,
     targets: &GitBranchPushTargets,
     callbacks: RemoteCallbacks<'_>,
-) -> Result<Option<String>, 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<Option<String>, 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<Option<String>, 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<String> = 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<PathBuf>>,
     pub get_password: Option<&'a mut dyn FnMut(&str, &str) -> Option<String>>,
     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;

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();