Merge pull request #1810 from jeckersb/nice-clap

nice: move from getopts to clap #1794

Cargo.lock

@@ -1823,8 +1823,9 @@ dependencies = [
 name = "uu_nice"
 version = "0.0.4"
 dependencies = [
- "getopts 0.2.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "clap 2.33.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.85 (registry+https://github.com/rust-lang/crates.io-index)",
+ "nix 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "uucore 0.0.7",
  "uucore_procs 0.0.5",
 ]

src/uu/nice/Cargo.toml

@@ -15,8 +15,9 @@ edition = "2018"
 path = "src/nice.rs"
 
 [dependencies]
-getopts = "0.2.18"
+clap = "2.33"
 libc = "0.2.42"
+nix = { version="<=0.13" }
 uucore = { version=">=0.0.7", package="uucore", path="../../uucore" }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 

src/uu/nice/src/nice.rs

@@ -15,7 +15,7 @@ use std::ffi::CString;
 use std::io::Error;
 use std::ptr;
 
-const NAME: &str = "nice";
+use clap::{App, AppSettings, Arg};
 const VERSION: &str = env!("CARGO_PKG_VERSION");
 
 // XXX: PRIO_PROCESS is 0 on at least FreeBSD and Linux.  Don't know about Mac OS X.
@@ -26,64 +26,57 @@ extern "C" {
     fn setpriority(which: c_int, who: c_int, prio: c_int) -> c_int;
 }
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
-    let args = args.collect_str();
+pub mod options {
+    pub static ADJUSTMENT: &str = "adjustment";
+    pub static COMMAND: &str = "COMMAND";
+}
 
-    let mut opts = getopts::Options::new();
-
-    opts.optopt(
-        "n",
-        "adjustment",
-        "add N to the niceness (default is 10)",
-        "N",
-    );
-    opts.optflag("h", "help", "display this help and exit");
-    opts.optflag("V", "version", "output version information and exit");
-
-    let matches = match opts.parse(&args[1..]) {
-        Ok(m) => m,
-        Err(err) => {
-            show_error!("{}", err);
-            return 125;
-        }
-    };
-
-    if matches.opt_present("version") {
-        println!("{} {}", NAME, VERSION);
-        return 0;
-    }
-
-    if matches.opt_present("help") {
-        let msg = format!(
-            "{0} {1}
-
-Usage:
+fn get_usage() -> String {
+    format!(
+        "
   {0} [OPTIONS] [COMMAND [ARGS]]
 
 Run COMMAND with an adjusted niceness, which affects process scheduling.
 With no COMMAND, print the current niceness.  Niceness values range from at
 least -20 (most favorable to the process) to 19 (least favorable to the
 process).",
-            NAME, VERSION
-        );
+        executable!()
+    )
+}
 
-        print!("{}", opts.usage(&msg));
-        return 0;
-    }
+pub fn uumain(args: impl uucore::Args) -> i32 {
+    let usage = get_usage();
 
-    let mut niceness = unsafe { getpriority(PRIO_PROCESS, 0) };
+    let matches = App::new(executable!())
+        .setting(AppSettings::TrailingVarArg)
+        .version(VERSION)
+        .usage(&usage[..])
+        .arg(
+            Arg::with_name(options::ADJUSTMENT)
+                .short("n")
+                .long(options::ADJUSTMENT)
+                .help("add N to the niceness (default is 10)")
+                .takes_value(true)
+                .allow_hyphen_values(true),
+        )
+        .arg(Arg::with_name(options::COMMAND).multiple(true))
+        .get_matches_from(args);
+
+    let mut niceness = unsafe {
+        nix::errno::Errno::clear();
+        getpriority(PRIO_PROCESS, 0)
+    };
     if Error::last_os_error().raw_os_error().unwrap() != 0 {
-        show_error!("{}", Error::last_os_error());
+        show_error!("getpriority: {}", Error::last_os_error());
         return 125;
     }
 
-    let adjustment = match matches.opt_str("adjustment") {
+    let adjustment = match matches.value_of(options::ADJUSTMENT) {
         Some(nstr) => {
-            if matches.free.is_empty() {
+            if !matches.is_present(options::COMMAND) {
                 show_error!(
-                    "A command must be given with an adjustment.
-                                Try \"{} --help\" for more information.",
-                    args[0]
+                    "A command must be given with an adjustment.\nTry \"{} --help\" for more information.",
+                    executable!()
                 );
                 return 125;
             }
@@ -96,7 +89,7 @@ process).",
             }
         }
         None => {
-            if matches.free.is_empty() {
+            if !matches.is_present(options::COMMAND) {
                 println!("{}", niceness);
                 return 0;
             }
@@ -105,25 +98,23 @@ process).",
     };
 
     niceness += adjustment;
-    unsafe {
-        setpriority(PRIO_PROCESS, 0, niceness);
-    }
-    if Error::last_os_error().raw_os_error().unwrap() != 0 {
-        show_warning!("{}", Error::last_os_error());
+    if unsafe { setpriority(PRIO_PROCESS, 0, niceness) } == -1 {
+        show_warning!("setpriority: {}", Error::last_os_error());
     }
 
     let cstrs: Vec<CString> = matches
-        .free
-        .iter()
+        .values_of(options::COMMAND)
+        .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(ptr::null::<c_char>());
     unsafe {
         execvp(args[0], args.as_mut_ptr());
     }
 
-    show_error!("{}", Error::last_os_error());
+    show_error!("execvp: {}", Error::last_os_error());
     if Error::last_os_error().raw_os_error().unwrap() as c_int == libc::ENOENT {
         127
     } else {

tests/by-util/test_nice.rs

@@ -1 +1,56 @@
-// ToDO: add tests
+use crate::common::util::*;
+
+#[test]
+fn test_get_current_niceness() {
+    // NOTE: this assumes the test suite is being run with a default niceness
+    // of 0, which may not necessarily be true
+    new_ucmd!().run().stdout_is("0\n");
+}
+
+#[test]
+fn test_negative_adjustment() {
+    // This assumes the test suite is run as a normal (non-root) user, and as
+    // such attempting to set a negative niceness value will be rejected by
+    // the OS.  If it gets denied, then we know a negative value was parsed
+    // correctly.
+
+    let res = new_ucmd!().args(&["-n", "-1", "true"]).run();
+    assert!(res.stderr.starts_with("nice: warning: setpriority: Permission denied"));
+}
+
+#[test]
+fn test_adjustment_with_no_command_should_error() {
+    new_ucmd!()
+    .args(&["-n", "19"])
+    .run()
+    .stderr_is("nice: error: A command must be given with an adjustment.\nTry \"nice --help\" for more information.\n");
+}
+
+#[test]
+fn test_command_with_no_adjustment() {
+    new_ucmd!().args(&["echo", "a"]).run().stdout_is("a\n");
+}
+
+#[test]
+fn test_command_with_no_args() {
+    new_ucmd!()
+        .args(&["-n", "19", "echo"])
+        .run()
+        .stdout_is("\n");
+}
+
+#[test]
+fn test_command_with_args() {
+    new_ucmd!()
+        .args(&["-n", "19", "echo", "a", "b", "c"])
+        .run()
+        .stdout_is("a b c\n");
+}
+
+#[test]
+fn test_command_where_command_takes_n_flag() {
+    new_ucmd!()
+        .args(&["-n", "19", "echo", "-n", "a"])
+        .run()
+        .stdout_is("a");
+}