diff --git a/Cargo.lock b/Cargo.lock index d5810e7a..42f8dd01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -193,9 +193,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.31" +version = "4.5.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "027bb0d98429ae334a8698531da7077bdf906419543a35a55c2cb1b66437d767" +checksum = "6088f3ae8c3608d19260cd7445411865a485688711b78b5be70d78cd96136f83" dependencies = [ "clap_builder", "clap_derive", @@ -203,9 +203,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.31" +version = "4.5.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5589e0cba072e0f3d23791efac0fd8627b49c829c196a492e88168e6a669d863" +checksum = "22a7ef7f676155edfb82daa97f99441f3ebf4a58d5e32f295a56259f1b6facc8" dependencies = [ "anstream", "anstyle", @@ -225,9 +225,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.28" +version = "4.5.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf4ced95c6f4a675af3da73304b9ac4ed991640c36374e4b46795c49e17cf1ed" +checksum = "09176aae279615badda0765c0c0b3f6ed53f4709118af73cf4655d85d1530cd7" dependencies = [ "heck", "proc-macro2", @@ -575,6 +575,7 @@ dependencies = [ "is_executable", "lexiclean", "libc", + "nix", "num_cpus", "once_cell", "percent-encoding", @@ -607,9 +608,9 @@ checksum = "441225017b106b9f902e97947a6d31e44ebcf274b91bdbfb51e5c477fcd468e5" [[package]] name = "libc" -version = "0.2.170" +version = "0.2.171" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "875b3680cb2f8f71bdcf9a30f38d48282f5d3c95cbf9b3fa57269bb5d5c06828" +checksum = "c19937216e9d3aa9956d9bb8dfc0b0c8beb6058fc4f7a4dc4d850edf86a237d6" [[package]] name = "libredox" @@ -629,9 +630,9 @@ checksum = "d26c52dbd32dccf2d10cac7725f8eae5296885fb5703b261f7d0a0739ec807ab" [[package]] name = "linux-raw-sys" -version = "0.9.2" +version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6db9c683daf087dc577b7506e9695b3d556a9f3849903fa28186283afd6809e9" +checksum = "fe7db12097d22ec582439daf8618b8fdd1a7bef6270e9af3b1ebcd30893cf413" [[package]] name = "log" @@ -687,9 +688,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.20.3" +version = "1.21.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "945462a4b81e43c4e3ba96bd7b49d834c6f61198356aa858733bc4acf3cbe62e" +checksum = "d75b0bedcc4fe52caa0e03d9f1151a323e4aa5e2d78ba3580400cd3c9e2bc4bc" [[package]] name = "option-ext" @@ -754,9 +755,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.39" +version = "1.0.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1f1914ce909e1658d9907913b4b91947430c7d9be598b15a1912935b8c04801" +checksum = "1885c039570dc00dcb4ff087a89e185fd56bae234ddc7f056a945bf36467248d" dependencies = [ "proc-macro2", ] @@ -873,14 +874,14 @@ dependencies = [ [[package]] name = "rustix" -version = "1.0.1" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dade4812df5c384711475be5fcd8c162555352945401aed22a35bffeab61f657" +checksum = "f7178faa4b75a30e269c71e61c353ce2748cf3d76f0c44c393f4e60abf49b825" dependencies = [ "bitflags", "errno", "libc", - "linux-raw-sys 0.9.2", + "linux-raw-sys 0.9.3", "windows-sys 0.59.0", ] @@ -1021,9 +1022,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.99" +version = "2.0.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e02e925281e18ffd9d640e234264753c43edc62d64b2d4cf898f1bc5e75f3fc2" +checksum = "b09a44accad81e1ba1cd74a32461ba89dee89095ba17b32f5d03683b1b1fc2a0" dependencies = [ "proc-macro2", "quote", @@ -1038,15 +1039,14 @@ checksum = "1e8f05f774b2db35bdad5a8237a90be1102669f8ea013fea9777b366d34ab145" [[package]] name = "tempfile" -version = "3.18.0" +version = "3.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c317e0a526ee6120d8dabad239c8dadca62b24b6f168914bbbc8e2fb1f0e567" +checksum = "488960f40a3fd53d72c2a29a58722561dee8afdd175bd88e3db4677d7b2ba600" dependencies = [ - "cfg-if", "fastrand", "getrandom 0.3.1", "once_cell", - "rustix 1.0.1", + "rustix 1.0.2", "windows-sys 0.59.0", ] @@ -1065,7 +1065,7 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "45c6481c4829e4cc63825e62c49186a34538b7b2750b73b266581ffb612fb5ed" dependencies = [ - "rustix 1.0.1", + "rustix 1.0.2", "windows-sys 0.59.0", ] @@ -1166,9 +1166,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" -version = "1.15.1" +version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0f540e3240398cce6128b64ba83fdbdd86129c16a3aa1a3a252efd66eb3d587" +checksum = "458f7a779bf54acc9f347480ac654f68407d3aab21269a6e3c9f922acd9e2da9" dependencies = [ "getrandom 0.3.1", ] diff --git a/Cargo.toml b/Cargo.toml index 931f2b4a..ed7e8431 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,12 @@ typed-arena = "2.0.1" unicode-width = "0.2.0" uuid = { version = "1.0.0", features = ["v4"] } +[target.'cfg(unix)'.dependencies] +nix = { version = "0.29.0", features = ["user"] } + +[target.'cfg(windows)'.dependencies] +ctrlc = { version = "3.1.1", features = ["termination"] } + [dev-dependencies] executable-path = "1.0.0" pretty_assertions = "1.0.0" @@ -74,6 +80,7 @@ struct_excessive_bools = "allow" struct_field_names = "allow" too_many_arguments = "allow" too_many_lines = "allow" +undocumented_unsafe_blocks = "deny" unnecessary_wraps = "allow" wildcard_imports = "allow" diff --git a/README.md b/README.md index c10c3654..54339062 100644 --- a/README.md +++ b/README.md @@ -3976,6 +3976,57 @@ the [`chrono` library docs](https://docs.rs/chrono/latest/chrono/format/strftime/index.html) for details. +### Signal Handling + +[Signals](https://en.wikipedia.org/wiki/Signal_(IPC)) are messsages sent to +running programs to trigger specific behavior. For example, `SIGINT` is sent to +all processes in the terminal forground process group when `CTRL-C` is pressed. + +`just` tries to exit when requested by a signal, but it also tries to avoid +leaving behind running child proccesses, two goals which are somewhat in +conflict. + +If `just` exits leaving behind child processes, the user will have no recourse +but to `ps aux | grep` for the children and manually `kill` them, a tedious +endevour. + +#### Fatal Signals + +`SIGHUP`, `SIGINT`, and `SIGQUIT` are generated when the user closes the +terminal, types `ctrl-c`, or types `ctrl-\`, respectively, and are sent to all +processes in the foreground process group. + +`SIGTERM` is the default signal sent by the `kill` command, and is delivered +only to its intended victim. + +When a child process is not running, `just` will exit immediately on receipt of +any of the above signals. + +When a child process *is* running, `just` will wait until it terminates, to +avoid leaving it behind. + +Additionally, on receipt of `SIGTERM`, `just` will forward `SIGTERM` to any +running childrenmaster, since unlike other fatal signals, `SIGTERM`, +was likely sent to `just` alone. + +Regardless of whether a child process terminates successfully after `just` +receives a fatal signal, `just` halts execution. + +#### `SIGINFO` + +`SIGINFO` is sent to all processes in the foreground process group when the +user types `ctrl-t` on +[BSD](https://en.wikipedia.org/wiki/Berkeley_Software_Distribution)-derived +operating systems, including MacOS, but not Linux. + +`just` responds by printing a list of all child process IDs and +commandsmaster. + +#### Windows + +On Windows, `just` behaves as if it had received `SIGINT` when the user types +`ctrl-c`. Other signals are unsupported. + Changelog --------- diff --git a/justfile b/justfile index 513f319b..cfd9400d 100755 --- a/justfile +++ b/justfile @@ -17,7 +17,7 @@ test: cargo test --all [group: 'check'] -ci: forbid test build-book clippy +ci: test clippy build-book forbid cargo fmt --all -- --check cargo update --locked --package just diff --git a/src/command_ext.rs b/src/command_ext.rs index 40d9da16..1cd7ef03 100644 --- a/src/command_ext.rs +++ b/src/command_ext.rs @@ -7,9 +7,15 @@ pub(crate) trait CommandExt { dotenv: &BTreeMap, scope: &Scope, unexports: &HashSet, - ); + ) -> &mut Command; fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet); + + fn output_guard(self) -> (io::Result, Option); + + fn output_guard_stdout(self) -> Result; + + fn status_guard(self) -> (io::Result, Option); } impl CommandExt for Command { @@ -19,7 +25,7 @@ impl CommandExt for Command { dotenv: &BTreeMap, scope: &Scope, unexports: &HashSet, - ) { + ) -> &mut Command { for (name, value) in dotenv { self.env(name, value); } @@ -27,6 +33,8 @@ impl CommandExt for Command { if let Some(parent) = scope.parent() { self.export_scope(settings, parent, unexports); } + + self } fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet) { @@ -44,4 +52,34 @@ impl CommandExt for Command { } } } + + fn output_guard(self) -> (io::Result, Option) { + SignalHandler::spawn(self, process::Child::wait_with_output) + } + + fn output_guard_stdout(self) -> Result { + let (result, caught) = self.output_guard(); + + let output = result.map_err(OutputError::Io)?; + + OutputError::result_from_exit_status(output.status)?; + + let output = str::from_utf8(&output.stdout).map_err(OutputError::Utf8)?; + + if let Some(signal) = caught { + return Err(OutputError::Interrupted(signal)); + } + + Ok( + output + .strip_suffix("\r\n") + .or_else(|| output.strip_suffix("\n")) + .unwrap_or(output) + .into(), + ) + } + + fn status_guard(self) -> (io::Result, Option) { + SignalHandler::spawn(self, |mut child| child.wait()) + } } diff --git a/src/error.rs b/src/error.rs index 668f16a7..1e4c20ac 100644 --- a/src/error.rs +++ b/src/error.rs @@ -112,6 +112,9 @@ pub(crate) enum Error<'src> { Internal { message: String, }, + Interrupted { + signal: Signal, + }, Io { recipe: &'src str, io_error: io::Error, @@ -158,6 +161,23 @@ pub(crate) enum Error<'src> { line_number: Option, signal: i32, }, + #[cfg(windows)] + SignalHandlerInstall { + source: ctrlc::Error, + }, + #[cfg(unix)] + SignalHandlerPipeOpen { + io_error: io::Error, + }, + #[cfg(unix)] + SignalHandlerSigaction { + signal: Signal, + io_error: io::Error, + }, + #[cfg(unix)] + SignalHandlerSpawnThread { + io_error: io::Error, + }, StdoutIo { io_error: io::Error, }, @@ -194,12 +214,23 @@ pub(crate) enum Error<'src> { impl<'src> Error<'src> { pub(crate) fn code(&self) -> Option { match self { - Self::Code { code, .. } - | Self::Backtick { + Self::Backtick { output_error: OutputError::Code(code), .. - } => Some(*code), + } + | Self::Code { code, .. } => Some(*code), + Self::ChooserStatus { status, .. } | Self::EditorStatus { status, .. } => status.code(), + Self::Backtick { + output_error: OutputError::Signal(signal), + .. + } + | Self::Signal { signal, .. } => 128i32.checked_add(*signal), + Self::Backtick { + output_error: OutputError::Interrupted(signal), + .. + } + | Self::Interrupted { signal } => signal.code(), _ => None, } } @@ -291,6 +322,7 @@ impl ColorDisplay for Error<'_> { OutputError::Code(code) => write!(f, "Backtick failed with exit code {code}")?, OutputError::Signal(signal) => write!(f, "Backtick was terminated by signal {signal}")?, OutputError::Unknown => write!(f, "Backtick failed for an unknown reason")?, + OutputError::Interrupted(signal) => write!(f, "Backtick succeeded but `just` was interrupted by signal {signal}")?, OutputError::Io(io_error) => match io_error.kind() { io::ErrorKind::NotFound => write!(f, "Backtick could not be run because just could not find the shell:\n{io_error}"), io::ErrorKind::PermissionDenied => write!(f, "Backtick could not be run because just could not run the shell:\n{io_error}"), @@ -340,6 +372,7 @@ impl ColorDisplay for Error<'_> { OutputError::Code(code) => write!(f, "Cygpath failed with exit code {code} while translating recipe `{recipe}` shebang interpreter path")?, OutputError::Signal(signal) => write!(f, "Cygpath terminated by signal {signal} while translating recipe `{recipe}` shebang interpreter path")?, OutputError::Unknown => write!(f, "Cygpath experienced an unknown failure while translating recipe `{recipe}` shebang interpreter path")?, + OutputError::Interrupted(signal) => write!(f, "Cygpath succeeded but `just` was interrupted by {signal}")?, OutputError::Io(io_error) => { match io_error.kind() { io::ErrorKind::NotFound => write!(f, "Could not find `cygpath` executable to translate recipe `{recipe}` shebang interpreter path:\n{io_error}"), @@ -402,6 +435,9 @@ impl ColorDisplay for Error<'_> { write!(f, "Internal runtime error, this may indicate a bug in just: {message} \ consider filing an issue: https://github.com/casey/just/issues/new")?; } + Interrupted { signal } => { + write!(f, "Interrupted by {signal}")?; + } Io { recipe, io_error } => { match io_error.kind() { io::ErrorKind::NotFound => write!(f, "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"), @@ -442,8 +478,24 @@ impl ColorDisplay for Error<'_> { write!(f, "Recipe `{recipe}` was terminated by signal {signal}")?; } } + #[cfg(windows)] + SignalHandlerInstall { source } => { + write!(f, "Could not install signal handler: {source}")?; + } + #[cfg(unix)] + SignalHandlerPipeOpen { io_error } => { + write!(f, "I/O error opening pipe for signal handler: {io_error}")?; + } + #[cfg(unix)] + SignalHandlerSigaction { io_error, signal } => { + write!(f, "I/O error setting sigaction for {signal}: {io_error}")?; + } + #[cfg(unix)] + SignalHandlerSpawnThread { io_error } => { + write!(f, "I/O error spawning thread for signal handler: {io_error}")?; + } StdoutIo { io_error } => { - write!(f, "I/O error writing to stdout: {io_error}?")?; + write!(f, "I/O error writing to stdout: {io_error}")?; } TempdirIo { recipe, io_error } => { write!(f, "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \ diff --git a/src/evaluator.rs b/src/evaluator.rs index 1558da80..84171f0e 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -273,22 +273,26 @@ impl<'src, 'run> Evaluator<'src, 'run> { .module .settings .shell_command(self.context.config); - cmd.arg(command); - cmd.args(args); - cmd.current_dir(self.context.working_directory()); - cmd.export( - &self.context.module.settings, - self.context.dotenv, - &self.scope, - &self.context.module.unexports, - ); - cmd.stdin(Stdio::inherit()); - cmd.stderr(if self.context.config.verbosity.quiet() { - Stdio::null() - } else { - Stdio::inherit() - }); - InterruptHandler::guard(|| output(cmd)) + + cmd + .arg(command) + .args(args) + .current_dir(self.context.working_directory()) + .export( + &self.context.module.settings, + self.context.dotenv, + &self.scope, + &self.context.module.unexports, + ) + .stdin(Stdio::inherit()) + .stderr(if self.context.config.verbosity.quiet() { + Stdio::null() + } else { + Stdio::inherit() + }) + .stdout(Stdio::piped()); + + cmd.output_guard_stdout() } pub(crate) fn evaluate_line( diff --git a/src/interrupt_guard.rs b/src/interrupt_guard.rs deleted file mode 100644 index 23afa1dd..00000000 --- a/src/interrupt_guard.rs +++ /dev/null @@ -1,16 +0,0 @@ -use super::*; - -pub(crate) struct InterruptGuard; - -impl InterruptGuard { - pub(crate) fn new() -> Self { - InterruptHandler::instance().block(); - Self - } -} - -impl Drop for InterruptGuard { - fn drop(&mut self) { - InterruptHandler::instance().unblock(); - } -} diff --git a/src/interrupt_handler.rs b/src/interrupt_handler.rs deleted file mode 100644 index 1105954b..00000000 --- a/src/interrupt_handler.rs +++ /dev/null @@ -1,86 +0,0 @@ -use super::*; - -pub(crate) struct InterruptHandler { - blocks: u32, - interrupted: bool, - verbosity: Verbosity, -} - -impl InterruptHandler { - pub(crate) fn install(verbosity: Verbosity) -> Result<(), ctrlc::Error> { - let mut instance = Self::instance(); - instance.verbosity = verbosity; - ctrlc::set_handler(|| Self::instance().interrupt()) - } - - pub(crate) fn instance() -> MutexGuard<'static, Self> { - static INSTANCE: Mutex = Mutex::new(InterruptHandler::new()); - - match INSTANCE.lock() { - Ok(guard) => guard, - Err(poison_error) => { - eprintln!( - "{}", - Error::Internal { - message: format!("interrupt handler mutex poisoned: {poison_error}"), - } - .color_display(Color::auto().stderr()) - ); - process::exit(EXIT_FAILURE); - } - } - } - - const fn new() -> Self { - Self { - blocks: 0, - interrupted: false, - verbosity: Verbosity::default(), - } - } - - fn interrupt(&mut self) { - self.interrupted = true; - - if self.blocks > 0 { - return; - } - - Self::exit(); - } - - fn exit() { - process::exit(130); - } - - pub(crate) fn block(&mut self) { - self.blocks += 1; - } - - pub(crate) fn unblock(&mut self) { - if self.blocks == 0 { - if self.verbosity.loud() { - eprintln!( - "{}", - Error::Internal { - message: "attempted to unblock interrupt handler, but handler was not blocked" - .to_owned(), - } - .color_display(Color::auto().stderr()) - ); - } - process::exit(EXIT_FAILURE); - } - - self.blocks -= 1; - - if self.interrupted { - Self::exit(); - } - } - - pub(crate) fn guard T>(function: F) -> T { - let _guard = InterruptGuard::new(); - function() - } -} diff --git a/src/justfile.rs b/src/justfile.rs index 342a6109..4eb474fb 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -109,20 +109,20 @@ impl<'src> Justfile<'src> { Command::new(binary) }; - command.args(arguments); - - command.current_dir(&search.working_directory); + command + .args(arguments) + .current_dir(&search.working_directory); let scope = scope.child(); command.export(&self.settings, &dotenv, &scope, &self.unexports); - let status = InterruptHandler::guard(|| command.status()).map_err(|io_error| { - Error::CommandInvoke { - binary: binary.clone(), - arguments: arguments.clone(), - io_error, - } + let (result, caught) = command.status_guard(); + + let status = result.map_err(|io_error| Error::CommandInvoke { + binary: binary.clone(), + arguments: arguments.clone(), + io_error, })?; if !status.success() { @@ -133,6 +133,10 @@ impl<'src> Justfile<'src> { }); }; + if let Some(signal) = caught { + return Err(Error::Interrupted { signal }); + } + return Ok(()); } Subcommand::Evaluate { variable, .. } => { diff --git a/src/lib.rs b/src/lib.rs index 1830ac71..1bdf9ad1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,8 +42,6 @@ pub(crate) use { fragment::Fragment, function::Function, interpreter::Interpreter, - interrupt_guard::InterruptGuard, - interrupt_handler::InterruptHandler, item::Item, justfile::Justfile, keyed::Keyed, @@ -57,7 +55,6 @@ pub(crate) use { name::Name, namepath::Namepath, ordinal::Ordinal, - output::output, output_error::OutputError, parameter::Parameter, parameter_kind::ParameterKind, @@ -80,6 +77,8 @@ pub(crate) use { settings::Settings, shebang::Shebang, show_whitespace::ShowWhitespace, + signal::Signal, + signal_handler::SignalHandler, source::Source, string_delimiter::StringDelimiter, string_kind::StringKind, @@ -221,8 +220,6 @@ mod expression; mod fragment; mod function; mod interpreter; -mod interrupt_guard; -mod interrupt_handler; mod item; mod justfile; mod keyed; @@ -236,7 +233,6 @@ mod module_path; mod name; mod namepath; mod ordinal; -mod output; mod output_error; mod parameter; mod parameter_kind; @@ -260,6 +256,10 @@ mod setting; mod settings; mod shebang; mod show_whitespace; +mod signal; +mod signal_handler; +#[cfg(unix)] +mod signals; mod source; mod string_delimiter; mod string_kind; diff --git a/src/output.rs b/src/output.rs deleted file mode 100644 index f9f4ab02..00000000 --- a/src/output.rs +++ /dev/null @@ -1,34 +0,0 @@ -use super::*; - -/// Run a command and return the data it wrote to stdout as a string -pub(crate) fn output(mut command: Command) -> Result { - match command.output() { - Ok(output) => { - if let Some(code) = output.status.code() { - if code != 0 { - return Err(OutputError::Code(code)); - } - } else { - let signal = Platform::signal_from_exit_status(output.status); - return Err(match signal { - Some(signal) => OutputError::Signal(signal), - None => OutputError::Unknown, - }); - } - match str::from_utf8(&output.stdout) { - Err(error) => Err(OutputError::Utf8(error)), - Ok(output) => Ok( - if output.ends_with("\r\n") { - &output[0..output.len() - 2] - } else if output.ends_with('\n') { - &output[0..output.len() - 1] - } else { - output - } - .to_owned(), - ), - } - } - Err(io_error) => Err(OutputError::Io(io_error)), - } -} diff --git a/src/output_error.rs b/src/output_error.rs index b1a16558..84c4b93b 100644 --- a/src/output_error.rs +++ b/src/output_error.rs @@ -4,6 +4,8 @@ use super::*; pub(crate) enum OutputError { /// Non-zero exit code Code(i32), + /// Interrupted by signal + Interrupted(Signal), /// IO error Io(io::Error), /// Terminated by signal @@ -14,10 +16,27 @@ pub(crate) enum OutputError { Utf8(str::Utf8Error), } +impl OutputError { + pub(crate) fn result_from_exit_status(exit_status: ExitStatus) -> Result<(), OutputError> { + match exit_status.code() { + Some(0) => Ok(()), + Some(code) => Err(Self::Code(code)), + None => match Platform::signal_from_exit_status(exit_status) { + Some(signal) => Err(Self::Signal(signal)), + None => Err(Self::Unknown), + }, + } + } +} + impl Display for OutputError { fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { Self::Code(code) => write!(f, "Process exited with status code {code}"), + Self::Interrupted(signal) => write!( + f, + "Process succeded but `just` was interrupted by signal {signal}" + ), Self::Io(ref io_error) => write!(f, "Error executing process: {io_error}"), Self::Signal(signal) => write!(f, "Process terminated by signal {signal}"), Self::Unknown => write!(f, "Process experienced an unknown failure"), diff --git a/src/platform.rs b/src/platform.rs index ece80fe1..973062ab 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -3,113 +3,7 @@ use super::*; pub(crate) struct Platform; #[cfg(unix)] -impl PlatformInterface for Platform { - fn make_shebang_command( - path: &Path, - working_directory: Option<&Path>, - _shebang: Shebang, - ) -> Result { - // shebang scripts can be executed directly on unix - let mut command = Command::new(path); - - if let Some(working_directory) = working_directory { - command.current_dir(working_directory); - } - - Ok(command) - } - - fn set_execute_permission(path: &Path) -> io::Result<()> { - use std::os::unix::fs::PermissionsExt; - - // get current permissions - let mut permissions = fs::metadata(path)?.permissions(); - - // set the execute bit - let current_mode = permissions.mode(); - permissions.set_mode(current_mode | 0o100); - - // set the new permissions - fs::set_permissions(path, permissions) - } - - fn signal_from_exit_status(exit_status: ExitStatus) -> Option { - use std::os::unix::process::ExitStatusExt; - exit_status.signal() - } - - fn convert_native_path(_working_directory: &Path, path: &Path) -> FunctionResult { - path - .to_str() - .map(str::to_string) - .ok_or_else(|| String::from("Error getting current directory: unicode decode error")) - } -} +mod unix; #[cfg(windows)] -impl PlatformInterface for Platform { - fn make_shebang_command( - path: &Path, - working_directory: Option<&Path>, - shebang: Shebang, - ) -> Result { - use std::borrow::Cow; - - // If the path contains forward slashes… - let command = if shebang.interpreter.contains('/') { - // …translate path to the interpreter from unix style to windows style. - let mut cygpath = Command::new("cygpath"); - if let Some(working_directory) = working_directory { - cygpath.current_dir(working_directory); - } - cygpath.arg("--windows"); - cygpath.arg(shebang.interpreter); - - Cow::Owned(output(cygpath)?) - } else { - // …otherwise use it as-is. - Cow::Borrowed(shebang.interpreter) - }; - - let mut cmd = Command::new(command.as_ref()); - - if let Some(working_directory) = working_directory { - cmd.current_dir(working_directory); - } - - if let Some(argument) = shebang.argument { - cmd.arg(argument); - } - - cmd.arg(path); - Ok(cmd) - } - - fn set_execute_permission(_path: &Path) -> io::Result<()> { - // it is not necessary to set an execute permission on a script on windows, so - // this is a nop - Ok(()) - } - - fn signal_from_exit_status(_exit_status: process::ExitStatus) -> Option { - // The rust standard library does not expose a way to extract a signal from a - // windows process exit status, so just return None - None - } - - fn convert_native_path(working_directory: &Path, path: &Path) -> FunctionResult { - // Translate path from windows style to unix style - let mut cygpath = Command::new("cygpath"); - cygpath.current_dir(working_directory); - cygpath.arg("--unix"); - cygpath.arg(path); - - match output(cygpath) { - Ok(shell_path) => Ok(shell_path), - Err(_) => path - .to_str() - .map(str::to_string) - .ok_or_else(|| String::from("Error getting current directory: unicode decode error")), - } - } -} +mod windows; diff --git a/src/platform/unix.rs b/src/platform/unix.rs new file mode 100644 index 00000000..26ee625c --- /dev/null +++ b/src/platform/unix.rs @@ -0,0 +1,62 @@ +use super::*; + +impl PlatformInterface for Platform { + fn make_shebang_command( + path: &Path, + working_directory: Option<&Path>, + _shebang: Shebang, + ) -> Result { + // shebang scripts can be executed directly on unix + let mut command = Command::new(path); + + if let Some(working_directory) = working_directory { + command.current_dir(working_directory); + } + + Ok(command) + } + + fn set_execute_permission(path: &Path) -> io::Result<()> { + use std::os::unix::fs::PermissionsExt; + + // get current permissions + let mut permissions = fs::metadata(path)?.permissions(); + + // set the execute bit + let current_mode = permissions.mode(); + permissions.set_mode(current_mode | 0o100); + + // set the new permissions + fs::set_permissions(path, permissions) + } + + fn signal_from_exit_status(exit_status: ExitStatus) -> Option { + use std::os::unix::process::ExitStatusExt; + exit_status.signal() + } + + fn convert_native_path(_working_directory: &Path, path: &Path) -> FunctionResult { + path + .to_str() + .map(str::to_string) + .ok_or_else(|| String::from("Error getting current directory: unicode decode error")) + } + + fn install_signal_handler(handler: T) -> RunResult<'static> { + let signals = crate::signals::Signals::new()?; + + std::thread::Builder::new() + .name("signal handler".into()) + .spawn(move || { + for signal in signals { + match signal { + Ok(signal) => handler(signal), + Err(io_error) => eprintln!("warning: I/O error reading from signal pipe: {io_error}"), + } + } + }) + .map_err(|io_error| Error::SignalHandlerSpawnThread { io_error })?; + + Ok(()) + } +} diff --git a/src/platform/windows.rs b/src/platform/windows.rs new file mode 100644 index 00000000..07438d9a --- /dev/null +++ b/src/platform/windows.rs @@ -0,0 +1,84 @@ +use super::*; + +impl PlatformInterface for Platform { + fn make_shebang_command( + path: &Path, + working_directory: Option<&Path>, + shebang: Shebang, + ) -> Result { + use std::borrow::Cow; + + // If the path contains forward slashes… + let command = if shebang.interpreter.contains('/') { + // …translate path to the interpreter from unix style to windows style. + let mut cygpath = Command::new("cygpath"); + + if let Some(working_directory) = working_directory { + cygpath.current_dir(working_directory); + } + + cygpath + .arg("--windows") + .arg(shebang.interpreter) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + Cow::Owned(cygpath.output_guard_stdout()?) + } else { + // …otherwise use it as-is. + Cow::Borrowed(shebang.interpreter) + }; + + let mut cmd = Command::new(command.as_ref()); + + if let Some(working_directory) = working_directory { + cmd.current_dir(working_directory); + } + + if let Some(argument) = shebang.argument { + cmd.arg(argument); + } + + cmd.arg(path); + Ok(cmd) + } + + fn set_execute_permission(_path: &Path) -> io::Result<()> { + // it is not necessary to set an execute permission on a script on windows, so + // this is a nop + Ok(()) + } + + fn signal_from_exit_status(_exit_status: process::ExitStatus) -> Option { + // The rust standard library does not expose a way to extract a signal from a + // windows process exit status, so just return None + None + } + + fn convert_native_path(working_directory: &Path, path: &Path) -> FunctionResult { + // Translate path from windows style to unix style + let mut cygpath = Command::new("cygpath"); + + cygpath + .current_dir(working_directory) + .arg("--unix") + .arg(path) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + match cygpath.output_guard_stdout() { + Ok(shell_path) => Ok(shell_path), + Err(_) => path + .to_str() + .map(str::to_string) + .ok_or_else(|| String::from("Error getting current directory: unicode decode error")), + } + } + + fn install_signal_handler(handler: T) -> RunResult<'static> { + ctrlc::set_handler(move || handler(Signal::Interrupt)) + .map_err(|source| Error::SignalHandlerInstall { source }) + } +} diff --git a/src/platform_interface.rs b/src/platform_interface.rs index 573d6abb..67b5a92c 100644 --- a/src/platform_interface.rs +++ b/src/platform_interface.rs @@ -1,21 +1,23 @@ use super::*; pub(crate) trait PlatformInterface { - /// Translate a path from a "native" path to a path the interpreter expects + /// translate path from "native" path to path interpreter expects fn convert_native_path(working_directory: &Path, path: &Path) -> FunctionResult; - /// Construct a command equivalent to running the script at `path` with the - /// shebang line `shebang` + /// install handler, may only be called once + fn install_signal_handler(handler: T) -> RunResult<'static>; + + /// construct command equivalent to running script at `path` with shebang + /// line `shebang` fn make_shebang_command( path: &Path, working_directory: Option<&Path>, shebang: Shebang, ) -> Result; - /// Set the execute permission on the file pointed to by `path` + /// set the execute permission on file pointed to by `path` fn set_execute_permission(path: &Path) -> io::Result<()>; - /// Extract the signal from a process exit status, if it was terminated by a - /// signal + /// extract signal from process exit status fn signal_from_exit_status(exit_status: ExitStatus) -> Option; } diff --git a/src/recipe.rs b/src/recipe.rs index 304d1295..83bb9ee3 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -300,7 +300,9 @@ impl<'src, D> Recipe<'src, D> { &context.module.unexports, ); - match InterruptHandler::guard(|| cmd.status()) { + let (result, caught) = cmd.status_guard(); + + match result { Ok(exit_status) => { if let Some(code) = exit_status.code() { if code != 0 && !infallible_line { @@ -311,7 +313,7 @@ impl<'src, D> Recipe<'src, D> { print_message: self.print_exit_message(&context.module.settings), }); } - } else { + } else if !infallible_line { return Err(error_from_signal( self.name(), Some(line_number), @@ -326,6 +328,12 @@ impl<'src, D> Recipe<'src, D> { }); } }; + + if !infallible_line { + if let Some(signal) = caught { + return Err(Error::Interrupted { signal }); + } + } } } @@ -442,7 +450,9 @@ impl<'src, D> Recipe<'src, D> { ); // run it! - match InterruptHandler::guard(|| command.status()) { + let (result, caught) = command.status_guard(); + + match result { Ok(exit_status) => exit_status.code().map_or_else( || Err(error_from_signal(self.name(), None, exit_status)), |code| { @@ -457,9 +467,15 @@ impl<'src, D> Recipe<'src, D> { }) } }, - ), - Err(io_error) => Err(executor.error(io_error, self.name())), + )?, + Err(io_error) => return Err(executor.error(io_error, self.name())), } + + if let Some(signal) = caught { + return Err(Error::Interrupted { signal }); + } + + Ok(()) } pub(crate) fn groups(&self) -> BTreeSet { diff --git a/src/request.rs b/src/request.rs index a84a84c2..de66986a 100644 --- a/src/request.rs +++ b/src/request.rs @@ -4,10 +4,14 @@ use super::*; #[serde(rename_all = "kebab-case")] pub enum Request { EnvironmentVariable(String), + #[cfg(not(windows))] + Signal, } #[derive(Debug, Deserialize, PartialEq, Serialize)] #[serde(rename_all = "kebab-case")] pub enum Response { EnvironmentVariable(Option), + #[cfg(not(windows))] + Signal(String), } diff --git a/src/run.rs b/src/run.rs index a07b73bb..53e37fb5 100644 --- a/src/run.rs +++ b/src/run.rs @@ -24,7 +24,7 @@ pub fn run(args: impl Iterator + Clone>) -> Result<() config .and_then(|config| { - InterruptHandler::install(config.verbosity).ok(); + SignalHandler::install(config.verbosity)?; config.subcommand.execute(&config, &loader) }) .map_err(|error| { diff --git a/src/signal.rs b/src/signal.rs new file mode 100644 index 00000000..d8e34112 --- /dev/null +++ b/src/signal.rs @@ -0,0 +1,155 @@ +use super::*; + +#[derive(Clone, Copy, Debug, PartialEq)] +#[repr(i32)] +pub(crate) enum Signal { + Hangup = 1, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Info = 29, + Interrupt = 2, + Quit = 3, + Terminate = 15, +} + +impl Signal { + #[cfg(not(windows))] + pub(crate) const ALL: &[Signal] = &[ + Signal::Hangup, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info, + Signal::Interrupt, + Signal::Quit, + Signal::Terminate, + ]; + + pub(crate) fn code(self) -> Option { + 128i32.checked_add(self.number()) + } + + pub(crate) fn number(self) -> i32 { + self as libc::c_int + } +} + +impl Display for Signal { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + write!( + f, + "{}", + match self { + Signal::Hangup => "SIGHUP", + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info => "SIGINFO", + Signal::Interrupt => "SIGINT", + Signal::Quit => "SIGQUIT", + Signal::Terminate => "SIGTERM", + } + ) + } +} + +#[cfg(not(windows))] +impl From for nix::sys::signal::Signal { + fn from(signal: Signal) -> Self { + match signal { + Signal::Hangup => Self::SIGHUP, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info => Self::SIGINFO, + Signal::Interrupt => Self::SIGINT, + Signal::Quit => Self::SIGQUIT, + Signal::Terminate => Self::SIGTERM, + } + } +} + +impl TryFrom for Signal { + type Error = io::Error; + + fn try_from(n: u8) -> Result { + match n { + 1 => Ok(Signal::Hangup), + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + 29 => Ok(Signal::Info), + 2 => Ok(Signal::Interrupt), + 3 => Ok(Signal::Quit), + 15 => Ok(Signal::Terminate), + _ => Err(io::Error::new( + io::ErrorKind::Other, + format!("unexpected signal: {n}"), + )), + } + } +} + +#[cfg(test)] +#[cfg(not(windows))] +mod tests { + use super::*; + + #[test] + fn signals_fit_in_u8() { + for signal in Signal::ALL { + assert!(signal.number() <= i32::from(u8::MAX)); + } + } + + #[test] + fn signal_numbers_are_correct() { + for &signal in Signal::ALL { + let n = match signal { + Signal::Hangup => libc::SIGHUP, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info => libc::SIGINFO, + Signal::Interrupt => libc::SIGINT, + Signal::Quit => libc::SIGQUIT, + Signal::Terminate => libc::SIGTERM, + }; + + assert_eq!(signal as i32, n); + + assert_eq!(Signal::try_from(u8::try_from(n).unwrap()).unwrap(), signal); + } + } +} diff --git a/src/signal_handler.rs b/src/signal_handler.rs new file mode 100644 index 00000000..c42f3701 --- /dev/null +++ b/src/signal_handler.rs @@ -0,0 +1,147 @@ +use super::*; + +pub(crate) struct SignalHandler { + caught: Option, + children: BTreeMap, + verbosity: Verbosity, +} + +impl SignalHandler { + pub(crate) fn install(verbosity: Verbosity) -> RunResult<'static> { + let mut instance = Self::instance(); + instance.verbosity = verbosity; + Platform::install_signal_handler(|signal| Self::instance().interrupt(signal)) + } + + pub(crate) fn instance() -> MutexGuard<'static, Self> { + static INSTANCE: Mutex = Mutex::new(SignalHandler::new()); + + match INSTANCE.lock() { + Ok(guard) => guard, + Err(poison_error) => { + eprintln!( + "{}", + Error::Internal { + message: format!("signal handler mutex poisoned: {poison_error}"), + } + .color_display(Color::auto().stderr()) + ); + process::exit(EXIT_FAILURE); + } + } + } + + const fn new() -> Self { + Self { + caught: None, + children: BTreeMap::new(), + verbosity: Verbosity::default(), + } + } + + fn interrupt(&mut self, signal: Signal) { + if self.children.is_empty() { + process::exit(signal.code().unwrap_or(1)); + } + + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + if signal != Signal::Info && self.caught.is_none() { + self.caught = Some(signal); + } + + match signal { + // SIGHUP, SIGINT, and SIGQUIT are normally sent on terminal close, + // ctrl-c, and ctrl-\, respectively, and are sent to all processes in the + // foreground process group. this includes child processes, so we ignore + // the signal and wait for them to exit + Signal::Hangup | Signal::Interrupt | Signal::Quit => {} + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info => { + let id = process::id(); + if self.children.is_empty() { + eprintln!("just {id}: no child processes"); + } else { + let n = self.children.len(); + + let mut message = format!( + "just {id}: {n} child {}:\n", + if n == 1 { "process" } else { "processes" } + ); + + for (&child, command) in &self.children { + message.push_str(&format!("{child}: {command:?}\n")); + } + + eprint!("{message}"); + } + } + // SIGTERM is the default signal sent by kill. forward it to child + // processes and wait for them to exit + Signal::Terminate => + { + #[cfg(not(windows))] + for &child in self.children.keys() { + if self.verbosity.loquacious() { + eprintln!("just: sending SIGTERM to child process {child}"); + } + nix::sys::signal::kill( + nix::unistd::Pid::from_raw(child), + Some(Signal::Terminate.into()), + ) + .ok(); + } + } + } + } + + pub(crate) fn spawn( + mut command: Command, + f: impl Fn(process::Child) -> io::Result, + ) -> (io::Result, Option) { + let mut instance = Self::instance(); + + let child = match command.spawn() { + Err(err) => return (Err(err), None), + Ok(child) => child, + }; + + let pid = match child.id().try_into() { + Err(err) => { + return ( + Err(io::Error::new( + io::ErrorKind::Other, + format!("invalid child PID: {err}"), + )), + None, + ) + } + Ok(pid) => pid, + }; + + instance.children.insert(pid, command); + + drop(instance); + + let result = f(child); + + let mut instance = Self::instance(); + + instance.children.remove(&pid); + + (result, instance.caught) + } +} diff --git a/src/signals.rs b/src/signals.rs new file mode 100644 index 00000000..23f2477e --- /dev/null +++ b/src/signals.rs @@ -0,0 +1,122 @@ +use { + super::*, + nix::{ + errno::Errno, + sys::signal::{SaFlags, SigAction, SigHandler, SigSet}, + }, + std::{ + fs::File, + os::fd::{BorrowedFd, IntoRawFd}, + sync::atomic::{self, AtomicI32}, + }, +}; + +const INVALID_FILENO: i32 = -1; + +static WRITE: AtomicI32 = AtomicI32::new(INVALID_FILENO); + +fn die(message: &str) -> ! { + // SAFETY: + // + // Standard error is open for the duration of the program. + const STDERR: BorrowedFd = unsafe { BorrowedFd::borrow_raw(libc::STDERR_FILENO) }; + + let mut i = 0; + let mut buffer = [0; 512]; + + let mut append = |s: &str| { + let remaining = buffer.len() - i; + let n = s.len().min(remaining); + let end = i + n; + buffer[i..end].copy_from_slice(&s.as_bytes()[0..n]); + i = end; + }; + + append("error: "); + append(message); + append("\n"); + + nix::unistd::write(STDERR, &buffer[0..i]).ok(); + + process::abort(); +} + +extern "C" fn handler(signal: libc::c_int) { + let errno = Errno::last(); + + let Ok(signal) = u8::try_from(signal) else { + die("unexpected signal"); + }; + + // SAFETY: + // + // `WRITE` is initialized before the signal handler can run and remains open + // for the duration of the program. + let fd = unsafe { BorrowedFd::borrow_raw(WRITE.load(atomic::Ordering::Relaxed)) }; + + if let Err(err) = nix::unistd::write(fd, &[signal]) { + die(err.desc()); + } + + errno.set(); +} + +pub(crate) struct Signals(File); + +impl Signals { + pub(crate) fn new() -> RunResult<'static, Self> { + let (read, write) = nix::unistd::pipe().map_err(|errno| Error::SignalHandlerPipeOpen { + io_error: errno.into(), + })?; + + if WRITE + .compare_exchange( + INVALID_FILENO, + write.into_raw_fd(), + atomic::Ordering::Relaxed, + atomic::Ordering::Relaxed, + ) + .is_err() + { + panic!("signal iterator cannot be initialized twice"); + } + + let sa = SigAction::new( + SigHandler::Handler(handler), + SaFlags::SA_RESTART, + SigSet::empty(), + ); + + for &signal in Signal::ALL { + // SAFETY: + // + // This is the only place we modify signal handlers, and + // `nix::sys::signal::sigaction` is unsafe only if an invalid signal + // handler has already been installed. + unsafe { + nix::sys::signal::sigaction(signal.into(), &sa).map_err(|errno| { + Error::SignalHandlerSigaction { + signal, + io_error: errno.into(), + } + })?; + } + } + + Ok(Self(File::from(read))) + } +} + +impl Iterator for Signals { + type Item = io::Result; + + fn next(&mut self) -> Option { + let mut signal = [0]; + Some( + self + .0 + .read_exact(&mut signal) + .and_then(|()| Signal::try_from(signal[0])), + ) + } +} diff --git a/src/subcommand.rs b/src/subcommand.rs index 654fd846..9f54a46d 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -406,6 +406,16 @@ impl Subcommand { fn request(request: &Request) -> RunResult<'static> { let response = match request { Request::EnvironmentVariable(key) => Response::EnvironmentVariable(env::var_os(key)), + #[cfg(not(windows))] + Request::Signal => { + let sigset = nix::sys::signal::SigSet::all(); + + sigset.thread_block().unwrap(); + + let received = sigset.wait().unwrap(); + + Response::Signal(received.as_str().into()) + } }; serde_json::to_writer(io::stdout(), &response).map_err(|source| Error::DumpJson { source })?; diff --git a/tests/format.rs b/tests/format.rs index 43ea38aa..f1e225fc 100644 --- a/tests/format.rs +++ b/tests/format.rs @@ -128,7 +128,7 @@ fn write_error() { // skip this test if running as root, since root can write files even if // permissions would otherwise forbid it #[cfg(not(windows))] - if unsafe { libc::getuid() } == 0 { + if nix::unistd::getuid() == nix::unistd::ROOT { return; } diff --git a/tests/interrupts.rs b/tests/interrupts.rs deleted file mode 100644 index bd0077c2..00000000 --- a/tests/interrupts.rs +++ /dev/null @@ -1,90 +0,0 @@ -use { - super::*, - std::time::{Duration, Instant}, -}; - -fn kill(process_id: u32) { - unsafe { - libc::kill(process_id.try_into().unwrap(), libc::SIGINT); - } -} - -fn interrupt_test(arguments: &[&str], justfile: &str) { - let tmp = tempdir(); - let mut justfile_path = tmp.path().to_path_buf(); - justfile_path.push("justfile"); - fs::write(justfile_path, unindent(justfile)).unwrap(); - - let start = Instant::now(); - - let mut child = Command::new(executable_path("just")) - .current_dir(&tmp) - .args(arguments) - .spawn() - .expect("just invocation failed"); - - while start.elapsed() < Duration::from_millis(500) {} - - kill(child.id()); - - let status = child.wait().unwrap(); - - let elapsed = start.elapsed(); - - assert!( - elapsed <= Duration::from_secs(2), - "process returned too late: {elapsed:?}" - ); - - assert!( - elapsed >= Duration::from_millis(100), - "process returned too early : {elapsed:?}" - ); - - assert_eq!(status.code(), Some(130)); -} - -#[test] -#[ignore] -fn interrupt_shebang() { - interrupt_test( - &[], - " - default: - #!/usr/bin/env sh - sleep 1 - ", - ); -} - -#[test] -#[ignore] -fn interrupt_line() { - interrupt_test( - &[], - " - default: - @sleep 1 - ", - ); -} - -#[test] -#[ignore] -fn interrupt_backtick() { - interrupt_test( - &[], - " - foo := `sleep 1` - - default: - @echo {{foo}} - ", - ); -} - -#[test] -#[ignore] -fn interrupt_command() { - interrupt_test(&["--command", "sleep", "1"], ""); -} diff --git a/tests/lib.rs b/tests/lib.rs index fb8d6d01..45845d58 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -1,4 +1,4 @@ -pub(crate) use { +use { crate::{ assert_stdout::assert_stdout, assert_success::assert_success, @@ -29,6 +29,12 @@ pub(crate) use { which::which, }; +#[cfg(not(windows))] +use std::{ + thread, + time::{Duration, Instant}, +}; + fn default() -> T { Default::default() } @@ -75,8 +81,6 @@ mod groups; mod ignore_comments; mod imports; mod init; -#[cfg(unix)] -mod interrupts; mod invocation_directory; mod json; mod line_prefixes; @@ -111,6 +115,8 @@ mod shebang; mod shell; mod shell_expansion; mod show; +#[cfg(unix)] +mod signals; mod slash_operator; mod string; mod subsequents; diff --git a/tests/shell.rs b/tests/shell.rs index 3f4f0185..22106c0d 100644 --- a/tests/shell.rs +++ b/tests/shell.rs @@ -37,7 +37,12 @@ fn flag() { assert_stdout(&output, stdout); } -const JUSTFILE_CMD: &str = r#" +/// Test that we can use `set shell` to use cmd.exe on windows +#[test] +#[cfg(windows)] +fn cmd() { + let tmp = temptree! { + justfile: r#" set shell := ["cmd.exe", "/C"] @@ -46,14 +51,7 @@ x := `Echo` recipe: REM foo Echo "{{x}}" -"#; - -/// Test that we can use `set shell` to use cmd.exe on windows -#[test] -#[cfg_attr(unix, ignore)] -fn cmd() { - let tmp = temptree! { - justfile: JUSTFILE_CMD, +"#, }; let output = Command::new(executable_path("just")) @@ -66,7 +64,12 @@ fn cmd() { assert_stdout(&output, stdout); } -const JUSTFILE_POWERSHELL: &str = r#" +/// Test that we can use `set shell` to use cmd.exe on windows +#[test] +#[cfg(windows)] +fn powershell() { + let tmp = temptree! { + justfile: r#" set shell := ["powershell.exe", "-c"] @@ -75,15 +78,9 @@ x := `Write-Host "Hello, world!"` recipe: For ($i=0; $i -le 10; $i++) { Write-Host $i } Write-Host "{{x}}" -"#; - -/// Test that we can use `set shell` to use cmd.exe on windows -#[test] -#[cfg_attr(unix, ignore)] -fn powershell() { - let tmp = temptree! { - justfile: JUSTFILE_POWERSHELL, - }; +"# + , + }; let output = Command::new(executable_path("just")) .current_dir(tmp.path()) diff --git a/tests/signals.rs b/tests/signals.rs new file mode 100644 index 00000000..69aa38e2 --- /dev/null +++ b/tests/signals.rs @@ -0,0 +1,215 @@ +use {super::*, nix::sys::signal::Signal, nix::unistd::Pid, std::process::Child}; + +fn kill(child: &Child, signal: Signal) { + nix::sys::signal::kill(Pid::from_raw(child.id().try_into().unwrap()), signal).unwrap(); +} + +fn interrupt_test(arguments: &[&str], justfile: &str) { + let tmp = tempdir(); + let mut justfile_path = tmp.path().to_path_buf(); + justfile_path.push("justfile"); + fs::write(justfile_path, unindent(justfile)).unwrap(); + + let start = Instant::now(); + + let mut child = Command::new(executable_path("just")) + .current_dir(&tmp) + .args(arguments) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("just invocation failed"); + + while start.elapsed() < Duration::from_millis(500) {} + + kill(&child, Signal::SIGINT); + + let status = child.wait().unwrap(); + + let elapsed = start.elapsed(); + + assert!( + elapsed <= Duration::from_secs(2), + "process returned too late: {elapsed:?}" + ); + + assert!( + elapsed >= Duration::from_millis(100), + "process returned too early : {elapsed:?}" + ); + + assert_eq!(status.code(), Some(130)); +} + +#[test] +#[ignore] +fn interrupt_shebang() { + interrupt_test( + &[], + " + default: + #!/usr/bin/env sh + sleep 1 + ", + ); +} + +#[test] +#[ignore] +fn interrupt_line() { + interrupt_test( + &[], + " + default: + @sleep 1 + ", + ); +} + +#[test] +#[ignore] +fn interrupt_backtick() { + interrupt_test( + &[], + " + foo := `sleep 1` + + default: + @echo {{foo}} + ", + ); +} + +#[test] +#[ignore] +fn interrupt_command() { + interrupt_test(&["--command", "sleep", "1"], ""); +} + +/// This test is sensitive to the process signal mask. Programs like +/// `watchexec` and `cargo-watch` change the signal mask to ignore `SIGHUP`, +/// which causes this test to fail. +#[test] +#[ignore] +fn forwarding() { + let just = executable_path("just"); + + let tempdir = tempdir(); + + fs::write( + tempdir.path().join("justfile"), + "foo:\n @{{just_executable()}} --request '\"signal\"'", + ) + .unwrap(); + + for signal in [Signal::SIGINT, Signal::SIGQUIT, Signal::SIGHUP] { + let mut child = Command::new(&just) + .current_dir(&tempdir) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + + // wait for child to start + thread::sleep(Duration::from_millis(500)); + + // send non-forwarded signal + kill(&child, signal); + + // wait for child to receive signal + thread::sleep(Duration::from_millis(500)); + + // assert that child does not exit, because signal is not forwarded + assert!(child.try_wait().unwrap().is_none()); + + // send forwarded signal + kill(&child, Signal::SIGTERM); + + // child exits + let output = child.wait_with_output().unwrap(); + + let status = output.status; + let stderr = str::from_utf8(&output.stderr).unwrap(); + let stdout = str::from_utf8(&output.stdout).unwrap(); + + let mut failures = 0; + + if status.code() != Some(128 + signal as i32) { + failures += 1; + eprintln!("unexpected status: {status}"); + } + + // just reports that it was interrupted by first, non-forwarded signal + if stderr != format!("error: Interrupted by {signal}\n") { + failures += 1; + eprintln!("unexpected stderr: {stderr}"); + } + + // child reports that it was terminated by forwarded signal + if stdout != r#"{"signal":"SIGTERM"}"# { + failures += 1; + eprintln!("unexpected stdout: {stdout}"); + } + + assert!(failures == 0, "{failures} failures"); + } +} + +/// This test is ignored because it includes a 500ms wait, and because signal +/// tests are often flakey. +#[test] +#[ignore] +#[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", +))] +fn siginfo_prints_current_process() { + let just = executable_path("just"); + + let tempdir = tempdir(); + + fs::write(tempdir.path().join("justfile"), "foo:\n @sleep 1").unwrap(); + + let child = Command::new(&just) + .current_dir(&tempdir) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + + thread::sleep(Duration::from_millis(500)); + + kill(&child, Signal::SIGINFO); + + let output = child.wait_with_output().unwrap(); + + let status = output.status; + let stderr = str::from_utf8(&output.stderr).unwrap(); + let stdout = str::from_utf8(&output.stdout).unwrap(); + + let mut failures = 0; + + if !status.success() { + failures += 1; + eprintln!("unexpected status: {status}"); + } + + let re = + Regex::new(r#"just \d+: 1 child process:\n\d+: cd ".*" && "sh" "-cu" "sleep 1"\n"#).unwrap(); + + if !re.is_match(stderr) { + failures += 1; + eprintln!("unexpected stderr: {stderr}"); + } + + if !stdout.is_empty() { + failures += 1; + eprintln!("unexpected stdout: {stdout}"); + } + + assert!(failures == 0, "{failures} failures"); +}