Skip to content

Commit

Permalink
git-push: Display messages from remote
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 23, 2024
1 parent 2831459 commit 3034dbb
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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,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 {
Expand All @@ -1061,6 +1065,7 @@ fn cmd_git_push(
),
_ => user_error(err),
})?;
writer.flush(ui)?;
tx.finish(ui, tx_description)?;
Ok(())
}
Expand Down
100 changes: 95 additions & 5 deletions cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,108 @@ 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>,
}

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 =
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 3034dbb

Please sign in to comment.