From 852e6f6aed74449250456fdc138438174ae09f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= Date: Sat, 3 Oct 2026 01:42:06 +0200 Subject: [PATCH] Share progress reporting for bundle synchronization --- .github/workflows/ci.yml | 1 + Cargo.lock | 1 + crates/rb-cli/Cargo.toml | 1 + crates/rb-cli/src/commands/exec.rs | 29 +--- crates/rb-cli/src/commands/mod.rs | 1 + crates/rb-cli/src/commands/sync.rs | 98 +----------- crates/rb-cli/src/commands/sync_report.rs | 144 ++++++++++++++++++ crates/rb-cli/tests/sync_ui_test.py | 175 ++++++++++++++++++++++ crates/rb-core/src/bundler/mod.rs | 168 +++++++++++++-------- crates/rb-progress/src/render.rs | 41 ++++- crates/rb-progress/src/reporter.rs | 32 +++- crates/rb-progress/src/reporter/tests.rs | 101 +++++++++++++ crates/rb-progress/src/state.rs | 11 ++ crates/rb-progress/src/task.rs | 9 +- crates/rb-progress/tests/pty_ui_test.py | 9 +- spec/commands/exec/bundler_spec.sh | 10 +- spec/commands/sync_spec.sh | 18 +-- tests/commands/Sync.Integration.Tests.ps1 | 4 +- 18 files changed, 634 insertions(+), 219 deletions(-) create mode 100644 crates/rb-cli/src/commands/sync_report.rs create mode 100644 crates/rb-cli/tests/sync_ui_test.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 595858d..7ad4977 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -133,6 +133,7 @@ jobs: cargo test -p rb-progress --features rb-task cargo build -p rb-progress --features rb-task --example progress_ui_probe --example tasks python3 crates/rb-progress/tests/pty_ui_test.py target/debug/examples/progress_ui_probe target/debug/examples/parallel + python3 crates/rb-cli/tests/sync_ui_test.py target/debug/rb - name: Build release run: cargo build --release diff --git a/Cargo.lock b/Cargo.lock index 3942645..a7a26ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -556,6 +556,7 @@ dependencies = [ "kdl", "log", "rb-core", + "rb-progress", "rb-tests", "semver", "serde", diff --git a/crates/rb-cli/Cargo.toml b/crates/rb-cli/Cargo.toml index 4ef6e48..075234d 100644 --- a/crates/rb-cli/Cargo.toml +++ b/crates/rb-cli/Cargo.toml @@ -22,6 +22,7 @@ path = "src/bin/rb.rs" clap = { version = "4.0", features = ["derive", "color", "help", "usage", "env"] } clap_complete = "4.0" rb-core = { path = "../rb-core" } +rb-progress = { path = "../rb-progress" } home = "0.5" colored = "2.0" log = "0.4" diff --git a/crates/rb-cli/src/commands/exec.rs b/crates/rb-cli/src/commands/exec.rs index 26b1b44..cebca32 100644 --- a/crates/rb-cli/src/commands/exec.rs +++ b/crates/rb-cli/src/commands/exec.rs @@ -1,4 +1,4 @@ -use colored::*; +use super::sync_report::{Completion, report_sync, sync_reporter}; use log::{debug, info}; use rb_core::butler::{ButlerError, ButlerRuntime, Command}; @@ -24,30 +24,9 @@ pub fn exec_command(butler: ButlerRuntime, program_args: Vec) -> Result< if let Some(bundler_runtime) = butler.bundler_runtime() { match bundler_runtime.check_sync(&butler) { Ok(false) => { - println!( - "{} {}", - "🎩 Butler Notice:".bright_blue().bold(), - "Bundler environment requires synchronization. Preparing now...".dimmed() - ); - - match bundler_runtime.synchronize(&butler, |line| { - println!("{}", line.dimmed()); - }) { - Ok(_) => { - println!( - "{} {}", - "✨".bright_green(), - "Environment meticulously prepared. Proceeding with execution..." - .green() - ); - } - Err(e) => { - return Err(ButlerError::General(format!( - "Failed to prepare bundler environment: {}", - e - ))); - } - } + report_sync(sync_reporter(), Completion::Compact, |handler| { + bundler_runtime.synchronize_with_events(&butler, handler) + })?; } Ok(true) => { debug!("Bundler environment already synchronized"); diff --git a/crates/rb-cli/src/commands/mod.rs b/crates/rb-cli/src/commands/mod.rs index 7236d57..0bd260e 100644 --- a/crates/rb-cli/src/commands/mod.rs +++ b/crates/rb-cli/src/commands/mod.rs @@ -5,6 +5,7 @@ pub mod new; pub mod run; pub mod shell_integration; pub mod sync; +mod sync_report; pub mod version; pub use exec::exec_command; diff --git a/crates/rb-cli/src/commands/sync.rs b/crates/rb-cli/src/commands/sync.rs index 7e400fe..8c0b96e 100644 --- a/crates/rb-cli/src/commands/sync.rs +++ b/crates/rb-cli/src/commands/sync.rs @@ -1,5 +1,5 @@ +use super::sync_report::{Completion, report_sync, sync_reporter}; use log::debug; -use rb_core::bundler::SyncResult; use rb_core::butler::{ButlerError, ButlerRuntime}; pub fn sync_command(butler_runtime: ButlerRuntime) -> Result<(), ButlerError> { @@ -14,99 +14,9 @@ pub fn sync_command(butler_runtime: ButlerRuntime) -> Result<(), ButlerError> { } }; - println!("🔄 Synchronizing Bundler Environment"); - println!(); - println!("📂 Project: {}", bundler_runtime.root.display()); - println!("📄 Gemfile: {}", bundler_runtime.gemfile_path().display()); - println!("📦 Vendor: {}", bundler_runtime.vendor_dir().display()); - println!(); - - match bundler_runtime.synchronize(&butler_runtime, |line| { - println!("{}", line); - }) { - Ok(SyncResult::AlreadySynced) => { - println!("✅ Environment Already Synchronized"); - println!(); - println!( - "Your bundler environment is meticulously prepared and ready for distinguished service." - ); - println!("All dependencies are satisfied and properly installed."); - } - Ok(SyncResult::Synchronized) => { - println!(); - println!("✅ Environment Successfully Synchronized"); - println!(); - println!( - "Your bundler environment has been meticulously prepared with all required dependencies." - ); - println!("The installation is complete and ready for distinguished service."); - } - Err(e) => { - println!(); - println!("❌ Synchronization Failed"); - println!(); - - let error_msg = e.to_string(); - - if error_msg.contains("extconf.rb failed") - || error_msg.contains("native extension") - || error_msg.contains("development tools") - || error_msg.contains("compiler failed") - || error_msg.contains("Makefile") - { - println!("🔧 Native Extension Compilation Failed"); - println!(); - println!("Some gems in your Gemfile require native extensions to be compiled."); - println!("This requires development tools to be installed on your system."); - println!(); - println!("📋 Required Development Tools:"); - println!(" • Build essentials (gcc, make, etc.)"); - println!(" • Ruby development headers"); - println!(" • Platform-specific libraries"); - println!(); - println!("🚀 Installation Commands:"); - println!(" Ubuntu/Debian: sudo apt-get install build-essential ruby-dev"); - println!( - " CentOS/RHEL: sudo yum groupinstall 'Development Tools' && sudo yum install ruby-devel" - ); - println!(" Alpine Linux: sudo apk add build-base ruby-dev"); - println!(" macOS: xcode-select --install"); - println!(); - println!("💡 Alternative Solutions:"); - println!(" • Use pre-compiled gem versions if available"); - println!(" • Consider using --platform ruby to force source compilation"); - println!(" • Use Docker with a development-ready base image"); - } else if error_msg.contains("not found") && error_msg.contains("bundler") { - println!("📦 Bundler Not Found"); - println!(); - println!("The bundler executable is not available in your Ruby environment."); - println!(); - println!("🚀 Installation:"); - println!(" gem install bundler"); - } else if error_msg.contains("permission") || error_msg.contains("Permission") { - println!("🔒 Permission Denied"); - println!(); - println!("Unable to write to the gem installation directory."); - println!(); - println!("💡 Solutions:"); - println!(" • Ensure write permissions to the vendor directory"); - println!(" • Check file system permissions"); - println!(" • Consider using a user-specific gem directory"); - } else { - println!("⚠️ Bundle Installation Error"); - println!(); - println!("Details: {}", error_msg); - } - - println!(); - println!("🔍 For detailed error information, run:"); - println!(" rb exec bundle install --verbose"); - - return Err(ButlerError::General(e.to_string())); - } - } - - Ok(()) + report_sync(sync_reporter(), Completion::Tree, |handler| { + bundler_runtime.synchronize_with_events(&butler_runtime, handler) + }) } #[cfg(test)] diff --git a/crates/rb-cli/src/commands/sync_report.rs b/crates/rb-cli/src/commands/sync_report.rs new file mode 100644 index 0000000..0b74238 --- /dev/null +++ b/crates/rb-cli/src/commands/sync_report.rs @@ -0,0 +1,144 @@ +use rb_core::bundler::{SyncEvent, SyncResult}; +use rb_core::butler::ButlerError; +use rb_progress::Reporter; +use std::io::IsTerminal; + +pub(super) fn sync_reporter() -> Reporter { + if std::io::stderr().is_terminal() && std::io::stdout().is_terminal() { + Reporter::with_writer("Preparing your bundle", std::io::stderr()) + } else { + Reporter::with_plain_writer("Preparing your bundle", std::io::stderr()) + } +} + +pub(super) enum Completion { + Tree, + Compact, +} + +pub(super) fn report_sync( + reporter: Reporter, + completion: Completion, + run: impl FnOnce(&mut dyn FnMut(SyncEvent<'_>)) -> std::io::Result, +) -> Result<(), ButlerError> { + let progress = reporter.handle(); + progress.worker_started(0, "Bundler"); + let mut active_phase = None; + let mut active_detail = None; + let result = run(&mut |event| { + let phase = match event { + SyncEvent::Checking => "checking dependencies", + SyncEvent::CheckingLockfile => "checking lockfile", + SyncEvent::LockfileChecked { changed } => { + active_detail = Some( + if changed { + "updated lockfile" + } else { + "unchanged" + } + .into(), + ); + return; + } + SyncEvent::Installing => "installing dependencies", + SyncEvent::Output { line, .. } => { + progress.worker_output(0, line); + return; + } + }; + if let Some(previous) = active_phase.replace(phase) { + progress.event(previous, 1, 1, active_detail.take()); + } + progress.event(phase, 0, 1, None); + progress.worker_event(Some(0), phase, 0, 0, Some("Bundler".into())); + }); + match result { + Ok(result) => { + progress.worker_finished(0); + if let Some(phase) = active_phase { + progress.event(phase, 1, 1, active_detail); + } + let summary = match result { + SyncResult::AlreadySynced => "Everything is already in order", + SyncResult::Synchronized => "Your bundle is ready", + }; + match completion { + Completion::Tree => reporter.finish_with_summary(summary), + Completion::Compact => reporter.finish_compact(summary), + } + Ok(()) + } + Err(error) => { + progress.worker_failed(0, error.to_string()); + reporter.finish(); + Err(ButlerError::General(error.to_string())) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[derive(Clone, Default)] + struct Buffer(std::sync::Arc>>); + + impl std::io::Write for Buffer { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + #[test] + fn reporter_maps_bundler_phases_and_success() { + for result in [SyncResult::AlreadySynced, SyncResult::Synchronized] { + let buffer = Buffer::default(); + let reporter = Reporter::with_writer("sync", buffer.clone()); + report_sync(reporter, Completion::Tree, |handler| { + handler(SyncEvent::Checking); + handler(SyncEvent::CheckingLockfile); + handler(SyncEvent::Installing); + handler(SyncEvent::Output { + line: "compiler output", + stderr: true, + }); + Ok(result.clone()) + }) + .unwrap(); + let text = String::from_utf8(buffer.0.lock().unwrap().clone()).unwrap(); + for phase in [ + "checking dependencies", + "checking lockfile", + "installing dependencies", + "compiler output", + ] { + assert!(text.contains(phase), "{text}"); + } + assert!(text.contains("worker 0")); + assert!(text.contains("│ compiler output")); + assert!(!text.contains("| compiler output")); + assert!(text.contains(match result { + SyncResult::AlreadySynced => "Everything is already in order", + SyncResult::Synchronized => "Your bundle is ready", + })); + } + } + + #[test] + fn reporter_retains_bundler_failure() { + let buffer = Buffer::default(); + let reporter = Reporter::with_writer("sync", buffer.clone()); + let result = report_sync(reporter, Completion::Tree, |handler| { + handler(SyncEvent::Installing); + Err(std::io::Error::other("compiler failed")) + }); + assert!(result.unwrap_err().to_string().contains("compiler failed")); + let text = String::from_utf8(buffer.0.lock().unwrap().clone()).unwrap(); + assert!(text.contains("compiler failed")); + assert!(text.contains("└─ × failed")); + assert!(!text.contains("Your bundle is ready")); + } +} diff --git a/crates/rb-cli/tests/sync_ui_test.py b/crates/rb-cli/tests/sync_ui_test.py new file mode 100644 index 0000000..c4d06ee --- /dev/null +++ b/crates/rb-cli/tests/sync_ui_test.py @@ -0,0 +1,175 @@ +#!/usr/bin/env python3 +import importlib.util +import os +from pathlib import Path +import subprocess +import sys +import tempfile + +sys.dont_write_bytecode = True +spec = importlib.util.spec_from_file_location( + 'progress_ui', Path(__file__).resolve().parents[2] / 'rb-progress/tests/pty_ui_test.py' +) +ui = importlib.util.module_from_spec(spec) +spec.loader.exec_module(ui) + +binary = str(Path(sys.argv[1]).resolve()) +with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + bin_dir = root / 'rubies/ruby-3.4.5/bin' + bin_dir.mkdir(parents=True) + (root / 'Gemfile').write_text("source 'https://rubygems.org'\n") + bundle = bin_dir / 'bundle' + bundle.write_text('''#!/bin/sh +printf '%s\\n' "$*" >> calls +case "$1" in + exec) echo 'Rails console fixture'; exit 0 ;; + check) sleep 0.3; test -f ready ;; + lock) + if [ -f lock-fail ]; then echo 'lock reconciliation failed' >&2; exit 1; fi + if [ -f desired-lock ]; then cat desired-lock > Gemfile.lock; fi + echo 'lockfile updated'; echo 'lock warning' >&2 ;; + install) + echo 'installing fixture' + sleep 0.3 + i=0 + while [ "$i" -lt 3000 ]; do + echo 'compiler diagnostic: testing concurrent stdout and stderr pipe drainage' >&2 + i=$((i + 1)) + done + if [ -f fail ]; then echo 'compiler failed' >&2; exit 1; fi + printf '%s\\n' 'preview one' 'preview two' 'preview three' >&2 + sleep 0.3 + echo 'bundle complete' + ;; +esac +''') + bundle.chmod(0o755) + args = ['-R', str(root / 'rubies'), 'sync'] + previous = os.getcwd() + os.chdir(root) + try: + for ready in [False, True]: + if ready: + (root / 'ready').touch() + (root / 'calls').write_text('') + plain = subprocess.run([binary] + args, capture_output=True, timeout=20) + assert plain.returncode == 0, plain.stderr + summary = 'Everything is already in order' if ready else 'Your bundle is ready' + assert plain.stdout == b'', plain.stdout + assert b'[overall] Preparing your bundle' in plain.stderr + assert b'[overall] complete: ' + summary.encode() in plain.stderr + assert b'[worker: 0]' in plain.stderr + assert b'\x1b[' not in plain.stdout + plain.stderr + assert (b'lock warning' if ready else b'compiler diagnostic') in plain.stderr + assert (b'lockfile updated' if ready else b'bundle complete') in plain.stderr + expected = ['check', 'lock --local'] if ready else ['check', 'install'] + assert (root / 'calls').read_text().splitlines() == expected + for rows in [8, 24]: + preview_seen = [] + def observe(screen): + first = screen.find('│ preview one') + if first is not None and first + 2 < rows: + if (screen.line(first + 1).strip() == '│ preview two' + and screen.line(first + 2).strip() == '│ preview three'): + preview_seen.append(True) + screen, output = ui.capture(binary, args, rows, 80, 0, observe) + if not ready and rows == 24: + assert preview_seen, 'three-line Bundler preview was never displayed' + for line in ['preview one', 'preview two', 'preview three']: + assert ('\x1b[2m│ ' + line + '\x1b[22m').encode() in output + + assert screen.find('[shell]$ rb --db sync') == 0 + assert screen.find('┌─ ✓ Preparing your bundle') == 1 + end = screen.find('└─ ✓ ' + ('Everything is already in order' if ready else 'Your bundle is ready')) + assert end is not None + if ready: + assert any('checking lockfile 1/1 | unchanged' in screen.line(row) for row in range(rows)) + assert b'updated lockfile' not in output + for phase in ['checking dependencies', 'checking lockfile' if ready else 'installing dependencies']: + assert sum(phase in screen.line(row) for row in range(rows)) == 1 + assert screen.row == end + 1 and screen.column == 0 + assert not any(screen.line(row) for row in range(end + 1, rows)) + assert b'worker 0' in output + assert b'checking dependencies' in output + assert (b'checking lockfile' if ready else b'installing dependencies') in output + assert b'\x1b[2J' not in output + print(f'PASS sync PTY ready={ready} rows={rows}') + for before, after, changed in [(None, 'first lock', True), + ('old lock', 'new lock', True), + ('same lock', 'same lock', False)]: + lockfile = root / 'Gemfile.lock' + if before is None: + lockfile.unlink(missing_ok=True) + else: + lockfile.write_text(before) + (root / 'desired-lock').write_text(after) + (root / 'calls').write_text('') + screen, output = ui.capture(binary, args, 24, 80, 0) + assert (root / 'calls').read_text().splitlines() == ['check', 'lock --local'] + assert lockfile.read_text() == after + detail = 'updated lockfile' if changed else 'unchanged' + assert any('checking lockfile 1/1 | ' + detail in screen.line(row) for row in range(24)) + summary = 'Your bundle is ready' if changed else 'Everything is already in order' + assert screen.find('└─ ✓ ' + summary) is not None + if not changed: + assert b'updated lockfile' not in output + (root / 'desired-lock').unlink() + (root / 'lock-fail').touch() + (root / 'calls').write_text('') + failed_lock = subprocess.run([binary] + args, capture_output=True, timeout=20) + assert failed_lock.returncode != 0 + assert b'Bundle lock failed' in failed_lock.stdout + failed_lock.stderr + assert (root / 'calls').read_text().splitlines() == ['check', 'lock --local'] + (root / 'lock-fail').unlink() + print('PASS lockfile creation, changes, identical rewrites and failures') + exec_args = ['-R', str(root / 'rubies'), 'x', 'rails', 'c'] + for ready in [True, False]: + if not ready: + (root / 'ready').unlink() + plain_exec = subprocess.run([binary] + exec_args, capture_output=True, timeout=20) + assert plain_exec.returncode == 0, plain_exec.stderr + assert plain_exec.stdout == b'Rails console fixture\n' + assert b'\x1b[' not in plain_exec.stderr + if ready: + assert plain_exec.stderr == b'' + else: + for message in [b'[overall] Preparing your bundle', + b'[worker: 0] installing dependencies', + b'[worker: 0] bundle complete', + b'[overall] complete: Your bundle is ready']: + assert message in plain_exec.stderr + for rows in [8, 24]: + (root / 'calls').write_text('') + screen, output = ui.capture(binary, exec_args, rows, 80, 0) + expected_row = 1 if ready else 2 + assert screen.find('Rails console fixture') == expected_row + assert screen.row == expected_row + 1 and screen.column == 0 + assert not any(screen.line(row) for row in range(expected_row + 1, rows)) + calls = (root / 'calls').read_text().splitlines() + assert calls == (['check', 'exec rails c'] if ready + else ['check', 'check', 'install', 'exec rails c']) + if ready: + assert b'Preparing your bundle' not in output + assert b'worker 0' not in output + else: + assert screen.find('✓ Your bundle is ready') == 1 + assert b'worker 0' in output + assert b'preview three' in output + assert not any('┌─' in screen.line(row) for row in range(rows)) + print(f'PASS exec PTY ready={ready} rows={rows}') + (root / 'fail').touch() + failed = subprocess.run([binary] + args, capture_output=True, timeout=20) + assert failed.returncode != 0 + assert b'compiler failed' in failed.stdout + failed.stderr + assert failed.stdout == b'' + assert b'[worker: 0] failed:' in failed.stderr + (root / 'calls').write_text('') + failed_exec = subprocess.run([binary] + exec_args, capture_output=True, timeout=20) + assert failed_exec.returncode != 0 + assert b'compiler failed' in failed_exec.stdout + failed_exec.stderr + assert 'exec rails c' not in (root / 'calls').read_text().splitlines() + assert b'Rails console fixture' not in failed_exec.stdout + print('PASS sync and exec failures prevent launching the command') + finally: + os.chdir(previous) diff --git a/crates/rb-core/src/bundler/mod.rs b/crates/rb-core/src/bundler/mod.rs index cdaf0d9..c530c3a 100644 --- a/crates/rb-core/src/bundler/mod.rs +++ b/crates/rb-core/src/bundler/mod.rs @@ -5,6 +5,25 @@ use log::debug; use semver::Version; use std::path::{Path, PathBuf}; +#[derive(Debug)] +pub enum SyncEvent<'a> { + Checking, + CheckingLockfile, + LockfileChecked { changed: bool }, + Installing, + Output { line: &'a str, stderr: bool }, +} + +fn forward_output(event: SyncEvent<'_>, handler: &mut impl FnMut(&str)) { + if let SyncEvent::Output { line, stderr } = event { + if stderr { + eprintln!("{line}"); + } else { + handler(line); + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct BundlerRuntime { /// Root directory containing the Gemfile @@ -77,7 +96,6 @@ impl BundlerRuntime { } /// Check if bundler environment is synchronized (dependencies satisfied) - /// Also updates Gemfile.lock if check passes to handle removed gems pub fn check_sync( &self, butler_runtime: &crate::butler::ButlerRuntime, @@ -98,11 +116,6 @@ impl BundlerRuntime { output.status.code().unwrap_or(-1) ); - if is_synced { - debug!("Bundle check passed, updating lockfile to match Gemfile"); - self.update_lockfile_quietly(butler_runtime)?; - } - Ok(is_synced) } Err(e) => { @@ -126,6 +139,16 @@ impl BundlerRuntime { where F: FnMut(&str), { + self.install_with_events(butler_runtime, |event| { + forward_output(event, &mut output_handler) + }) + } + + fn install_with_events( + &self, + butler_runtime: &crate::butler::ButlerRuntime, + mut output_handler: impl FnMut(SyncEvent<'_>), + ) -> std::io::Result<()> { use std::io::{BufRead, BufReader}; use std::process::Stdio; @@ -152,26 +175,53 @@ impl BundlerRuntime { } }; - if let Some(stdout) = child.stdout.take() { - let reader = BufReader::new(stdout); - for line in reader.lines() { - let line = line?; - output_handler(&line); - } - } - let mut stderr_content = String::new(); - if let Some(stderr) = child.stderr.take() { - let reader = BufReader::new(stderr); - for line in reader.lines() { - let line = line?; - eprintln!("{}", line); // Still show stderr to user - stderr_content.push_str(&line); - stderr_content.push('\n'); + let read_result = std::thread::scope(|scope| { + let (sender, receiver) = std::sync::mpsc::sync_channel(64); + if let Some(stdout) = child.stdout.take() { + let sender = sender.clone(); + scope.spawn(move || { + for line in BufReader::new(stdout).lines() { + if sender.send((false, line)).is_err() { + break; + } + } + }); } - } + if let Some(stderr) = child.stderr.take() { + let sender = sender.clone(); + scope.spawn(move || { + for line in BufReader::new(stderr).lines() { + if sender.send((true, line)).is_err() { + break; + } + } + }); + } + drop(sender); + let mut error = None; + for (stderr, line) in receiver { + match line { + Ok(line) => { + if stderr { + stderr_content.push_str(&line); + stderr_content.push('\n'); + } + output_handler(SyncEvent::Output { + line: &line, + stderr, + }); + } + Err(err) => { + error.get_or_insert(err); + } + } + } + error.map_or(Ok(()), Err) + }); let status = child.wait()?; + read_result?; if status.success() { debug!("Bundle install completed successfully"); @@ -192,45 +242,14 @@ impl BundlerRuntime { } } - /// Update Gemfile.lock to match Gemfile quietly (no output) - /// Used by check_sync to ensure lockfile is up to date - fn update_lockfile_quietly( - &self, - butler_runtime: &crate::butler::ButlerRuntime, - ) -> std::io::Result<()> { - debug!("Quietly updating Gemfile.lock to match Gemfile"); - - // Run bundle lock --local to regenerate lockfile based on Gemfile - // Uses --local to avoid network access since bundle check already passed - let output = Command::new("bundle") - .arg("lock") - .arg("--local") - .current_dir(&self.root) - .output_with_context(butler_runtime)?; - - if output.status.success() { - debug!("Gemfile.lock updated successfully"); - Ok(()) - } else { - // Silently ignore errors - lockfile update is best-effort - // The bundle check already passed, so environment is functional - debug!( - "Bundle lock failed but continuing (exit code: {})", - output.status.code().unwrap_or(-1) - ); - Ok(()) - } - } - /// Update Gemfile.lock to match Gemfile (handles removed gems) - /// Used by sync command with output streaming fn update_lockfile( &self, butler_runtime: &crate::butler::ButlerRuntime, output_handler: &mut F, ) -> std::io::Result<()> where - F: FnMut(&str), + F: FnMut(SyncEvent<'_>), { debug!("Updating Gemfile.lock to match Gemfile"); @@ -245,14 +264,17 @@ impl BundlerRuntime { if !output.stdout.is_empty() { let stdout_str = String::from_utf8_lossy(&output.stdout); for line in stdout_str.lines() { - output_handler(line); + output_handler(SyncEvent::Output { + line, + stderr: false, + }); } } if !output.stderr.is_empty() { let stderr_str = String::from_utf8_lossy(&output.stderr); for line in stderr_str.lines() { - eprintln!("{}", line); + output_handler(SyncEvent::Output { line, stderr: true }); } } @@ -267,6 +289,14 @@ impl BundlerRuntime { } } + fn lockfile_contents(&self) -> std::io::Result>> { + match std::fs::read(self.root.join("Gemfile.lock")) { + Ok(contents) => Ok(Some(contents)), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(error), + } + } + pub fn synchronize( &self, butler_runtime: &crate::butler::ButlerRuntime, @@ -275,22 +305,40 @@ impl BundlerRuntime { where F: FnMut(&str), { + self.synchronize_with_events(butler_runtime, |event| { + forward_output(event, &mut output_handler) + }) + } + + pub fn synchronize_with_events( + &self, + butler_runtime: &crate::butler::ButlerRuntime, + mut output_handler: impl FnMut(SyncEvent<'_>), + ) -> std::io::Result { + output_handler(SyncEvent::Checking); debug!("Starting bundler synchronization"); - // check_sync already updates lockfile quietly, but for sync command - // we want to show output, so we call update_lockfile explicitly match self.check_sync(butler_runtime)? { true => { debug!("Bundler environment already synchronized"); + output_handler(SyncEvent::CheckingLockfile); + let before = self.lockfile_contents()?; self.update_lockfile(butler_runtime, &mut output_handler)?; + let changed = before != self.lockfile_contents()?; + output_handler(SyncEvent::LockfileChecked { changed }); - Ok(SyncResult::AlreadySynced) + Ok(if changed { + SyncResult::Synchronized + } else { + SyncResult::AlreadySynced + }) } false => { debug!("Bundler environment requires synchronization"); - self.install_dependencies(butler_runtime, output_handler)?; + output_handler(SyncEvent::Installing); + self.install_with_events(butler_runtime, output_handler)?; Ok(SyncResult::Synchronized) } diff --git a/crates/rb-progress/src/render.rs b/crates/rb-progress/src/render.rs index 0b8ee5c..49029b6 100644 --- a/crates/rb-progress/src/render.rs +++ b/crates/rb-progress/src/render.rs @@ -70,7 +70,7 @@ pub(crate) fn tree_frame(state: &ProgressState) -> Vec { .filter(|worker| worker.status == WorkerStatus::Running) .count(); fit_terminal(&format!( - "└─ {icon} completed {} | active {} | failed {} | elapsed {}", + "└─ {icon} completed {} | active {} | failed {} ({})", state.completed_tasks, active, state.failed_tasks, @@ -78,7 +78,7 @@ pub(crate) fn tree_frame(state: &ProgressState) -> Vec { )) } else { fit_terminal(&format!( - "└─ {icon} overall: {} {}/{} | workers {} | elapsed {}", + "└─ {icon} {} {}/{} | workers {} ({})", state.phase, state.done, state.total, @@ -114,6 +114,7 @@ pub(crate) fn body_lines(state: &ProgressState) -> Vec { format_duration(state.phase_elapsed()) )); } + let mut lines: Vec<_> = lines.into_iter().map(|line| fit_terminal(&line)).collect(); for (position, (worker, worker_state)) in state.workers.iter().enumerate() { let branch = if position + 1 == state.workers.len() { "└─" @@ -129,7 +130,7 @@ pub(crate) fn body_lines(state: &ProgressState) -> Vec { WorkerStatus::Succeeded => "✓", WorkerStatus::Failed => "×", }; - lines.push(format!( + lines.push(fit_terminal(&format!( "│ {} {marker} worker {} ({}){} ({})", branch, worker, @@ -143,12 +144,13 @@ pub(crate) fn body_lines(state: &ProgressState) -> Vec { elapsed + worker_state.started.elapsed() } ),) - )); + ))); for line in &worker_state.output { - lines.push(format!("│ {line}")); + let line = fit_terminal(&format!("│ {line}")); + lines.push(format!("\x1b[2m{line}\x1b[22m")); } } - lines.into_iter().map(|line| fit_terminal(&line)).collect() + lines } #[cfg(test)] @@ -237,6 +239,21 @@ fn clear_render(state: &mut ProgressState) { state.frame_capacity = 0; } +pub(crate) fn finish_compact(state: &mut ProgressState, summary: Option) { + if state.plain { + crate::plain::finish(state, summary.as_deref()); + return; + } + clear_render(state); + let line = fit_terminal(&format!( + "✓ {} ({})", + summary.as_deref().unwrap_or("complete"), + format_duration(state.elapsed()) + )); + println(state, format_args!("{line}")); + flush(state); +} + pub(crate) fn finish(state: &mut ProgressState, summary: Option) { if state.plain { crate::plain::finish(state, summary.as_deref()); @@ -306,20 +323,28 @@ pub(crate) fn finish(state: &mut ProgressState, summary: Option) { .iter() .map(|record| { fit_terminal(&format!( - "│ ├─ ✓ {} {}/{} ({})", + "│ ├─ ✓ {} {}/{}{} ({})", record.phase, record.done, record.total, + record + .detail + .as_deref() + .map_or(String::new(), |detail| format!(" | {detail}")), format_duration(record.elapsed) )) }) .collect::>(); if !state.phase.is_empty() { body.push(fit_terminal(&format!( - "│ ├─ {marker} {} {}/{} ({})", + "│ ├─ {marker} {} {}/{}{} ({})", state.phase, state.done, state.total, + state + .phase_detail + .as_deref() + .map_or(String::new(), |detail| format!(" | {detail}")), format_duration(state.phase_elapsed()) ))); } diff --git a/crates/rb-progress/src/reporter.rs b/crates/rb-progress/src/reporter.rs index 5aeeba0..de229eb 100644 --- a/crates/rb-progress/src/reporter.rs +++ b/crates/rb-progress/src/reporter.rs @@ -80,6 +80,20 @@ impl ProgressHandle { render::draw(&mut progress); } + /// Appends output to an existing worker's three-line terminal preview. + /// Plain reporters write every line with its worker prefix. + pub fn worker_output(&self, worker: usize, output: &str) { + let mut progress = self.progress.lock().unwrap(); + if progress.closed { + return; + } + if progress.plain { + crate::plain::line(&progress, &format!("worker: {worker}"), output); + } + progress.worker_output(worker, output); + render::draw(&mut progress); + } + pub fn worker_finished(&self, worker: usize) { self.end_worker(worker, None); } @@ -192,11 +206,15 @@ impl Reporter { } pub fn finish(mut self) { - self.stop(None); + self.stop(None, false); } pub fn finish_with_summary(mut self, summary: impl Into) { - self.stop(Some(summary.into())); + self.stop(Some(summary.into()), false); + } + + pub fn finish_compact(mut self, summary: impl Into) { + self.stop(Some(summary.into()), true); } pub fn handle(&self) -> ProgressHandle { @@ -212,7 +230,7 @@ impl Reporter { }) } - fn stop(&mut self, summary: Option) { + fn stop(&mut self, summary: Option, compact: bool) { if self.finished { return; } @@ -222,14 +240,18 @@ impl Reporter { let _ = ticker.join(); } let mut progress = self.progress.lock().unwrap(); - render::finish(&mut progress, summary); + if compact && progress.failed_tasks == 0 && !progress.external_output { + render::finish_compact(&mut progress, summary); + } else { + render::finish(&mut progress, summary); + } progress.closed = true; } } impl Drop for Reporter { fn drop(&mut self) { - self.stop(None); + self.stop(None, false); } } diff --git a/crates/rb-progress/src/reporter/tests.rs b/crates/rb-progress/src/reporter/tests.rs index 879ff6f..bdf5fab 100644 --- a/crates/rb-progress/src/reporter/tests.rs +++ b/crates/rb-progress/src/reporter/tests.rs @@ -497,3 +497,104 @@ fn standalone_threads_share_workers_and_completion_counts() { drop(state); reporter.finish(); } + +#[test] +fn standalone_worker_output_keeps_three_lines_without_changing_its_label() { + let buffer = Buffer::default(); + let reporter = Reporter::with_writer("service", buffer.clone()); + let handle = reporter.handle(); + handle.worker_started(0, "Bundler"); + handle.worker_started(1, "other job"); + handle.worker_output(1, "other output"); + handle.worker_output(0, "one\ntwo\nthree\nfour\n"); + { + let state = reporter.progress.lock().unwrap(); + assert_eq!(state.workers[&0].detail.as_deref(), Some("Bundler")); + assert_eq!(state.workers[&0].output, ["two", "three", "four"]); + assert_eq!(state.workers[&1].output, ["other output"]); + let lines = render::body_lines(&state); + assert!(lines.windows(3).any(|lines| lines + == [ + "\x1b[2m│ two\x1b[22m", + "\x1b[2m│ three\x1b[22m", + "\x1b[2m│ four\x1b[22m" + ])); + } + handle.worker_started(0, "next job"); + assert!( + reporter.progress.lock().unwrap().workers[&0] + .output + .is_empty() + ); + reporter.finish(); + let finished = buffer.text(); + handle.worker_output(0, "late output"); + assert_eq!(buffer.text(), finished); +} + +#[test] +fn standalone_plain_worker_output_appends_every_line_with_a_prefix() { + let buffer = Buffer::default(); + let reporter = Reporter::with_plain_writer("service", buffer.clone()); + let handle = reporter.handle(); + handle.worker_started(2, "Bundler"); + handle.worker_output(2, "one\ntwo\nthree\nfour\n"); + reporter.finish(); + let output = buffer.text(); + assert!( + output.contains("[worker: 2] one\n[worker: 2] two\n[worker: 2] three\n[worker: 2] four\n") + ); + assert!(!output.contains('\x1b')); + handle.worker_output(2, "late output"); + assert_eq!(buffer.text(), output); +} + +#[test] +fn compact_completion_keeps_failure_details() { + let buffer = Buffer::default(); + let reporter = Reporter::with_writer("service", buffer.clone()); + let handle = reporter.handle(); + handle.worker_started(0, "inspection"); + handle.worker_failed(0, "Replace the teacup"); + buffer.0.lock().unwrap().clear(); + reporter.finish_compact("service failed"); + let output = buffer.text(); + assert!(output.contains("┌─ × service")); + assert!(output.contains("│ × Replace the teacup")); + assert!(output.contains("└─ × service failed")); + handle.worker_output(0, "late output"); + assert_eq!(buffer.text(), output); +} + +#[test] +fn compact_plain_completion_preserves_append_only_output() { + let buffer = Buffer::default(); + let reporter = Reporter::with_plain_writer("service", buffer.clone()); + let handle = reporter.handle(); + handle.worker_started(0, "inspection"); + handle.worker_output(0, "Checking the china"); + handle.worker_finished(0); + reporter.finish_compact("ready"); + let output = buffer.text(); + assert!(output.contains("[worker: 0] Checking the china\n")); + assert!(output.contains("[overall] complete: ready")); + assert!(!output.contains('\x1b')); + handle.worker_output(0, "late output"); + assert_eq!(buffer.text(), output); +} + +#[test] +fn live_footer_uses_direct_status_and_trailing_duration() { + let mut state = ProgressState::default(); + assert_eq!( + render::tree_frame(&state).last().unwrap(), + "└─ ⠋ completed 0 | active 0 | failed 0 (0.0s)" + ); + state.phase = "checking dependencies"; + state.done = 1; + state.total = 2; + assert_eq!( + render::tree_frame(&state).last().unwrap(), + "└─ ⠋ checking dependencies 1/2 | workers 0 (0.0s)" + ); +} diff --git a/crates/rb-progress/src/state.rs b/crates/rb-progress/src/state.rs index a944771..a052268 100644 --- a/crates/rb-progress/src/state.rs +++ b/crates/rb-progress/src/state.rs @@ -58,6 +58,17 @@ pub(crate) struct WorkerProgress { } impl ProgressState { + pub(crate) fn worker_output(&mut self, worker: usize, output: &str) { + if let Some(state) = self.workers.get_mut(&worker) { + for line in output.lines() { + state.output.push_back(line.to_owned()); + while state.output.len() > 3 { + state.output.pop_front(); + } + } + } + } + pub(crate) fn set_worker_status(&mut self, worker: usize, status: WorkerStatus) -> bool { let Some(state) = self.workers.get_mut(&worker) else { return false; diff --git a/crates/rb-progress/src/task.rs b/crates/rb-progress/src/task.rs index c1a1aa1..649669c 100644 --- a/crates/rb-progress/src/task.rs +++ b/crates/rb-progress/src/task.rs @@ -49,14 +49,7 @@ impl TaskEventSink for TaskBridge { elapsed: Some(elapsed), }, TaskEvent::Output { worker, line, .. } => { - if let Some(state) = progress.workers.get_mut(&worker) { - for line in line.lines() { - state.output.push_back(line.to_owned()); - while state.output.len() > 3 { - state.output.pop_front(); - } - } - } + progress.worker_output(worker, &line); crate::render::draw(&mut progress); return; } diff --git a/crates/rb-progress/tests/pty_ui_test.py b/crates/rb-progress/tests/pty_ui_test.py index f821d70..2281893 100644 --- a/crates/rb-progress/tests/pty_ui_test.py +++ b/crates/rb-progress/tests/pty_ui_test.py @@ -144,9 +144,12 @@ def capture(binary, args, rows, columns, prompt_row, observe=None): if not chunk: break output.extend(chunk) - screen.feed(chunk) - if observe is not None: - observe(screen) + if observe is None: + screen.feed(chunk) + else: + for line in chunk.splitlines(keepends=True): + screen.feed(line) + observe(screen) if not screen.has('[shell]$ rb --db sync'): raise AssertionError('prompt was erased or scrolled during rendering') if not progress_started and b'resolved' in output: diff --git a/spec/commands/exec/bundler_spec.sh b/spec/commands/exec/bundler_spec.sh index 82e7859..a70269d 100644 --- a/spec/commands/exec/bundler_spec.sh +++ b/spec/commands/exec/bundler_spec.sh @@ -236,13 +236,13 @@ Describe "Ruby Butler Exec Command - Bundler Environment" The output should include "Bundle complete" End - It "executes bundle list after install" - # First install, then test list in separate test + It "installs dependencies before listing bundled gems" When run rb -R "$RUBIES_DIR" exec bundle list The status should equal 0 - The lines of stderr should be valid number - # Bundle list may trigger install, so expect bundler output - The output should include "Butler Notice" + The output should include "Gems included by the bundle:" + The output should include "rake" + The stderr should include "[worker: 0] installing dependencies" + The stderr should include "[overall] complete: Your bundle is ready" End It "executes bundle exec rake after install" diff --git a/spec/commands/sync_spec.sh b/spec/commands/sync_spec.sh index 0737c47..d3542ba 100644 --- a/spec/commands/sync_spec.sh +++ b/spec/commands/sync_spec.sh @@ -14,14 +14,14 @@ Describe 'rb sync command' When run rb -R "$RUBIES_DIR" sync The status should be success The lines of stderr should be valid number - The output should include "Environment Successfully Synchronized" - The output should include "Bundle complete!" + The stderr should include "[overall] complete: Your bundle is ready" + The stderr should include "[worker: 0] Bundle complete!" End It 'does not emit bundler deprecation warnings' When run rb -R "$RUBIES_DIR" sync The status should be success - The output should include "Environment Successfully Synchronized" + The stderr should include "[overall] complete: Your bundle is ready" The stderr should not include "[DEPRECATED]" End @@ -65,7 +65,7 @@ Describe 'rb sync command' When run rb -R "$RUBIES_DIR" s The status should be success The lines of stderr should be valid number - The output should include "Environment Successfully Synchronized" + The stderr should include "[overall] complete: Your bundle is ready" End End End @@ -83,7 +83,7 @@ Describe 'rb sync command' When run rb -R "$RUBIES_DIR" sync The status should be success The lines of stderr should be valid number - The output should include "Synchronizing" + The stderr should include "[overall] Preparing your bundle" End End @@ -113,7 +113,7 @@ EOF When run rb -R "$RUBIES_DIR" sync The status should be success The lines of stderr should be valid number - The output should include "Synchronizing" + The stderr should include "[overall] Preparing your bundle" # Verify rake is still in lockfile but minitest is removed The path Gemfile.lock should be exist @@ -129,7 +129,7 @@ EOF When run rb sync The status should be success The lines of stderr should be valid number - The output should include "Environment Successfully Synchronized" + The stderr should include "[overall] complete: Your bundle is ready" End It 'respects RB_RUBY_VERSION environment variable' @@ -138,7 +138,7 @@ EOF When run rb -R "$RUBIES_DIR" sync The status should be success The lines of stderr should be valid number - The output should include "Synchronizing" + The stderr should include "[overall] Preparing your bundle" End It 'respects RB_NO_BUNDLER environment variable (disables sync)' @@ -155,7 +155,7 @@ EOF When run rb -R "$RUBIES_DIR" -r "$OLDER_RUBY" sync The status should be success The lines of stderr should be valid number - The output should include "Synchronizing" + The stderr should include "[overall] Preparing your bundle" End End End diff --git a/tests/commands/Sync.Integration.Tests.ps1 b/tests/commands/Sync.Integration.Tests.ps1 index a246f4c..e7f879a 100644 --- a/tests/commands/Sync.Integration.Tests.ps1 +++ b/tests/commands/Sync.Integration.Tests.ps1 @@ -35,7 +35,7 @@ gem 'rake' try { $Output = & $Script:RbPath sync 2>&1 $LASTEXITCODE | Should -Be 0 - ($Output -join " ") | Should -Match "Environment Successfully Synchronized|Bundle complete" + ($Output -join " ") | Should -Match "Your bundle is ready|Bundle complete" } finally { Pop-Location } @@ -54,7 +54,7 @@ gem 'rake' try { $Output = & $Script:RbPath s 2>&1 $LASTEXITCODE | Should -Be 0 - ($Output -join " ") | Should -Match "Environment Successfully Synchronized|Bundle complete" + ($Output -join " ") | Should -Match "Your bundle is ready|Bundle complete" } finally { Pop-Location }