diff --git a/Cargo.lock b/Cargo.lock index 821c5493eb..a96bbd3c3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2509,7 +2509,6 @@ dependencies = [ "bitflags", "cfg-if 1.0.0", "libc", - "memoffset", ] [[package]] @@ -2869,13 +2868,11 @@ dependencies = [ name = "nu-system" version = "0.68.1" dependencies = [ - "atty", "chrono", "errno", "libc", "libproc", "mach2", - "nix", "ntapi", "once_cell", "procfs", diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index f29d0b3a52..788181f230 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -7,7 +7,6 @@ use nu_protocol::did_you_mean; use nu_protocol::engine::{EngineState, Stack}; use nu_protocol::{ast::Call, engine::Command, ShellError, Signature, SyntaxShape, Value}; use nu_protocol::{Category, Example, ListStream, PipelineData, RawStream, Span, Spanned}; -use nu_system::ForegroundProcess; use pathdiff::diff_paths; use std::collections::HashMap; use std::io::{BufRead, BufReader, Write}; @@ -142,7 +141,7 @@ impl ExternalCommand { let ctrlc = engine_state.ctrlc.clone(); - let mut fg_process = ForegroundProcess::new(self.create_process(&input, false, head)?); + let mut process = self.create_process(&input, false, head)?; // mut is used in the windows branch only, suppress warning on other platforms #[allow(unused_mut)] let mut child; @@ -157,7 +156,8 @@ impl ExternalCommand { // fails to be run as a normal executable: // 1. "shell out" to cmd.exe if the command is a known cmd.exe internal command // 2. Otherwise, use `which-rs` to look for batch files etc. then run those in cmd.exe - match fg_process.spawn() { + + match process.spawn() { Err(err) => { // set the default value, maybe we'll override it later child = Err(err); @@ -174,8 +174,7 @@ impl ExternalCommand { .any(|&cmd| command_name_upper == cmd); if looks_like_cmd_internal { - let mut cmd_process = - ForegroundProcess::new(self.create_process(&input, true, head)?); + let mut cmd_process = self.create_process(&input, true, head)?; child = cmd_process.spawn(); } else { #[cfg(feature = "which-support")] @@ -203,10 +202,8 @@ impl ExternalCommand { item: file_name.to_string_lossy().to_string(), span: self.name.span, }; - let mut cmd_process = ForegroundProcess::new( - new_command - .create_process(&input, true, head)?, - ); + let mut cmd_process = new_command + .create_process(&input, true, head)?; child = cmd_process.spawn(); } } @@ -224,7 +221,7 @@ impl ExternalCommand { #[cfg(not(windows))] { - child = fg_process.spawn() + child = process.spawn() } match child { @@ -276,7 +273,7 @@ impl ExternalCommand { engine_state.config.use_ansi_coloring = false; // if there is a string or a stream, that is sent to the pipe std - if let Some(mut stdin_write) = child.as_mut().stdin.take() { + if let Some(mut stdin_write) = child.stdin.take() { std::thread::spawn(move || { let input = crate::Table::run( &crate::Table, @@ -317,7 +314,7 @@ impl ExternalCommand { // and we create a ListStream that can be consumed if redirect_stderr { - let stderr = child.as_mut().stderr.take().ok_or_else(|| { + let stderr = child.stderr.take().ok_or_else(|| { ShellError::ExternalCommand( "Error taking stderr from external".to_string(), "Redirects need access to stderr of an external command" @@ -356,7 +353,7 @@ impl ExternalCommand { } if redirect_stdout { - let stdout = child.as_mut().stdout.take().ok_or_else(|| { + let stdout = child.stdout.take().ok_or_else(|| { ShellError::ExternalCommand( "Error taking stdout from external".to_string(), "Redirects need access to stdout of an external command" @@ -394,7 +391,7 @@ impl ExternalCommand { } } - match child.as_mut().wait() { + match child.wait() { Err(err) => Err(ShellError::ExternalCommand( "External command exited with error".into(), err.to_string(), diff --git a/crates/nu-system/Cargo.toml b/crates/nu-system/Cargo.toml index 4b96b9232c..e59efabc59 100644 --- a/crates/nu-system/Cargo.toml +++ b/crates/nu-system/Cargo.toml @@ -16,10 +16,6 @@ path = "src/main.rs" [dependencies] libc = "0.2" -[target.'cfg(target_family = "unix")'.dependencies] -nix = "0.24" -atty = "0.2" - [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] procfs = "0.14.0" diff --git a/crates/nu-system/src/foreground.rs b/crates/nu-system/src/foreground.rs deleted file mode 100644 index b9fdcc46af..0000000000 --- a/crates/nu-system/src/foreground.rs +++ /dev/null @@ -1,117 +0,0 @@ -use std::process::{Child, Command}; - -/// A simple wrapper for `std::process::Command` -/// -/// ## spawn behavior -/// ### Unix -/// When invoke `spawn`, current process will block `SIGTSTP`, `SIGTTOU`, `SIGTTIN`, `SIGCHLD` -/// -/// spawned child process will get it's own process group id, and it's going to foreground(by making stdin belong's to child's process group). -/// -/// When child is to over, unblock `SIGTSTP`, `SIGTTOU`, `SIGTTIN`, `SIGCHLD`, foreground process is back to callers' process. -/// It bahaves something like `SignalHandler` in ion(https://gitlab.redox-os.org/redox-os/ion/-/tree/master/). -/// -/// ### Windows -/// It does nothing special on windows system, `spawn` is the same as [std::process::Command::spawn](std::process::Command::spawn) -pub struct ForegroundProcess { - inner: Command, -} - -/// A simple wrapper for `std::process::Child` -/// -/// It can only be created by `ForegroundProcess::spawn`. -pub struct ForegroundChild { - inner: Child, -} - -impl ForegroundProcess { - pub fn new(cmd: Command) -> Self { - Self { inner: cmd } - } - - pub fn spawn(&mut self) -> std::io::Result { - fg_process_setup::prepare_to_foreground(&mut self.inner); - self.inner.spawn().map(|child| { - fg_process_setup::set_foreground(&child); - ForegroundChild { inner: child } - }) - } -} - -impl AsMut for ForegroundChild { - fn as_mut(&mut self) -> &mut Child { - &mut self.inner - } -} - -impl Drop for ForegroundChild { - fn drop(&mut self) { - // It's ok to use here because we have called `set_foreground` during creation. - unsafe { fg_process_setup::reset_foreground_id() } - } -} - -// It's a simpler version of fish shell's external process handling. -#[cfg(target_family = "unix")] -mod fg_process_setup { - use crate::signal::{block, unblock}; - use nix::unistd::{self, Pid}; - use std::os::unix::prelude::CommandExt; - - pub(super) fn prepare_to_foreground(external_command: &mut std::process::Command) { - unsafe { - block(); - // Safety: - // POSIX only allows async-signal-safe functions to be called. - // And `setpgid` is async-signal-safe function according to: - // https://manpages.ubuntu.com/manpages/bionic/man7/signal-safety.7.html - // So we're ok to invoke `libc::setpgid` inside `pre_exec`. - external_command.pre_exec(|| { - // make the command startup with new process group. - // The process group id must be the same as external commands' pid. - // Or else we'll failed to set it as foreground process. - // For more information, check `fork_child_for_process` function: - // https://github.com/fish-shell/fish-shell/blob/023042098396aa450d2c4ea1eb9341312de23126/src/exec.cpp#L398 - if let Err(e) = unistd::setpgid(Pid::from_raw(0), Pid::from_raw(0)) { - println!("ERROR: setpgid for external failed, result: {e:?}"); - } - Ok(()) - }); - } - } - - // If `prepare_to_foreground` function is not called, the function will fail with silence and do nothing. - pub(super) fn set_foreground(process: &std::process::Child) { - if atty::is(atty::Stream::Stdin) { - if let Err(e) = - nix::unistd::tcsetpgrp(nix::libc::STDIN_FILENO, Pid::from_raw(process.id() as i32)) - { - println!("ERROR: set foreground id failed, tcsetpgrp result: {e:?}"); - } - } - } - - /// Reset foreground to current process, unblock `SIGTSTP`, `SIGTTOU`, `SIGTTIN`, `SIGCHLD` - /// - /// ## Safety - /// It can only be called when you have called `set_foreground`, or results in undefined behavior. - pub(super) unsafe fn reset_foreground_id() { - if atty::is(atty::Stream::Stdin) { - if let Err(e) = nix::unistd::tcsetpgrp(nix::libc::STDIN_FILENO, unistd::getpgrp()) { - println!("ERROR: reset foreground id failed, tcsetpgrp result: {e:?}"); - } - } - unblock() - } -} - -// TODO: investigate if we can set foreground process through windows system call. -#[cfg(target_family = "windows")] -mod fg_process_setup { - - pub(super) fn prepare_to_foreground(_external_command: &mut std::process::Command) {} - - pub(super) fn set_foreground(_process: &std::process::Child) {} - - pub(super) unsafe fn reset_foreground_id() {} -} diff --git a/crates/nu-system/src/lib.rs b/crates/nu-system/src/lib.rs index 40f218e7d9..1ad89d73d5 100644 --- a/crates/nu-system/src/lib.rs +++ b/crates/nu-system/src/lib.rs @@ -1,15 +1,10 @@ -mod foreground; #[cfg(any(target_os = "android", target_os = "linux"))] mod linux; #[cfg(target_os = "macos")] mod macos; -#[cfg(target_family = "unix")] -pub mod signal; - #[cfg(target_os = "windows")] mod windows; -pub use self::foreground::{ForegroundChild, ForegroundProcess}; #[cfg(any(target_os = "android", target_os = "linux"))] pub use self::linux::*; #[cfg(target_os = "macos")] diff --git a/crates/nu-system/src/signal.rs b/crates/nu-system/src/signal.rs deleted file mode 100644 index bb696d34a8..0000000000 --- a/crates/nu-system/src/signal.rs +++ /dev/null @@ -1,41 +0,0 @@ -use nix::sys::signal::{self, SigHandler, Signal}; - -/// Blocks the SIGTSTP/SIGTTOU/SIGTTIN/SIGCHLD signals so that the shell never receives -/// them. -pub fn block() { - let mut sigset = signal::SigSet::empty(); - sigset.add(signal::Signal::SIGTSTP); - sigset.add(signal::Signal::SIGTTOU); - sigset.add(signal::Signal::SIGTTIN); - sigset.add(signal::Signal::SIGCHLD); - if let Err(e) = signal::sigprocmask(signal::SigmaskHow::SIG_BLOCK, Some(&sigset), None) { - println!("ERROR: Could not block the signals, error message: {e:?}"); - } -} - -/// Unblocks the SIGTSTP/SIGTTOU/SIGTTIN/SIGCHLD signals so children processes can be -/// controlled -/// by the shell. -pub fn unblock() { - let mut sigset = signal::SigSet::empty(); - sigset.add(signal::Signal::SIGTSTP); - sigset.add(signal::Signal::SIGTTOU); - sigset.add(signal::Signal::SIGTTIN); - sigset.add(signal::Signal::SIGCHLD); - if let Err(e) = signal::sigprocmask(signal::SigmaskHow::SIG_UNBLOCK, Some(&sigset), None) { - println!("ERROR: Could not unblock the signals, error message: {e:?}"); - } -} - -// It's referenced from `set_unique_pid` function in `ion`. -pub fn set_terminal_leader() { - let stdin_is_a_tty = atty::is(atty::Stream::Stdin); - if stdin_is_a_tty { - // We have make sure that stdin is a tty, it's ok to ignore SIGTTOU. - unsafe { - if let Err(e) = signal::signal(Signal::SIGTTOU, SigHandler::SigIgn) { - println!("WARN: ignore SIGTTOU failed, error message: {e:?}"); - } - } - } -} diff --git a/src/main.rs b/src/main.rs index 12c67e9f92..a16c4fd1c6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -135,14 +135,6 @@ fn main() -> Result<()> { let parsed_nu_cli_args = parse_commandline_args(&nushell_commandline_args, &mut engine_state); - #[cfg(target_family = "unix")] - { - // This will block SIGTSTP, SIGTTOU, SIGTTIN, and SIGCHLD, which is required - // for this shell to manage its own process group / children / etc. - nu_system::signal::block(); - nu_system::signal::set_terminal_leader() - } - if let Ok(ref args) = parsed_nu_cli_args { set_config_path( &mut engine_state,