From ee5e93e6af1cb89dae7b85dbc5f1682bd1b21903 Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Thu, 6 Apr 2017 22:31:52 +0200 Subject: [PATCH 1/7] Add support for --no-shell option --- src/cli.rs | 6 ++++++ src/main.rs | 8 ++++---- src/process.rs | 30 +++++++++++++++++++++++++----- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 3ef1a7f..c197e59 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -14,6 +14,7 @@ pub struct Args { pub restart: bool, pub debug: bool, pub run_initially: bool, + pub no_shell: bool, pub no_vcs_ignore: bool, pub once: bool, pub poll: bool, @@ -102,6 +103,10 @@ pub fn get_args() -> Args { .help("Forces polling mode") .long("force-poll") .value_name("interval")) + .arg(Arg::with_name("no-shell") + .help("Do not wrap command in 'sh -c'") + .short("n") + .long("no-shell")) .arg(Arg::with_name("once").short("1").hidden(true)) .get_matches(); @@ -164,6 +169,7 @@ pub fn get_args() -> Args { restart: args.is_present("restart"), debug: args.is_present("debug"), run_initially: !args.is_present("postpone"), + no_shell: args.is_present("no-shell"), no_vcs_ignore: args.is_present("no-vcs-ignore"), once: args.is_present("once"), poll: args.occurrences_of("poll") > 0, diff --git a/src/main.rs b/src/main.rs index ed1ec3f..bc5dfeb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -107,7 +107,7 @@ fn main() { } let mut guard = child_process.write().unwrap(); - *guard = Some(process::spawn(&args.cmd, vec![])); + *guard = Some(process::spawn(&args.cmd, vec![], args.no_shell)); } loop { @@ -140,7 +140,7 @@ fn main() { debug!("Launching child process"); { let mut guard = child_process.write().unwrap(); - *guard = Some(process::spawn(&args.cmd, paths)); + *guard = Some(process::spawn(&args.cmd, paths, args.no_shell)); } } @@ -158,7 +158,7 @@ fn main() { debug!("Launching child process"); { let mut guard = child_process.write().unwrap(); - *guard = Some(process::spawn(&args.cmd, paths)); + *guard = Some(process::spawn(&args.cmd, paths, args.no_shell)); } } @@ -179,7 +179,7 @@ fn main() { debug!("Launching child process"); { let mut guard = child_process.write().unwrap(); - *guard = Some(process::spawn(&args.cmd, paths)); + *guard = Some(process::spawn(&args.cmd, paths, args.no_shell)); } } } diff --git a/src/process.rs b/src/process.rs index c95ffc4..31e9cb1 100644 --- a/src/process.rs +++ b/src/process.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; -pub fn spawn(cmd: &str, updated_paths: Vec) -> Process { - self::imp::Process::new(cmd, updated_paths).expect("unable to spawn process") +pub fn spawn(cmd: &str, updated_paths: Vec, no_shell: bool) -> Process { + self::imp::Process::new(cmd, updated_paths, no_shell).expect("unable to spawn process") } pub use self::imp::Process; @@ -24,13 +24,33 @@ mod imp { #[allow(unknown_lints)] #[allow(mutex_atomic)] impl Process { - pub fn new(cmd: &str, updated_paths: Vec) -> Result { + pub fn new(cmd: &str, updated_paths: Vec, no_shell: bool) -> Result { use nix::unistd::*; use std::io; use std::os::unix::process::CommandExt; - let mut command = Command::new("sh"); - command.arg("-c").arg(cmd); + // Assemble command to run. + // This is either the first argument from cmd (if no_shell was given) or "sh". + // Using "sh -c" gives us features like supportin pipes and redirects, + // but is a little less performant and can cause trouble when using custom signals + // (e.g. --signal SIGHUP) + let mut iter_args = cmd.split_whitespace(); + let arg0 = match no_shell { + true => iter_args.next().unwrap(), + false => "sh", + }; + + // TODO: There might be a better way of doing this with &str. + // I've had to fall back to String, as I wasn't able to join(" ") a Vec<&str> + // into a &str + let args: Vec = match no_shell { + true => iter_args.map(str::to_string).collect(), + false => vec!["-c".to_string(), iter_args.collect::>().join(" ")], + }; + + let mut command = Command::new(arg0); + command.args(args); + debug!("Assembled command {:?}", command); if let Some(single_path) = super::get_single_updated_path(&updated_paths) { command.env("WATCHEXEC_UPDATED_PATH", single_path); From ea130dd021ec15483b02d02bcd610421f890a711 Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Thu, 6 Apr 2017 22:51:52 +0200 Subject: [PATCH 2/7] Add documentation for --no-shell --- README.md | 4 ++-- doc/watchexec.1 | 4 ++++ doc/watchexec.1.html | 1 + doc/watchexec.1.ronn | 3 +++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4db681c..367b79c 100644 --- a/README.md +++ b/README.md @@ -58,9 +58,9 @@ Call/restart `my_server` when any file in the current directory (and all subdire $ watchexec -r -s SIGKILL my_server -Send a SIGHUP to the child process upon changes: +Send a SIGHUP to the child process upon changes (Note: with using `-n | --no-shell` here, we're executing `my_server` directly, instead of wrapping it in a shell: - $ watchexec -s SIGHUP my_server + $ watchexec -n -s SIGHUP my_server Run `make` when any file changes, using the `.gitignore` file in the current directory to filter: diff --git a/doc/watchexec.1 b/doc/watchexec.1 index c95a17d..0ff7805 100644 --- a/doc/watchexec.1 +++ b/doc/watchexec.1 @@ -34,6 +34,10 @@ Ignores modifications from paths that do not match \fIpattern\fR\. This option c Sends the specified signal (e\.g\. \fBSIGKILL\fR) to the child process\. Defaults to \fBSIGTERM\fR\. . .TP +\fB\-n\fR, \fB\-\-no\-shell\fR +Execute command directly, do not use \fBsh -c\fR\. This is especially useful in combination with \fB--signal\fR, as the signal is then send directly to the specified command\. While \fB--no-shell\fR is a little more performant than the default, it prevents using shell-features like pipes and redirects\. +. +.TP \fB\-i\fR, \fB\-\-ignore\fR \fIpattern\fR Ignores modifications from paths that match \fIpattern\fR\. This option can be specified multiple times, and a match on any pattern causes the path to be ignored\. . diff --git a/doc/watchexec.1.html b/doc/watchexec.1.html index 071e13d..ccd0f27 100644 --- a/doc/watchexec.1.html +++ b/doc/watchexec.1.html @@ -89,6 +89,7 @@
-e, --exts extensions

Comma-separated list of file extensions to filter by. Leading dots are allowed (.rs) are allowed. (This is a shorthand for -f).

-f, --filter pattern

Ignores modifications from paths that do not match pattern. This option can be specified multiple times, where a match on any given pattern causes the path to trigger command.

-s, --signal SIGNAL

Sends the specified signal (e.g. SIGKILl) to the child process. Defaults to SIGTERM.

+
-n, --no-shell

Execute command directly, do not use sh -c. This is especially useful in combination with --signal, as the signal is then send directly to the specified command. While --no-shell is a little more performant than the default, it prevents using shell-features like pipes and redirects.

-i, --ignore pattern

Ignores modifications from paths that match pattern. This option can be specified multiple times, and a match on any pattern causes the path to be ignored.

-w, --watch path

Monitor a specific path for changes. By default, the current working directory is watched. This may be specified multiple times, where a change in any watched directory (and subdirectories) causes command to be executed.

-r, --restart

Terminates the child process group if it is still running when subsequent file modifications are detected. By default, sends SIGTERM; use --kill to send SIGKILL.

diff --git a/doc/watchexec.1.ronn b/doc/watchexec.1.ronn index 1b8bbe3..0ce9e35 100644 --- a/doc/watchexec.1.ronn +++ b/doc/watchexec.1.ronn @@ -25,6 +25,9 @@ Ignores modifications from paths that do not match . This option can be * `-s`, `--signal`: Sends the specified signal (e.g. `SIGKILL`) to the child process. Defaults to `SIGTERM`. +* `-n`, `--no-shell`: +Execute command directly, do not use `sh -c`. This is especially useful in combination with `--signal`, as the signal is then send directly to the specified command. While `--no-shell` is a little more performant than the default, it prevents using shell-features like pipes and redirects. + * `-i`, `--ignore` : Ignores modifications from paths that match . This option can be specified multiple times, and a match on any pattern causes the path to be ignored. From 5a7ccf759b60104eece4b37d454b9b86a5e415d1 Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Thu, 6 Apr 2017 23:05:53 +0200 Subject: [PATCH 3/7] Adapt spawn() test for Unix --- src/process.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/process.rs b/src/process.rs index 31e9cb1..ed6be69 100644 --- a/src/process.rs +++ b/src/process.rs @@ -261,7 +261,7 @@ mod tests { #[test] fn test_start() { - let _ = spawn("echo hi", vec![]); + let _ = spawn("echo hi", vec![], true); } #[test] From 780b54b34e14024a7bf5e72e9bc83d75b4cb10dc Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Thu, 6 Apr 2017 23:04:57 +0200 Subject: [PATCH 4/7] Port --no-shell to Windows --- doc/watchexec.1 | 2 +- doc/watchexec.1.html | 2 +- doc/watchexec.1.ronn | 2 +- src/cli.rs | 2 +- src/process.rs | 21 ++++++++++++++++++--- 5 files changed, 22 insertions(+), 7 deletions(-) diff --git a/doc/watchexec.1 b/doc/watchexec.1 index 0ff7805..393c6c0 100644 --- a/doc/watchexec.1 +++ b/doc/watchexec.1 @@ -35,7 +35,7 @@ Sends the specified signal (e\.g\. \fBSIGKILL\fR) to the child process\. Default . .TP \fB\-n\fR, \fB\-\-no\-shell\fR -Execute command directly, do not use \fBsh -c\fR\. This is especially useful in combination with \fB--signal\fR, as the signal is then send directly to the specified command\. While \fB--no-shell\fR is a little more performant than the default, it prevents using shell-features like pipes and redirects\. +Execute command directly, do not wrap it in \fBsh -c\fR resp\. \fBcmd.exec /C\R\. This is especially useful in combination with \fB--signal\fR, as the signal is then send directly to the specified command\. While \fB--no-shell\fR is a little more performant than the default, it prevents using shell-features like pipes and redirects\. . .TP \fB\-i\fR, \fB\-\-ignore\fR \fIpattern\fR diff --git a/doc/watchexec.1.html b/doc/watchexec.1.html index ccd0f27..4fc28b7 100644 --- a/doc/watchexec.1.html +++ b/doc/watchexec.1.html @@ -89,7 +89,7 @@
-e, --exts extensions

Comma-separated list of file extensions to filter by. Leading dots are allowed (.rs) are allowed. (This is a shorthand for -f).

-f, --filter pattern

Ignores modifications from paths that do not match pattern. This option can be specified multiple times, where a match on any given pattern causes the path to trigger command.

-s, --signal SIGNAL

Sends the specified signal (e.g. SIGKILl) to the child process. Defaults to SIGTERM.

-
-n, --no-shell

Execute command directly, do not use sh -c. This is especially useful in combination with --signal, as the signal is then send directly to the specified command. While --no-shell is a little more performant than the default, it prevents using shell-features like pipes and redirects.

+
-n, --no-shell

Execute command directly, do not wrap it in sh -c resp. cmd.exec /C. This is especially useful in combination with --signal, as the signal is then send directly to the specified command. While --no-shell is a little more performant than the default, it prevents using shell-features like pipes and redirects.

-i, --ignore pattern

Ignores modifications from paths that match pattern. This option can be specified multiple times, and a match on any pattern causes the path to be ignored.

-w, --watch path

Monitor a specific path for changes. By default, the current working directory is watched. This may be specified multiple times, where a change in any watched directory (and subdirectories) causes command to be executed.

-r, --restart

Terminates the child process group if it is still running when subsequent file modifications are detected. By default, sends SIGTERM; use --kill to send SIGKILL.

diff --git a/doc/watchexec.1.ronn b/doc/watchexec.1.ronn index 0ce9e35..7a1cf34 100644 --- a/doc/watchexec.1.ronn +++ b/doc/watchexec.1.ronn @@ -26,7 +26,7 @@ Ignores modifications from paths that do not match . This option can be Sends the specified signal (e.g. `SIGKILL`) to the child process. Defaults to `SIGTERM`. * `-n`, `--no-shell`: -Execute command directly, do not use `sh -c`. This is especially useful in combination with `--signal`, as the signal is then send directly to the specified command. While `--no-shell` is a little more performant than the default, it prevents using shell-features like pipes and redirects. +Execute command directly, do not wrap it in `sh -c` resp. `cmd.exe /C`. This is especially useful in combination with `--signal`, as the signal is then send directly to the specified command. While `--no-shell` is a little more performant than the default, it prevents using shell-features like pipes and redirects. * `-i`, `--ignore` : Ignores modifications from paths that match . This option can be specified multiple times, and a match on any pattern causes the path to be ignored. diff --git a/src/cli.rs b/src/cli.rs index c197e59..52d2e34 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -104,7 +104,7 @@ pub fn get_args() -> Args { .long("force-poll") .value_name("interval")) .arg(Arg::with_name("no-shell") - .help("Do not wrap command in 'sh -c'") + .help("Do not wrap command in 'sh -c' resp. 'cmd.exe /C'") .short("n") .long("no-shell")) .arg(Arg::with_name("once").short("1").hidden(true)) diff --git a/src/process.rs b/src/process.rs index ed6be69..95661be 100644 --- a/src/process.rs +++ b/src/process.rs @@ -139,7 +139,7 @@ mod imp { } impl Process { - pub fn new(cmd: &str, updated_paths: Vec) -> Result { + pub fn new(cmd: &str, updated_paths: Vec, no_shell: bool) -> Result { use std::os::windows::io::IntoRawHandle; fn last_err() -> io::Error { @@ -163,8 +163,23 @@ mod imp { panic!("failed to set job info: {}", last_err()); } - let mut command = Command::new("cmd.exe"); - command.arg("/C").arg(cmd); + let mut iter_args = cmd.split_whitespace(); + let arg0 = match no_shell { + true => iter_args.next().unwrap(), + false => "cmd.exe", + }; + + // TODO: There might be a better way of doing this with &str. + // I've had to fall back to String, as I wasn't able to join(" ") a Vec<&str> + // into a &str + let args: Vec = match no_shell { + true => iter_args.map(str::to_string).collect(), + false => vec!["/C".to_string(), iter_args.collect::>().join(" ")], + }; + + let mut command = Command::new(arg0); + command.args(args); + debug!("Assembled command {:?}", command); if let Some(single_path) = super::get_single_updated_path(&updated_paths) { command.env("WATCHEXEC_UPDATED_PATH", single_path); From df72aaf9779ac1443ef58da1e14dfececd9dc496 Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Mon, 10 Apr 2017 00:17:03 +0200 Subject: [PATCH 5/7] Use if-else statements instead of boolean match This was a hint by `cargo clippy` --- src/cli.rs | 8 +++++--- src/process.rs | 14 ++++++++------ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 52d2e34..60d45d3 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -116,9 +116,11 @@ pub fn get_args() -> Args { let paths = values_t!(args.values_of("path"), String).unwrap_or(vec![String::from(".")]); // Treat --kill as --signal SIGKILL (for compatibility with older syntax) - let signal = match args.is_present("kill") { - true => Some("SIGKILL".to_string()), - false => args.value_of("signal").map(str::to_string), // Convert Option<&str> to Option + let signal = if args.is_present("kill") { + Some("SIGKILL".to_string()) + } else { + // Convert Option<&str> to Option + args.value_of("signal").map(str::to_string) }; let mut filters = values_t!(args.values_of("filter"), String).unwrap_or(vec![]); diff --git a/src/process.rs b/src/process.rs index 95661be..7f73b32 100644 --- a/src/process.rs +++ b/src/process.rs @@ -35,17 +35,19 @@ mod imp { // but is a little less performant and can cause trouble when using custom signals // (e.g. --signal SIGHUP) let mut iter_args = cmd.split_whitespace(); - let arg0 = match no_shell { - true => iter_args.next().unwrap(), - false => "sh", + let arg0 = if no_shell { + iter_args.next().unwrap() + } else { + "sh" }; // TODO: There might be a better way of doing this with &str. // I've had to fall back to String, as I wasn't able to join(" ") a Vec<&str> // into a &str - let args: Vec = match no_shell { - true => iter_args.map(str::to_string).collect(), - false => vec!["-c".to_string(), iter_args.collect::>().join(" ")], + let args: Vec = if no_shell { + iter_args.map(str::to_string).collect() + } else { + vec!["-c".to_string(), iter_args.collect::>().join(" ")] }; let mut command = Command::new(arg0); From a395ed84cd810759459ba5dea3a688affa37c4da Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Mon, 10 Apr 2017 00:17:29 +0200 Subject: [PATCH 6/7] Remove useless format!() This was a hint by `cargo clippy` --- src/cli.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 60d45d3..1dfe292 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -151,13 +151,13 @@ pub fn get_args() -> Args { if signal.is_some() && args.is_present("postpone") { // TODO: Error::argument_conflict() might be the better fit, usage was unclear, though - Error::value_validation_auto(format!("--postpone and --signal are mutually exclusive")) + Error::value_validation_auto("--postpone and --signal are mutually exclusive".to_string()) .exit(); } if signal.is_some() && args.is_present("kill") { // TODO: Error::argument_conflict() might be the better fit, usage was unclear, though - Error::value_validation_auto(format!("--kill and --signal is ambiguous.\n Hint: Use only '--signal SIGKILL' without --kill")) + Error::value_validation_auto("--kill and --signal is ambiguous.\n Hint: Use only '--signal SIGKILL' without --kill".to_string()) .exit(); } From f88f1b36c31a62e4dcf92e8601a823a43846d6ce Mon Sep 17 00:00:00 2001 From: Chris Aumann Date: Mon, 10 Apr 2017 14:13:27 +0200 Subject: [PATCH 7/7] Refactor command assembly Kudos to https://www.reddit.com/user/burntsushi from the /r/rust community! See: https://www.reddit.com/r/rust/comments/64ic0q/idiomatic_string_processing/ --- src/process.rs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/process.rs b/src/process.rs index 7f73b32..a5393e8 100644 --- a/src/process.rs +++ b/src/process.rs @@ -34,24 +34,17 @@ mod imp { // Using "sh -c" gives us features like supportin pipes and redirects, // but is a little less performant and can cause trouble when using custom signals // (e.g. --signal SIGHUP) - let mut iter_args = cmd.split_whitespace(); - let arg0 = if no_shell { - iter_args.next().unwrap() + let mut command = if no_shell { + let mut split = cmd.split_whitespace(); + let mut command = Command::new(split.next().unwrap()); + command.args(split); + command } else { - "sh" + let mut command = Command::new("sh"); + command.arg("-c").arg(cmd); + command }; - // TODO: There might be a better way of doing this with &str. - // I've had to fall back to String, as I wasn't able to join(" ") a Vec<&str> - // into a &str - let args: Vec = if no_shell { - iter_args.map(str::to_string).collect() - } else { - vec!["-c".to_string(), iter_args.collect::>().join(" ")] - }; - - let mut command = Command::new(arg0); - command.args(args); debug!("Assembled command {:?}", command); if let Some(single_path) = super::get_single_updated_path(&updated_paths) {