From b7c8bddca16807cb1dd6d006fcb7aa35835193bf Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 17 Jan 2025 16:50:34 -0800 Subject: [PATCH] dont hang in background - ignore suspend commands when tty is disabled --- packages/cli/src/cli/serve.rs | 4 +-- packages/cli/src/logging.rs | 7 ++-- packages/cli/src/serve/mod.rs | 2 +- packages/cli/src/serve/output.rs | 56 ++++++++++++++++++++++++-------- packages/cli/src/serve/server.rs | 35 ++++++++++---------- 5 files changed, 69 insertions(+), 35 deletions(-) diff --git a/packages/cli/src/cli/serve.rs b/packages/cli/src/cli/serve.rs index 48ee57753c..1ab9e22c05 100644 --- a/packages/cli/src/cli/serve.rs +++ b/packages/cli/src/cli/serve.rs @@ -96,8 +96,8 @@ impl ServeArgs { } pub(crate) fn is_interactive_tty(&self) -> bool { - use crossterm::tty::IsTty; - std::io::stdout().is_tty() && self.interactive.unwrap_or(true) + use std::io::IsTerminal; + std::io::stdout().is_terminal() && self.interactive.unwrap_or(true) } pub(crate) fn should_proxy_build(&self) -> bool { diff --git a/packages/cli/src/logging.rs b/packages/cli/src/logging.rs index 45d6a4cbc1..7b702bb05a 100644 --- a/packages/cli/src/logging.rs +++ b/packages/cli/src/logging.rs @@ -18,7 +18,7 @@ use crate::{serve::ServeUpdate, Cli, Commands, Platform as TargetPlatform, Verbo use cargo_metadata::{diagnostic::DiagnosticLevel, CompilerMessage}; use clap::Parser; use futures_channel::mpsc::{unbounded, UnboundedReceiver, UnboundedSender}; -use once_cell::sync::OnceCell; +use once_cell::sync::{Lazy, OnceCell}; use std::{ collections::HashMap, env, @@ -27,7 +27,7 @@ use std::{ path::PathBuf, sync::{ atomic::{AtomicBool, Ordering}, - Mutex, + Arc, Mutex, }, time::Instant, }; @@ -46,6 +46,8 @@ const LOG_ENV: &str = "DIOXUS_LOG"; const LOG_FILE_NAME: &str = "dx.log"; const DX_SRC_FLAG: &str = "dx_src"; +pub static TUI_INTERACTIVE_DISABLED: Lazy> = + Lazy::new(|| Arc::new(AtomicBool::new(false))); static TUI_ACTIVE: AtomicBool = AtomicBool::new(false); static TUI_TX: OnceCell> = OnceCell::new(); pub static VERBOSITY: OnceCell = OnceCell::new(); @@ -135,6 +137,7 @@ impl TraceController { let (tui_tx, tui_rx) = unbounded(); TUI_ACTIVE.store(true, Ordering::Relaxed); TUI_TX.set(tui_tx.clone()).unwrap(); + Self { tui_rx } } diff --git a/packages/cli/src/serve/mod.rs b/packages/cli/src/serve/mod.rs index e4bc0b3008..9ddc51aa35 100644 --- a/packages/cli/src/serve/mod.rs +++ b/packages/cli/src/serve/mod.rs @@ -45,11 +45,11 @@ pub(crate) async fn serve_all(mut args: ServeArgs) -> Result<()> { let krate = args.load_krate().await?; // Note that starting the builder will queue up a build immediately + let mut screen = Output::start(&args).await?; let mut builder = Builder::start(&krate, args.build_args())?; let mut devserver = WebServer::start(&krate, &args)?; let mut watcher = Watcher::start(&krate, &args); let mut runner = AppRunner::start(&krate); - let mut screen = Output::start(&args)?; // This is our default splash screen. We might want to make this a fancier splash screen in the future // Also, these commands might not be the most important, but it's all we've got enabled right now diff --git a/packages/cli/src/serve/output.rs b/packages/cli/src/serve/output.rs index f2ad1bc085..abaf34c3a9 100644 --- a/packages/cli/src/serve/output.rs +++ b/packages/cli/src/serve/output.rs @@ -76,7 +76,7 @@ struct RenderState<'a> { } impl Output { - pub(crate) fn start(cfg: &ServeArgs) -> io::Result { + pub(crate) async fn start(cfg: &ServeArgs) -> io::Result { let mut output = Self { term: Rc::new(RefCell::new(None)), interactive: cfg.is_interactive_tty(), @@ -100,14 +100,14 @@ impl Output { }, }; - output.startup()?; + output.startup().await?; Ok(output) } /// Call the startup functions that might mess with the terminal settings. /// This is meant to be paired with "shutdown" to restore the terminal to its original state. - fn startup(&mut self) -> io::Result<()> { + async fn startup(&mut self) -> io::Result<()> { if self.interactive { // set the panic hook to fix the terminal in the event of a panic // The terminal might be left in a wonky state if a panic occurs, and we don't want it to be completely broken @@ -118,6 +118,15 @@ impl Output { original_hook(info); })); + // Check if writing the terminal is going to block infinitely. + // If it does, we should disable interactive mode. This ensures we work with programs like `bg` + // which suspend the process and cause us to block when writing output. + if !Self::enable_raw_mode().await.unwrap_or(false) { + self.term.take(); + self.interactive = false; + return Ok(()); + } + self.term.replace( Terminal::with_options( CrosstermBackend::new(stdout()), @@ -128,12 +137,6 @@ impl Output { .ok(), ); - enable_raw_mode()?; - stdout() - .execute(Hide)? - .execute(EnableFocusChange)? - .execute(EnableBracketedPaste)?; - // Initialize the event stream here - this is optional because an EvenStream in a non-interactive // terminal will cause a panic instead of simply doing nothing. // https://github.com/crossterm-rs/crossterm/issues/659 @@ -143,6 +146,33 @@ impl Output { Ok(()) } + /// Enable raw mode, but don't let it block forever. + /// + /// This lets us check if writing to tty is going to block forever and then recover, allowing + /// interopability with programs like `bg`. + async fn enable_raw_mode() -> io::Result { + use tokio::signal::unix::{signal, SignalKind}; + + // Ignore SIGTSTP, SIGTTIN, and SIGTTOU + _ = signal(SignalKind::from_raw(20))?; // SIGTSTP + _ = signal(SignalKind::from_raw(21))?; // SIGTTIN + _ = signal(SignalKind::from_raw(22))?; // SIGTTOU + + use std::io::IsTerminal; + + if !stdout().is_terminal() { + return Ok(false); + } + + enable_raw_mode()?; + stdout() + .execute(Hide)? + .execute(EnableFocusChange)? + .execute(EnableBracketedPaste)?; + + Ok(true) + } + /// Call the shutdown functions that might mess with the terminal settings - see the related code /// in "startup" for more details about what we need to unset pub(crate) fn shutdown(&self) -> io::Result<()> { @@ -164,6 +194,10 @@ impl Output { use futures_util::future::OptionFuture; use futures_util::StreamExt; + if !self.interactive { + return std::future::pending().await; + } + // Wait for the next user event or animation tick loop { let next = OptionFuture::from(self.events.as_mut().map(|f| f.next())); @@ -324,10 +358,6 @@ impl Output { /// re-render when external state changes. Ratatui will diff the intermediate buffer, so we at least /// we won't be drawing it. pub(crate) fn new_build_update(&mut self, update: &BuildUpdate) { - if !self.interactive { - return; - } - match update { BuildUpdate::Progress { stage: BuildStage::Starting { .. }, diff --git a/packages/cli/src/serve/server.rs b/packages/cli/src/serve/server.rs index 0aeb622675..5ea6397777 100644 --- a/packages/cli/src/serve/server.rs +++ b/packages/cli/src/serve/server.rs @@ -82,7 +82,23 @@ impl WebServer { .then(|| get_available_port(devserver_bind_ip)) .flatten(); - let proxied_address = proxied_port.map(|port| SocketAddr::new(devserver_bind_ip, port)); + // Create the listener that we'll pass into the devserver, but save its IP here so + // we can display it to the user in the tui + let listener = std::net::TcpListener::bind(devserver_bind_address).with_context(|| { + anyhow::anyhow!( + "Failed to bind server to: {devserver_bind_address}, is there another devserver running?\nTo run multiple devservers, use the --port flag to specify a different port" + ) + })?; + + // If the IP is 0.0.0.0, we need to get the actual IP of the machine + // This will let ios/android/network clients connect to the devserver + let devserver_exposed_ip = if devserver_bind_ip == IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)) { + local_ip_address::local_ip().unwrap_or(devserver_bind_ip) + } else { + devserver_bind_ip + }; + + let proxied_address = proxied_port.map(|port| SocketAddr::new(devserver_exposed_ip, port)); // Set up the router with some shared state that we'll update later to reflect the current state of the build let build_status = SharedStatus::new_with_starting_build(); @@ -95,14 +111,6 @@ impl WebServer { build_status.clone(), )?; - // Create the listener that we'll pass into the devserver, but save its IP here so - // we can display it to the user in the tui - let listener = std::net::TcpListener::bind(devserver_bind_address).with_context(|| { - anyhow::anyhow!( - "Failed to bind server to: {devserver_bind_address}, is there another devserver running?\nTo run multiple devservers, use the --port flag to specify a different port" - ) - })?; - // And finally, start the server mainloop tokio::spawn(devserver_mainloop( krate.config.web.https.clone(), @@ -110,14 +118,6 @@ impl WebServer { router, )); - // If the IP is 0.0.0.0, we need to get the actual IP of the machine - // This will let ios/android/network clients connect to the devserver - let devserver_exposed_ip = if devserver_bind_ip == IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)) { - local_ip_address::local_ip().unwrap_or(devserver_bind_ip) - } else { - devserver_bind_ip - }; - Ok(Self { build_status, proxied_port, @@ -391,6 +391,7 @@ fn build_devserver_router( if args.should_proxy_build() { // For fullstack, liveview, and server, forward all requests to the inner server let address = fullstack_address.unwrap(); + tracing::debug!("Proxying requests to fullstack server at {address}"); router = router.nest_service("/",super::proxy::proxy_to( format!("http://{address}").parse().unwrap(), true,