brush: fix minimal-backend input (persistent line buffer) + drop debug traces
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.
This commit is contained in:
@@ -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<ReadResult, ShellError> {
|
||||
@@ -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<String> = 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<u8, brush_interactive::ShellError> {
|
||||
+ 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 {
|
||||
|
||||
Reference in New Issue
Block a user