Feat: Update Cmp

* use clap parser
* internationalization of error and
  info messages, e.g. --help
* moved tests into tests by-util
* separated and moved benches
* adapted fuzzer
* common parser error handling for all modules
* some fixes, e.g. error on selection of a directory

Author: Gunter Schmidt

Cargo.lock

@@ -63,13 +63,9 @@ feat_common_core = [
   "cmp", "diff"
 ]
 
-# default = ["feat_bench_not_diff"]
-# Turn bench for diffutils cmp off
-feat_bench_not_cmp = []
 # Turn bench for diffutils diff off
 feat_bench_not_diff = []
 
-
 [workspace]
 resolver = "3"
 members = [
@@ -78,7 +74,7 @@ members = [
 #   "src/uu/stdbuf/src/libstdbuf",
   "src/uudiff",
 #   "src/uucore_procs",
-  # "tests/uutests",
+  "tests/uutests",
   # "fuzz",  # TODO fuzz
 ]
 
@@ -98,7 +94,7 @@ version = "0.5.1"
 
 [workspace.dependencies]
 assert_cmd = "2.2.0"
-# bytecount = "0.6.9"
+bytecount = "0.6.9"
 chrono = "0.4.0"
 clap = { version = "4.6", features = ["wrap_help", "cargo", "color"] }
 const_format = "0.2.35"
@@ -116,22 +112,24 @@ predicates = "3.1.0"
 pretty_assertions = "1.4.0"
 rand = "0.10.0"
 regex = "1.10.4"
-
+rlimit = "0.11.0"
 same-file = "1.0.6"
 tempfile = "3.27.0"
 textwrap = { version = "0.16.1", features = ["terminal_size"] }
 thiserror = "2.0.3"
 unicode-width = "0.2.0"
-uucore = "0.7.0"
-uudiff = { package = "uudiff", path = "src/uudiff" }
-# sdiff = {package = "uu_sdiff", path = "src/uu/sdiff" }
+xattr = "1.3.1"
 
 # Fluent dependencies
 fluent = "0.17.0"
 fluent-bundle = "0.16.0"
 unic-langid = "0.9.6"
 fluent-syntax = "0.12.0"
 
+uucore = { version = "0.7.0", features = ["parser-size"]}
+uudiff = { package = "uudiff", path = "src/uudiff" }
+uutests = { version = "0.5.1", package = "uutests", path = "tests/uutests" }
+
 [dependencies]
 # clap_complete = { workspace = true, optional = true }
 # clap_mangen = { workspace = true, optional = true }
@@ -160,7 +158,7 @@ predicates.workspace = true
 rand.workspace = true
 regex.workspace = true
 tempfile.workspace = true
-# uutests.workspace = true
+uutests.workspace = true
 
 [build-dependencies]
 phf_codegen.workspace = true

Cargo.toml

@@ -19,65 +19,6 @@ const NUM_DIFF: u64 = 4;
 const MB: u64 = 1_000;
 const CHANGE_CHAR: u8 = b'#';
 
-#[cfg(not(feature = "feat_bench_not_cmp"))]
-mod diffutils_cmp {
-    use std::hint::black_box;
-
-    use divan::Bencher;
-
-    use crate::{FILE_SIZE_KILO_BYTES, binary, prepare::*};
-
-    #[divan::bench(args = FILE_SIZE_KILO_BYTES)]
-    fn cmp_compare_files_equal(bencher: Bencher, kb: u64) {
-        let (from, to) = get_context().get_test_files_equal(kb);
-        let cmd = format!("cmp {from} {to}");
-        let opts = str_to_options(&cmd).into_iter().peekable();
-        let params = cmp::parse_params(opts).unwrap();
-
-        bencher
-            // .with_inputs(|| prepare::cmp_params_identical_testfiles(lines))
-            .with_inputs(|| params.clone())
-            .bench_refs(|params| black_box(cmp::cmp_compare(&params).unwrap()));
-    }
-
-    // bench the actual compare; cmp exits on first difference
-    #[divan::bench(args = FILE_SIZE_KILO_BYTES)]
-    fn cmp_compare_files_different(bencher: Bencher, bytes: u64) {
-        let (from, to) = get_context().get_test_files_different(bytes);
-        let cmd = format!("cmp {from} {to} -s");
-        let opts = str_to_options(&cmd).into_iter().peekable();
-        let params = cmp::parse_params(opts).unwrap();
-
-        bencher
-            // .with_inputs(|| prepare::cmp_params_identical_testfiles(lines))
-            .with_inputs(|| params.clone())
-            .bench_refs(|params| black_box(cmp::cmp_compare(&params).unwrap()));
-    }
-
-    // bench original GNU cmp
-    #[divan::bench(args = FILE_SIZE_KILO_BYTES)]
-    fn cmd_cmp_gnu_equal(bencher: Bencher, bytes: u64) {
-        let (from, to) = get_context().get_test_files_equal(bytes);
-        let args_str = format!("{from} {to}");
-        bencher
-            // .with_inputs(|| prepare::cmp_params_identical_testfiles(lines))
-            .with_inputs(|| args_str.clone())
-            .bench_refs(|cmd_args| binary::bench_binary("cmp", cmd_args));
-    }
-
-    // bench the compiled release version
-    #[divan::bench(args = FILE_SIZE_KILO_BYTES)]
-    fn cmd_cmp_release_equal(bencher: Bencher, bytes: u64) {
-        let (from, to) = get_context().get_test_files_equal(bytes);
-        let args_str = format!("cmp {from} {to}");
-
-        bencher
-            // .with_inputs(|| prepare::cmp_params_identical_testfiles(lines))
-            .with_inputs(|| args_str.clone())
-            .bench_refs(|cmd_args| binary::bench_binary("target/release/diffutils", cmd_args));
-    }
-}
-
 #[cfg(not(feature = "feat_bench_not_diff"))]
 mod diffutils_diff {
     // use std::hint::black_box;
@@ -131,24 +72,6 @@ mod parser {
 
     use crate::prepare::str_to_options;
 
-    // bench the time it takes to parse the command line arguments
-    #[divan::bench]
-    fn cmp_parser(bencher: Bencher) {
-        let cmd = "cmd file_1.txt file_2.txt -bl n10M --ignore-initial=100KiB:1MiB";
-        let args = str_to_options(&cmd).into_iter().peekable();
-        bencher
-            .with_inputs(|| args.clone())
-            .bench_values(|data| black_box(cmp::parse_params(data)));
-    }
-
-    // // test the impact on the benchmark if not converting the cmd to Vec<OsString> (doubles for parse)
-    // #[divan::bench]
-    // fn cmp_parser_no_prepare() {
-    //     let cmd = "cmd file_1.txt file_2.txt -bl n10M --ignore-initial=100KiB:1MiB";
-    //     let args = str_to_options(&cmd).into_iter().peekable();
-    //     let _ = cmp::parse_params(args);
-    // }
-
     // bench the time it takes to parse the command line arguments
     #[divan::bench]
     fn diff_parser(bencher: Bencher) {

benches/bench-diffutils.rs

@@ -13,6 +13,7 @@ libfuzzer-sys = "0.4.7"
 diffutils = { path = "../" }
 uu_cmp = { path = "../src/uu/cmp" }
 uu_diff = { path = "../src/uu/diff" }
+uudiff = { path = "../src/uudiff" }
 
 # Prevent this from interfering with workspaces
 [workspace]

fuzz/Cargo.lock

@@ -2,10 +2,14 @@
 #[macro_use]
 extern crate libfuzzer_sys;
 
+use std::convert::TryFrom;
 use std::ffi::OsString;
 use std::fs::File;
 use std::io::Write;
 
+use uu_cmp::params_cmp::Params;
+use uudiff::utils::CompareOk;
+
 fn os(s: &str) -> OsString {
     OsString::from(s)
 }
@@ -28,15 +32,23 @@ fuzz_target!(|x: (Vec<u8>, Vec<u8>)| {
         .write_all(&to)
         .unwrap();
 
+    // let params =
+    //     uu_cmp::parse_params(args).unwrap_or_else(|e| panic!("Failed to parse params: {}", e));
+    let matches = uudiff::clap_localization::handle_clap_result_with_exit_code(
+        uu_cmp::params_cmp::uu_app(),
+        args,
+        2,
+    )
+    .unwrap_or_else(|e| panic!("Failed to parse params: {}", e));
     let params =
-        uu_cmp::parse_params(args).unwrap_or_else(|e| panic!("Failed to parse params: {}", e));
-    let ret = uu_cmp::cmp(&params);
-    if from == to && !matches!(ret, Ok(uu_cmp::Cmp::Equal)) {
+        Params::try_from(matches).unwrap_or_else(|e| panic!("Failed to parse params: {}", e));
+    let ret = uu_cmp::cmp_compare(&params);
+    if from == to && !matches!(ret, Ok(CompareOk::Equal)) {
         panic!(
             "target/fuzz.cmp.a and target/fuzz.cmp.b are equal, but cmp returned {:?}.",
             ret
         );
-    } else if from != to && !matches!(ret, Ok(uu_cmp::Cmp::Different)) {
+    } else if from != to && !matches!(ret, Ok(CompareOk::Different)) {
         panic!(
             "target/fuzz.cmp.a and target/fuzz.cmp.b are different, but cmp returned {:?}.",
             ret

fuzz/Cargo.toml

@@ -3,20 +3,30 @@
 extern crate libfuzzer_sys;
 
 use libfuzzer_sys::Corpus;
-use std::ffi::OsString;
+use std::{convert::TryFrom, ffi::OsString};
+use uu_cmp::params_cmp::Params;
 
 fn os(s: &str) -> OsString {
     OsString::from(s)
 }
 
-fuzz_target!(|x: Vec<OsString>| -> Corpus {
-    if x.len() > 6 {
+fuzz_target!(|args: Vec<OsString>| -> Corpus {
+    if args.len() > 6 {
         // Make sure we try to parse an option when we get longer args. x[0] will be
         // the executable name.
-        if ![os("-l"), os("-b"), os("-s"), os("-n"), os("-i")].contains(&x[1]) {
+        if ![os("-l"), os("-b"), os("-s"), os("-n"), os("-i")].contains(&args[1]) {
             return Corpus::Reject;
         }
     }
-    let _ = uu_cmp::parse_params(x.into_iter().peekable());
+    // not sure what this does, mostly empty args
+    // dbg!(&args);
+    // let _ = uu_cmp::parse_params(x.into_iter().peekable());
+    if let Ok(matches) = uudiff::clap_localization::handle_clap_result_with_exit_code(
+        uu_cmp::params_cmp::uu_app(),
+        args,
+        2,
+    ) {
+        let _params = Params::try_from(matches);
+    }
     Corpus::Keep
 });

fuzz/fuzz_targets/fuzz_cmp.rs

@@ -25,14 +25,14 @@ path = "src/main.rs"
 path = "src/cmp.rs"
 
 [features]
-# default = ["feat_run_binary_bench" ]
+default = ["feat_run_binary_bench" ]
 # The cmd benchmarks start the binaries and take a lot of runtime on the github checks.
 # Only run them locally.
 feat_run_binary_bench = []
 
 
 [dependencies]
-# bytecount.workspace = true
+bytecount.workspace = true
 clap.workspace = true
 diff_crate.workspace = true
 fluent.workspace = true
@@ -55,3 +55,6 @@ tempfile.workspace = true
 # for flamegraph
 # debug = true
 
+[[bench]]
+name = "cmp_bench"
+harness = false