mkfifo: general refactor, move to clap, add tests (#1945)

* mkfifo: general refactor, move to clap, add unimplemented flags

* chore: update Cargo.lock

* chore: delete unused variables, simplify multiple lines with crash

* test: add tests

* chore: revert the use of crash

* test: use even more invalid mod mode

Cargo.lock

@@ -1760,7 +1760,7 @@ dependencies = [
 name = "uu_mkfifo"
 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)",
  "uucore 0.0.7",
  "uucore_procs 0.0.5",

src/uu/mkfifo/Cargo.toml

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

src/uu/mkfifo/src/mkfifo.rs

@@ -8,56 +8,55 @@
 #[macro_use]
 extern crate uucore;
 
+use clap::{App, Arg};
 use libc::mkfifo;
 use std::ffi::CString;
-use std::io::Error;
 
 static NAME: &str = "mkfifo";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static USAGE: &str = "mkfifo [OPTION]... NAME...";
+static SUMMARY: &str = "Create a FIFO with the given name.";
+
+mod options {
+    pub static MODE: &str = "mode";
+    pub static SE_LINUX_SECURITY_CONTEXT: &str = "Z";
+    pub static CONTEXT: &str = "context";
+    pub static FIFO: &str = "fifo";
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let args = args.collect_str();
 
-    let mut opts = getopts::Options::new();
+    let matches = App::new(executable!())
+        .name(NAME)
+        .version(VERSION)
+        .usage(USAGE)
+        .about(SUMMARY)
+        .arg(
+            Arg::with_name(options::MODE)
+                .short("m")
+                .long(options::MODE)
+                .help("file permissions for the fifo")
+                .default_value("0666")
+                .value_name("0666"),
+        )
+        .arg(
+            Arg::with_name(options::SE_LINUX_SECURITY_CONTEXT)
+                .short(options::SE_LINUX_SECURITY_CONTEXT)
+                .help("set the SELinux security context to default type")
+        )
+        .arg(Arg::with_name(options::CONTEXT).long(options::CONTEXT).value_name("CTX").help("like -Z, or if CTX is specified then set the SELinux\nor SMACK security context to CTX"))
+        .arg(Arg::with_name(options::FIFO).hidden(true).multiple(true))
+        .get_matches_from(args);
 
-    opts.optopt(
+    if matches.is_present(options::CONTEXT) {
-        "m",
+        crash!(1, "--context is not implemented");
-        "mode",
+    }
-        "file permissions for the fifo",
+    if matches.is_present(options::SE_LINUX_SECURITY_CONTEXT) {
-        "(default 0666)",
+        crash!(1, "-Z is not implemented");
-    );
-    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) => panic!("{}", err),
-    };
-
-    if matches.opt_present("version") {
-        println!("{} {}", NAME, VERSION);
-        return 0;
     }
 
-    if matches.opt_present("help") || matches.free.is_empty() {
+    let mode = match matches.value_of(options::MODE) {
-        let msg = format!(
-            "{0} {1}
-
-Usage:
-  {0} [OPTIONS] NAME...
-
-Create a FIFO with the given name.",
-            NAME, VERSION
-        );
-
-        print!("{}", opts.usage(&msg));
-        if matches.free.is_empty() {
-            return 1;
-        }
-        return 0;
-    }
-
-    let mode = match matches.opt_str("m") {
         Some(m) => match usize::from_str_radix(&m, 8) {
             Ok(m) => m,
             Err(e) => {
@@ -68,21 +67,22 @@ Create a FIFO with the given name.",
         None => 0o666,
     };
 
-    let mut exit_status = 0;
+    let fifos: Vec<String> = match matches.values_of(options::FIFO) {
-    for f in &matches.free {
+        Some(v) => v.clone().map(|s| s.to_owned()).collect(),
+        None => crash!(1, "missing operand"),
+    };
+
+    let mut exit_code = 0;
+    for f in fifos {
         let err = unsafe {
             let name = CString::new(f.as_bytes()).unwrap();
             mkfifo(name.as_ptr(), mode as libc::mode_t)
         };
         if err == -1 {
-            show_error!(
+            show_error!("cannot create fifo '{}': File exists", f);
-                "creating '{}': {}",
+            exit_code = 1;
-                f,
-                Error::last_os_error().raw_os_error().unwrap()
-            );
-            exit_status = 1;
         }
     }
 
-    exit_status
+    exit_code
 }

tests/by-util/test_mkfifo.rs

@@ -1 +1,48 @@
-// ToDO: add tests
+use crate::common::util::*;
+
+#[test]
+fn test_create_fifo_missing_operand() {
+    new_ucmd!()
+        .fails()
+        .stderr_is("mkfifo: error: missing operand");
+}
+
+#[test]
+fn test_create_one_fifo() {
+    new_ucmd!().arg("abc").succeeds();
+}
+
+#[test]
+fn test_create_one_fifo_with_invalid_mode() {
+    new_ucmd!()
+        .arg("abcd")
+        .arg("-m")
+        .arg("invalid")
+        .fails()
+        .stderr
+        .contains("invalid mode");
+}
+
+#[test]
+fn test_create_multiple_fifos() {
+    new_ucmd!()
+        .arg("abcde")
+        .arg("def")
+        .arg("sed")
+        .arg("dum")
+        .succeeds();
+}
+
+#[test]
+fn test_create_one_fifo_with_mode() {
+    new_ucmd!().arg("abcde").arg("-m600").succeeds();
+}
+
+#[test]
+fn test_create_one_fifo_already_exists() {
+    new_ucmd!()
+        .arg("abcdef")
+        .arg("abcdef")
+        .fails()
+        .stderr_is("mkfifo: error: cannot create fifo 'abcdef': File exists");
+}