From 5c9bce4b889d65fd6966fc4944f2b96a914a197a Mon Sep 17 00:00:00 2001 From: vasilito Date: Fri, 24 Jul 2026 07:48:31 +0900 Subject: [PATCH] brush: fix minimal-backend input (persistent line buffer) + drop debug traces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to the Redox minimal input backend patch: 1. read_input_line accumulated into a *fresh* String each retry. On the Redox console pty, a `read_line` can return WouldBlock (or Ok(0)) after only a partial line — the characters just typed — and those bytes, left in the read_line buffer, were discarded on every retry. Only a fully-queued line (e.g. a leftover newline) ever came through, so the shell echoed keystrokes but never executed the command. Fix: accumulate partial reads into a thread-local buffer that persists across retries and only yield a line once a newline arrives. 2. Removed all BRUSH-DBG / rb_dbg startup and read tracing (the "debug mode" output on the console). The functional Redox changes stay: tokio current-thread runtime and the minimal input backend selection. Verified: brush-interactive + brush-shell compile. Runtime verification of the interactive shell still pending a boot test. --- .../brush-redox-runtime-and-input.patch | 173 +++++------------- 1 file changed, 47 insertions(+), 126 deletions(-) diff --git a/local/recipes/shells/brush/patches/brush-redox-runtime-and-input.patch b/local/recipes/shells/brush/patches/brush-redox-runtime-and-input.patch index 274493fb64..5f56cdc0ee 100644 --- a/local/recipes/shells/brush/patches/brush-redox-runtime-and-input.patch +++ b/local/recipes/shells/brush/patches/brush-redox-runtime-and-input.patch @@ -1,8 +1,8 @@ diff --git a/brush-interactive/src/minimal/input_backend.rs b/brush-interactive/src/minimal/input_backend.rs -index 48732fb..70b4049 100644 +index 48732fb..24d6ced 100644 --- a/brush-interactive/src/minimal/input_backend.rs +++ b/brush-interactive/src/minimal/input_backend.rs -@@ -48,15 +48,53 @@ impl MinimalInputBackend { +@@ -48,15 +48,58 @@ impl MinimalInputBackend { } fn read_input_line() -> Result { @@ -15,80 +15,66 @@ index 48732fb..70b4049 100644 - Ok(ReadResult::Eof) - } else { - Ok(ReadResult::Input(input)) -+ // On Redox the interactive console is a pty slave whose reads can -+ // return Ok(0) with no bytes even though the master side is still open -+ // (e.g. a non-blocking read with no data currently queued). std's -+ // `read_line` reports that as end-of-input; if the shell treated it as -+ // EOF it would exit immediately and getty would respawn login in a -+ // tight loop. Distinguish a *spurious* empty read (retry, briefly -+ // yielding to avoid a busy spin) from a real terminal by only treating -+ // 0 bytes as EOF when stdin is not a tty. ++ // On Redox the interactive console pty often presents as non-blocking, ++ // so a single `read_line` returns WouldBlock (or Ok(0)) after only a ++ // partial line — the characters the user has typed so far. The bytes ++ // read before that error stay in the `read_line` buffer, so reading ++ // into a *fresh* String each retry silently dropped every keystroke and ++ // only a fully-queued line (e.g. a leftover newline) ever came through: ++ // the shell echoed input but never executed the command. Accumulate ++ // partial reads into a buffer that persists across retries, and only ++ // yield a line once a newline has actually arrived. ++ // ++ // The buffer is per-thread; the minimal backend reads on one thread. ++ use std::cell::RefCell; + use std::io::ErrorKind; -+ let mut dbg_iters: u32 = 0; ++ thread_local! { ++ static PENDING: RefCell = const { RefCell::new(String::new()) }; ++ } + loop { -+ let mut input = String::new(); -+ let dbg_res = std::io::stdin().read_line(&mut input); -+ if dbg_iters < 6 { -+ use std::io::Write; -+ let _ = std::io::stderr().write_all( -+ format!( -+ "BRUSH-DBG: read iter={} tty={} res={:?}\n", -+ dbg_iters, -+ std::io::stdin().is_terminal(), -+ dbg_res.as_ref().map(|n| *n).map_err(|e| e.kind()), -+ ) -+ .as_bytes(), -+ ); -+ } -+ dbg_iters = dbg_iters.wrapping_add(1); -+ match dbg_res { -+ // Got a line. -+ Ok(n) if n != 0 => return Ok(ReadResult::Input(input)), -+ // Zero bytes: real EOF only on a non-interactive input. On an -+ // interactive Redox pty a non-blocking read with no data queued -+ // can also surface as Ok(0); retry rather than exiting (which -+ // would drop back to a getty login loop). -+ Ok(_) => { -+ if !std::io::stdin().is_terminal() { -+ return Ok(ReadResult::Eof); -+ } ++ let mut chunk = String::new(); ++ let res = std::io::stdin().read_line(&mut chunk); ++ // Preserve whatever was read before any error/would-block. ++ let complete = PENDING.with(|p| { ++ let mut p = p.borrow_mut(); ++ p.push_str(&chunk); ++ p.ends_with('\n') ++ }); ++ match res { ++ // Real EOF only on a non-interactive input. On an interactive ++ // Redox pty a non-blocking read with no data queued can also ++ // surface as Ok(0); retry rather than exiting (which would drop ++ // back to a getty login loop). Flush any partial line first. ++ Ok(0) if !std::io::stdin().is_terminal() => { ++ let line = PENDING.with(|p| std::mem::take(&mut *p.borrow_mut())); ++ return Ok(if line.is_empty() { ++ ReadResult::Eof ++ } else { ++ ReadResult::Input(line) ++ }); + } + // A non-blocking read with no data currently available (or an + // interrupted syscall) is not fatal on an interactive terminal; -+ // keep polling for input. ++ // the partial bytes are already saved in PENDING, so keep polling. + Err(e) + if std::io::stdin().is_terminal() + && matches!(e.kind(), ErrorKind::WouldBlock | ErrorKind::Interrupted) => {} + Err(e) => return Err(ShellError::InputError(e)), ++ _ => {} ++ } ++ if complete { ++ let line = PENDING.with(|p| std::mem::take(&mut *p.borrow_mut())); ++ return Ok(ReadResult::Input(line)); + } + std::thread::sleep(std::time::Duration::from_millis(20)); } } } diff --git a/brush-shell/src/entry.rs b/brush-shell/src/entry.rs -index 15340ad..46774c3 100644 +index 15340ad..384f0ca 100644 --- a/brush-shell/src/entry.rs +++ b/brush-shell/src/entry.rs -@@ -117,8 +117,18 @@ impl CommandLineArgs { - } - } - -+/// Temporary mini-diagnostic: write to fd 2 (console) so brush startup is -+/// visible on serial. -+#[doc(hidden)] -+fn rb_dbg(m: &str) { -+ use std::io::Write; -+ let df = std::fs::OpenOptions::new().write(true).open("/scheme/debug"); -+ if let Ok(mut df) = df { let _ = df.write_all(m.as_bytes()); } -+} -+ - /// Main entry point for the `brush` shell. - pub fn run() { -+ rb_dbg("BRUSH-DBG: run() entered\n"); - // - // Install the bundled-command registry so it's available both for - // bundled dispatch (handled next) and for builtin shim registration -@@ -173,17 +183,30 @@ pub fn run() { +@@ -173,9 +173,12 @@ pub fn run() { // // Run. // @@ -102,73 +88,8 @@ index 15340ad..46774c3 100644 + #[cfg(any(not(any(unix, windows)), target_os = "redox"))] let mut builder = tokio::runtime::Builder::new_current_thread(); -- let Ok(runtime) = builder.enable_all().build() else { -- tracing::error!("error: failed to create Tokio runtime"); -- std::process::exit(1); -+ rb_dbg("BRUSH-DBG: building tokio runtime (enable_all)\n"); -+ let runtime = match builder.enable_all().build() { -+ Ok(rt) => { -+ rb_dbg("BRUSH-DBG: tokio runtime built OK\n"); -+ rt -+ } -+ Err(e) => { -+ rb_dbg(&format!("BRUSH-DBG: tokio enable_all build FAILED: {e}\n")); -+ tracing::error!("error: failed to create Tokio runtime"); -+ std::process::exit(1); -+ } - }; - -+ rb_dbg("BRUSH-DBG: entering block_on(run_async)\n"); - let result = runtime.block_on(run_async(&args, parsed_args)); -+ rb_dbg("BRUSH-DBG: block_on returned (shell exiting)\n"); - - let exit_code = match result { - Ok(code) => code, -@@ -242,6 +265,7 @@ async fn run_async( - cli_args: &[String], - args: CommandLineArgs, - ) -> Result { -+ rb_dbg("BRUSH-DBG: run_async: entered\n"); - // Initializing tracing. - let mut event_config = TRACE_EVENT_CONFIG.lock().await; - *event_config = Some(events::TraceEventConfig::init( -@@ -249,15 +273,18 @@ async fn run_async( - &args.disabled_events, - )); - drop(event_config); -+ rb_dbg("BRUSH-DBG: run_async: tracing init done, loading config\n"); - - // Load configuration file. - let file_config = config::load_config(args.no_config, args.config_file.as_deref()) - .into_config_or_log() - .map_err(|e| brush_interactive::ShellError::IoError(std::io::Error::other(e)))?; -+ rb_dbg("BRUSH-DBG: run_async: config loaded\n"); - - // Instantiate an appropriately configured shell and wrap it in an `Arc`. Note that we do - // *not* run any code in the shell yet. We'll delay loading profiles and such until after - // we've set up everything else (in `run_in_shell`). -+ rb_dbg("BRUSH-DBG: run_async: instantiating shell\n"); - let shell: BrushShell = instantiate_shell(&args, cli_args).await?; - let shell = Arc::new(Mutex::new(shell)); - -@@ -265,6 +292,16 @@ async fn run_async( - // backend type and calls `run_in_shell`, preserving static dispatch. - let default_backend = get_default_input_backend_type(&args); - let selected_backend = args.input_backend.unwrap_or(default_backend); -+ rb_dbg(&format!( -+ "BRUSH-DBG: backend={} stdin_tty={} interactive={}\n", -+ match selected_backend { -+ InputBackendType::Reedline => "reedline", -+ InputBackendType::Basic => "basic", -+ InputBackendType::Minimal => "minimal", -+ }, -+ std::io::IsTerminal::is_terminal(&std::io::stdin()), -+ will_run_interactively(&args), -+ )); - - // Build UI options by merging config file with CLI args. - #[allow(unused_variables, reason = "not used when no backend features enabled")] -@@ -623,7 +660,18 @@ const fn new_error_behavior(args: &CommandLineArgs) -> error_formatter::Formatte + let Ok(runtime) = builder.enable_all().build() else { +@@ -623,7 +626,18 @@ const fn new_error_behavior(args: &CommandLineArgs) -> error_formatter::Formatte } fn get_default_input_backend_type(args: &CommandLineArgs) -> InputBackendType {