Merge pull request #3014 from HeroicKatora/main

Make true return false less frequently

src/uu/false/src/false.rs

@@ -4,16 +4,62 @@
 //  *
 //  * For the full copyright and license information, please view the LICENSE
 //  * file that was distributed with this source code.
+use clap::{App, AppSettings, Arg};
+use std::io::Write;
+use uucore::error::{set_exit_code, UResult};
 
-use clap::App;
+static ABOUT: &str = "\
-use uucore::error::UResult;
+Returns false, an unsuccessful exit status.
+
+Immediately returns with the exit status `1`. When invoked with one of the recognized options it
+will try to write the help or version text. Any IO error during this operation is diagnosed, yet
+the program will also return `1`.
+";
 
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    uu_app().get_matches_from(args);
+    let mut app = uu_app();
-    Err(1.into())
+
+    // Mirror GNU options, always return `1`. In particular even the 'successful' cases of no-op,
+    // and the interrupted display of help and version should return `1`. Also, we return Ok in all
+    // paths to avoid the allocation of an error object, an operation that could, in theory, fail
+    // and unwind through the standard library allocation handling machinery.
+    set_exit_code(1);
+
+    if let Ok(matches) = app.try_get_matches_from_mut(args) {
+        let error = if matches.index_of("help").is_some() {
+            app.print_long_help()
+        } else if matches.index_of("version").is_some() {
+            writeln!(std::io::stdout(), "{}", app.render_version())
+        } else {
+            Ok(())
+        };
+
+        // Try to display this error.
+        if let Err(print_fail) = error {
+            // Completely ignore any error here, no more failover and we will fail in any case.
+            let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail);
+        }
+    }
+
+    Ok(())
 }
 
 pub fn uu_app<'a>() -> App<'a> {
     App::new(uucore::util_name())
+        .version(clap::crate_version!())
+        .about(ABOUT)
+        // We provide our own help and version options, to ensure maximum compatibility with GNU.
+        .setting(AppSettings::DisableHelpFlag | AppSettings::DisableVersionFlag)
+        .arg(
+            Arg::new("help")
+                .long("help")
+                .help("Print help information")
+                .exclusive(true),
+        )
+        .arg(
+            Arg::new("version")
+                .long("version")
+                .help("Print version information"),
+        )
 }

src/uu/true/src/true.rs

@@ -4,16 +4,59 @@
 //  *
 //  * For the full copyright and license information, please view the LICENSE
 //  * file that was distributed with this source code.
+use clap::{App, AppSettings, Arg};
+use std::io::Write;
+use uucore::error::{set_exit_code, UResult};
 
-use clap::{App, AppSettings};
+static ABOUT: &str = "\
-use uucore::error::UResult;
+Returns true, a successful exit status.
+
+Immediately returns with the exit status `0`, except when invoked with one of the recognized
+options. In those cases it will try to write the help or version text. Any IO error during this
+operation causes the program to return `1` instead.
+";
 
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    uu_app().get_matches_from(args);
+    let mut app = uu_app();
+
+    if let Ok(matches) = app.try_get_matches_from_mut(args) {
+        let error = if matches.index_of("help").is_some() {
+            app.print_long_help()
+        } else if matches.index_of("version").is_some() {
+            writeln!(std::io::stdout(), "{}", app.render_version())
+        } else {
+            Ok(())
+        };
+
+        if let Err(print_fail) = error {
+            // Try to display this error.
+            let _ = writeln!(std::io::stderr(), "{}: {}", uucore::util_name(), print_fail);
+            // Mirror GNU options. When failing to print warnings or version flags, then we exit
+            // with FAIL. This avoids allocation some error information which may result in yet
+            // other types of failure.
+            set_exit_code(1);
+        }
+    }
+
     Ok(())
 }
 
 pub fn uu_app<'a>() -> App<'a> {
-    App::new(uucore::util_name()).setting(AppSettings::InferLongArgs)
+    App::new(uucore::util_name())
+        .version(clap::crate_version!())
+        .about(ABOUT)
+        // We provide our own help and version options, to ensure maximum compatibility with GNU.
+        .setting(AppSettings::DisableHelpFlag | AppSettings::DisableVersionFlag)
+        .arg(
+            Arg::new("help")
+                .long("help")
+                .help("Print help information")
+                .exclusive(true),
+        )
+        .arg(
+            Arg::new("version")
+                .long("version")
+                .help("Print version information"),
+        )
 }

tests/by-util/test_false.rs

@@ -1,6 +1,53 @@
 use crate::common::util::*;
+#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))]
+use std::fs::OpenOptions;
 
 #[test]
 fn test_exit_code() {
     new_ucmd!().fails();
 }
+
+#[test]
+fn test_version() {
+    new_ucmd!()
+        .args(&["--version"])
+        .fails()
+        .stdout_contains("false");
+}
+
+#[test]
+fn test_help() {
+    new_ucmd!()
+        .args(&["--help"])
+        .fails()
+        .stdout_contains("false");
+}
+
+#[test]
+fn test_short_options() {
+    for option in ["-h", "-V"] {
+        new_ucmd!().arg(option).fails().stdout_is("");
+    }
+}
+
+#[test]
+fn test_conflict() {
+    new_ucmd!()
+        .args(&["--help", "--version"])
+        .fails()
+        .stdout_is("");
+}
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))]
+fn test_full() {
+    for option in ["--version", "--help"] {
+        let dev_full = OpenOptions::new().write(true).open("/dev/full").unwrap();
+
+        new_ucmd!()
+            .arg(option)
+            .set_stdout(dev_full)
+            .fails()
+            .stderr_contains("No space left on device");
+    }
+}

tests/by-util/test_true.rs

@@ -1,6 +1,53 @@
 use crate::common::util::*;
+#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))]
+use std::fs::OpenOptions;
 
 #[test]
 fn test_exit_code() {
     new_ucmd!().succeeds();
 }
+
+#[test]
+fn test_version() {
+    new_ucmd!()
+        .args(&["--version"])
+        .succeeds()
+        .stdout_contains("true");
+}
+
+#[test]
+fn test_help() {
+    new_ucmd!()
+        .args(&["--help"])
+        .succeeds()
+        .stdout_contains("true");
+}
+
+#[test]
+fn test_short_options() {
+    for option in ["-h", "-V"] {
+        new_ucmd!().arg(option).succeeds().stdout_is("");
+    }
+}
+
+#[test]
+fn test_conflict() {
+    new_ucmd!()
+        .args(&["--help", "--version"])
+        .succeeds()
+        .stdout_is("");
+}
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))]
+fn test_full() {
+    for option in ["--version", "--help"] {
+        let dev_full = OpenOptions::new().write(true).open("/dev/full").unwrap();
+
+        new_ucmd!()
+            .arg(option)
+            .set_stdout(dev_full)
+            .fails()
+            .stderr_contains("No space left on device");
+    }
+}