cli: Display messages from remote after jj git push

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

@@ -17,7 +17,6 @@
 use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
 use std::process::Stdio;
-use std::sync::Mutex;
 use std::time::Instant;
 use std::{error, iter};
 
@@ -60,11 +59,11 @@ pub fn is_colocated_git_workspace(workspace: &Workspace, repo: &ReadonlyRepo) ->
     git_workdir.canonicalize().ok().as_deref() == dot_git_path.parent()
 }
 
-fn terminal_get_username(ui: &mut Ui, url: &str) -> Option<String> {
+fn terminal_get_username(ui: &Ui, url: &str) -> Option<String> {
     ui.prompt(&format!("Username for {url}")).ok()
 }
 
-fn terminal_get_pw(ui: &mut Ui, url: &str) -> Option<String> {
+fn terminal_get_pw(ui: &Ui, url: &str) -> Option<String> {
     ui.prompt_password(&format!("Passphrase for {url}: ")).ok()
 }
 
@@ -138,32 +137,115 @@ fn get_ssh_keys(_username: &str) -> Vec<PathBuf> {
     paths
 }
 
+// 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>,
+}
+
+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: &mut Ui,
+    ui: &Ui,
+    sideband_progress_callback: Option<SidebandProgressCallback<'_>>,
     f: impl FnOnce(git::RemoteCallbacks<'_>) -> T,
 ) -> T {
-    let mut ui = Mutex::new(ui);
-    let mut callback = None;
-    if let Some(mut output) = ui.get_mut().unwrap().progress_output() {
+    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 = |url: &str, _username: &str| {
-        pinentry_get_pw(url).or_else(|| terminal_get_pw(*ui.lock().unwrap(), url))
-    };
+    let mut get_pw =
+        |url: &str, _username: &str| pinentry_get_pw(url).or_else(|| terminal_get_pw(ui, url));
     callbacks.get_password = Some(&mut get_pw);
-    let mut get_user_pw = |url: &str| {
-        let ui = &mut *ui.lock().unwrap();
-        Some((terminal_get_username(ui, url)?, terminal_get_pw(ui, url)?))
-    };
+    let mut get_user_pw =
+        |url: &str| Some((terminal_get_username(ui, url)?, terminal_get_pw(ui, url)?));
     callbacks.get_username_password = Some(&mut get_user_pw);
     f(callbacks)
 }

cli/src/ui.rs

@@ -415,7 +415,7 @@ impl Ui {
                 .unwrap_or(false)
     }
 
-    pub fn prompt(&mut self, prompt: &str) -> io::Result<String> {
+    pub fn prompt(&self, prompt: &str) -> io::Result<String> {
         if !Self::can_prompt() {
             return Err(io::Error::new(
                 io::ErrorKind::Unsupported,
@@ -441,7 +441,7 @@ impl Ui {
 
     /// Repeat the given prompt until the input is one of the specified choices.
     pub fn prompt_choice(
-        &mut self,
+        &self,
         prompt: &str,
         choices: &[impl AsRef<str>],
         default: Option<&str>,
@@ -470,7 +470,7 @@ impl Ui {
     }
 
     /// Prompts for a yes-or-no response, with yes = true and no = false.
-    pub fn prompt_yes_no(&mut self, prompt: &str, default: Option<bool>) -> io::Result<bool> {
+    pub fn prompt_yes_no(&self, prompt: &str, default: Option<bool>) -> io::Result<bool> {
         let default_str = match &default {
             Some(true) => "(Yn)",
             Some(false) => "(yN)",
@@ -486,7 +486,7 @@ impl Ui {
         Ok(choice.starts_with(['y', 'Y']))
     }
 
-    pub fn prompt_password(&mut self, prompt: &str) -> io::Result<String> {
+    pub fn prompt_password(&self, prompt: &str) -> io::Result<String> {
         if !io::stdout().is_terminal() {
             return Err(io::Error::new(
                 io::ErrorKind::Unsupported,

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;