Skip to content

Commit

Permalink
cli: Display message from remote after jj git push
Browse files Browse the repository at this point in the history
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 #3236.
  • Loading branch information
bnjmnt4n committed Mar 20, 2024
1 parent c45d64e commit 908f006
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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
Expand Down
15 changes: 10 additions & 5 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ use crate::command_error::{
CommandError,
};
use crate::git_util::{
get_git_repo, is_colocated_git_workspace, print_failed_git_export, print_git_import_stats,
with_remote_git_callbacks,
get_git_repo, get_git_sideband_progress_message_writer, is_colocated_git_workspace,
print_failed_git_export, print_git_import_stats, with_remote_git_callbacks,
};
use crate::ui::Ui;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1049,7 +1049,12 @@ fn cmd_git_push(
branch_updates,
force_pushed_branches,
};
with_remote_git_callbacks(ui, |cb| {
let mut scratch: Vec<u8> = Vec::new();
let mut write_message = get_git_sideband_progress_message_writer();
let mut sideband_progress_callback = |ui: &Ui, progress_message: &[u8]| {
write_message(ui, &mut scratch, 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 {
Expand Down
90 changes: 84 additions & 6 deletions cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@

//! Git utilities shared by various commands.

use std::io::{Read, Write};
use std::io::{IsTerminal, Read, Write};
use std::path::{Path, PathBuf};
use std::process::Stdio;
use std::time::Instant;
use std::{error, iter};

use gix::bstr::ByteSlice;
use itertools::Itertools;
use jj_lib::git::{self, FailedRefExport, FailedRefExportReason, GitImportStats, RefName};
use jj_lib::git_backend::GitBackend;
Expand Down Expand Up @@ -137,18 +138,95 @@ 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 fn get_git_sideband_progress_message_writer() -> impl FnMut(&Ui, &mut Vec<u8>, &[u8]) {
let display_prefix = "remote: ";
let suffix = if std::io::stderr().is_terminal() {
"\x1B[K"
} else {
" "
};

|ui: &Ui, scratch: &mut Vec<u8>, progress_message: &[u8]| {
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..]
.find_char('\n')
.or_else(|| progress_message[index..].find_char('\r'))
.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 !scratch.is_empty() && line_length == 0 {
scratch.extend_from_slice(suffix.as_bytes());
}

if scratch.is_empty() {
scratch.extend_from_slice(display_prefix.as_bytes());
}

// 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 {
scratch.extend_from_slice(&progress_message[index..i]);
scratch.extend_from_slice(suffix.as_bytes());
}
scratch.extend_from_slice(&progress_message[i..i + 1]);

_ = write!(ui.stderr(), "{}", String::from_utf8_lossy(scratch));
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 scratch.is_empty() {
scratch.extend_from_slice(display_prefix.as_bytes());
}
scratch.extend_from_slice(&progress_message[index..]);
}
}
}

type SidebandProgressCallback<'a> = &'a mut dyn FnMut(&Ui, &[u8]);

pub fn with_remote_git_callbacks<T>(
ui: &Ui,
mut 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));
let mut internal_sideband_progress_callback = None;
if let Some(ref mut sideband_progress_callback) = sideband_progress_callback {
internal_sideband_progress_callback = Some(|data: &[u8]| {
sideband_progress_callback(ui, data);
});
}
callbacks.sideband_progress = internal_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 =
Expand Down
7 changes: 7 additions & 0 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)>>,
Expand All @@ -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;
Expand Down

0 comments on commit 908f006

Please sign in to comment.