diff --git a/Cargo.lock b/Cargo.lock index d7d78420af2..416d6bbcac2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -178,9 +178,9 @@ dependencies = [ [[package]] name = "bytecount" -version = "0.6.2" +version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72feb31ffc86498dacdbd0fcebb56138e7177a8cc5cea4516031d15ae85a742e" +checksum = "2c676a478f63e9fa2dd5368a42f28bba0d6c560b775f38583c8bbaa7fcd67c9c" [[package]] name = "byteorder" diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 5bd478fd9da..ca42b56ed48 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -87,6 +87,9 @@ struct Options { /// types will *not* be listed. exclude: Option>, + /// Whether to sync before operating. + sync: bool, + /// Whether to show a final row comprising the totals for each column. show_total: bool, @@ -104,6 +107,7 @@ impl Default for Options { header_mode: Default::default(), include: Default::default(), exclude: Default::default(), + sync: Default::default(), show_total: Default::default(), columns: vec![ Column::Source, @@ -178,6 +182,7 @@ impl Options { Ok(Self { show_local_fs: matches.is_present(OPT_LOCAL), show_all_fs: matches.is_present(OPT_ALL), + sync: matches.is_present(OPT_SYNC), block_size: read_block_size(matches).map_err(|e| match e { ParseSizeError::InvalidSuffix(s) => OptionsError::InvalidSuffix(s), ParseSizeError::SizeTooBig(_) => OptionsError::BlockSizeTooLarge( @@ -325,9 +330,21 @@ fn filter_mount_list(vmi: Vec, opt: &Options) -> Vec { /// Get all currently mounted filesystems. /// -/// `opt` excludes certain filesystems from consideration; see +/// `opt` excludes certain filesystems from consideration and allows for the synchronization of filesystems before running; see /// [`Options`] for more information. + fn get_all_filesystems(opt: &Options) -> Vec { + // Run a sync call before any operation if so instructed. + if opt.sync { + #[cfg(not(windows))] + unsafe { + #[cfg(not(target_os = "android"))] + uucore::libc::sync(); + #[cfg(target_os = "android")] + uucore::libc::syscall(uucore::libc::SYS_sync); + } + } + // The list of all mounted filesystems. // // Filesystems excluded by the command-line options are @@ -559,7 +576,7 @@ pub fn uu_app<'a>() -> Command<'a> { Arg::new(OPT_SYNC) .long("sync") .overrides_with_all(&[OPT_NO_SYNC, OPT_SYNC]) - .help("invoke sync before getting usage info"), + .help("invoke sync before getting usage info (non-windows only)"), ) .arg( Arg::new(OPT_TYPE) diff --git a/src/uu/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index 6bc96adbf38..4062b0b7a3a 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.rs @@ -135,13 +135,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let result; if files.is_empty() { result = parse(INTERNAL_DB.lines(), &out_format, ""); + } else if files.len() > 1 { + return Err(UUsageError::new( + 1, + format!("extra operand {}", files[1].quote()), + )); + } else if files[0].eq("-") { + let fin = BufReader::new(std::io::stdin()); + result = parse(fin.lines().filter_map(Result::ok), &out_format, files[0]); } else { - if files.len() > 1 { - return Err(UUsageError::new( - 1, - format!("extra operand {}", files[1].quote()), - )); - } match File::open(files[0]) { Ok(f) => { let fin = BufReader::new(f); diff --git a/src/uu/expand/src/expand.rs b/src/uu/expand/src/expand.rs index 1700920d381..d81d2d313d0 100644 --- a/src/uu/expand/src/expand.rs +++ b/src/uu/expand/src/expand.rs @@ -13,12 +13,15 @@ extern crate uucore; use clap::{crate_version, Arg, ArgMatches, Command}; +use std::error::Error; +use std::fmt; use std::fs::File; use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; +use std::num::IntErrorKind; use std::str::from_utf8; use unicode_width::UnicodeWidthChar; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult}; +use uucore::error::{FromIo, UError, UResult}; use uucore::format_usage; static ABOUT: &str = "Convert tabs in each FILE to spaces, writing to standard output. @@ -57,6 +60,38 @@ fn is_space_or_comma(c: char) -> bool { c == ' ' || c == ',' } +/// Errors that can occur when parsing a `--tabs` argument. +#[derive(Debug)] +enum ParseError { + InvalidCharacter(String), + SpecifierNotAtStartOfNumber(String, String), + TabSizeCannotBeZero, + TabSizeTooLarge(String), + TabSizesMustBeAscending, +} + +impl Error for ParseError {} +impl UError for ParseError {} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::InvalidCharacter(s) => { + write!(f, "tab size contains invalid character(s): {}", s.quote()) + } + Self::SpecifierNotAtStartOfNumber(specifier, s) => write!( + f, + "{} specifier not at start of number: {}", + specifier.quote(), + s.quote(), + ), + Self::TabSizeCannotBeZero => write!(f, "tab size cannot be 0"), + Self::TabSizeTooLarge(s) => write!(f, "tab stop is too large {}", s.quote()), + Self::TabSizesMustBeAscending => write!(f, "tab sizes must be ascending"), + } + } +} + /// Parse a list of tabstops from a `--tabs` argument. /// /// This function returns both the vector of numbers appearing in the @@ -65,14 +100,14 @@ fn is_space_or_comma(c: char) -> bool { /// in the list. This mode defines the strategy to use for computing the /// number of spaces to use for columns beyond the end of the tab stop /// list specified here. -fn tabstops_parse(s: &str) -> (RemainingMode, Vec) { +fn tabstops_parse(s: &str) -> Result<(RemainingMode, Vec), ParseError> { // Leading commas and spaces are ignored. let s = s.trim_start_matches(is_space_or_comma); // If there were only commas and spaces in the string, just use the // default tabstops. if s.is_empty() { - return (RemainingMode::None, vec![DEFAULT_TABSTOP]); + return Ok((RemainingMode::None, vec![DEFAULT_TABSTOP])); } let mut nums = vec![]; @@ -89,23 +124,41 @@ fn tabstops_parse(s: &str) -> (RemainingMode, Vec) { } _ => { // Parse a number from the byte sequence. - let num = from_utf8(&bytes[i..]).unwrap().parse::().unwrap(); + let s = from_utf8(&bytes[i..]).unwrap(); + match s.parse::() { + Ok(num) => { + // Tab size must be positive. + if num == 0 { + return Err(ParseError::TabSizeCannotBeZero); + } - // Tab size must be positive. - if num == 0 { - crash!(1, "tab size cannot be 0"); - } + // Tab sizes must be ascending. + if let Some(last_stop) = nums.last() { + if *last_stop >= num { + return Err(ParseError::TabSizesMustBeAscending); + } + } - // Tab sizes must be ascending. - if let Some(last_stop) = nums.last() { - if *last_stop >= num { - crash!(1, "tab sizes must be ascending"); + // Append this tab stop to the list of all tabstops. + nums.push(num); + break; } - } + Err(e) => { + if *e.kind() == IntErrorKind::PosOverflow { + return Err(ParseError::TabSizeTooLarge(s.to_string())); + } - // Append this tab stop to the list of all tabstops. - nums.push(num); - break; + let s = s.trim_start_matches(char::is_numeric); + if s.starts_with('/') || s.starts_with('+') { + return Err(ParseError::SpecifierNotAtStartOfNumber( + s[0..1].to_string(), + s.to_string(), + )); + } else { + return Err(ParseError::InvalidCharacter(s.to_string())); + } + } + } } } } @@ -115,7 +168,7 @@ fn tabstops_parse(s: &str) -> (RemainingMode, Vec) { if nums.is_empty() { nums = vec![DEFAULT_TABSTOP]; } - (remaining_mode, nums) + Ok((remaining_mode, nums)) } struct Options { @@ -131,9 +184,9 @@ struct Options { } impl Options { - fn new(matches: &ArgMatches) -> Self { + fn new(matches: &ArgMatches) -> Result { let (remaining_mode, tabstops) = match matches.values_of(options::TABS) { - Some(s) => tabstops_parse(&s.collect::>().join(",")), + Some(s) => tabstops_parse(&s.collect::>().join(","))?, None => (RemainingMode::None, vec![DEFAULT_TABSTOP]), }; @@ -158,14 +211,14 @@ impl Options { None => vec!["-".to_owned()], }; - Self { + Ok(Self { files, tabstops, tspaces, iflag, uflag, remaining_mode, - } + }) } } @@ -173,7 +226,7 @@ impl Options { pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().get_matches_from(args); - expand(&Options::new(&matches)).map_err_context(|| "failed to write output".to_string()) + expand(&Options::new(&matches)?).map_err_context(|| "failed to write output".to_string()) } pub fn uu_app<'a>() -> Command<'a> { diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 52a5a114526..2ebd2fb973d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2505,9 +2505,12 @@ fn display_file_name( let mut width = name.width(); if let Some(ls_colors) = &config.color { - if let Ok(metadata) = path.p_buf.symlink_metadata() { - name = color_name(ls_colors, &path.p_buf, &name, &metadata, config); - } + name = color_name( + name, + &path.p_buf, + path.p_buf.symlink_metadata().ok().as_ref(), + ls_colors, + ); } if config.format != Format::Long && !more_info.is_empty() { @@ -2588,11 +2591,10 @@ fn display_file_name( }; name.push_str(&color_name( - ls_colors, + escape_name(target.as_os_str(), &config.quoting_style), &target_data.p_buf, - &target.to_string_lossy(), - &target_metadata, - config, + Some(&target_metadata), + ls_colors, )); } } else { @@ -2623,19 +2625,12 @@ fn display_file_name( } } -fn color_name( - ls_colors: &LsColors, - path: &Path, - name: &str, - md: &Metadata, - config: &Config, -) -> String { - match ls_colors.style_for_path_with_metadata(path, Some(md)) { +fn color_name(name: String, path: &Path, md: Option<&Metadata>, ls_colors: &LsColors) -> String { + match ls_colors.style_for_path_with_metadata(path, md) { Some(style) => { - let p = escape_name(OsStr::new(&name), &config.quoting_style); - return style.to_ansi_term_style().paint(p).to_string(); + return style.to_ansi_term_style().paint(name).to_string(); } - None => escape_name(OsStr::new(&name), &config.quoting_style), + None => name, } } diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index dd1f57b8b44..78e36874ea6 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -6,7 +6,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (paths) GPGHome +// spell-checker:ignore (paths) GPGHome findxs use clap::{crate_version, Arg, ArgMatches, Command}; use uucore::display::{println_verbatim, Quotable}; @@ -205,12 +205,29 @@ struct Params { suffix: String, } +/// Find the start and end indices of the last contiguous block of Xs. +/// +/// If no contiguous block of at least three Xs could be found, this +/// function returns `None`. +/// +/// # Examples +/// +/// ```rust,ignore +/// assert_eq!(find_last_contiguous_block_of_xs("XXX_XXX"), Some((4, 7))); +/// assert_eq!(find_last_contiguous_block_of_xs("aXbXcX"), None); +/// ``` +fn find_last_contiguous_block_of_xs(s: &str) -> Option<(usize, usize)> { + let j = s.rfind("XXX")? + 3; + let i = s[..j].rfind(|c| c != 'X').map_or(0, |i| i + 1); + Some((i, j)) +} + impl Params { fn from(options: Options) -> Result { // Get the start and end indices of the randomized part of the template. // // For example, if the template is "abcXXXXyz", then `i` is 3 and `j` is 7. - let i = match options.template.find("XXX") { + let (i, j) = match find_last_contiguous_block_of_xs(&options.template) { None => { let s = match options.suffix { None => options.template, @@ -218,9 +235,8 @@ impl Params { }; return Err(MkTempError::TooFewXs(s)); } - Some(i) => i, + Some(indices) => indices, }; - let j = options.template.rfind("XXX").unwrap() + 3; // Combine the directory given as an option and the prefix of the template. // @@ -450,3 +466,21 @@ fn exec(dir: &str, prefix: &str, rand: usize, suffix: &str, make_dir: bool) -> U println_verbatim(path).map_err_context(|| "failed to print directory name".to_owned()) } + +#[cfg(test)] +mod tests { + use crate::find_last_contiguous_block_of_xs as findxs; + + #[test] + fn test_find_last_contiguous_block_of_xs() { + assert_eq!(findxs("XXX"), Some((0, 3))); + assert_eq!(findxs("XXX_XXX"), Some((4, 7))); + assert_eq!(findxs("XXX_XXX_XXX"), Some((8, 11))); + assert_eq!(findxs("aaXXXbb"), Some((2, 5))); + assert_eq!(findxs(""), None); + assert_eq!(findxs("X"), None); + assert_eq!(findxs("XX"), None); + assert_eq!(findxs("aXbXcX"), None); + assert_eq!(findxs("aXXbXXcXX"), None); + } +} diff --git a/src/uu/wc/Cargo.toml b/src/uu/wc/Cargo.toml index d224329b487..2b0417239d5 100644 --- a/src/uu/wc/Cargo.toml +++ b/src/uu/wc/Cargo.toml @@ -17,7 +17,7 @@ path = "src/wc.rs" [dependencies] clap = { version = "3.1", features = ["wrap_help", "cargo"] } uucore = { version=">=0.0.11", package="uucore", path="../../uucore", features=["pipes"] } -bytecount = "0.6.2" +bytecount = "0.6.3" utf-8 = "0.7.6" unicode-width = "0.1.8" diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 4206c4c827a..13cfdcef4c8 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -28,6 +28,11 @@ fn test_df_compatible_si() { new_ucmd!().arg("-aH").succeeds(); } +#[test] +fn test_df_compatible_sync() { + new_ucmd!().arg("--sync").succeeds(); +} + #[test] fn test_df_arguments_override_themselves() { new_ucmd!().args(&["--help", "--help"]).succeeds(); diff --git a/tests/by-util/test_dircolors.rs b/tests/by-util/test_dircolors.rs index a4ad0df32bd..f34f5573bb9 100644 --- a/tests/by-util/test_dircolors.rs +++ b/tests/by-util/test_dircolors.rs @@ -131,6 +131,25 @@ fn test_exclusive_option() { .stderr_contains("mutually exclusive"); } +#[test] +fn test_stdin() { + new_ucmd!() + .pipe_in("owt 40;33\n") + .args(&["-b", "-"]) + .succeeds() + .stdout_is("LS_COLORS='tw=40;33:';\nexport LS_COLORS\n") + .no_stderr(); +} + +#[test] +fn test_extra_operand() { + new_ucmd!() + .args(&["-c", "file1", "file2"]) + .fails() + .stderr_contains("dircolors: extra operand 'file2'\n") + .no_stdout(); +} + fn test_helper(file_name: &str, term: &str) { new_ucmd!() .env("TERM", term) diff --git a/tests/by-util/test_expand.rs b/tests/by-util/test_expand.rs index 4566e4176b6..289743ce8dd 100644 --- a/tests/by-util/test_expand.rs +++ b/tests/by-util/test_expand.rs @@ -1,4 +1,5 @@ use crate::common::util::*; +use uucore::display::Quotable; // spell-checker:ignore (ToDO) taaaa tbbbb tcccc #[test] @@ -179,6 +180,14 @@ fn test_tabs_must_be_ascending() { .stderr_contains("tab sizes must be ascending"); } +#[test] +fn test_tabs_cannot_be_zero() { + new_ucmd!() + .arg("--tabs=0") + .fails() + .stderr_contains("tab size cannot be 0"); +} + #[test] fn test_tabs_keep_last_trailing_specifier() { // If there are multiple trailing specifiers, use only the last one @@ -200,3 +209,39 @@ fn test_tabs_comma_separated_no_numbers() { .succeeds() .stdout_is(" a b c"); } + +#[test] +fn test_tabs_with_specifier_not_at_start() { + fn run_cmd(arg: &str, expected_prefix: &str, expected_suffix: &str) { + let expected_msg = format!( + "{} specifier not at start of number: {}", + expected_prefix.quote(), + expected_suffix.quote() + ); + new_ucmd!().arg(arg).fails().stderr_contains(expected_msg); + } + run_cmd("--tabs=1/", "/", "/"); + run_cmd("--tabs=1/2", "/", "/2"); + run_cmd("--tabs=1+", "+", "+"); + run_cmd("--tabs=1+2", "+", "+2"); +} + +#[test] +fn test_tabs_with_invalid_chars() { + new_ucmd!() + .arg("--tabs=x") + .fails() + .stderr_contains("tab size contains invalid character(s): 'x'"); + new_ucmd!() + .arg("--tabs=1x2") + .fails() + .stderr_contains("tab size contains invalid character(s): 'x2'"); +} + +#[test] +fn test_tabs_with_too_large_size() { + let arg = format!("--tabs={}", u128::MAX); + let expected_error = format!("tab stop is too large '{}'", u128::MAX); + + new_ucmd!().arg(arg).fails().stderr_contains(expected_error); +} diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index ba95fa6a1a6..db2c3d445f2 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2324,6 +2324,20 @@ fn test_ls_quoting_style() { } } +#[test] +fn test_ls_quoting_and_color() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.touch("one two"); + scene + .ucmd() + .arg("--color") + .arg("one two") + .succeeds() + .stdout_only("'one two'\n"); +} + #[test] fn test_ls_ignore_hide() { let scene = TestScenario::new(util_name!()); diff --git a/tests/by-util/test_mktemp.rs b/tests/by-util/test_mktemp.rs index 55ca021c105..df630e715a4 100644 --- a/tests/by-util/test_mktemp.rs +++ b/tests/by-util/test_mktemp.rs @@ -27,6 +27,18 @@ const TMPDIR: &str = "TMPDIR"; #[cfg(windows)] const TMPDIR: &str = "TMP"; +/// An assertion that uses [`matches_template`] and adds a helpful error message. +macro_rules! assert_matches_template { + ($template:expr, $s:expr) => {{ + assert!( + matches_template($template, $s), + "\"{}\" != \"{}\"", + $template, + $s + ); + }}; +} + #[test] fn test_mktemp_mktemp() { let scene = TestScenario::new(util_name!()); @@ -417,6 +429,21 @@ fn test_mktemp_directory_tmpdir() { assert!(PathBuf::from(result.stdout_str().trim()).is_dir()); } +/// Test for combining `--tmpdir` and a template with a subdirectory. +#[test] +fn test_tmpdir_template_has_subdirectory() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + #[cfg(not(windows))] + let (template, joined) = ("a/bXXXX", "./a/bXXXX"); + #[cfg(windows)] + let (template, joined) = (r"a\bXXXX", r".\a\bXXXX"); + let result = ucmd.args(&["--tmpdir=.", template]).succeeds(); + let filename = result.no_stderr().stdout_str().trim_end(); + assert_matches_template!(joined, filename); + assert!(at.file_exists(filename)); +} + /// Test that an absolute path is disallowed when --tmpdir is provided. #[test] fn test_tmpdir_absolute_path() { @@ -466,18 +493,6 @@ fn matches_template(template: &str, s: &str) -> bool { true } -/// An assertion that uses [`matches_template`] and adds a helpful error message. -macro_rules! assert_matches_template { - ($template:expr, $s:expr) => {{ - assert!( - matches_template($template, $s), - "\"{}\" != \"{}\"", - $template, - $s - ); - }}; -} - /// Test that the file is created in the directory given by the template. #[test] fn test_respect_template() { @@ -550,6 +565,16 @@ fn test_suffix_path_separator() { .arg(r"aXXX\b") .fails() .stderr_only("mktemp: invalid suffix '\\b', contains directory separator\n"); + #[cfg(not(windows))] + new_ucmd!() + .arg("XXX/..") + .fails() + .stderr_only("mktemp: invalid suffix '/..', contains directory separator\n"); + #[cfg(windows)] + new_ucmd!() + .arg(r"XXX\..") + .fails() + .stderr_only("mktemp: invalid suffix '\\..', contains directory separator\n"); } #[test] @@ -572,3 +597,25 @@ fn test_too_few_xs_suffix_directory() { fn test_too_many_arguments() { new_ucmd!().args(&["-q", "a", "b"]).fails().code_is(1); } + +#[test] +fn test_two_contiguous_wildcard_blocks() { + let (at, mut ucmd) = at_and_ucmd!(); + let template = "XXX_XXX"; + let result = ucmd.arg(template).succeeds(); + let filename = result.no_stderr().stdout_str().trim_end(); + assert_eq!(&filename[..4], "XXX_"); + assert_matches_template!(template, filename); + assert!(at.file_exists(filename)); +} + +#[test] +fn test_three_contiguous_wildcard_blocks() { + let (at, mut ucmd) = at_and_ucmd!(); + let template = "XXX_XXX_XXX"; + let result = ucmd.arg(template).succeeds(); + let filename = result.no_stderr().stdout_str().trim_end(); + assert_eq!(&filename[..8], "XXX_XXX_"); + assert_matches_template!(template, filename); + assert!(at.file_exists(filename)); +} diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 5e8ffdbd925..3b809114f56 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -1738,7 +1738,7 @@ fn test_follow_name_truncate4() { let mut args = vec!["-s.1", "--max-unchanged-stats=1", "-F", "file"]; - let delay = 100; + let delay = 300; for _ in 0..2 { at.append("file", "foobar\n"); @@ -1761,7 +1761,7 @@ fn test_follow_name_truncate4() { } #[test] -#[cfg(unix)] +#[cfg(all(unix, not(target_os = "android")))] // NOTE: Should work on Android but CI VM is too slow. fn test_follow_truncate_fast() { // inspired by: "gnu/tests/tail-2/truncate.sh" // Ensure all logs are output upon file truncation @@ -1775,7 +1775,7 @@ fn test_follow_truncate_fast() { let mut args = vec!["-s.1", "--max-unchanged-stats=1", "f", "---disable-inotify"]; let follow = vec!["-f", "-F"]; - let delay = 100; + let delay = 150; for _ in 0..2 { for mode in &follow { args.push(mode);