Merge pull request #2036 from joppich/issue1930

stdbuf: move from getopts to clap

Cargo.lock

@@ -2316,7 +2316,7 @@ dependencies = [
 name = "uu_stdbuf"
 version = "0.0.6"
 dependencies = [
- "getopts",
+ "clap",
  "tempfile",
  "uu_stdbuf_libstdbuf",
  "uucore",

src/uu/stdbuf/Cargo.toml

@@ -15,7 +15,7 @@ edition = "2018"
 path = "src/stdbuf.rs"
 
 [dependencies]
-getopts = "0.2.18"
+clap = "2.33"
 tempfile = "3.1"
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore" }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }

src/uu/stdbuf/src/stdbuf.rs

@@ -10,7 +10,8 @@
 #[macro_use]
 extern crate uucore;
 
-use getopts::{Matches, Options};
+use clap::{App, AppSettings, Arg, ArgMatches};
+use std::convert::TryFrom;
 use std::fs::File;
 use std::io::{self, Write};
 use std::os::unix::process::ExitStatusExt;
@@ -19,8 +20,35 @@ use std::process::Command;
 use tempfile::tempdir;
 use tempfile::TempDir;
 
-static NAME: &str = "stdbuf";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static ABOUT: &str =
+    "Run COMMAND, with modified buffering operations for its standard streams.\n\n\
+                      Mandatory arguments to long options are mandatory for short options too.";
+static LONG_HELP: &str = "If MODE is 'L' the corresponding stream will be line buffered.\n\
+                          This option is invalid with standard input.\n\n\
+                          If MODE is '0' the corresponding stream will be unbuffered.\n\n\
+                          Otherwise MODE is a number which may be followed by one of the following:\n\n\
+                          KB 1000, K 1024, MB 1000*1000, M 1024*1024, and so on for G, T, P, E, Z, Y.\n\
+                          In this case the corresponding stream will be fully buffered with the buffer size set to \
+                          MODE bytes.\n\n\
+                          NOTE: If COMMAND adjusts the buffering of its standard streams ('tee' does for e.g.) then \
+                          that will override corresponding settings changed by 'stdbuf'.\n\
+                          Also some filters (like 'dd' and 'cat' etc.) don't use streams for I/O, \
+                          and are thus unaffected by 'stdbuf' settings.\n";
+
+mod options {
+    pub const INPUT: &str = "input";
+    pub const INPUT_SHORT: &str = "i";
+    pub const OUTPUT: &str = "output";
+    pub const OUTPUT_SHORT: &str = "o";
+    pub const ERROR: &str = "error";
+    pub const ERROR_SHORT: &str = "e";
+    pub const COMMAND: &str = "command";
+}
+
+fn get_usage() -> String {
+    format!("{0} OPTION... COMMAND", executable!())
+}
 
 const STDBUF_INJECT: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/libstdbuf.so"));
 
@@ -36,16 +64,19 @@ struct ProgramOptions {
     stderr: BufferType,
 }
 
-enum ErrMsg {
+impl<'a> TryFrom<&ArgMatches<'a>> for ProgramOptions {
-    Retry,
+    type Error = ProgramOptionsError;
-    Fatal,
+
+    fn try_from(matches: &ArgMatches) -> Result<Self, Self::Error> {
+        Ok(ProgramOptions {
+            stdin: check_option(&matches, options::INPUT)?,
+            stdout: check_option(&matches, options::OUTPUT)?,
+            stderr: check_option(&matches, options::ERROR)?,
+        })
+    }
 }
 
-enum OkMsg {
+struct ProgramOptionsError(String);
-    Buffering,
-    Help,
-    Version,
-}
 
 #[cfg(any(
     target_os = "linux",
@@ -73,31 +104,6 @@ fn preload_strings() -> (&'static str, &'static str) {
     crash!(1, "Command not supported for this operating system!")
 }
 
-fn print_version() {
-    println!("{} {}", NAME, VERSION);
-}
-
-fn print_usage(opts: &Options) {
-    let brief = "Run COMMAND, with modified buffering operations for its standard streams\n \
-                 Mandatory arguments to long options are mandatory for short options too.";
-    let explanation = "If MODE is 'L' the corresponding stream will be line buffered.\n \
-         This option is invalid with standard input.\n\n \
-         If MODE is '0' the corresponding stream will be unbuffered.\n\n \
-         Otherwise MODE is a number which may be followed by one of the following:\n\n \
-         KB 1000, K 1024, MB 1000*1000, M 1024*1024, and so on for G, T, P, E, Z, Y.\n \
-         In this case the corresponding stream will be fully buffered with the buffer size set to \
-         MODE bytes.\n\n \
-         NOTE: If COMMAND adjusts the buffering of its standard streams ('tee' does for e.g.) then \
-         that will override corresponding settings changed by 'stdbuf'.\n \
-         Also some filters (like 'dd' and 'cat' etc.) don't use streams for I/O, \
-         and are thus unaffected by 'stdbuf' settings.\n";
-    println!("{} {}", NAME, VERSION);
-    println!();
-    println!("Usage: stdbuf OPTION... COMMAND");
-    println!();
-    println!("{}\n{}", opts.usage(brief), explanation);
-}
-
 fn parse_size(size: &str) -> Option<u64> {
     let ext = size.trim_start_matches(|c: char| c.is_digit(10));
     let num = size.trim_end_matches(char::is_alphabetic);
@@ -133,65 +139,30 @@ fn parse_size(size: &str) -> Option<u64> {
     Some(buf_size * base.pow(power))
 }
 
-fn check_option(matches: &Matches, name: &str, modified: &mut bool) -> Option<BufferType> {
+fn check_option(matches: &ArgMatches, name: &str) -> Result<BufferType, ProgramOptionsError> {
-    match matches.opt_str(name) {
+    match matches.value_of(name) {
-        Some(value) => {
+        Some(value) => match &value[..] {
-            *modified = true;
+            "L" => {
-            match &value[..] {
+                if name == options::INPUT {
-                "L" => {
+                    Err(ProgramOptionsError(format!(
-                    if name == "input" {
+                        "line buffering stdin is meaningless"
-                        show_info!("line buffering stdin is meaningless");
+                    )))
-                        None
+                } else {
-                    } else {
+                    Ok(BufferType::Line)
-                        Some(BufferType::Line)
-                    }
-                }
-                x => {
-                    let size = match parse_size(x) {
-                        Some(m) => m,
-                        None => {
-                            show_error!("Invalid mode {}", x);
-                            return None;
-                        }
-                    };
-                    Some(BufferType::Size(size))
                 }
             }
-        }
+            x => {
-        None => Some(BufferType::Default),
+                let size = match parse_size(x) {
+                    Some(m) => m,
+                    None => return Err(ProgramOptionsError(format!("invalid mode {}", x))),
+                };
+                Ok(BufferType::Size(size))
+            }
+        },
+        None => Ok(BufferType::Default),
     }
 }
 
-fn parse_options(
-    args: &[String],
-    options: &mut ProgramOptions,
-    optgrps: &Options,
-) -> Result<OkMsg, ErrMsg> {
-    let matches = match optgrps.parse(args) {
-        Ok(m) => m,
-        Err(_) => return Err(ErrMsg::Retry),
-    };
-    if matches.opt_present("help") {
-        return Ok(OkMsg::Help);
-    }
-    if matches.opt_present("version") {
-        return Ok(OkMsg::Version);
-    }
-    let mut modified = false;
-    options.stdin = check_option(&matches, "input", &mut modified).ok_or(ErrMsg::Fatal)?;
-    options.stdout = check_option(&matches, "output", &mut modified).ok_or(ErrMsg::Fatal)?;
-    options.stderr = check_option(&matches, "error", &mut modified).ok_or(ErrMsg::Fatal)?;
-
-    if matches.free.len() != 1 {
-        return Err(ErrMsg::Retry);
-    }
-    if !modified {
-        show_error!("you must specify a buffering mode option");
-        return Err(ErrMsg::Fatal);
-    }
-    Ok(OkMsg::Buffering)
-}
-
 fn set_command_env(command: &mut Command, buffer_name: &str, buffer_type: BufferType) {
     match buffer_type {
         BufferType::Size(m) => {
@@ -215,72 +186,62 @@ fn get_preload_env(tmp_dir: &mut TempDir) -> io::Result<(String, PathBuf)> {
 }
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
-    let args = args.collect_str();
+    let usage = get_usage();
 
-    let mut opts = Options::new();
+    let matches = App::new(executable!())
+        .version(VERSION)
+        .about(ABOUT)
+        .usage(&usage[..])
+        .after_help(LONG_HELP)
+        .setting(AppSettings::TrailingVarArg)
+        .arg(
+            Arg::with_name(options::INPUT)
+                .long(options::INPUT)
+                .short(options::INPUT_SHORT)
+                .help("adjust standard input stream buffering")
+                .value_name("MODE")
+                .required_unless_one(&[options::OUTPUT, options::ERROR]),
+        )
+        .arg(
+            Arg::with_name(options::OUTPUT)
+                .long(options::OUTPUT)
+                .short(options::OUTPUT_SHORT)
+                .help("adjust standard output stream buffering")
+                .value_name("MODE")
+                .required_unless_one(&[options::INPUT, options::ERROR]),
+        )
+        .arg(
+            Arg::with_name(options::ERROR)
+                .long(options::ERROR)
+                .short(options::ERROR_SHORT)
+                .help("adjust standard error stream buffering")
+                .value_name("MODE")
+                .required_unless_one(&[options::INPUT, options::OUTPUT]),
+        )
+        .arg(
+            Arg::with_name(options::COMMAND)
+                .multiple(true)
+                .takes_value(true)
+                .hidden(true)
+                .required(true),
+        )
+        .get_matches_from(args);
 
-    opts.optopt(
+    let options = ProgramOptions::try_from(&matches)
-        "i",
+        .unwrap_or_else(|e| crash!(125, "{}\nTry 'stdbuf --help' for more information.", e.0));
-        "input",
-        "adjust standard input stream buffering",
-        "MODE",
-    );
-    opts.optopt(
-        "o",
-        "output",
-        "adjust standard output stream buffering",
-        "MODE",
-    );
-    opts.optopt(
-        "e",
-        "error",
-        "adjust standard error stream buffering",
-        "MODE",
-    );
-    opts.optflag("", "help", "display this help and exit");
-    opts.optflag("", "version", "output version information and exit");
 
-    let mut options = ProgramOptions {
+    let mut command_values = matches.values_of::<&str>(options::COMMAND).unwrap();
-        stdin: BufferType::Default,
+    let mut command = Command::new(command_values.next().unwrap());
-        stdout: BufferType::Default,
+    let command_params: Vec<&str> = command_values.collect();
-        stderr: BufferType::Default,
-    };
-    let mut command_idx: i32 = -1;
-    for i in 1..=args.len() {
-        match parse_options(&args[1..i], &mut options, &opts) {
-            Ok(OkMsg::Buffering) => {
-                command_idx = (i as i32) - 1;
-                break;
-            }
-            Ok(OkMsg::Help) => {
-                print_usage(&opts);
-                return 0;
-            }
-            Ok(OkMsg::Version) => {
-                print_version();
-                return 0;
-            }
-            Err(ErrMsg::Fatal) => break,
-            Err(ErrMsg::Retry) => continue,
-        }
-    }
-    if command_idx == -1 {
-        crash!(
-            125,
-            "Invalid options\nTry 'stdbuf --help' for more information."
-        );
-    }
-    let command_name = &args[command_idx as usize];
-    let mut command = Command::new(command_name);
 
     let mut tmp_dir = tempdir().unwrap();
     let (preload_env, libstdbuf) = return_if_err!(1, get_preload_env(&mut tmp_dir));
-    command
+    command.env(preload_env, libstdbuf);
-        .args(&args[(command_idx as usize) + 1..])
-        .env(preload_env, libstdbuf);
     set_command_env(&mut command, "_STDBUF_I", options.stdin);
     set_command_env(&mut command, "_STDBUF_O", options.stdout);
     set_command_env(&mut command, "_STDBUF_E", options.stderr);
+    command.args(command_params);
+
     let mut process = match command.spawn() {
         Ok(p) => p,
         Err(e) => crash!(1, "failed to execute process: {}", e),

tests/by-util/test_stdbuf.rs

@@ -1,13 +1,71 @@
+#[cfg(not(target_os = "windows"))]
 use crate::common::util::*;
 
+#[cfg(not(target_os = "windows"))]
 #[test]
 fn test_stdbuf_unbuffered_stdout() {
-    if cfg!(target_os = "linux") {
+    // This is a basic smoke test
-        // This is a basic smoke test
+    new_ucmd!()
-        new_ucmd!()
+        .args(&["-o0", "head"])
-            .args(&["-o0", "head"])
+        .pipe_in("The quick brown fox jumps over the lazy dog.")
-            .pipe_in("The quick brown fox jumps over the lazy dog.")
+        .run()
-            .run()
+        .stdout_is("The quick brown fox jumps over the lazy dog.");
-            .stdout_is("The quick brown fox jumps over the lazy dog.");
+}
-    }
+
+#[cfg(not(target_os = "windows"))]
+#[test]
+fn test_stdbuf_line_buffered_stdout() {
+    new_ucmd!()
+        .args(&["-oL", "head"])
+        .pipe_in("The quick brown fox jumps over the lazy dog.")
+        .run()
+        .stdout_is("The quick brown fox jumps over the lazy dog.");
+}
+
+#[cfg(not(target_os = "windows"))]
+#[test]
+fn test_stdbuf_no_buffer_option_fails() {
+    new_ucmd!()
+        .args(&["head"])
+        .pipe_in("The quick brown fox jumps over the lazy dog.")
+        .fails()
+        .stderr_is(
+            "error: The following required arguments were not provided:\n    \
+            --error <MODE>\n    \
+            --input <MODE>\n    \
+            --output <MODE>\n\n\
+            USAGE:\n    \
+            stdbuf OPTION... COMMAND\n\n\
+            For more information try --help",
+        );
+}
+
+#[cfg(not(target_os = "windows"))]
+#[test]
+fn test_stdbuf_trailing_var_arg() {
+    new_ucmd!()
+        .args(&["-i", "1024", "tail", "-1"])
+        .pipe_in("The quick brown fox\njumps over the lazy dog.")
+        .run()
+        .stdout_is("jumps over the lazy dog.");
+}
+
+#[cfg(not(target_os = "windows"))]
+#[test]
+fn test_stdbuf_line_buffering_stdin_fails() {
+    new_ucmd!()
+        .args(&["-i", "L", "head"])
+        .pipe_in("The quick brown fox jumps over the lazy dog.")
+        .fails()
+        .stderr_is("stdbuf: error: line buffering stdin is meaningless\nTry 'stdbuf --help' for more information.");
+}
+
+#[cfg(not(target_os = "windows"))]
+#[test]
+fn test_stdbuf_invalid_mode_fails() {
+    new_ucmd!()
+        .args(&["-i", "1024R", "head"])
+        .pipe_in("The quick brown fox jumps over the lazy dog.")
+        .fails()
+        .stderr_is("stdbuf: error: invalid mode 1024R\nTry 'stdbuf --help' for more information.");
 }