diff --git a/Cargo.lock b/Cargo.lock index 033812eec99c8..f5d0ab0ad6a13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7335,16 +7335,13 @@ dependencies = [ "clap 4.1.11", "clap_complete", "command-group", - "ctrlc", "dunce", "itertools", - "libc", "log", "pretty_assertions", "serde", "serde_json", "serde_yaml", - "shared_child", "tiny-gradient", "tokio-util", "turborepo-lib", @@ -7972,6 +7969,7 @@ dependencies = [ "config", "console", "const_format", + "ctrlc", "dialoguer", "dirs-next", "dunce", @@ -7984,6 +7982,7 @@ dependencies = [ "indicatif", "itertools", "lazy_static", + "libc", "log", "notify 5.1.0", "pidlock", @@ -7997,6 +7996,7 @@ dependencies = [ "serde_json", "serde_yaml", "sha2", + "shared_child", "sysinfo", "tempfile", "test-case", diff --git a/cli/integration_tests/signal_handling/ctrlc.t b/cli/integration_tests/signal_handling/ctrlc.t new file mode 100644 index 0000000000000..b91173550176d --- /dev/null +++ b/cli/integration_tests/signal_handling/ctrlc.t @@ -0,0 +1,31 @@ +Setup + $ . ${TESTDIR}/../setup.sh + $ . ${TESTDIR}/setup.sh $(pwd) + +Run script with INT handler and verify that INT gets passed to script + +Start turbo in the background + $ ${TURBO} trap & +Save the PID of turbo + $ TURBO_PID=$! +We send INT to turbo, but with a delay to give us time to bring turbo back to +the foreground. + $ sh -c "sleep 1 && kill -2 ${TURBO_PID}" & +Bring turbo back to the foreground + $ fg 1 + ${TURBO} trap + \xe2\x80\xa2 Packages in scope: test (esc) + \xe2\x80\xa2 Running trap in 1 packages (esc) + \xe2\x80\xa2 Remote caching disabled (esc) + test:trap: cache miss, executing d25759ee0a8e12ae + test:trap: + test:trap: > trap + test:trap: > trap 'echo trap hit; sleep 1; echo trap finish' INT; sleep 5 && echo 'script finish' + test:trap: + test:trap: trap hit + test:trap: trap finish + test:trap: npm ERR! Lifecycle script `trap` failed with error: + test:trap: npm ERR! Error: command failed + test:trap: npm ERR! in workspace: test + test:trap: npm ERR! at location: .*ctrlc.t/apps/test (re) + [1] diff --git a/cli/integration_tests/signal_handling/setup.sh b/cli/integration_tests/signal_handling/setup.sh new file mode 100644 index 0000000000000..e5d67a073b72d --- /dev/null +++ b/cli/integration_tests/signal_handling/setup.sh @@ -0,0 +1,8 @@ +#!/bin/bash +# Enable jobcontrol as we need it for fg to work +set -m + +SCRIPT_DIR=$(dirname ${BASH_SOURCE[0]}) +TARGET_DIR=$1 +cp -a ${SCRIPT_DIR}/test_repo/. ${TARGET_DIR}/ +${SCRIPT_DIR}/../setup_git.sh ${TARGET_DIR} diff --git a/cli/integration_tests/signal_handling/test_repo/apps/test/package.json b/cli/integration_tests/signal_handling/test_repo/apps/test/package.json new file mode 100644 index 0000000000000..9e423f68dfcde --- /dev/null +++ b/cli/integration_tests/signal_handling/test_repo/apps/test/package.json @@ -0,0 +1,6 @@ +{ + "name": "test", + "scripts": { + "trap": "trap 'echo trap hit; sleep 1; echo trap finish' INT; sleep 5 && echo 'script finish'" + } +} diff --git a/cli/integration_tests/signal_handling/test_repo/package.json b/cli/integration_tests/signal_handling/test_repo/package.json new file mode 100644 index 0000000000000..65c13eadeccaa --- /dev/null +++ b/cli/integration_tests/signal_handling/test_repo/package.json @@ -0,0 +1,7 @@ +{ + "name": "signal-test", + "workspaces": [ + "apps/*" + ], + "packageManager": "npm@8.0.0" +} diff --git a/cli/integration_tests/signal_handling/test_repo/turbo.json b/cli/integration_tests/signal_handling/test_repo/turbo.json new file mode 100644 index 0000000000000..d4b5cd2d0b1cc --- /dev/null +++ b/cli/integration_tests/signal_handling/test_repo/turbo.json @@ -0,0 +1,6 @@ +{ + "$schema": "https://turbo.build/schema.json", + "pipeline": { + "trap": {} + } +} diff --git a/cli/internal/ffi/libgit2.a b/cli/internal/ffi/libgit2.a new file mode 100644 index 0000000000000..886f2542da0df Binary files /dev/null and b/cli/internal/ffi/libgit2.a differ diff --git a/crates/turborepo-lib/Cargo.toml b/crates/turborepo-lib/Cargo.toml index 2ab76a70f4fde..2f8aed127387a 100644 --- a/crates/turborepo-lib/Cargo.toml +++ b/crates/turborepo-lib/Cargo.toml @@ -31,6 +31,7 @@ clap_complete = { workspace = true } command-group = { version = "2.1.0", features = ["with-tokio"] } config = "0.13" console = { workspace = true } +ctrlc = { version = "3.2.5", features = ["termination"] } dialoguer = { workspace = true, features = ["fuzzy-select"] } dirs-next = "2.0.0" dunce = { workspace = true } @@ -42,6 +43,7 @@ hostname = "0.3.1" humantime = "2.1.0" indicatif = { workspace = true } lazy_static = { workspace = true } +libc = "0.2.140" log = { workspace = true } notify = { version = "5.1.0", default-features = false, features = [ "macos_kqueue", @@ -55,6 +57,7 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_yaml = { workspace = true } sha2 = "0.10.6" +shared_child = "1.0.0" sysinfo = "0.27.7" thiserror = "1.0.38" tiny-gradient = { workspace = true } diff --git a/crates/turborepo-lib/src/child.rs b/crates/turborepo-lib/src/child.rs new file mode 100644 index 0000000000000..404c91bc8cdce --- /dev/null +++ b/crates/turborepo-lib/src/child.rs @@ -0,0 +1,29 @@ +use std::{process::Command, sync::Arc}; + +use anyhow::Result; +use shared_child::SharedChild; + +/// Spawns a child in a way where SIGINT is correctly forwarded to the child +pub fn spawn_child(mut command: Command) -> Result> { + let shared_child = Arc::new(SharedChild::spawn(&mut command)?); + let handler_shared_child = shared_child.clone(); + + ctrlc::set_handler(move || { + // on windows, we can't send signals so just kill + // we are quiting anyways so just ignore + #[cfg(target_os = "windows")] + handler_shared_child.kill().ok(); + + // on unix, we should send a SIGTERM to the child + // so that go can gracefully shut down process groups + // SAFETY: we could pull in the nix crate to handle this + // 'safely' but nix::sys::signal::kill just calls libc::kill + #[cfg(not(target_os = "windows"))] + unsafe { + libc::kill(handler_shared_child.id() as i32, libc::SIGTERM); + } + }) + .expect("handler set"); + + Ok(shared_child) +} diff --git a/crates/turborepo-lib/src/lib.rs b/crates/turborepo-lib/src/lib.rs index 21ad6c8bef370..bec2f4aae4a34 100644 --- a/crates/turborepo-lib/src/lib.rs +++ b/crates/turborepo-lib/src/lib.rs @@ -1,5 +1,6 @@ #![feature(assert_matches)] +mod child; mod cli; mod client; mod commands; @@ -11,6 +12,7 @@ mod shim; mod ui; use anyhow::Result; +pub use child::spawn_child; use log::error; pub use crate::cli::Args; diff --git a/crates/turborepo-lib/src/shim.rs b/crates/turborepo-lib/src/shim.rs index 4d02d8bc873af..04dde21ac1ed6 100644 --- a/crates/turborepo-lib/src/shim.rs +++ b/crates/turborepo-lib/src/shim.rs @@ -21,7 +21,7 @@ use serde::{Deserialize, Serialize}; use tiny_gradient::{GradientStr, RGB}; use turbo_updater::check_for_updates; -use crate::{cli, get_version, package_manager::Globs, PackageManager, Payload}; +use crate::{cli, get_version, package_manager::Globs, spawn_child, PackageManager, Payload}; // all arguments that result in a stdout that much be directly parsable and // should not be paired with additional output (from the update notifier for @@ -649,18 +649,21 @@ impl RepoState { // We spawn a process that executes the local turbo // that we've found in node_modules/.bin/turbo. - let mut command = process::Command::new(local_turbo_path) + let mut command = process::Command::new(local_turbo_path); + command .args(&raw_args) // rather than passing an argument that local turbo might not understand, set // an environment variable that can be optionally used .env(cli::INVOCATION_DIR_ENV_VAR, &shim_args.invocation_dir) .current_dir(cwd) .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .spawn() - .expect("Failed to execute turbo."); + .stderr(Stdio::inherit()); - Ok(command.wait()?.code().unwrap_or(2)) + let child = spawn_child(command)?; + + let exit_code = child.wait()?.code().unwrap_or(2); + + Ok(exit_code) } } diff --git a/crates/turborepo/Cargo.toml b/crates/turborepo/Cargo.toml index d59bd9bcd076f..5fafeaa8baf31 100644 --- a/crates/turborepo/Cargo.toml +++ b/crates/turborepo/Cargo.toml @@ -27,14 +27,11 @@ anyhow = { workspace = true, features = ["backtrace"] } clap = { workspace = true, features = ["derive"] } clap_complete = { workspace = true } command-group = { version = "2.0.1", features = ["with-tokio"] } -ctrlc = { version = "3.2.5", features = ["termination"] } dunce = { workspace = true } -libc = "0.2.140" log = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_yaml = { workspace = true } -shared_child = "1.0.0" tiny-gradient = { workspace = true } tokio-util = { version = "0.7.7", features = ["io"] } turborepo-lib = { workspace = true } diff --git a/crates/turborepo/src/main.rs b/crates/turborepo/src/main.rs index 2c23ade38b1d5..84ee07a7bde3c 100644 --- a/crates/turborepo/src/main.rs +++ b/crates/turborepo/src/main.rs @@ -7,7 +7,7 @@ use std::{ use anyhow::Result; use dunce::canonicalize as fs_canonicalize; use log::{debug, error, trace}; -use turborepo_lib::{Args, Payload}; +use turborepo_lib::{spawn_child, Args, Payload}; fn run_go_binary(args: Args) -> Result { // canonicalize the binary path to ensure we can find go-turbo @@ -49,28 +49,8 @@ fn run_go_binary(args: Args) -> Result { .stdout(Stdio::inherit()) .stderr(Stdio::inherit()); - let shared_child = shared_child::SharedChild::spawn(&mut command).unwrap(); - let child_arc = std::sync::Arc::new(shared_child); - - let child_arc_clone = child_arc.clone(); - ctrlc::set_handler(move || { - // on windows, we can't send signals so just kill - // we are quiting anyways so just ignore - #[cfg(target_os = "windows")] - child_arc_clone.kill().ok(); - - // on unix, we should send a SIGTERM to the child - // so that go can gracefully shut down process groups - // SAFETY: we could pull in the nix crate to handle this - // 'safely' but nix::sys::signal::kill just calls libc::kill - #[cfg(not(target_os = "windows"))] - unsafe { - libc::kill(child_arc_clone.id() as i32, libc::SIGTERM); - } - }) - .expect("handler set"); - - let exit_code = child_arc.wait()?.code().unwrap_or(2); + let child = spawn_child(command)?; + let exit_code = child.wait()?.code().unwrap_or(2); Ok(exit_code) }