mirror of
https://github.com/nushell/nushell.git
synced 2026-10-28 17:05:03 +08:00
feat(job spawn) auto-kill flag (#19141)
## Description closes #18896 when calling `exit`, if the flag is passed, we suppress the warning for quitting that job. if all remaining jobs have this flag enabled, we don't emit a warning. I chose the name `auto-kill` because we already use the terminology "kill" for jobs ## User-facing changes (Release notes) Allow using the flag `-k`/`--auto-kill` with `job spawn` for jobs that are expected to stay alive. This suppresses warnings about that job still running on `exit`. `job list` now shows which jobs have this `auto-kill` flag set, and a hint has been added to `exit`.
This commit is contained in:
1 parent
e5ebc68190
commit
394445e89d
11 files changed
+166
-30
No files matched your search
Generated
+1
@@ -5115,6 +5115,7 @@ dependencies = [
|
||||
"nu-protocol",
|
||||
"nu-test-support",
|
||||
"nu-utils",
|
||||
"rstest",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -56,6 +56,7 @@ impl Command for JobList {
|
||||
Value::list(vec![ Value::int(unfreeze.pid() as i64, head) ], head)
|
||||
}
|
||||
},
|
||||
"auto_kill" => Value::bool(job.is_auto_kill(), head),
|
||||
};
|
||||
|
||||
if let Some(description) = job.description() {
|
||||
|
||||
@@ -36,6 +36,11 @@ impl Command for JobSpawn {
|
||||
"An optional description for this job.",
|
||||
Some('d'),
|
||||
)
|
||||
.switch(
|
||||
"auto-kill",
|
||||
"Kill this job on `exit` without warning that it is still running.",
|
||||
Some('k'),
|
||||
)
|
||||
.required(
|
||||
"closure",
|
||||
SyntaxShape::Closure(Some(vec![SyntaxShape::Any])),
|
||||
@@ -64,6 +69,7 @@ impl Command for JobSpawn {
|
||||
engine_state.get_closure_block(&closure, head)?;
|
||||
|
||||
let description: Option<String> = call.get_flag(engine_state, stack, "description")?;
|
||||
let auto_kill = call.has_flag(engine_state, stack, "auto-kill")?;
|
||||
let job_stack = stack.clone();
|
||||
|
||||
let mut job_state = engine_state.clone();
|
||||
@@ -86,7 +92,7 @@ impl Command for JobSpawn {
|
||||
let (send, recv) = mpsc::channel();
|
||||
|
||||
let id = {
|
||||
let thread_job = ThreadJob::new(job_signals, description, send);
|
||||
let thread_job = ThreadJob::new(job_signals, description, auto_kill, send);
|
||||
|
||||
let id = jobs.add_job(Job::Thread(thread_job.clone()));
|
||||
|
||||
@@ -143,11 +149,18 @@ impl Command for JobSpawn {
|
||||
}
|
||||
|
||||
fn examples(&self) -> Vec<Example<'_>> {
|
||||
vec![Example {
|
||||
example: "job spawn { sleep 5sec; rm evidence.pdf }",
|
||||
description: "Spawn a background job to do some time consuming work.",
|
||||
result: None,
|
||||
}]
|
||||
vec![
|
||||
Example {
|
||||
example: "job spawn { sleep 5sec; rm evidence.pdf }",
|
||||
description: "Spawn a background job to do some time consuming work.",
|
||||
result: None,
|
||||
},
|
||||
Example {
|
||||
example: "job spawn --auto-kill { loop { sleep 1sec } }",
|
||||
description: "Spawn a job that `exit` kills without warning about it.",
|
||||
result: None,
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
fn extra_description(&self) -> &str {
|
||||
@@ -155,6 +168,10 @@ impl Command for JobSpawn {
|
||||
and registers this task in the background job table, which can be retrieved with `job list`.
|
||||
|
||||
This command returns the job id of the newly created job.
|
||||
|
||||
The --auto-kill flag applies to the spawned thread job. If an external process
|
||||
inside it is suspended and registered as a separate frozen job, it would still
|
||||
trigger the exit warning.
|
||||
"
|
||||
}
|
||||
}
|
||||
@@ -5,6 +5,32 @@ use nu_test_support::prelude::*;
|
||||
use rstest::rstest;
|
||||
use std::time::Duration;
|
||||
|
||||
#[rstest]
|
||||
#[case(None)]
|
||||
#[case(Some("-k"))]
|
||||
#[case(Some("--auto-kill"))]
|
||||
#[nu_test_support::test]
|
||||
#[cfg_attr(ci, serial)]
|
||||
fn job_list_reports_auto_kill(#[case] auto_kill_flag: Option<&str>) -> Result {
|
||||
let is_auto_kill = auto_kill_flag.is_some();
|
||||
|
||||
let spawn_command = match auto_kill_flag {
|
||||
Some(auto_kill_flag) => format!("job spawn {auto_kill_flag} {{ job recv }}"),
|
||||
None => "job spawn { job recv }".to_owned(),
|
||||
};
|
||||
|
||||
let code = format!(
|
||||
"
|
||||
let id = {spawn_command}
|
||||
let auto_kill = job list | where id == $id | get auto_kill.0
|
||||
job kill $id
|
||||
$auto_kill
|
||||
"
|
||||
);
|
||||
|
||||
test().run(&code).expect_value_eq(is_auto_kill)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn job_spawn_runs_with_empty_pipeline_not_null_value() -> Result {
|
||||
let code = "
|
||||
|
||||
@@ -39,3 +39,4 @@ plugin = [
|
||||
|
||||
[dev-dependencies]
|
||||
nu-test-support = { path = "../nu-test-support" }
|
||||
rstest.workspace = true
|
||||
@@ -1,12 +1,12 @@
|
||||
use std::sync::{Mutex, PoisonError, atomic::Ordering};
|
||||
|
||||
use nu_protocol::engine::{EngineState, Jobs};
|
||||
use nu_protocol::{ShellWarning, report_shell_warning};
|
||||
use std::sync::{Mutex, PoisonError, atomic::Ordering};
|
||||
|
||||
/// Exit the process or clean jobs if appropriate.
|
||||
///
|
||||
/// Drops `tag` and exits the current process if there are no running jobs, or if `exit_warning_given` is true.
|
||||
/// When running in a REPL session, warns the user if there
|
||||
/// were jobs and sets `exit_warning_given` instead, returning `tag` itself in that case.
|
||||
/// In a REPL session, warns if any jobs are not `auto_kill` and
|
||||
/// `exit_warning_given` is false. Sets that flag and returns `tag` in that case.
|
||||
/// Otherwise, kills all jobs, drops `tag`, and exits the current process.
|
||||
///
|
||||
// Currently, this `tag` argument exists mostly so that a LineEditor can be dropped before exiting the process.
|
||||
pub fn cleanup_exit<T>(tag: T, engine_state: &EngineState, exit_code: i32) -> T {
|
||||
@@ -19,24 +19,26 @@ pub fn cleanup_exit<T>(tag: T, engine_state: &EngineState, exit_code: i32) -> T
|
||||
|
||||
/// Clean up jobs before exiting, unless the REPL needs to warn about running jobs.
|
||||
///
|
||||
/// On the first REPL exit with running jobs, sets `exit_warning_given` and returns
|
||||
/// `Some(tag)` so the caller can return to the prompt. Otherwise, kills the jobs, drops
|
||||
/// `tag`, and returns `None` so the caller can exit.
|
||||
/// In a REPL session, warns if any jobs are not `auto_kill` and
|
||||
/// `exit_warning_given` is false. Sets that flag and returns `Some(tag)` in that case.
|
||||
/// Otherwise, kills all jobs, drops `tag`, and returns `None`.
|
||||
pub fn cleanup<T>(tag: T, engine_state: &EngineState) -> Option<T> {
|
||||
let jobs = engine_state
|
||||
.jobs
|
||||
.lock()
|
||||
.unwrap_or_else(PoisonError::into_inner);
|
||||
|
||||
let job_count = jobs.iter().filter(|(_, job)| !job.is_auto_kill()).count();
|
||||
|
||||
if engine_state.is_repl
|
||||
&& jobs.iter().next().is_some()
|
||||
&& job_count > 0
|
||||
&& !engine_state.exit_warning_given.load(Ordering::SeqCst)
|
||||
{
|
||||
let job_count = jobs.iter().count();
|
||||
|
||||
println!("There are still background jobs running ({job_count}).");
|
||||
|
||||
println!("Running `exit` a second time will kill all of them.");
|
||||
report_shell_warning(
|
||||
None,
|
||||
engine_state,
|
||||
&ShellWarning::BackgroundJobsRunning { job_count },
|
||||
);
|
||||
|
||||
engine_state
|
||||
.exit_warning_given
|
||||
@@ -71,3 +73,65 @@ pub fn kill_all_jobs(jobs: &Mutex<Jobs>) {
|
||||
let mut jobs = jobs.lock().unwrap_or_else(PoisonError::into_inner);
|
||||
let _ = jobs.kill_all();
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use nu_protocol::Signals;
|
||||
use nu_protocol::engine::Job;
|
||||
use nu_protocol::engine::ThreadJob;
|
||||
use rstest::rstest;
|
||||
use std::ops::Not;
|
||||
use std::sync::mpsc;
|
||||
|
||||
#[rstest]
|
||||
#[case(&[true])]
|
||||
#[case(&[false])]
|
||||
#[case(&[true, false])]
|
||||
#[case(&[false, false, true])]
|
||||
fn interactive_cleanup_respects_auto_kill(#[case] auto_kill_flags: &[bool]) {
|
||||
let mut engine_state = EngineState::new();
|
||||
engine_state.is_repl = true;
|
||||
|
||||
let mut signals_handles = vec![];
|
||||
|
||||
for auto_kill in auto_kill_flags {
|
||||
let signals_handle = Signals::new(Default::default());
|
||||
signals_handles.push(signals_handle.clone());
|
||||
|
||||
let (send, _) = mpsc::channel();
|
||||
|
||||
let job = ThreadJob::new(signals_handle, None, *auto_kill, send);
|
||||
let job = Job::Thread(job);
|
||||
engine_state.jobs.lock().unwrap().add_job(job);
|
||||
}
|
||||
|
||||
let should_warn = auto_kill_flags.contains(&false);
|
||||
|
||||
let exited = cleanup((), &engine_state).is_none();
|
||||
assert_eq!(exited, !should_warn);
|
||||
|
||||
let warned = engine_state.exit_warning_given.load(Ordering::SeqCst);
|
||||
assert_eq!(warned, should_warn);
|
||||
|
||||
if should_warn {
|
||||
assert_eq!(
|
||||
engine_state.jobs.lock().unwrap().iter().count(),
|
||||
signals_handles.len()
|
||||
);
|
||||
|
||||
assert!(
|
||||
signals_handles
|
||||
.iter()
|
||||
.all(|signals| signals.interrupted().not())
|
||||
);
|
||||
|
||||
let exited = cleanup((), &engine_state).is_none();
|
||||
assert!(exited);
|
||||
}
|
||||
|
||||
assert!(engine_state.jobs.lock().unwrap().is_empty());
|
||||
|
||||
assert!(signals_handles.iter().all(Signals::interrupted));
|
||||
}
|
||||
}
|
||||
@@ -521,7 +521,8 @@ fn promote_to_background_job(
|
||||
) -> Result<EvalOutput, rmcp::ErrorData> {
|
||||
let signals = Signals::new(interrupt);
|
||||
let (sender, _receiver) = mpsc::channel();
|
||||
let thread_job = ThreadJob::new(signals, Some(description), sender);
|
||||
let auto_kill = false;
|
||||
let thread_job = ThreadJob::new(signals, Some(description), auto_kill, sender);
|
||||
|
||||
let job_id = match jobs.lock() {
|
||||
Ok(mut jobs) => jobs.add_job(Job::Thread(thread_job)),
|
||||
|
||||
@@ -230,10 +230,10 @@ pub struct EngineState {
|
||||
|
||||
pub root_job_sender: Sender<Mail>,
|
||||
|
||||
// When there are background jobs running, the interactive behavior of `exit` changes depending on
|
||||
// the value of this flag:
|
||||
// - if this is false, then a warning about running jobs is shown and `exit` enables this flag
|
||||
// - if this is true, then `exit` will `std::process::exit`
|
||||
// When there are background jobs that are not auto_kill, interactive `exit` checks this flag:
|
||||
// - if false, it warns about those jobs and sets this flag
|
||||
// - if true, it kills all jobs and exits
|
||||
// If all jobs have auto_kill, exit kills them and exits without warning.
|
||||
//
|
||||
// This ensures that running exit twice will terminate the program correctly
|
||||
pub exit_warning_given: Arc<AtomicBool>,
|
||||
|
||||
@@ -160,16 +160,23 @@ pub struct ThreadJob {
|
||||
signals: Signals,
|
||||
pids: Arc<Mutex<HashSet<u32>>>,
|
||||
description: Option<String>,
|
||||
auto_kill: bool,
|
||||
pub sender: Sender<Mail>,
|
||||
}
|
||||
|
||||
impl ThreadJob {
|
||||
pub fn new(signals: Signals, description: Option<String>, sender: Sender<Mail>) -> Self {
|
||||
pub fn new(
|
||||
signals: Signals,
|
||||
description: Option<String>,
|
||||
auto_kill: bool,
|
||||
sender: Sender<Mail>,
|
||||
) -> Self {
|
||||
ThreadJob {
|
||||
signals,
|
||||
pids: Arc::new(Mutex::new(HashSet::default())),
|
||||
sender,
|
||||
description,
|
||||
auto_kill,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -253,6 +260,13 @@ impl Job {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn is_auto_kill(&self) -> bool {
|
||||
match self {
|
||||
Job::Thread(job) => job.auto_kill,
|
||||
Job::Frozen(_) => false,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn assign_description(&mut self, description: Option<String>) {
|
||||
match self {
|
||||
Job::Thread(thread_job) => thread_job.description = description,
|
||||
@@ -452,7 +466,7 @@ mod tests {
|
||||
fn interrupted_job_does_not_spawn() {
|
||||
let (sender, _) = mpsc::channel();
|
||||
let signals = Signals::new(Arc::new(AtomicBool::new(false)));
|
||||
let job = ThreadJob::new(signals.clone(), None, sender);
|
||||
let job = ThreadJob::new(signals.clone(), None, false, sender);
|
||||
signals.trigger();
|
||||
|
||||
let child = job.try_spawn(|| panic!("interrupted job must not spawn"));
|
||||
@@ -463,7 +477,7 @@ mod tests {
|
||||
fn spawned_process_is_registered_before_returning() {
|
||||
let (sender, _) = mpsc::channel();
|
||||
let signals = Signals::new(Arc::new(AtomicBool::new(false)));
|
||||
let job = ThreadJob::new(signals, None, sender);
|
||||
let job = ThreadJob::new(signals, None, false, sender);
|
||||
// Listing this test binary's tests needs no platform-specific executable, and the
|
||||
// child terminates by itself if an assertion fails.
|
||||
let mut command = std::process::Command::new(std::env::current_exe().unwrap());
|
||||
@@ -503,7 +517,7 @@ mod tests {
|
||||
fn spawn_holds_pid_lock_until_registration() {
|
||||
let (sender, _) = mpsc::channel();
|
||||
let signals = Signals::new(Arc::new(AtomicBool::new(false)));
|
||||
let job = ThreadJob::new(signals, None, sender);
|
||||
let job = ThreadJob::new(signals, None, false, sender);
|
||||
let (started_tx, started_rx) = mpsc::channel();
|
||||
let (release_tx, release_rx) = mpsc::channel();
|
||||
|
||||
|
||||
@@ -9,6 +9,15 @@ use crate::{ConfigWarning, ReportMode, Reportable};
|
||||
#[derive(Clone, Debug, Error, Diagnostic)]
|
||||
#[diagnostic(severity(Warning))]
|
||||
pub enum ShellWarning {
|
||||
/// Running jobs (without auto-kill) prevented interactive shell from exiting.
|
||||
#[error("Running background jobs that won't be killed automatically: {job_count}.")]
|
||||
#[diagnostic(
|
||||
code(nu::shell::background_jobs_running),
|
||||
help(
|
||||
"Run `exit` again to kill all jobs.\nUse `job spawn --auto-kill` when spawning thread jobs to exclude them from this warning."
|
||||
)
|
||||
)]
|
||||
BackgroundJobsRunning { job_count: usize },
|
||||
/// A parse-time deprecation. Indicates that something will be removed in a future release.
|
||||
///
|
||||
/// Use [`ParseWarning::Deprecated`](crate::ParseWarning::Deprecated) if this is a deprecation
|
||||
@@ -55,6 +64,7 @@ impl Reportable for ShellWarning {
|
||||
ShellWarning::Deprecated { report_mode, .. }
|
||||
| ShellWarning::LastResultTruncated { report_mode, .. } => *report_mode,
|
||||
ShellWarning::InvalidConfig { .. } => ReportMode::FirstUse,
|
||||
ShellWarning::BackgroundJobsRunning { .. } => ReportMode::EveryUse,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -63,6 +73,7 @@ impl Reportable for ShellWarning {
|
||||
impl Hash for ShellWarning {
|
||||
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
|
||||
match self {
|
||||
ShellWarning::BackgroundJobsRunning { job_count } => job_count.hash(state),
|
||||
ShellWarning::Deprecated {
|
||||
dep_type, label, ..
|
||||
} => {
|
||||
|
||||
+1
-1
@@ -55,7 +55,7 @@ mod tests {
|
||||
let jobs = Arc::new(Mutex::new(Jobs::default()));
|
||||
let job_signals = Signals::new(Arc::new(AtomicBool::new(false)));
|
||||
let (sender, _receiver) = mpsc::channel();
|
||||
let job = ThreadJob::new(job_signals.clone(), None, sender);
|
||||
let job = ThreadJob::new(job_signals.clone(), None, false, sender);
|
||||
let id = jobs.lock().unwrap().add_job(Job::Thread(job));
|
||||
|
||||
(jobs, id, job_signals)
|
||||
|
||||
Reference in new issue
Block a user