nohup: move from getopts to clap (#1871)

- changed some error return codes to match GNU implementation
- changed warning/error messages to match GNU nohup
- replaced getopts dependency with clap
- added a test

src/uu/nohup/Cargo.toml

@@ -15,7 +15,7 @@ edition = "2018"
 path = "src/nohup.rs"
 
 [dependencies]
-getopts = "0.2.18"
+clap = "2.33"
 libc = "0.2.42"
 uucore = { version=">=0.0.7", package="uucore", path="../../uucore", features=["fs"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }

src/uu/nohup/src/nohup.rs

@@ -10,6 +10,7 @@
 #[macro_use]
 extern crate uucore;
 
+use clap::{App, AppSettings, Arg};
 use libc::{c_char, dup2, execvp, signal};
 use libc::{SIGHUP, SIG_IGN};
 use std::env;
@@ -20,50 +21,42 @@ use std::os::unix::prelude::*;
 use std::path::{Path, PathBuf};
 use uucore::fs::{is_stderr_interactive, is_stdin_interactive, is_stdout_interactive};
 
-static NAME: &str = "nohup";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static ABOUT: &str = "Run COMMAND ignoring hangup signals.";
+static LONG_HELP: &str = "
+If standard input is terminal, it'll be replaced with /dev/null.
+If standard output is terminal, it'll be appended to nohup.out instead,
+or $HOME/nohup.out, if nohup.out open failed.
+If standard error is terminal, it'll be redirected to stdout.
+";
+static NOHUP_OUT: &str = "nohup.out";
+// exit codes that match the GNU implementation
+static EXIT_CANCELED: i32 = 125;
+static EXIT_CANNOT_INVOKE: i32 = 126;
+static EXIT_ENOENT: i32 = 127;
+static POSIX_NOHUP_FAILURE: i32 = 127;
 
-#[cfg(target_vendor = "apple")]
+mod options {
-extern "C" {
+    pub const CMD: &str = "cmd";
-    fn _vprocmgr_detach_from_console(flags: u32) -> *const libc::c_int;
-}
-
-#[cfg(any(target_os = "linux", target_os = "freebsd"))]
-unsafe fn _vprocmgr_detach_from_console(_: u32) -> *const libc::c_int {
-    std::ptr::null()
 }
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
-    let args = args.collect_str();
+    let usage = get_usage();
 
-    let mut opts = getopts::Options::new();
+    let matches = App::new(executable!())
+        .version(VERSION)
+        .about(ABOUT)
+        .usage(&usage[..])
+        .after_help(LONG_HELP)
+        .arg(
+            Arg::with_name(options::CMD)
+                .hidden(true)
+                .required(true)
+                .multiple(true),
+        )
+        .setting(AppSettings::TrailingVarArg)
+        .get_matches_from(args);
 
-    opts.optflag("h", "help", "Show help and exit");
-    opts.optflag("V", "version", "Show version and exit");
-
-    let matches = match opts.parse(&args[1..]) {
-        Ok(m) => m,
-        Err(f) => {
-            show_error!("{}", f);
-            show_usage(&opts);
-            return 1;
-        }
-    };
-
-    if matches.opt_present("V") {
-        println!("{} {}", NAME, VERSION);
-        return 0;
-    }
-    if matches.opt_present("h") {
-        show_usage(&opts);
-        return 0;
-    }
-
-    if matches.free.is_empty() {
-        show_error!("Missing operand: COMMAND");
-        println!("Try `{} --help` for more information.", NAME);
-        return 1;
-    }
     replace_fds();
 
     unsafe { signal(SIGHUP, SIG_IGN) };
@@ -73,13 +66,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     };
 
     let cstrs: Vec<CString> = matches
-        .free
+        .values_of(options::CMD)
-        .iter()
+        .unwrap()
         .map(|x| CString::new(x.as_bytes()).unwrap())
         .collect();
     let mut args: Vec<*const c_char> = cstrs.iter().map(|s| s.as_ptr()).collect();
     args.push(std::ptr::null());
-    unsafe { execvp(args[0], args.as_mut_ptr()) }
+
+    let ret = unsafe { execvp(args[0], args.as_mut_ptr()) };
+    match ret {
+        libc::ENOENT => EXIT_ENOENT,
+        _ => EXIT_CANNOT_INVOKE,
+    }
 }
 
 fn replace_fds() {
@@ -108,23 +106,32 @@ fn replace_fds() {
 }
 
 fn find_stdout() -> File {
+    let internal_failure_code = match std::env::var("POSIXLY_CORRECT") {
+        Ok(_) => POSIX_NOHUP_FAILURE,
+        Err(_) => EXIT_CANCELED,
+    };
+
     match OpenOptions::new()
         .write(true)
         .create(true)
         .append(true)
-        .open(Path::new("nohup.out"))
+        .open(Path::new(NOHUP_OUT))
     {
         Ok(t) => {
-            show_warning!("Output is redirected to: nohup.out");
+            show_info!("ignoring input and appending output to '{}'", NOHUP_OUT);
             t
         }
-        Err(e) => {
+        Err(e1) => {
             let home = match env::var("HOME") {
-                Err(_) => crash!(2, "Cannot replace STDOUT: {}", e),
+                Err(_) => {
+                    show_info!("failed to open '{}': {}", NOHUP_OUT, e1);
+                    exit!(internal_failure_code)
+                }
                 Ok(h) => h,
             };
             let mut homeout = PathBuf::from(home);
-            homeout.push("nohup.out");
+            homeout.push(NOHUP_OUT);
+            let homeout_str = homeout.to_str().unwrap();
             match OpenOptions::new()
                 .write(true)
                 .create(true)
@@ -132,30 +139,29 @@ fn find_stdout() -> File {
                 .open(&homeout)
             {
                 Ok(t) => {
-                    show_warning!("Output is redirected to: {:?}", homeout);
+                    show_info!("ignoring input and appending output to '{}'", homeout_str);
                     t
                 }
-                Err(e) => crash!(2, "Cannot replace STDOUT: {}", e),
+                Err(e2) => {
+                    show_info!("failed to open '{}': {}", NOHUP_OUT, e1);
+                    show_info!("failed to open '{}': {}", homeout_str, e2);
+                    exit!(internal_failure_code)
+                }
             }
         }
     }
 }
 
-fn show_usage(opts: &getopts::Options) {
+fn get_usage() -> String {
-    let msg = format!(
+    format!("{0} COMMAND [ARG]...\n    {0} FLAG", executable!())
-        "{0} {1}
+}
-
+
-Usage:
+#[cfg(target_vendor = "apple")]
-  {0} COMMAND [ARG]...
+extern "C" {
-  {0} OPTION
+    fn _vprocmgr_detach_from_console(flags: u32) -> *const libc::c_int;
-
+}
-Run COMMAND ignoring hangup signals.
+
-If standard input is terminal, it'll be replaced with /dev/null.
+#[cfg(any(target_os = "linux", target_os = "freebsd"))]
-If standard output is terminal, it'll be appended to nohup.out instead,
+unsafe fn _vprocmgr_detach_from_console(_: u32) -> *const libc::c_int {
-or $HOME/nohup.out, if nohup.out open failed.
+    std::ptr::null()
-If standard error is terminal, it'll be redirected to stdout.",
-        NAME, VERSION
-    );
-
-    print!("{}", opts.usage(&msg));
 }

tests/by-util/test_nohup.rs

@@ -1 +1,19 @@
-// ToDO: add tests
+use crate::common::util::*;
+use std::thread::sleep;
+
+// General observation: nohup.out will not be created in tests run by cargo test
+// because stdin/stdout is not attached to a TTY.
+// All that can be tested is the side-effects.
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "freebsd", target_vendor = "apple"))]
+fn test_nohup_multiple_args_and_flags() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    ucmd.args(&["touch", "-t", "1006161200", "file1", "file2"])
+        .succeeds();
+    sleep(std::time::Duration::from_millis(10));
+
+    assert!(at.file_exists("file1"));
+    assert!(at.file_exists("file2"));
+}