From c4e034a987867b3114fd4aca703b0bc05c50b17d Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sun, 12 Jul 2026 09:02:21 +0200 Subject: [PATCH] feat(cli): expand file lists, output dirs, and --list in Rust frontend The Rust CLI frontend rejected -r, --filelist, --output-dir-flat, --output-dir-mirror, -l and --list, which tests/playTests.sh exercises heavily (32 recursive runs, 20 file-list runs, 32 output-dir runs, 29 list runs). These are pure parser/dispatch features: the actual directory traversal, file-of-names reading, and frame inspection remain in C. Mirror the pre-migration C zstdcli.c control flow: - --filelist names are collected during parsing and expanded after the symbolic-link filter through UTIL_createFileNamesTable_fromFileName, so list contents are never symlink-filtered and an unreadable, irregular, empty, or over-long list aborts with "error reading NAME" and exit code 1, exactly like the C loop. - -r expands the collected inputs through UTIL_createExpandedFNT, following links only under --force (the C followLinks flag). The input-name count is saved before expansion: when expansion leaves no files but names were given (only empty directories), the run prints "please provide correct input file(s) or non-empty directories -- ignored" and exits 0 instead of falling back to stdin. -r parses only on unix/windows targets, mirroring UTIL_HAS_CREATEFILELIST. - --output-dir-flat / --output-dir-mirror pass through to the existing output-dir arguments of FIO_{,de}compressMultipleFilenames (test mode included). Both use exact NEXT_FIELD semantics via a new next_field helper: an attached empty value (--output-dir-flat=) stays empty and is rejected with the C "output dir cannot be empty string" message instead of consuming the next argument. --output-dir-mirror parses only on POSIX targets, mirroring UTIL_HAS_MIRRORFILELIST. - -l/--list dispatches to FIO_listMultipleFiles after expansion and before the console checks, so "No files given", the stdin refusal, display-level gating, and per-file exit codes are the C code paths. Builds without the decompression feature report "file information is not supported" like ZSTD_NODECOMPRESS builds. - g_utilDisplayLevel is published before any expansion so traversal warnings honor -q/-v, and benchmark dispatch moved after expansion so -rqi0b1e2 style aggregates benchmark expanded directories like C. FIO_setNbFilesTotal keeps receiving the post-expansion count because apply_preferences reads cli.inputs after all expansion steps. The FileNamesTable struct is mirrored in Rust (pointer/pointer/size/ size); tables returned by the UTIL helpers are drained into owned CStrings and freed immediately, so no C buffer lifetime escapes. Test Plan: - cd rust/cli && cargo clippy --all-targets -- -D warnings && cargo test --all-targets, plus the compression-only and decompression-only feature matrices; new parser tests cover -r aggregation, both --filelist syntaxes, option-as-argument rejection, empty output-dir rejection, --list forms, and --list=x rejection. - make -C programs zstd, then diffed behavior against a pre-migration C binary (f8745da6): -r on a nested tree produces identical file sets and identical -o output; --filelist with one and two lists, missing list ("zstd: error reading X", exit 1), and binary-garbage list (exit 1); --output-dir-flat compress + decompress round-trip; --output-dir-mirror round-trip and '..' rejection; -l/-lv identical stdout, identical exit codes for no-files, non-zst, and stdin cases; -r on an empty directory exits 0 with the same message; -c -r with stdin fallback produces identical frames. --- rust/src/zstd_cli.rs | 355 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 329 insertions(+), 26 deletions(-) diff --git a/rust/src/zstd_cli.rs b/rust/src/zstd_cli.rs index 8504051ac..a1cb45104 100644 --- a/rust/src/zstd_cli.rs +++ b/rust/src/zstd_cli.rs @@ -16,9 +16,13 @@ //! (`benchzstd.c`) remain C behind the preprocessor-gated bridge, so builds //! with `ZSTD_NOBENCH` never reference benchmark symbols. //! +//! Recursive expansion (`-r`), `--filelist`, the output-directory modes, and +//! `--list` reuse the C `FileNamesTable` helpers from `programs/util.c` and +//! `FIO_listMultipleFiles`, so directory traversal and frame inspection stay +//! byte-identical with the C CLI. +//! //! Remaining C-only CLI boundaries are called out in `unsupported()` below: -//! dictionary training, recursive/file-list expansion, tracing, -//! alternate-format selection, and the advanced directory modes. +//! dictionary training, tracing, and alternate-format selection. use std::env; use std::ffi::{CStr, CString, OsStr, OsString}; @@ -79,6 +83,16 @@ struct ZSTD_compressionParameters { strategy: c_int, } +/// Mirror of the `FileNamesTable` in `programs/util.h`; the tables returned +/// by the `UTIL_*FNT` helpers are only read and released here, never resized. +#[repr(C)] +struct FileNamesTable { + fileNames: *mut *const c_char, + buf: *mut c_char, + tableSize: usize, + tableCapacity: usize, +} + unsafe extern "C" { fn ZSTD_versionString() -> *const c_char; fn ZSTD_rust_cli_expected_version() -> *const c_char; @@ -91,6 +105,15 @@ unsafe extern "C" { fn UTIL_countPhysicalCores() -> c_int; #[cfg(feature = "compression")] fn UTIL_countLogicalCores() -> c_int; + fn UTIL_createFileNamesTable_fromFileName( + input_file_name: *const c_char, + ) -> *mut FileNamesTable; + fn UTIL_createExpandedFNT( + file_names: *const *const c_char, + nb_file_names: usize, + follow_links: c_int, + ) -> *mut FileNamesTable; + fn UTIL_freeFileNamesTable(table: *mut FileNamesTable); fn FIO_createPreferences() -> *mut FIO_prefs_t; fn FIO_freePreferences(prefs: *mut FIO_prefs_t); @@ -180,6 +203,12 @@ unsafe extern "C" { output: *const c_char, dict: *const c_char, ) -> c_int; + #[cfg(feature = "decompression")] + fn FIO_listMultipleFiles( + nb_files: c_uint, + file_names: *const *const c_char, + display_level: c_int, + ) -> c_int; /// Benchmark bridge implemented by the `programs/zstdcli.c` shim, which /// owns the `ZSTD_NOBENCH` preprocessor decision. Returns the benchmark @@ -204,6 +233,7 @@ enum Operation { Decompress, Test, Bench, + List, } #[derive(Debug)] @@ -256,6 +286,10 @@ struct Cli { compression_params: ZSTD_compressionParameters, bench_end_level: Option, bench_nb_seconds: Option, + recursive: bool, + file_lists: Vec, + output_dir_flat: Option, + output_dir_mirror: Option, unsupported_program: Option, } @@ -303,6 +337,10 @@ impl Cli { compression_params: ZSTD_compressionParameters::default(), bench_end_level: None, bench_nb_seconds: None, + recursive: false, + file_lists: Vec::new(), + output_dir_flat: None, + output_dir_mirror: None, unsupported_program: None, }; @@ -432,6 +470,28 @@ fn usage(advanced: bool) { out, " --adapt[=min=#,max=#], --rsyncable, --[no-]row-match-finder" ); + let _ = writeln!(out, "\nFile selection and placement:"); + let _ = writeln!( + out, + " -r Operate recursively on directories" + ); + let _ = writeln!( + out, + " --filelist LIST Read a list of input files from LIST" + ); + let _ = writeln!( + out, + " --output-dir-flat DIR Store processed files in DIR" + ); + #[cfg(unix)] + let _ = writeln!( + out, + " --output-dir-mirror DIR Store processed files in DIR, mirroring the input tree" + ); + let _ = writeln!( + out, + " -l, --list Print information about .zst file(s)" + ); let _ = writeln!(out, "\nBenchmark options:"); let _ = writeln!( out, @@ -447,9 +507,8 @@ fn usage(advanced: bool) { ); let _ = writeln!( out, - "\nNot yet migrated: dictionary training, recursive/file-list expansion," + "\nNot yet migrated: dictionary training, trace, and alternate formats." ); - let _ = writeln!(out, "trace, alternate formats, and output-directory modes."); } } @@ -561,6 +620,32 @@ fn next_os_value(args: &[OsString], index: &mut usize, option: &str) -> Result, + args: &[OsString], + index: &mut usize, + option: &str, +) -> Result { + if let Some(value) = attached { + return Ok(value.to_owned()); + } + *index += 1; + let Some(value) = args.get(*index) else { + return Err(format!("missing argument for {option}")); + }; + let rendered = value.to_string_lossy().into_owned(); + if rendered.starts_with('-') { + return Err(format!( + "{option} cannot be separated from its argument by another option" + )); + } + Ok(rendered) +} + #[cfg(unix)] fn short_attached_value(value: &OsStr, start: usize) -> Option { let bytes = &value.as_bytes()[start..]; @@ -689,6 +774,7 @@ fn parse_long_option( | "--no-compress-literals" | "--exclude-compressed" | "--no-name" + | "--list" ) { return Err(format!("{name} does not take an argument")); @@ -894,17 +980,47 @@ fn parse_long_option( parse_compression_parameters(&value, cli)?; Ok(None) } - "--list" - | "--train" + "--list" => { + cli.operation = Operation::List; + Ok(None) + } + "--filelist" => { + let value = next_field(attached, args, index, name)?; + cli.file_lists.push(cstring(&value)?); + Ok(None) + } + "--output-dir-flat" => { + let value = next_field(attached, args, index, name)?; + if value.is_empty() { + return Err( + "output dir cannot be empty string (did you mean to pass '.' instead?)" + .to_owned(), + ); + } + cli.output_dir_flat = Some(cstring(&value)?); + Ok(None) + } + #[cfg(unix)] + "--output-dir-mirror" => { + /* Parsed only where the C CLI defines UTIL_HAS_MIRRORFILELIST + * (POSIX); elsewhere the option falls through as unknown. */ + let value = next_field(attached, args, index, name)?; + if value.is_empty() { + return Err( + "output dir cannot be empty string (did you mean to pass '.' instead?)" + .to_owned(), + ); + } + cli.output_dir_mirror = Some(cstring(&value)?); + Ok(None) + } + "--train" | "--train-cover" | "--train-fastcover" | "--train-legacy" | "--max" | "--maxdict" | "--dictID" - | "--filelist" - | "--output-dir-flat" - | "--output-dir-mirror" | "--patch-from" | "--trace" | "--format" @@ -947,6 +1063,9 @@ fn parse_short_options( 'z' => cli.operation = Operation::Compress, 't' => cli.operation = Operation::Test, 'b' => cli.operation = Operation::Bench, + 'l' => cli.operation = Operation::List, + #[cfg(any(unix, windows))] + 'r' => cli.recursive = true, 'e' | 'i' => { // Benchmark range end (-e#) and duration (-i#): like the C // parser, digits attach directly and default to 0. @@ -1008,7 +1127,7 @@ fn parse_short_options( } break; } - 'l' | 'p' | 'P' | 'r' | 's' | 'S' => { + 'p' | 'P' | 's' | 'S' => { unsupported(&format!("-{option}"))?; } _ => return Err(format!("unknown option -{option}")), @@ -1248,8 +1367,8 @@ unsafe fn run_compress( ctx, prefs, inputs.as_ptr(), - ptr::null(), - ptr::null(), + output_dir_mirror(cli), + output_dir_flat(cli), output, ZSTD_SUFFIX.as_ptr().cast(), dictionary, @@ -1260,16 +1379,28 @@ unsafe fn run_compress( } } +fn output_dir_flat(cli: &Cli) -> *const c_char { + cli.output_dir_flat + .as_ref() + .map_or(ptr::null(), |value| value.as_ptr()) +} + +fn output_dir_mirror(cli: &Cli) -> *const c_char { + cli.output_dir_mirror + .as_ref() + .map_or(ptr::null(), |value| value.as_ptr()) +} + #[cfg(feature = "decompression")] unsafe fn run_decompress( - operation: Operation, + cli: &Cli, ctx: *mut FIO_ctx_t, prefs: *mut FIO_prefs_t, inputs: &[*const c_char], output: *const c_char, dictionary: *const c_char, ) -> c_int { - match operation { + match cli.operation { Operation::Test => { let null_output = cstring(NULL_MARK).expect("static null marker"); unsafe { @@ -1278,8 +1409,8 @@ unsafe fn run_decompress( ctx, prefs, inputs.as_ptr(), - ptr::null(), - ptr::null(), + output_dir_mirror(cli), + output_dir_flat(cli), null_output.as_ptr(), dictionary, ) @@ -1293,18 +1424,74 @@ unsafe fn run_decompress( ctx, prefs, inputs.as_ptr(), - ptr::null(), - ptr::null(), + output_dir_mirror(cli), + output_dir_flat(cli), output, dictionary, ) }, - Operation::Compress | Operation::Bench => { - unreachable!("compression and benchmark are dispatched separately") + Operation::Compress | Operation::Bench | Operation::List => { + unreachable!("compression, benchmark, and list are dispatched separately") } } } +/// Copies every name out of a C `FileNamesTable` and releases the table. +unsafe fn drain_file_names_table(table: *mut FileNamesTable, into: &mut Vec) { + unsafe { + for entry in 0..(*table).tableSize { + let name = *(*table).fileNames.add(entry); + if !name.is_null() { + into.push(CStr::from_ptr(name).to_owned()); + } + } + UTIL_freeFileNamesTable(table); + } +} + +/// Appends the names read from every `--filelist` argument to the inputs. +/// As in the C CLI, a list that cannot be read (missing, irregular, empty, +/// or with over-long lines) aborts the whole run. +fn expand_file_lists(cli: &mut Cli) -> Result<(), String> { + let file_lists = std::mem::take(&mut cli.file_lists); + for list in &file_lists { + let table = unsafe { UTIL_createFileNamesTable_fromFileName(list.as_ptr()) }; + if table.is_null() { + return Err(format!("error reading {}", list.to_string_lossy())); + } + unsafe { drain_file_names_table(table, &mut cli.inputs) }; + } + Ok(()) +} + +/// Replaces the inputs with their recursive directory expansion. Symbolic +/// links are followed only under --force, matching the C `followLinks` flag. +fn expand_recursive_inputs(cli: &mut Cli) -> Result<(), String> { + let pointers: Vec<*const c_char> = cli.inputs.iter().map(|value| value.as_ptr()).collect(); + let names = if pointers.is_empty() { + ptr::null() + } else { + pointers.as_ptr() + }; + let table = unsafe { UTIL_createExpandedFNT(names, pointers.len(), c_int::from(cli.force)) }; + if table.is_null() { + /* The C CLI treats this allocation failure as fatal (CONTROL). */ + return Err("recursive directory expansion failed".to_owned()); + } + let mut expanded = Vec::new(); + unsafe { drain_file_names_table(table, &mut expanded) }; + cli.inputs = expanded; + Ok(()) +} + +/// Prints frame information for every input through `FIO_listMultipleFiles`, +/// which owns the display-level gating, the stdin refusal, and the totals. +#[cfg(feature = "decompression")] +fn run_list(cli: &Cli) -> i32 { + let inputs: Vec<*const c_char> = cli.inputs.iter().map(|value| value.as_ptr()).collect(); + unsafe { FIO_listMultipleFiles(inputs.len() as c_uint, inputs.as_ptr(), cli.display_level) } +} + /// Runs benchmark mode through the C bridge. Level clamping against /// `ZSTD_maxCLevel()` happens on the C side, where the symbol is always /// available when benchmarking is compiled in. No input file means a @@ -1342,19 +1529,45 @@ fn run_cli(mut cli: Cli) -> Result { "{program_name} compatibility mode is not yet implemented by the Rust CLI frontend" )); } - if cli.operation == Operation::Bench { - return run_bench(&cli); + unsafe { + /* The C CLI publishes the display level to util.c before any table + * expansion, so traversal warnings obey -q/-v. */ + g_utilDisplayLevel = cli.display_level; } let explicit_input_count = cli.inputs.len(); filter_symlink_inputs(&mut cli); if explicit_input_count > 0 && cli.inputs.is_empty() { return Ok(1); } + expand_file_lists(&mut cli)?; + /* Names known before recursive expansion: an input set that named only + * (possibly empty) directories must not fall back to stdin below. */ + let input_name_count = cli.inputs.len(); + if cli.recursive { + expand_recursive_inputs(&mut cli)?; + } + if cli.operation == Operation::List { + #[cfg(feature = "decompression")] + return Ok(run_list(&cli)); + #[cfg(not(feature = "decompression"))] + return Err("file information is not supported".to_owned()); + } + if cli.operation == Operation::Bench { + return run_bench(&cli); + } if cli.operation == Operation::Test { cli.output = Some(cstring(NULL_MARK)?); cli.remove_source = false; } if cli.inputs.is_empty() { + if input_name_count > 0 { + if cli.display_level >= 1 { + eprintln!( + "please provide correct input file(s) or non-empty directories -- ignored" + ); + } + return Ok(0); + } cli.inputs.push(cstring(STDIN_MARK)?); if cli.output.is_none() { cli.output = Some(cstring(STDOUT_MARK)?); @@ -1444,14 +1657,14 @@ fn run_cli(mut cli: Cli) -> Result { Operation::Decompress | Operation::Test => { #[cfg(feature = "decompression")] { - unsafe { - run_decompress(cli.operation, ctx, prefs, &inputs, output, dictionary) - } + unsafe { run_decompress(&cli, ctx, prefs, &inputs, output, dictionary) } } #[cfg(not(feature = "decompression"))] unreachable!("unsupported decompression was rejected above") } - Operation::Bench => unreachable!("benchmark mode was dispatched earlier"), + Operation::Bench | Operation::List => { + unreachable!("benchmark and list modes were dispatched earlier") + } } }; @@ -1687,6 +1900,96 @@ mod tests { assert!(error.contains("not yet implemented")); } + #[test] + fn recursive_flag_can_be_aggregated_with_other_shorts() { + let cli = parse(&["zstd", "-rq", "dir"]); + + assert!(cli.recursive); + assert_eq!(cli.display_level, 1); + } + + #[test] + fn list_mode_is_selected_by_short_and_long_flags() { + let short = parse(&["zstd", "-l", "file.zst"]); + let long = parse(&["zstd", "--list", "file.zst"]); + + assert_eq!(short.operation, Operation::List); + assert_eq!(long.operation, Operation::List); + } + + #[test] + fn list_rejects_attached_values() { + let error = parse_args(vec![OsString::from("zstd"), OsString::from("--list=x")]) + .expect_err("an attached value must not activate --list"); + + assert!(error.contains("does not take an argument")); + } + + #[test] + fn filelist_accumulates_both_syntaxes() { + let cli = parse(&["zstd", "--filelist=one.txt", "--filelist", "two.txt", "in"]); + + assert_eq!( + cli.file_lists + .iter() + .map(|list| list.as_bytes()) + .collect::>(), + vec![&b"one.txt"[..], &b"two.txt"[..]] + ); + } + + #[test] + fn filelist_argument_must_not_be_another_option() { + let error = parse_args(vec![ + OsString::from("zstd"), + OsString::from("--filelist"), + OsString::from("-q"), + ]) + .expect_err("an option must not be consumed as the list name"); + + assert!(error.contains("cannot be separated from its argument")); + } + + #[test] + fn output_dir_flat_accepts_attached_and_separate_values() { + let attached = parse(&["zstd", "--output-dir-flat=out", "in"]); + let separate = parse(&["zstd", "--output-dir-flat", "out", "in"]); + + assert_eq!( + attached.output_dir_flat.as_deref().map(CStr::to_bytes), + Some(&b"out"[..]) + ); + assert_eq!( + separate.output_dir_flat.as_deref().map(CStr::to_bytes), + Some(&b"out"[..]) + ); + } + + #[test] + fn output_dir_flat_rejects_an_empty_name() { + /* Mirrors the C NEXT_FIELD semantics: `--output-dir-flat=` keeps the + * empty attached value instead of consuming the next argument. */ + let error = parse_args(vec![ + OsString::from("zstd"), + OsString::from("--output-dir-flat="), + OsString::from("out"), + ]) + .expect_err("an empty output dir must be rejected"); + + assert!(error.contains("output dir cannot be empty string")); + } + + #[cfg(unix)] + #[test] + fn output_dir_mirror_is_parsed_on_posix_targets() { + let cli = parse(&["zstd", "--output-dir-mirror", "out", "in"]); + + assert_eq!( + cli.output_dir_mirror.as_deref().map(CStr::to_bytes), + Some(&b"out"[..]) + ); + } + #[test] fn bench_mode_parses_level_duration_and_defaults() { let cli = parse(&["zstd", "-b1", "-i0", "input"]);