cli: Display message from remote after jj git push

The implementation of sideband progress message printing is aligned with
Git's implementation. See
https://github.com/git/git/blob/43072b4ca132437f21975ac6acc6b72dc22fd398/sideband.c#L178.

Closes martinvonz#3236.

CHANGELOG.md

@@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Timestamps are now shown in local timezone and without milliseconds and
   timezone offset by default.
 
+* `jj git push` now prints messages from the remote.
+
 ### Fixed bugs
 
 ## [0.15.1] - 2024-03-06

cli/src/commands/git.rs

@@ -50,7 +50,7 @@ use crate::command_error::{
 };
 use crate::git_util::{
     get_git_repo, is_colocated_git_workspace, print_failed_git_export, print_git_import_stats,
-    with_remote_git_callbacks,
+    with_remote_git_callbacks, GitSidebandProgressMessageWriter,
 };
 use crate::ui::Ui;
 
@@ -535,7 +535,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, None, |cb| {
             git::fetch(
                 tx.mut_repo(),
                 &git_repo,
@@ -758,7 +758,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, None, |cb| {
         git::fetch(
             fetch_tx.mut_repo(),
             &git_repo,
@@ -1049,7 +1049,11 @@ fn cmd_git_push(
         branch_updates,
         force_pushed_branches,
     };
-    with_remote_git_callbacks(ui, |cb| {
+    let mut writer = GitSidebandProgressMessageWriter::new(ui);
+    let mut sideband_progress_callback = |progress_message: &[u8]| {
+        _ = writer.write(ui, progress_message);
+    };
+    with_remote_git_callbacks(ui, Some(&mut sideband_progress_callback), |cb| {
         git::push_branches(tx.mut_repo(), &git_repo, &remote, &targets, cb)
     })
     .map_err(|err| match err {
@@ -1061,6 +1065,7 @@ fn cmd_git_push(
         ),
         _ => user_error(err),
     })?;
+    writer.flush(ui)?;
     tx.finish(ui, tx_description)?;
     Ok(())
 }

cli/src/git_util.rs

@@ -137,18 +137,109 @@ fn get_ssh_keys(_username: &str) -> Vec<PathBuf> {
     paths
 }
 
-pub fn with_remote_git_callbacks<T>(ui: &Ui, f: impl FnOnce(git::RemoteCallbacks<'_>) -> T) -> T {
-    let mut callback = None;
+// Based on Git's implementation: https://github.com/git/git/blob/43072b4ca132437f21975ac6acc6b72dc22fd398/sideband.c#L178
+pub struct GitSidebandProgressMessageWriter {
+    display_prefix: &'static [u8],
+    suffix: &'static [u8],
+    scratch: Vec<u8>,
+}
+
+#[allow(clippy::new_without_default)]
+impl GitSidebandProgressMessageWriter {
+    pub fn new(ui: &Ui) -> Self {
+        let is_terminal = ui.use_progress_indicator();
+
+        GitSidebandProgressMessageWriter {
+            display_prefix: "remote: ".as_bytes(),
+            suffix: if is_terminal { "\x1B[K" } else { "        " }.as_bytes(),
+            scratch: Vec::new(),
+        }
+    }
+
+    pub fn write(&mut self, ui: &Ui, progress_message: &[u8]) -> std::io::Result<()> {
+        let mut index = 0;
+        // Append a suffix to each nonempty line to clear the end of the screen line.
+        loop {
+            let Some(i) = progress_message[index..]
+                .iter()
+                .position(|&c| c == b'\r' || c == b'\n')
+                .map(|i| index + i)
+            else {
+                break;
+            };
+            let line_length = i - index;
+
+            // For messages sent across the packet boundary, there would be a nonempty
+            // "scratch" buffer from last call of this function, and there may be a leading
+            // CR/LF in this message. For this case we should add a clear-to-eol suffix to
+            // clean leftover letters we previously have written on the same line.
+            if !self.scratch.is_empty() && line_length == 0 {
+                self.scratch.extend_from_slice(self.suffix);
+            }
+
+            if self.scratch.is_empty() {
+                self.scratch.extend_from_slice(self.display_prefix);
+            }
+
+            // Do not add the clear-to-eol suffix to empty lines:
+            // For progress reporting we may receive a bunch of percentage updates
+            // followed by '\r' to remain on the same line, and at the end receive a single
+            // '\n' to move to the next line. We should preserve the final
+            // status report line by not appending clear-to-eol suffix to this single line
+            // break.
+            if line_length > 0 {
+                self.scratch.extend_from_slice(&progress_message[index..i]);
+                self.scratch.extend_from_slice(self.suffix);
+            }
+            self.scratch.extend_from_slice(&progress_message[i..i + 1]);
+
+            ui.stderr().write_all(&self.scratch)?;
+            self.scratch.clear();
+
+            index = i + 1;
+        }
+
+        // Add leftover message to "scratch" buffer to be printed in next call.
+        if index < progress_message.len() && progress_message[index] != 0 {
+            if self.scratch.is_empty() {
+                self.scratch.extend_from_slice(self.display_prefix);
+            }
+            self.scratch.extend_from_slice(&progress_message[index..]);
+        }
+
+        Ok(())
+    }
+
+    pub fn flush(&mut self, ui: &Ui) -> std::io::Result<()> {
+        if !self.scratch.is_empty() {
+            self.scratch.push(b'\n');
+            ui.stderr().write_all(&self.scratch)?;
+            self.scratch.clear();
+        }
+
+        Ok(())
+    }
+}
+
+type SidebandProgressCallback<'a> = &'a mut dyn FnMut(&[u8]);
+
+pub fn with_remote_git_callbacks<T>(
+    ui: &Ui,
+    sideband_progress_callback: Option<SidebandProgressCallback<'_>>,
+    f: impl FnOnce(git::RemoteCallbacks<'_>) -> T,
+) -> T {
+    let mut callbacks = git::RemoteCallbacks::default();
+    let mut progress_callback = None;
     if let Some(mut output) = ui.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));
+    callbacks.sideband_progress = sideband_progress_callback.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 =

lib/src/git.rs

@@ -1362,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)>>,
@@ -1381,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;