From c98d0e6cfd4331f455f6abee460c848ee8dccfa0 Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Thu, 23 Mar 2017 23:50:39 +0100 Subject: [PATCH] Decouple --restart and --signal, so they both make sense This change takes account of the following four use cases: 1. Make sure the previous run was ended, then run the command again (default) 2. Just send a specified signal to the child, do nothing more (--signal given) 3. Send SIGTERM to the child, wait for it to exit, then run the command again (--restart given) 4. Send a specified signal to the child, wait for it to exit, then run the command again (--restart and --signal given) --- src/cli.rs | 141 +++++++++++++++++++++++++------------------------- src/main.rs | 91 +++++++++++++++++++++++++------- src/signal.rs | 30 ++++++----- 3 files changed, 161 insertions(+), 101 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 1846bbe..7f4e6b2 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -10,7 +10,7 @@ pub struct Args { pub filters: Vec, pub ignores: Vec, pub clear_screen: bool, - pub signal: String, + pub signal: Option, pub restart: bool, pub debug: bool, pub run_initially: bool, @@ -33,79 +33,78 @@ pub fn clear_screen() { #[allow(unknown_lints)] #[allow(or_fun_call)] pub fn get_args() -> Args { - let args = App::new("watchexec") - .version(crate_version!()) - .about("Execute commands when watched files change") - .arg(Arg::with_name("command") - .help("Command to execute") - .multiple(true) - .required(true)) - .arg(Arg::with_name("extensions") - .help("Comma-separated list of file extensions to watch (js,css,html)") - .short("e") - .long("exts") - .takes_value(true)) - .arg(Arg::with_name("path") - .help("Watch a specific directory") - .short("w") - .long("watch") - .number_of_values(1) - .multiple(true) - .takes_value(true)) - .arg(Arg::with_name("clear") - .help("Clear screen before executing command") - .short("c") - .long("clear")) - .arg(Arg::with_name("restart") - .help("Restart the process if it's still running") - .short("r") - .long("restart")) - .arg(Arg::with_name("signal") // TODO: --signal only makes sense when used with --restart - .help("Signal to send when --restart is used, defaults to SIGTERM") - .short("s") - .long("signal") - .takes_value(true) - .number_of_values(1) - .value_name("signal")) - .arg(Arg::with_name("debug") - .help("Print debugging messages to stderr") - .short("d") - .long("debug")) - .arg(Arg::with_name("filter") - .help("Ignore all modifications except those matching the pattern") - .short("f") - .long("filter") - .number_of_values(1) - .multiple(true) - .takes_value(true) - .value_name("pattern")) - .arg(Arg::with_name("ignore") - .help("Ignore modifications to paths matching the pattern") - .short("i") - .long("ignore") - .number_of_values(1) - .multiple(true) - .takes_value(true) - .value_name("pattern")) - .arg(Arg::with_name("no-vcs-ignore") - .help("Skip auto-loading of .gitignore files for filtering") - .long("no-vcs-ignore")) - .arg(Arg::with_name("postpone") - .help("Wait until first change to execute command") - .short("p") - .long("postpone")) - .arg(Arg::with_name("poll") - .help("Forces polling mode") - .long("force-poll") - .value_name("interval")) - .arg(Arg::with_name("once") - .short("1") - .hidden(true)) - .get_matches(); + let args = + App::new("watchexec") + .version(crate_version!()) + .about("Execute commands when watched files change") + .arg(Arg::with_name("command") + .help("Command to execute") + .multiple(true) + .required(true)) + .arg(Arg::with_name("extensions") + .help("Comma-separated list of file extensions to watch (js,css,html)") + .short("e") + .long("exts") + .takes_value(true)) + .arg(Arg::with_name("path") + .help("Watch a specific directory") + .short("w") + .long("watch") + .number_of_values(1) + .multiple(true) + .takes_value(true)) + .arg(Arg::with_name("clear") + .help("Clear screen before executing command") + .short("c") + .long("clear")) + .arg(Arg::with_name("restart") + .help("Restart the process if it's still running") + .short("r") + .long("restart")) + .arg(Arg::with_name("signal") + .help("Send signal to process upon changes, e.g. SIGHUP") + .short("s") + .long("signal") + .takes_value(true) + .number_of_values(1) + .value_name("signal")) + .arg(Arg::with_name("debug") + .help("Print debugging messages to stderr") + .short("d") + .long("debug")) + .arg(Arg::with_name("filter") + .help("Ignore all modifications except those matching the pattern") + .short("f") + .long("filter") + .number_of_values(1) + .multiple(true) + .takes_value(true) + .value_name("pattern")) + .arg(Arg::with_name("ignore") + .help("Ignore modifications to paths matching the pattern") + .short("i") + .long("ignore") + .number_of_values(1) + .multiple(true) + .takes_value(true) + .value_name("pattern")) + .arg(Arg::with_name("no-vcs-ignore") + .help("Skip auto-loading of .gitignore files for filtering") + .long("no-vcs-ignore")) + .arg(Arg::with_name("postpone") + .help("Wait until first change to execute command") + .short("p") + .long("postpone")) + .arg(Arg::with_name("poll") + .help("Forces polling mode") + .long("force-poll") + .value_name("interval")) + .arg(Arg::with_name("once").short("1").hidden(true)) + .get_matches(); let cmd = values_t!(args.values_of("command"), String).unwrap().join(" "); let paths = values_t!(args.values_of("path"), String).unwrap_or(vec![String::from(".")]); - let signal = args.value_of("signal").unwrap_or("SIGTERM").to_owned(); + let signal = args.value_of("signal").map(str::to_string); // Convert Option<&str> to Option let mut filters = values_t!(args.values_of("filter"), String).unwrap_or(vec![]); diff --git a/src/main.rs b/src/main.rs index 614761f..da8c964 100644 --- a/src/main.rs +++ b/src/main.rs @@ -57,7 +57,7 @@ fn main() { let weak_child = Arc::downgrade(&child_process); // Convert signal string to the corresponding integer - let signal = signal::new(&*args.signal); + let signal = signal::new(args.signal); signal::install_handler(move |sig: Signal| { if let Some(lock) = weak_child.upgrade() { @@ -117,24 +117,76 @@ fn main() { debug!("Path updated: {:?}", path); } - // Wait for current child process to exit - // Note: signal is cloned here automatically - wait_process(&child_process, signal, args.restart); + // We have three scenarios here: + // + // 1. Make sure the previous run was ended, then run the command again + // 2. Just send a specified signal to the child, do nothing more + // 3. Send SIGTERM to the child, wait for it to exit, then run the command again + // 4. Send a specified signal to the child, wait for it to exit, then run the command again + // + match args.restart { + // Custom restart behaviour (--restart was given, and --signal specified): + // Send specified signal to the child, wait for it to exit, then run the command again + true if signal.is_some() => { + wait_process(&child_process, signal, true); - // Launch child process - if args.clear_screen { - cli::clear_screen(); - } + // Launch child process + if args.clear_screen { + cli::clear_screen(); + } - debug!("Launching child process"); - { - let mut guard = child_process.write().unwrap(); - *guard = Some(process::spawn(&args.cmd, paths)); + debug!("Launching child process"); + { + let mut guard = child_process.write().unwrap(); + *guard = Some(process::spawn(&args.cmd, paths)); + } + } + + // Default restart behaviour (--restart was given, but --signal wasn't specified): + // Send SIGTERM to the child, wait for it to exit, then run the command again + true if signal.is_none() => { + let sigterm = signal::new(Some("SIGTERM".to_owned())); + wait_process(&child_process, sigterm, true); + + // Launch child process + if args.clear_screen { + cli::clear_screen(); + } + + debug!("Launching child process"); + { + let mut guard = child_process.write().unwrap(); + *guard = Some(process::spawn(&args.cmd, paths)); + } + } + + // SIGHUP scenario: --signal was given, but --restart was not + // Just send a signal (e.g. SIGHUP) to the child, do nothing more + false if signal.is_some() => wait_process(&child_process, signal, false), + + // Default behaviour (neither --signal nor --restart specified): + // Make sure the previous run was ended, then run the command again + false if signal.is_none() => { + wait_process(&child_process, None, true); + + // Launch child process + if args.clear_screen { + cli::clear_screen(); + } + + debug!("Launching child process"); + { + let mut guard = child_process.write().unwrap(); + *guard = Some(process::spawn(&args.cmd, paths)); + } + } + + // Catch everything else, just to be sure. + _ => panic!("This should never be called. Please file a bug report!"), } // Handle once option for integration testing if args.once { - // Note: signal is cloned here automatically wait_process(&child_process, signal, false); break; } @@ -185,15 +237,18 @@ fn wait_fs(rx: &Receiver, filter: &NotificationFilter) -> Vec { paths } -fn wait_process(process: &RwLock>, signal: Signal, restart: bool) { +// wait_process sends signal to process. It waits for the process to exit if wait is true +fn wait_process(process: &RwLock>, signal: Option, wait: bool) { let guard = process.read().unwrap(); if let Some(ref child) = *guard { - if restart { - child.signal(signal); + if let Some(s) = signal { + child.signal(s); } - debug!("Waiting for process to exit..."); - child.wait(); + if wait { + debug!("Waiting for process to exit..."); + child.wait(); + } } } diff --git a/src/signal.rs b/src/signal.rs index fd61c8b..ea1c99e 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -49,18 +49,24 @@ impl ConvertToLibc for Signal { } } -pub fn new(signal_name: &str) -> Signal { - match signal_name { - "SIGKILL" | "KILL" => Signal::SIGKILL, - "SIGTERM" | "TERM" => Signal::SIGTERM, - "SIGINT" | "INT" => Signal::SIGINT, - "SIGHUP" | "HUP" => Signal::SIGHUP, - "SIGSTOP" | "STOP" => Signal::SIGSTOP, - "SIGCONT" | "CONT" => Signal::SIGCONT, - "SIGCHLD" | "CHLD" => Signal::SIGCHLD, - "SIGUSR1" | "USR1" => Signal::SIGUSR1, - "SIGUSR2" | "USR2" => Signal::SIGUSR2, - _ => panic!("unsupported signal: {}", signal_name), +pub fn new(signal_name: Option) -> Option { + if let Some(signame) = signal_name { + let signal = match signame.as_ref() { + "SIGKILL" | "KILL" => Signal::SIGKILL, + "SIGTERM" | "TERM" => Signal::SIGTERM, + "SIGINT" | "INT" => Signal::SIGINT, + "SIGHUP" | "HUP" => Signal::SIGHUP, + "SIGSTOP" | "STOP" => Signal::SIGSTOP, + "SIGCONT" | "CONT" => Signal::SIGCONT, + "SIGCHLD" | "CHLD" => Signal::SIGCHLD, + "SIGUSR1" | "USR1" => Signal::SIGUSR1, + "SIGUSR2" | "USR2" => Signal::SIGUSR2, + _ => panic!("unsupported signal: {}", signame), + }; + + Some(signal) + } else { + None } }