From 2acac0d558739729c5637c43326de6b84fb0c237 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 25 Mar 2022 09:41:01 +0100 Subject: [PATCH 01/26] CI: Force the rustc nightly version to fix issue 3305 --- .github/workflows/CICD.yml | 4 ++-- .github/workflows/GnuTests.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 74f1d8ce0..6623686b2 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -67,7 +67,7 @@ jobs: - name: Install `rust` toolchain uses: actions-rs/toolchain@v1 with: - toolchain: nightly + toolchain: nightly-2022-03-21 default: true profile: minimal - name: Install `cargo-udeps` @@ -483,7 +483,7 @@ jobs: - name: Install `rust` toolchain uses: actions-rs/toolchain@v1 with: - toolchain: nightly + toolchain: nightly-2022-03-21 default: true profile: minimal # minimal component installation (ie, no documentation) - name: Test diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index 1f24f3045..fbd6f4c0f 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -218,7 +218,7 @@ jobs: - name: Install `rust` toolchain uses: actions-rs/toolchain@v1 with: - toolchain: nightly + toolchain: nightly-2022-03-21 default: true profile: minimal # minimal component installation (ie, no documentation) components: rustfmt From c4d89ab1466200e61dccc709c013cc5139e89e4a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 25 Mar 2022 11:01:05 +0100 Subject: [PATCH 02/26] ci try to fix the error ? --- .github/workflows/CICD.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 6623686b2..eb347559d 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -86,7 +86,7 @@ jobs: fault_type="${{ steps.vars.outputs.FAULT_TYPE }}" fault_prefix=$(echo "$fault_type" | tr '[:lower:]' '[:upper:]') # - cargo +nightly udeps ${{ steps.vars.outputs.CARGO_FEATURES_OPTION }} --all-targets &> udeps.log || cat udeps.log + cargo +nightly-2022-03-21 udeps ${{ steps.vars.outputs.CARGO_FEATURES_OPTION }} --all-targets &> udeps.log || cat udeps.log grep --ignore-case "all deps seem to have been used" udeps.log || { printf "%s\n" "::${fault_type} ::${fault_prefix}: \`cargo udeps\`: style violation (unused dependency found)" ; fault=true ; } if [ -n "${{ steps.vars.outputs.FAIL_ON_FAULT }}" ] && [ -n "$fault" ]; then exit 1 ; fi From 22e06c2458a8916b100211aefdf67681eea2af79 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 25 Mar 2022 12:06:41 +0100 Subject: [PATCH 03/26] also fix coverage --- .github/workflows/CICD.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index eb347559d..0735e5782 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -936,7 +936,7 @@ jobs: ## VARs setup outputs() { step_id="vars"; for var in "$@" ; do echo steps.${step_id}.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } # toolchain - TOOLCHAIN="nightly" ## default to "nightly" toolchain (required for certain required unstable compiler flags) ## !maint: refactor when stable channel has needed support + TOOLCHAIN="nightly-2022-03-21" ## default to "nightly" toolchain (required for certain required unstable compiler flags) ## !maint: refactor when stable channel has needed support # * specify gnu-type TOOLCHAIN for windows; `grcov` requires gnu-style code coverage data files case ${{ matrix.job.os }} in windows-*) TOOLCHAIN="$TOOLCHAIN-x86_64-pc-windows-gnu" ;; esac; # * use requested TOOLCHAIN if specified From 9e86e566683f4c2724f932656a8c3f5ae4ee589d Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 25 Mar 2022 15:28:45 +0100 Subject: [PATCH 04/26] Remove a comment to retrigger the CI --- .github/workflows/CICD.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 0735e5782..d2cf890a2 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -919,7 +919,6 @@ jobs: strategy: fail-fast: true matrix: - # job: [ { os: ubuntu-latest }, { os: macos-latest }, { os: windows-latest } ] job: - { os: ubuntu-latest , features: unix } - { os: macos-latest , features: macos } From bd626df70ee0bb0f57c8893185820d3dd51814b3 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 6 Mar 2022 23:28:05 -0500 Subject: [PATCH 05/26] dd: replace cascading if/else if with enum match Replace a cascading `if/else if` chain in `conv_block_unblock_helper()` with a match statement on a new enum, `ConversionMode`, that enumerates the various modes in which `dd` can operate. --- src/uu/dd/src/blocks.rs | 147 +++++++++++++++++--------------- src/uu/dd/src/datastructures.rs | 27 ++++-- src/uu/dd/src/dd.rs | 2 +- 3 files changed, 95 insertions(+), 81 deletions(-) diff --git a/src/uu/dd/src/blocks.rs b/src/uu/dd/src/blocks.rs index 331bad56b..292f9ce09 100644 --- a/src/uu/dd/src/blocks.rs +++ b/src/uu/dd/src/blocks.rs @@ -6,7 +6,7 @@ // spell-checker:ignore datastructures rstat rposition cflags ctable use crate::conversion_tables::ConversionTable; -use crate::datastructures::InternalError; +use crate::datastructures::ConversionMode; use crate::progress::ReadStat; use crate::Input; use std::io::Read; @@ -65,6 +65,47 @@ fn unblock(buf: &[u8], cbs: usize) -> Vec { }) } +/// Given the various command-line parameters, determine the conversion mode. +/// +/// The `conv` command-line option can take many different values, +/// each of which may combine with others. For example, `conv=ascii`, +/// `conv=lcase`, `conv=sync`, and so on. The arguments to this +/// function represent the settings of those various command-line +/// parameters. This function translates those settings to a +/// [`ConversionMode`]. +fn conversion_mode( + ctable: Option<&ConversionTable>, + block: Option, + unblock: Option, + non_ascii: bool, + is_sync: bool, +) -> Option { + match (ctable, block, unblock) { + (Some(ct), None, None) => Some(ConversionMode::ConvertOnly(ct)), + (Some(ct), Some(cbs), None) => { + if non_ascii { + Some(ConversionMode::ConvertThenBlock(ct, cbs, is_sync)) + } else { + Some(ConversionMode::BlockThenConvert(ct, cbs, is_sync)) + } + } + (Some(ct), None, Some(cbs)) => { + if non_ascii { + Some(ConversionMode::ConvertThenUnblock(ct, cbs)) + } else { + Some(ConversionMode::UnblockThenConvert(ct, cbs)) + } + } + (None, Some(cbs), None) => Some(ConversionMode::BlockOnly(cbs, is_sync)), + (None, None, Some(cbs)) => Some(ConversionMode::UnblockOnly(cbs)), + (None, None, None) => None, + // The remaining variants should never happen because the + // argument parsing above should result in an error before + // getting to this line of code. + _ => unreachable!(), + } +} + /// A helper for teasing out which options must be applied and in which order. /// Some user options, such as the presence of conversion tables, will determine whether the input is assumed to be ascii. The parser sets the Input::non_ascii flag accordingly. /// Examples: @@ -76,94 +117,58 @@ pub(crate) fn conv_block_unblock_helper( mut buf: Vec, i: &mut Input, rstat: &mut ReadStat, -) -> Result, InternalError> { - // Local Predicate Fns ------------------------------------------------- - fn should_block_then_conv(i: &Input) -> bool { - !i.non_ascii && i.cflags.block.is_some() - } - fn should_conv_then_block(i: &Input) -> bool { - i.non_ascii && i.cflags.block.is_some() - } - fn should_unblock_then_conv(i: &Input) -> bool { - !i.non_ascii && i.cflags.unblock.is_some() - } - fn should_conv_then_unblock(i: &Input) -> bool { - i.non_ascii && i.cflags.unblock.is_some() - } - fn conv_only(i: &Input) -> bool { - i.cflags.ctable.is_some() && i.cflags.block.is_none() && i.cflags.unblock.is_none() - } - // Local Helper Fns ---------------------------------------------------- +) -> Vec { + // TODO This function has a mutable input `buf` but also returns a + // completely new `Vec`; that seems fishy. Could we either make + // the input immutable or make the function not return anything? + fn apply_conversion(buf: &mut [u8], ct: &ConversionTable) { for idx in 0..buf.len() { buf[idx] = ct[buf[idx] as usize]; } } - // -------------------------------------------------------------------- - if conv_only(i) { - // no block/unblock - let ct = i.cflags.ctable.unwrap(); - apply_conversion(&mut buf, ct); - Ok(buf) - } else if should_block_then_conv(i) { - // ascii input so perform the block first - let cbs = i.cflags.block.unwrap(); - - let mut blocks = block(&buf, cbs, i.cflags.sync.is_some(), rstat); - - if let Some(ct) = i.cflags.ctable { + let mode = conversion_mode( + i.cflags.ctable, + i.cflags.block, + i.cflags.unblock, + i.non_ascii, + i.cflags.sync.is_some(), + ) + .unwrap(); + match &mode { + ConversionMode::ConvertOnly(ct) => { + apply_conversion(&mut buf, ct); + buf + } + ConversionMode::BlockThenConvert(ct, cbs, sync) => { + let mut blocks = block(&buf, *cbs, *sync, rstat); for buf in &mut blocks { apply_conversion(buf, ct); } + blocks.into_iter().flatten().collect() } - - let blocks = blocks.into_iter().flatten().collect(); - - Ok(blocks) - } else if should_conv_then_block(i) { - // Non-ascii so perform the conversion first - let cbs = i.cflags.block.unwrap(); - - if let Some(ct) = i.cflags.ctable { + ConversionMode::ConvertThenBlock(ct, cbs, sync) => { apply_conversion(&mut buf, ct); + block(&buf, *cbs, *sync, rstat) + .into_iter() + .flatten() + .collect() } - - let blocks = block(&buf, cbs, i.cflags.sync.is_some(), rstat) + ConversionMode::BlockOnly(cbs, sync) => block(&buf, *cbs, *sync, rstat) .into_iter() .flatten() - .collect(); - - Ok(blocks) - } else if should_unblock_then_conv(i) { - // ascii input so perform the unblock first - let cbs = i.cflags.unblock.unwrap(); - - let mut buf = unblock(&buf, cbs); - - if let Some(ct) = i.cflags.ctable { + .collect(), + ConversionMode::UnblockThenConvert(ct, cbs) => { + let mut buf = unblock(&buf, *cbs); apply_conversion(&mut buf, ct); + buf } - - Ok(buf) - } else if should_conv_then_unblock(i) { - // Non-ascii input so perform the conversion first - let cbs = i.cflags.unblock.unwrap(); - - if let Some(ct) = i.cflags.ctable { + ConversionMode::ConvertThenUnblock(ct, cbs) => { apply_conversion(&mut buf, ct); + unblock(&buf, *cbs) } - - let buf = unblock(&buf, cbs); - - Ok(buf) - } else { - // The following error should not happen, as it results from - // insufficient command line data. This case should be caught - // by the parser before making it this far. - // Producing this error is an alternative to risking an unwrap call - // on 'cbs' if the required data is not provided. - Err(InternalError::InvalidConvBlockUnblockCase) + ConversionMode::UnblockOnly(cbs) => unblock(&buf, *cbs), } } diff --git a/src/uu/dd/src/datastructures.rs b/src/uu/dd/src/datastructures.rs index 6529f6602..17266ada1 100644 --- a/src/uu/dd/src/datastructures.rs +++ b/src/uu/dd/src/datastructures.rs @@ -14,6 +14,23 @@ use crate::conversion_tables::*; type Cbs = usize; +/// How to apply conversion, blocking, and/or unblocking. +/// +/// Certain settings of the `conv` parameter to `dd` require a +/// combination of conversion, blocking, or unblocking, applied in a +/// certain order. The variants of this enumeration give the different +/// ways of combining those three operations. +#[derive(Debug, PartialEq)] +pub(crate) enum ConversionMode<'a> { + ConvertOnly(&'a ConversionTable), + BlockOnly(Cbs, bool), + UnblockOnly(Cbs), + BlockThenConvert(&'a ConversionTable, Cbs, bool), + ConvertThenBlock(&'a ConversionTable, Cbs, bool), + UnblockThenConvert(&'a ConversionTable, Cbs), + ConvertThenUnblock(&'a ConversionTable, Cbs), +} + /// Stores all Conv Flags that apply to the input #[derive(Debug, Default, PartialEq)] pub struct IConvFlags { @@ -91,19 +108,11 @@ pub enum CountType { pub enum InternalError { WrongInputType, WrongOutputType, - InvalidConvBlockUnblockCase, } impl std::fmt::Display for InternalError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::WrongInputType | Self::WrongOutputType => { - write!(f, "Internal dd error: Wrong Input/Output data type") - } - Self::InvalidConvBlockUnblockCase => { - write!(f, "Invalid Conversion, Block, or Unblock data") - } - } + write!(f, "Internal dd error: Wrong Input/Output data type") } } diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index cdfcdb732..d004a592d 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -640,7 +640,7 @@ fn read_helper(i: &mut Input, bsize: usize) -> std::io::Result<(Read perform_swab(&mut buf); } if is_conv(i) || is_block(i) || is_unblock(i) { - let buf = conv_block_unblock_helper(buf, i, &mut rstat).unwrap(); + let buf = conv_block_unblock_helper(buf, i, &mut rstat); Ok((rstat, buf)) } else { Ok((rstat, buf)) From b98bccf9cc17ad8730e31a9cac200421bf6e2bb2 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 6 Mar 2022 23:35:03 -0500 Subject: [PATCH 06/26] dd: move ConversionMode parsing to dd.rs Move parsing of the `ConversionMode` outside of `conv_block_unblock_helper()` and up to the code that calls it. --- src/uu/dd/src/blocks.rs | 76 +++++++++-------------------------------- src/uu/dd/src/dd.rs | 70 +++++++++++++++++++++++++++++-------- 2 files changed, 71 insertions(+), 75 deletions(-) diff --git a/src/uu/dd/src/blocks.rs b/src/uu/dd/src/blocks.rs index 292f9ce09..3f7c59c27 100644 --- a/src/uu/dd/src/blocks.rs +++ b/src/uu/dd/src/blocks.rs @@ -8,8 +8,6 @@ use crate::conversion_tables::ConversionTable; use crate::datastructures::ConversionMode; use crate::progress::ReadStat; -use crate::Input; -use std::io::Read; const NEWLINE: u8 = b'\n'; const SPACE: u8 = b' '; @@ -65,57 +63,23 @@ fn unblock(buf: &[u8], cbs: usize) -> Vec { }) } -/// Given the various command-line parameters, determine the conversion mode. +/// Apply the specified conversion, blocking, and/or unblocking in the right order. /// -/// The `conv` command-line option can take many different values, -/// each of which may combine with others. For example, `conv=ascii`, -/// `conv=lcase`, `conv=sync`, and so on. The arguments to this -/// function represent the settings of those various command-line -/// parameters. This function translates those settings to a -/// [`ConversionMode`]. -fn conversion_mode( - ctable: Option<&ConversionTable>, - block: Option, - unblock: Option, - non_ascii: bool, - is_sync: bool, -) -> Option { - match (ctable, block, unblock) { - (Some(ct), None, None) => Some(ConversionMode::ConvertOnly(ct)), - (Some(ct), Some(cbs), None) => { - if non_ascii { - Some(ConversionMode::ConvertThenBlock(ct, cbs, is_sync)) - } else { - Some(ConversionMode::BlockThenConvert(ct, cbs, is_sync)) - } - } - (Some(ct), None, Some(cbs)) => { - if non_ascii { - Some(ConversionMode::ConvertThenUnblock(ct, cbs)) - } else { - Some(ConversionMode::UnblockThenConvert(ct, cbs)) - } - } - (None, Some(cbs), None) => Some(ConversionMode::BlockOnly(cbs, is_sync)), - (None, None, Some(cbs)) => Some(ConversionMode::UnblockOnly(cbs)), - (None, None, None) => None, - // The remaining variants should never happen because the - // argument parsing above should result in an error before - // getting to this line of code. - _ => unreachable!(), - } -} - -/// A helper for teasing out which options must be applied and in which order. -/// Some user options, such as the presence of conversion tables, will determine whether the input is assumed to be ascii. The parser sets the Input::non_ascii flag accordingly. -/// Examples: -/// - If conv=ebcdic or conv=ibm is specified then block, unblock or swab must be performed before the conversion happens since the source will start in ascii. -/// - If conv=ascii is specified then block, unblock or swab must be performed after the conversion since the source starts in ebcdic. -/// - If no conversion is specified then the source is assumed to be in ascii. -/// For more info see `info dd` -pub(crate) fn conv_block_unblock_helper( +/// The `mode` specifies the combination of conversion, blocking, and +/// unblocking to apply and the order in which to apply it. This +/// function is responsible only for applying the operations. +/// +/// `buf` is the buffer of input bytes to transform. This function +/// mutates this input and also returns a new buffer of bytes +/// representing the result of the transformation. +/// +/// `rstat` maintains a running total of the number of partial and +/// complete blocks read before calling this function. In certain +/// settings of `mode`, this function will update the number of +/// records truncated; that's why `rstat` is borrowed mutably. +pub(crate) fn conv_block_unblock_helper( mut buf: Vec, - i: &mut Input, + mode: &ConversionMode, rstat: &mut ReadStat, ) -> Vec { // TODO This function has a mutable input `buf` but also returns a @@ -128,15 +92,7 @@ pub(crate) fn conv_block_unblock_helper( } } - let mode = conversion_mode( - i.cflags.ctable, - i.cflags.block, - i.cflags.unblock, - i.non_ascii, - i.cflags.sync.is_some(), - ) - .unwrap(); - match &mode { + match mode { ConversionMode::ConvertOnly(ct) => { apply_conversion(&mut buf, ct); buf diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index d004a592d..33135bf02 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -605,18 +605,49 @@ impl Write for Output { } } +/// Given the various command-line parameters, determine the conversion mode. +/// +/// The `conv` command-line option can take many different values, +/// each of which may combine with others. For example, `conv=ascii`, +/// `conv=lcase`, `conv=sync`, and so on. The arguments to this +/// function represent the settings of those various command-line +/// parameters. This function translates those settings to a +/// [`ConversionMode`]. +fn conversion_mode( + ctable: Option<&ConversionTable>, + block: Option, + unblock: Option, + non_ascii: bool, + is_sync: bool, +) -> Option { + match (ctable, block, unblock) { + (Some(ct), None, None) => Some(ConversionMode::ConvertOnly(ct)), + (Some(ct), Some(cbs), None) => { + if non_ascii { + Some(ConversionMode::ConvertThenBlock(ct, cbs, is_sync)) + } else { + Some(ConversionMode::BlockThenConvert(ct, cbs, is_sync)) + } + } + (Some(ct), None, Some(cbs)) => { + if non_ascii { + Some(ConversionMode::ConvertThenUnblock(ct, cbs)) + } else { + Some(ConversionMode::UnblockThenConvert(ct, cbs)) + } + } + (None, Some(cbs), None) => Some(ConversionMode::BlockOnly(cbs, is_sync)), + (None, None, Some(cbs)) => Some(ConversionMode::UnblockOnly(cbs)), + (None, None, None) => None, + // The remaining variants should never happen because the + // argument parsing above should result in an error before + // getting to this line of code. + _ => unreachable!(), + } +} + /// Read helper performs read operations common to all dd reads, and dispatches the buffer to relevant helper functions as dictated by the operations requested by the user. fn read_helper(i: &mut Input, bsize: usize) -> std::io::Result<(ReadStat, Vec)> { - // Local Predicate Fns ----------------------------------------------- - fn is_conv(i: &Input) -> bool { - i.cflags.ctable.is_some() - } - fn is_block(i: &Input) -> bool { - i.cflags.block.is_some() - } - fn is_unblock(i: &Input) -> bool { - i.cflags.unblock.is_some() - } // Local Helper Fns ------------------------------------------------- fn perform_swab(buf: &mut [u8]) { for base in (1..buf.len()).step_by(2) { @@ -639,11 +670,20 @@ fn read_helper(i: &mut Input, bsize: usize) -> std::io::Result<(Read if i.cflags.swab { perform_swab(&mut buf); } - if is_conv(i) || is_block(i) || is_unblock(i) { - let buf = conv_block_unblock_helper(buf, i, &mut rstat); - Ok((rstat, buf)) - } else { - Ok((rstat, buf)) + + let mode = conversion_mode( + i.cflags.ctable, + i.cflags.block, + i.cflags.unblock, + i.non_ascii, + i.cflags.sync.is_some(), + ); + match mode { + Some(ref mode) => { + let buf = conv_block_unblock_helper(buf, mode, &mut rstat); + Ok((rstat, buf)) + } + None => Ok((rstat, buf)), } } From f856bfc479d300fef8515e11f5410bb3a2588c4f Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 6 Mar 2022 23:39:04 -0500 Subject: [PATCH 07/26] dd: move ConversionMode parsing to parseargs mod. Move the code for parsing the `ConversionMode` to use up to the `parseargs` module. This location makes more sense for it because the conversion mode can be determined entirely from the command-line arguments at the time of parsing just like the other parameters. Using an enum for this purpose also eliminates the amount of code we need later on. --- src/uu/dd/src/datastructures.rs | 6 +- src/uu/dd/src/dd.rs | 57 +------------------ src/uu/dd/src/parseargs.rs | 81 ++++++++++++++++++++++++--- src/uu/dd/src/parseargs/unit_tests.rs | 14 +++-- 4 files changed, 85 insertions(+), 73 deletions(-) diff --git a/src/uu/dd/src/datastructures.rs b/src/uu/dd/src/datastructures.rs index 17266ada1..ffcee4cb1 100644 --- a/src/uu/dd/src/datastructures.rs +++ b/src/uu/dd/src/datastructures.rs @@ -33,10 +33,8 @@ pub(crate) enum ConversionMode<'a> { /// Stores all Conv Flags that apply to the input #[derive(Debug, Default, PartialEq)] -pub struct IConvFlags { - pub ctable: Option<&'static ConversionTable>, - pub block: Option, - pub unblock: Option, +pub(crate) struct IConvFlags { + pub mode: Option>, pub swab: bool, pub sync: Option, pub noerror: bool, diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 33135bf02..354e4b261 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -45,7 +45,6 @@ const BUF_INIT_BYTE: u8 = 0xDD; struct Input { src: R, - non_ascii: bool, ibs: usize, print_level: Option, count: Option, @@ -56,7 +55,6 @@ struct Input { impl Input { fn new(matches: &Matches) -> UResult { let ibs = parseargs::parse_ibs(matches)?; - let non_ascii = parseargs::parse_input_non_ascii(matches)?; let print_level = parseargs::parse_status_level(matches)?; let cflags = parseargs::parse_conv_flag_input(matches)?; let iflags = parseargs::parse_iflags(matches)?; @@ -67,7 +65,6 @@ impl Input { let mut i = Self { src: io::stdin(), - non_ascii, ibs, print_level, count, @@ -131,7 +128,6 @@ fn make_linux_iflags(iflags: &IFlags) -> Option { impl Input { fn new(matches: &Matches) -> UResult { let ibs = parseargs::parse_ibs(matches)?; - let non_ascii = parseargs::parse_input_non_ascii(matches)?; let print_level = parseargs::parse_status_level(matches)?; let cflags = parseargs::parse_conv_flag_input(matches)?; let iflags = parseargs::parse_iflags(matches)?; @@ -163,7 +159,6 @@ impl Input { let i = Self { src, - non_ascii, ibs, print_level, count, @@ -605,47 +600,6 @@ impl Write for Output { } } -/// Given the various command-line parameters, determine the conversion mode. -/// -/// The `conv` command-line option can take many different values, -/// each of which may combine with others. For example, `conv=ascii`, -/// `conv=lcase`, `conv=sync`, and so on. The arguments to this -/// function represent the settings of those various command-line -/// parameters. This function translates those settings to a -/// [`ConversionMode`]. -fn conversion_mode( - ctable: Option<&ConversionTable>, - block: Option, - unblock: Option, - non_ascii: bool, - is_sync: bool, -) -> Option { - match (ctable, block, unblock) { - (Some(ct), None, None) => Some(ConversionMode::ConvertOnly(ct)), - (Some(ct), Some(cbs), None) => { - if non_ascii { - Some(ConversionMode::ConvertThenBlock(ct, cbs, is_sync)) - } else { - Some(ConversionMode::BlockThenConvert(ct, cbs, is_sync)) - } - } - (Some(ct), None, Some(cbs)) => { - if non_ascii { - Some(ConversionMode::ConvertThenUnblock(ct, cbs)) - } else { - Some(ConversionMode::UnblockThenConvert(ct, cbs)) - } - } - (None, Some(cbs), None) => Some(ConversionMode::BlockOnly(cbs, is_sync)), - (None, None, Some(cbs)) => Some(ConversionMode::UnblockOnly(cbs)), - (None, None, None) => None, - // The remaining variants should never happen because the - // argument parsing above should result in an error before - // getting to this line of code. - _ => unreachable!(), - } -} - /// Read helper performs read operations common to all dd reads, and dispatches the buffer to relevant helper functions as dictated by the operations requested by the user. fn read_helper(i: &mut Input, bsize: usize) -> std::io::Result<(ReadStat, Vec)> { // Local Helper Fns ------------------------------------------------- @@ -671,14 +625,7 @@ fn read_helper(i: &mut Input, bsize: usize) -> std::io::Result<(Read perform_swab(&mut buf); } - let mode = conversion_mode( - i.cflags.ctable, - i.cflags.block, - i.cflags.unblock, - i.non_ascii, - i.cflags.sync.is_some(), - ); - match mode { + match i.cflags.mode { Some(ref mode) => { let buf = conv_block_unblock_helper(buf, mode, &mut rstat); Ok((rstat, buf)) @@ -1129,7 +1076,6 @@ mod tests { src: LazyReader { src: File::open("./test-resources/deadbeef-16.test").unwrap(), }, - non_ascii: false, ibs: 16, print_level: None, count: None, @@ -1176,7 +1122,6 @@ mod tests { src: File::open("./test-resources/random-5828891cb1230748e146f34223bbd3b5.test") .unwrap(), }, - non_ascii: false, ibs: 521, print_level: None, count: None, diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index 8f2f10e70..4bc65bc1c 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -535,9 +535,50 @@ fn parse_flag_list>( .collect() } +/// Given the various command-line parameters, determine the conversion mode. +/// +/// The `conv` command-line option can take many different values, +/// each of which may combine with others. For example, `conv=ascii`, +/// `conv=lcase`, `conv=sync`, and so on. The arguments to this +/// function represent the settings of those various command-line +/// parameters. This function translates those settings to a +/// [`ConversionMode`]. +fn conversion_mode( + ctable: Option<&ConversionTable>, + block: Option, + unblock: Option, + non_ascii: bool, + is_sync: bool, +) -> Option { + match (ctable, block, unblock) { + (Some(ct), None, None) => Some(ConversionMode::ConvertOnly(ct)), + (Some(ct), Some(cbs), None) => { + if non_ascii { + Some(ConversionMode::ConvertThenBlock(ct, cbs, is_sync)) + } else { + Some(ConversionMode::BlockThenConvert(ct, cbs, is_sync)) + } + } + (Some(ct), None, Some(cbs)) => { + if non_ascii { + Some(ConversionMode::ConvertThenUnblock(ct, cbs)) + } else { + Some(ConversionMode::UnblockThenConvert(ct, cbs)) + } + } + (None, Some(cbs), None) => Some(ConversionMode::BlockOnly(cbs, is_sync)), + (None, None, Some(cbs)) => Some(ConversionMode::UnblockOnly(cbs)), + (None, None, None) => None, + // The remaining variants should never happen because the + // argument parsing above should result in an error before + // getting to this line of code. + _ => unreachable!(), + } +} + /// Parse Conversion Options (Input Variety) /// Construct and validate a IConvFlags -pub fn parse_conv_flag_input(matches: &Matches) -> Result { +pub(crate) fn parse_conv_flag_input(matches: &Matches) -> Result { let mut iconvflags = IConvFlags::default(); let mut fmt = None; let mut case = None; @@ -546,6 +587,9 @@ pub fn parse_conv_flag_input(matches: &Matches) -> Result { @@ -565,7 +609,7 @@ pub fn parse_conv_flag_input(matches: &Matches) -> Result Result Result match (cbs, iconvflags.unblock) { - (Some(cbs), None) => iconvflags.block = Some(cbs), + ConvFlag::Block => match (cbs, unblock) { + (Some(cbs), None) => block = Some(cbs), (None, _) => return Err(ParseError::BlockUnblockWithoutCBS), (_, Some(_)) => return Err(ParseError::MultipleBlockUnblock), }, - ConvFlag::Unblock => match (cbs, iconvflags.block) { - (Some(cbs), None) => iconvflags.unblock = Some(cbs), + ConvFlag::Unblock => match (cbs, block) { + (Some(cbs), None) => unblock = Some(cbs), (None, _) => return Err(ParseError::BlockUnblockWithoutCBS), (_, Some(_)) => return Err(ParseError::MultipleBlockUnblock), }, @@ -630,7 +674,7 @@ pub fn parse_conv_flag_input(matches: &Matches) -> Result Result Date: Fri, 25 Mar 2022 20:02:46 +0000 Subject: [PATCH 08/26] build(deps): bump redox_syscall from 0.2.10 to 0.2.12 Bumps redox_syscall from 0.2.10 to 0.2.12. --- updated-dependencies: - dependency-name: redox_syscall dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e159029a..762154050 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1663,9 +1663,9 @@ dependencies = [ [[package]] name = "redox_syscall" -version = "0.2.10" +version = "0.2.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8383f39639269cde97d255a32bdb68c047337295414940c68bdd30c2e13203ff" +checksum = "8ae183fc1b06c149f0c1793e1eb447c8b04bfe46d48e9e48bfb8d2d7ed64ecf0" dependencies = [ "bitflags", ] From fbb64b9c5c046dc1f66802e5ab2f362d24c2c9e0 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 22 Mar 2022 23:08:47 +0100 Subject: [PATCH 09/26] mkdir: gnu compat: add support of mkdir -p foo/. --- src/uu/mkdir/src/mkdir.rs | 16 ++++++++++++---- tests/by-util/test_mkdir.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index ed487bb58..35078d296 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -11,7 +11,7 @@ extern crate uucore; use clap::{crate_version, Arg, ArgMatches, Command, OsValues}; -use std::path::Path; +use std::path::{Path, PathBuf}; use uucore::display::Quotable; #[cfg(not(windows))] use uucore::error::FromIo; @@ -143,8 +143,17 @@ pub fn uu_app<'a>() -> Command<'a> { */ fn exec(dirs: OsValues, recursive: bool, mode: u32, verbose: bool) -> UResult<()> { for dir in dirs { - let path = Path::new(dir); - show_if_err!(mkdir(path, recursive, mode, verbose)); + // Special case to match GNU's behavior: + // mkdir -p foo/. should work and just create foo/ + // std::fs::create_dir("foo/."); fails in pure Rust + let path = if recursive && dir.to_string_lossy().ends_with("/.") { + // Do a simple dance to strip the "/." + Path::new(dir).components().collect::() + } else { + // Normal case + PathBuf::from(dir) + }; + show_if_err!(mkdir(path.as_path(), recursive, mode, verbose)); } Ok(()) } @@ -190,7 +199,6 @@ fn create_dir(path: &Path, recursive: bool, verbose: bool) -> UResult<()> { } } } - match std::fs::create_dir(path) { Ok(()) => { if verbose { diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index 011c7b25a..675ca6bea 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -11,6 +11,8 @@ static TEST_DIR6: &str = "mkdir_test6"; static TEST_FILE7: &str = "mkdir_test7"; static TEST_DIR8: &str = "mkdir_test8/mkdir_test8_1/mkdir_test8_2"; static TEST_DIR9: &str = "mkdir_test9/../mkdir_test9_1/../mkdir_test9_2"; +static TEST_DIR10: &str = "mkdir_test10/."; +static TEST_DIR11: &str = "mkdir_test11/.."; #[test] fn test_mkdir_mkdir() { @@ -123,3 +125,29 @@ fn test_recursive_reporting() { .stdout_contains("created directory 'mkdir_test9/../mkdir_test9_1'") .stdout_contains("created directory 'mkdir_test9/../mkdir_test9_1/../mkdir_test9_2'"); } + +#[test] +fn test_mkdir_trailing_dot() { + let scene2 = TestScenario::new("ls"); + new_ucmd!() + .arg("-p") + .arg("-v") + .arg("mkdir_test10-2") + .succeeds(); + + new_ucmd!() + .arg("-p") + .arg("-v") + .arg(TEST_DIR10) + .succeeds() + .stdout_contains("created directory 'mkdir_test10'"); + + new_ucmd!() + .arg("-p") + .arg("-v") + .arg(TEST_DIR11) + .succeeds() + .stdout_contains("created directory 'mkdir_test11'"); + let result = scene2.cmd("ls").arg("-al").run(); + println!("ls dest {}", result.stdout_str()); +} From 6a8ce447b7edd5eb6b040d45169e09604f9472f5 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 26 Mar 2022 10:39:00 -0400 Subject: [PATCH 10/26] uucore: no uppercase suffixes in parse_time Disallow uppercase unit suffixes "S", "M", "H", and "D" in `uucore::parse_time::from_str()` to match the behavior of GNU `sleep` and `timeout`. --- src/uucore/src/lib/parser/parse_time.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_time.rs b/src/uucore/src/lib/parser/parse_time.rs index 79387c0b1..e440a3c91 100644 --- a/src/uucore/src/lib/parser/parse_time.rs +++ b/src/uucore/src/lib/parser/parse_time.rs @@ -21,6 +21,13 @@ use crate::display::Quotable; /// one hundred twenty three seconds or "4.5d" meaning four and a half /// days. If no unit is specified, the unit is assumed to be seconds. /// +/// The only allowed suffixes are +/// +/// * "s" for seconds, +/// * "m" for minutes, +/// * "h" for hours, +/// * "d" for days. +/// /// This function uses [`Duration::saturating_mul`] to compute the /// number of seconds, so it does not overflow. If overflow would have /// occurred, [`Duration::MAX`] is returned instead. @@ -46,10 +53,10 @@ pub fn from_str(string: &str) -> Result { } let slice = &string[..len - 1]; let (numstr, times) = match string.chars().next_back().unwrap() { - 's' | 'S' => (slice, 1), - 'm' | 'M' => (slice, 60), - 'h' | 'H' => (slice, 60 * 60), - 'd' | 'D' => (slice, 60 * 60 * 24), + 's' => (slice, 1), + 'm' => (slice, 60), + 'h' => (slice, 60 * 60), + 'd' => (slice, 60 * 60 * 24), val if !val.is_alphabetic() => (string, 1), _ => { if string == "inf" || string == "infinity" { @@ -114,4 +121,13 @@ mod tests { fn test_negative() { assert!(from_str("-1").is_err()); } + + /// Test that capital letters are not allowed in suffixes. + #[test] + fn test_no_capital_letters() { + assert!(from_str("1S").is_err()); + assert!(from_str("1M").is_err()); + assert!(from_str("1H").is_err()); + assert!(from_str("1D").is_err()); + } } From 5f8567695c2db1e165aad7b719b45004e1d472fd Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 26 Mar 2022 09:21:42 -0400 Subject: [PATCH 11/26] util: use uutils timeout in GNU tests Remove the substitution of uutils `timeout` with GNU `timeout` before running the GNU test suite. Some of these replacements were not actually operational because the regex was not appropriate (for example, the test file had `timeout 10` but the regex would only match `timeout [0-9]`). Others no longer need to be used because the uutils version of `timeout` works well enough now to terminate a process within the given period of time. --- util/build-gnu.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/util/build-gnu.sh b/util/build-gnu.sh index adf69e3be..d53052167 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -104,7 +104,6 @@ sed -i '/INT_OFLOW/ D' tests/misc/printf.sh # Use the system coreutils where the test fails due to error in a util that is not the one being tested sed -i 's|stat|/usr/bin/stat|' tests/touch/60-seconds.sh tests/misc/sort-compress-proc.sh sed -i 's|ls -|/usr/bin/ls -|' tests/cp/same-file.sh tests/misc/mknod.sh tests/mv/part-symlink.sh -sed -i 's|timeout \([[:digit:]]\)| /usr/bin/timeout \1|' tests/tail-2/inotify-rotate.sh tests/tail-2/inotify-dir-recreate.sh tests/tail-2/inotify-rotate-resources.sh tests/cp/parent-perm-race.sh tests/ls/infloop.sh tests/misc/sort-exit-early.sh tests/misc/sort-NaN-infloop.sh tests/misc/uniq-perf.sh tests/tail-2/inotify-only-regular.sh tests/tail-2/pipe-f2.sh tests/tail-2/retry.sh tests/tail-2/symlink.sh tests/tail-2/wait.sh tests/tail-2/pid.sh tests/dd/stats.sh tests/tail-2/follow-name.sh tests/misc/shuf.sh # Don't break the function called 'grep_timeout' sed -i 's|chmod |/usr/bin/chmod |' tests/du/inacc-dir.sh tests/tail-2/tail-n0f.sh tests/cp/fail-perm.sh tests/mv/i-2.sh tests/misc/shuf.sh sed -i 's|sort |/usr/bin/sort |' tests/ls/hyperlink.sh tests/misc/test-N.sh sed -i 's|split |/usr/bin/split |' tests/misc/factor-parallel.sh From c43ef8b7049bce98f0c64699a6cb4e7eab80cd27 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 26 Mar 2022 10:18:30 -0400 Subject: [PATCH 12/26] timeout: support long form of --kill-after arg Add support for the long form of the `--kill-after` argument. Previously only the short form `-k` was supported. --- src/uu/timeout/src/timeout.rs | 2 ++ tests/by-util/test_timeout.rs | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index fbd955242..38187325c 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -134,7 +134,9 @@ pub fn uu_app<'a>() -> Command<'a> { ) .arg( Arg::new(options::KILL_AFTER) + .long(options::KILL_AFTER) .short('k') + .help("also send a KILL signal if COMMAND is still running this long after the initial signal was sent") .takes_value(true)) .arg( Arg::new(options::PRESERVE_STATUS) diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 80e7240fe..82582d2e4 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -105,3 +105,13 @@ fn test_invalid_signal() { .fails() .usage_error("'invalid': invalid signal"); } + +/// Test that the long form of the `--kill-after` argument is recognized. +#[test] +fn test_kill_after_long() { + new_ucmd!() + .args(&["--kill-after=1", "1", "sleep", "0"]) + .succeeds() + .no_stdout() + .no_stderr(); +} From bbee22bb1cc8a20d5eafa2f2b953f0b4efba9571 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 26 Mar 2022 11:30:07 +0100 Subject: [PATCH 13/26] ls: Add missing quote with --quoting-style=shell-escape Should fix GNU: tests/ls/symlink-quote.sh --- src/uu/ls/src/ls.rs | 3 ++- tests/by-util/test_ls.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index a3fdef344..ee653d4c4 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2568,7 +2568,8 @@ fn display_file_name( } } else { // If no coloring is required, we just use target as is. - name.push_str(&target.to_string_lossy()); + // Apply the right quoting + name.push_str(&escape_name(&target.as_os_str(), &config.quoting_style)); } } } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 3cfba4312..d4692b573 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2913,3 +2913,43 @@ fn test_ls_multiple_a_A() { .stdout_does_not_contain(".") .stdout_does_not_contain(".."); } + +#[test] +fn test_ls_quoting() { + let scene = TestScenario::new(util_name!()); + + scene + .ccmd("ln") + .arg("-s") + .arg("'need quoting'") + .arg("symlink") + .succeeds(); + scene + .ucmd() + .arg("-l") + .arg("--quoting-style=shell-escape") + .arg("symlink") + .succeeds() + .stdout_contains("\'need quoting\'"); +} + +//#[test] +// Enable when support with color is added +fn test_ls_quoting_auto() { + let scene = TestScenario::new(util_name!()); + + scene + .ccmd("ln") + .arg("-s") + .arg("'need quoting'") + .arg("symlink") + .succeeds(); + scene + .ucmd() + .arg("-l") + .arg("--quoting-style=shell-escape") + .arg("--color=auto") + .arg("symlink") + .succeeds() + .stdout_contains("\'need quoting\'"); +} From c79d146ddea52d0cd30d3d7a1ede1f1919443647 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 26 Mar 2022 11:45:47 +0100 Subject: [PATCH 14/26] ls: add support for --quoting-style=shell-escape b --color=auto --- src/uu/ls/src/ls.rs | 22 ++++++++++++++++------ tests/by-util/test_ls.rs | 5 ++--- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ee653d4c4..778283039 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2478,7 +2478,7 @@ fn display_file_name( 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); + name = color_name(ls_colors, &path.p_buf, &name, &metadata, config); } } @@ -2562,14 +2562,15 @@ fn display_file_name( name.push_str(&color_name( ls_colors, &target_data.p_buf, - target.to_string_lossy().into_owned(), + &target.to_string_lossy(), &target_metadata, + config, )); } } else { // If no coloring is required, we just use target as is. // Apply the right quoting - name.push_str(&escape_name(&target.as_os_str(), &config.quoting_style)); + name.push_str(&escape_name(target.as_os_str(), &config.quoting_style)); } } } @@ -2594,10 +2595,19 @@ fn display_file_name( } } -fn color_name(ls_colors: &LsColors, path: &Path, name: String, md: &Metadata) -> String { +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)) { - Some(style) => style.to_ansi_term_style().paint(name).to_string(), - None => name, + Some(style) => { + let p = escape_name(OsStr::new(&name), &config.quoting_style); + return style.to_ansi_term_style().paint(p).to_string(); + } + None => escape_name(OsStr::new(&name), &config.quoting_style), } } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index d4692b573..f60d53b6e 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2933,9 +2933,8 @@ fn test_ls_quoting() { .stdout_contains("\'need quoting\'"); } -//#[test] -// Enable when support with color is added -fn test_ls_quoting_auto() { +#[test] +fn test_ls_quoting_color() { let scene = TestScenario::new(util_name!()); scene From b34685f8a58bf92bce124450648925fb52eb0c4d Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 26 Mar 2022 10:22:23 -0400 Subject: [PATCH 15/26] timeout: return 125 on invalid time interval args Exit with status 125 (indicating an error in `timeout` itself) when the timeout duration is invalid or the "kill after" duration is invalid. --- src/uu/timeout/src/timeout.rs | 12 ++++++++---- tests/by-util/test_timeout.rs | 10 ++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 38187325c..8374c124c 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -68,14 +68,18 @@ impl Config { _ => uucore::signals::signal_by_name_or_value("TERM").unwrap(), }; - let kill_after = options - .value_of(options::KILL_AFTER) - .map(|time| uucore::parse_time::from_str(time).unwrap()); + let kill_after = match options.value_of(options::KILL_AFTER) { + None => None, + Some(kill_after) => match uucore::parse_time::from_str(kill_after) { + Ok(k) => Some(k), + Err(err) => return Err(UUsageError::new(ExitStatus::TimeoutFailed.into(), err)), + }, + }; let duration = match uucore::parse_time::from_str(options.value_of(options::DURATION).unwrap()) { Ok(duration) => duration, - Err(err) => return Err(UUsageError::new(1, err)), + Err(err) => return Err(UUsageError::new(ExitStatus::TimeoutFailed.into(), err)), }; let preserve_status: bool = options.is_present(options::PRESERVE_STATUS); diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 82582d2e4..fef5fd4ae 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -16,6 +16,16 @@ fn test_invalid_time_interval() { new_ucmd!() .args(&["xyz", "sleep", "0"]) .fails() + .code_is(125) + .usage_error("invalid time interval 'xyz'"); +} + +#[test] +fn test_invalid_kill_after() { + new_ucmd!() + .args(&["-k", "xyz", "1", "sleep", "0"]) + .fails() + .code_is(125) .usage_error("invalid time interval 'xyz'"); } From ab717ce370242fc82660dc800e1203a7c5aa686b Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 21 Mar 2022 22:03:10 -0400 Subject: [PATCH 16/26] df: implement the File column Implement the "File" column in the `df` output table. Before this commit, a blank entry appeared in the "File" column for each row. After this commit, a "-" entry appears when `df` is run with no positional arguments and the filename appears when run with positional arguments. For example: $ touch a b c && df --output=target,file a b c Mounted on File / a / b / c --- src/uu/df/src/df.rs | 5 ++++- src/uu/df/src/filesystem.rs | 18 +++++++++++++--- src/uu/df/src/table.rs | 17 +++++++++++++-- tests/by-util/test_df.rs | 43 ++++++++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 934a40a3d..6dd5ad43d 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -243,7 +243,10 @@ fn get_all_filesystems(opt: &Options) -> Vec { // Convert each `MountInfo` into a `Filesystem`, which contains // both the mount information and usage information. - mounts.into_iter().filter_map(Filesystem::new).collect() + mounts + .into_iter() + .filter_map(|m| Filesystem::new(m, None)) + .collect() } /// For each path, get the filesystem that contains that path. diff --git a/src/uu/df/src/filesystem.rs b/src/uu/df/src/filesystem.rs index bd9ff34eb..00b810073 100644 --- a/src/uu/df/src/filesystem.rs +++ b/src/uu/df/src/filesystem.rs @@ -23,6 +23,13 @@ use uucore::fsext::{FsUsage, MountInfo}; /// space available on the filesystem and the amount of space used. #[derive(Debug, Clone)] pub(crate) struct Filesystem { + /// The file given on the command line if any. + /// + /// When invoking `df` with a positional argument, it displays + /// usage information for the filesystem that contains the given + /// file. If given, this field contains that filename. + pub file: Option, + /// Information about the mounted device, mount directory, and related options. pub mount_info: MountInfo, @@ -66,7 +73,7 @@ where impl Filesystem { // TODO: resolve uuid in `mount_info.dev_name` if exists - pub(crate) fn new(mount_info: MountInfo) -> Option { + pub(crate) fn new(mount_info: MountInfo, file: Option) -> Option { let _stat_path = if !mount_info.mount_dir.is_empty() { mount_info.mount_dir.clone() } else { @@ -84,7 +91,11 @@ impl Filesystem { let usage = FsUsage::new(statfs(_stat_path).ok()?); #[cfg(windows)] let usage = FsUsage::new(Path::new(&_stat_path)); - Some(Self { mount_info, usage }) + Some(Self { + mount_info, + usage, + file, + }) } /// Find and create the filesystem that best matches a given path. @@ -107,11 +118,12 @@ impl Filesystem { where P: AsRef, { + let file = path.as_ref().display().to_string(); let canonicalize = true; let mount_info = mount_info_from_path(mounts, path, canonicalize)?; // TODO Make it so that we do not need to clone the `mount_info`. let mount_info = (*mount_info).clone(); - Self::new(mount_info) + Self::new(mount_info, Some(file)) } } diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index 00fe31caf..a99adaa6c 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -24,6 +24,9 @@ use std::ops::AddAssign; /// A row comprises several pieces of information, including the /// filesystem device, the mountpoint, the number of bytes used, etc. pub(crate) struct Row { + /// The filename given on the command-line, if given. + file: Option, + /// Name of the device on which the filesystem lives. fs_device: String, @@ -73,6 +76,7 @@ pub(crate) struct Row { impl Row { pub(crate) fn new(source: &str) -> Self { Self { + file: None, fs_device: source.into(), fs_type: "-".into(), fs_mount: "-".into(), @@ -101,6 +105,7 @@ impl AddAssign for Row { let inodes = self.inodes + rhs.inodes; let inodes_used = self.inodes_used + rhs.inodes_used; *self = Self { + file: None, fs_device: "total".into(), fs_type: "-".into(), fs_mount: "-".into(), @@ -145,6 +150,7 @@ impl From for Row { .. } = fs.usage; Self { + file: fs.file, fs_device: dev_name, fs_type, fs_mount: mount_dir, @@ -246,8 +252,9 @@ impl fmt::Display for DisplayRow<'_> { Column::Ipcent => { write!(f, "{0: >5} ", DisplayRow::percentage(self.row.inodes_usage))?; } - // TODO Implement this. - Column::File => {} + Column::File => { + write!(f, "{0: <16}", self.row.file.as_ref().unwrap_or(&"-".into()))?; + } Column::Fstype => write!(f, "{0: <5} ", self.row.fs_type)?, #[cfg(target_os = "macos")] Column::Capacity => write!( @@ -406,6 +413,7 @@ mod tests { ..Default::default() }; let row = Row { + file: Some("/path/to/file".to_string()), fs_device: "my_device".to_string(), fs_type: "my_type".to_string(), fs_mount: "my_mount".to_string(), @@ -437,6 +445,7 @@ mod tests { ..Default::default() }; let row = Row { + file: Some("/path/to/file".to_string()), fs_device: "my_device".to_string(), fs_type: "my_type".to_string(), fs_mount: "my_mount".to_string(), @@ -468,6 +477,7 @@ mod tests { ..Default::default() }; let row = Row { + file: Some("/path/to/file".to_string()), fs_device: "my_device".to_string(), fs_type: "my_type".to_string(), fs_mount: "my_mount".to_string(), @@ -499,6 +509,7 @@ mod tests { ..Default::default() }; let row = Row { + file: Some("/path/to/file".to_string()), fs_device: "my_device".to_string(), fs_type: "my_type".to_string(), fs_mount: "my_mount".to_string(), @@ -530,6 +541,7 @@ mod tests { ..Default::default() }; let row = Row { + file: Some("/path/to/file".to_string()), fs_device: "my_device".to_string(), fs_type: "my_type".to_string(), fs_mount: "my_mount".to_string(), @@ -560,6 +572,7 @@ mod tests { ..Default::default() }; let row = Row { + file: Some("/path/to/file".to_string()), fs_device: "my_device".to_string(), fs_type: "my_type".to_string(), fs_mount: "my_mount".to_string(), diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 4ba76385c..24277890d 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -222,4 +222,45 @@ fn test_output_selects_columns() { ); } -// ToDO: more tests... +// TODO Fix the spacing. +#[test] +fn test_output_file_all_filesystems() { + // When run with no positional arguments, `df` lets "-" represent + // the "File" entry for each row. + let output = new_ucmd!() + .arg("--output=file") + .succeeds() + .stdout_move_str(); + let mut lines = output.lines(); + assert_eq!(lines.next().unwrap(), "File "); + for line in lines { + assert_eq!(line, "- "); + } +} + +// TODO Fix the spacing. +#[test] +fn test_output_file_specific_files() { + // Create three files. + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("a"); + at.touch("b"); + at.touch("c"); + + // When run with positional arguments, the filesystems should + // appear in the "File" column. + let output = ucmd + .args(&["--output=file", "a", "b", "c"]) + .succeeds() + .stdout_move_str(); + let actual: Vec<&str> = output.lines().collect(); + assert_eq!( + actual, + vec![ + "File ", + "a ", + "b ", + "c " + ] + ); +} From 6f32a1921ae0727eb69a7f204eed0f222e1bd54a Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 27 Mar 2022 22:02:55 -0400 Subject: [PATCH 17/26] df: error on duplicate columns in --output arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Print a usage error when duplicat column names are specified to the `--output` command-line argument. For example, $ df --output=source,source df: option --output: field ‘source’ used more than once Try 'df --help' for more information. --- src/uu/df/src/columns.rs | 60 ++++++++++++++++++++++++++-------------- src/uu/df/src/df.rs | 41 ++++++++++++++++++++++++--- tests/by-util/test_df.rs | 8 ++++++ 3 files changed, 85 insertions(+), 24 deletions(-) diff --git a/src/uu/df/src/columns.rs b/src/uu/df/src/columns.rs index 89dd35220..91c52ed2e 100644 --- a/src/uu/df/src/columns.rs +++ b/src/uu/df/src/columns.rs @@ -55,19 +55,31 @@ pub(crate) enum Column { Capacity, } +/// An error while defining which columns to display in the output table. +#[derive(Debug)] +pub(crate) enum ColumnError { + /// If a column appears more than once in the `--output` argument. + MultipleColumns(String), +} + impl Column { /// Convert from command-line arguments to sequence of columns. /// /// The set of columns that will appear in the output table can be /// specified by command-line arguments. This function converts /// those arguments to a [`Vec`] of [`Column`] variants. - pub(crate) fn from_matches(matches: &ArgMatches) -> Vec { + /// + /// # Errors + /// + /// This function returns an error if a column is specified more + /// than once in the command-line argument. + pub(crate) fn from_matches(matches: &ArgMatches) -> Result, ColumnError> { match ( matches.is_present(OPT_PRINT_TYPE), matches.is_present(OPT_INODES), matches.occurrences_of(OPT_OUTPUT) > 0, ) { - (false, false, false) => vec![ + (false, false, false) => Ok(vec![ Self::Source, Self::Size, Self::Used, @@ -76,29 +88,37 @@ impl Column { Self::Capacity, Self::Pcent, Self::Target, - ], + ]), (false, false, true) => { - matches - .values_of(OPT_OUTPUT) - .unwrap() - .map(|s| { - // Unwrapping here should not panic because the - // command-line argument parsing library should be - // responsible for ensuring each comma-separated - // string is a valid column label. - Self::parse(s).unwrap() - }) - .collect() + // Unwrapping should not panic because in this arm of + // the `match` statement, we know that `OPT_OUTPUT` + // is non-empty. + let names = matches.values_of(OPT_OUTPUT).unwrap(); + let mut seen: Vec<&str> = vec![]; + let mut columns = vec![]; + for name in names { + if seen.contains(&name) { + return Err(ColumnError::MultipleColumns(name.to_string())); + } + seen.push(name); + // Unwrapping here should not panic because the + // command-line argument parsing library should be + // responsible for ensuring each comma-separated + // string is a valid column label. + let column = Self::parse(name).unwrap(); + columns.push(column); + } + Ok(columns) } - (false, true, false) => vec![ + (false, true, false) => Ok(vec![ Self::Source, Self::Itotal, Self::Iused, Self::Iavail, Self::Ipcent, Self::Target, - ], - (true, false, false) => vec![ + ]), + (true, false, false) => Ok(vec![ Self::Source, Self::Fstype, Self::Size, @@ -108,8 +128,8 @@ impl Column { Self::Capacity, Self::Pcent, Self::Target, - ], - (true, true, false) => vec![ + ]), + (true, true, false) => Ok(vec![ Self::Source, Self::Fstype, Self::Itotal, @@ -117,7 +137,7 @@ impl Column { Self::Iavail, Self::Ipcent, Self::Target, - ], + ]), // The command-line arguments -T and -i are each mutually // exclusive with --output, so the command-line argument // parser should reject those combinations before we get diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 6dd5ad43d..0afb55871 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -11,17 +11,19 @@ mod columns; mod filesystem; mod table; -use uucore::error::{UResult, USimpleError}; +use uucore::display::Quotable; +use uucore::error::{UError, UResult}; use uucore::format_usage; use uucore::fsext::{read_fs_list, MountInfo}; use clap::{crate_version, Arg, ArgMatches, Command}; +use std::error::Error; use std::fmt; use std::path::Path; use crate::blocks::{block_size_from_matches, BlockSize}; -use crate::columns::Column; +use crate::columns::{Column, ColumnError}; use crate::filesystem::Filesystem; use crate::table::{DisplayRow, Header, Row}; @@ -103,8 +105,12 @@ impl Default for Options { } } +#[derive(Debug)] enum OptionsError { InvalidBlockSize, + + /// An error getting the columns to display in the output table. + ColumnError(ColumnError), } impl fmt::Display for OptionsError { @@ -115,6 +121,11 @@ impl fmt::Display for OptionsError { // TODO This needs to vary based on whether `--block-size` // or `-B` were provided. Self::InvalidBlockSize => write!(f, "invalid --block-size argument"), + Self::ColumnError(ColumnError::MultipleColumns(s)) => write!( + f, + "option --output: field {} used more than once", + s.quote() + ), } } } @@ -131,7 +142,7 @@ impl Options { include: matches.values_of_lossy(OPT_TYPE), exclude: matches.values_of_lossy(OPT_EXCLUDE_TYPE), show_total: matches.is_present(OPT_TOTAL), - columns: Column::from_matches(matches), + columns: Column::from_matches(matches).map_err(OptionsError::ColumnError)?, }) } } @@ -273,6 +284,28 @@ where .collect() } +#[derive(Debug)] +enum DfError { + /// A problem while parsing command-line options. + OptionsError(OptionsError), +} + +impl Error for DfError {} + +impl UError for DfError { + fn usage(&self) -> bool { + matches!(self, Self::OptionsError(OptionsError::ColumnError(_))) + } +} + +impl fmt::Display for DfError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::OptionsError(e) => e.fmt(f), + } + } +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().get_matches_from(args); @@ -284,7 +317,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } - let opt = Options::from(&matches).map_err(|e| USimpleError::new(1, format!("{}", e)))?; + let opt = Options::from(&matches).map_err(DfError::OptionsError)?; // Get the list of filesystems to display in the output table. let filesystems: Vec = match matches.values_of(OPT_PATHS) { diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 24277890d..69652bed0 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -264,3 +264,11 @@ fn test_output_file_specific_files() { ] ); } + +#[test] +fn test_output_field_no_more_than_once() { + new_ucmd!() + .arg("--output=target,source,target") + .fails() + .usage_error("option --output: field 'target' used more than once"); +} From a1f300e8a72cb8304cc29f246af54ebd4ea2f2cf Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 27 Mar 2022 22:14:16 -0400 Subject: [PATCH 18/26] df: allow multiple occurrences of --output arg Allow multiple occurrences of the `--output` argument. For example, $ df --output=source --output=target | head -n1 Filesystem Mounted on --- src/uu/df/src/df.rs | 1 + tests/by-util/test_df.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 6dd5ad43d..ee2ea83a0 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -389,6 +389,7 @@ pub fn uu_app<'a>() -> Command<'a> { .long("output") .takes_value(true) .use_value_delimiter(true) + .multiple_occurrences(true) .possible_values(OUTPUT_FIELD_LIST) .default_missing_values(&OUTPUT_FIELD_LIST) .default_values(&["source", "size", "used", "avail", "pcent", "target"]) diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 24277890d..4c87fc63e 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -222,6 +222,18 @@ fn test_output_selects_columns() { ); } +#[test] +fn test_output_multiple_occurrences() { + let output = new_ucmd!() + .args(&["--output=source", "--output=target"]) + .succeeds() + .stdout_move_str(); + assert_eq!( + output.lines().next().unwrap(), + "Filesystem Mounted on " + ); +} + // TODO Fix the spacing. #[test] fn test_output_file_all_filesystems() { From a68d77b8cf30de1e7946fb26671a8086d4b76ed2 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Mon, 28 Mar 2022 10:13:54 +0200 Subject: [PATCH 19/26] df: --output w/o "=" doesn't expect further args "df --output ." was treated as "df --output=." and hence "." was interpreted as a column name. With this commit, "." is treated as an argument on its own. Fixes #3324 --- src/uu/df/src/df.rs | 2 ++ tests/by-util/test_df.rs | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index ee2ea83a0..525e2f086 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -388,6 +388,8 @@ pub fn uu_app<'a>() -> Command<'a> { Arg::new(OPT_OUTPUT) .long("output") .takes_value(true) + .min_values(0) + .require_equals(true) .use_value_delimiter(true) .multiple_occurrences(true) .possible_values(OUTPUT_FIELD_LIST) diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 4c87fc63e..b178c40c0 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -80,6 +80,11 @@ fn test_output_option() { new_ucmd!().arg("--output=invalid_option").fails(); } +#[test] +fn test_output_option_without_equals_sign() { + new_ucmd!().arg("--output").arg(".").succeeds(); +} + #[test] fn test_type_option() { new_ucmd!().args(&["-t", "ext4", "-t", "ext3"]).succeeds(); From f6cb42ee2d5379136611d41717c0e0034d42a67c Mon Sep 17 00:00:00 2001 From: DevSabb Date: Mon, 28 Mar 2022 10:17:07 -0400 Subject: [PATCH 20/26] shuf: accept multiple occurances of head-count argument --- src/uu/shuf/src/shuf.rs | 30 ++++++++++++++++++++---------- tests/by-util/test_shuf.rs | 14 ++++++++++++++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/uu/shuf/src/shuf.rs b/src/uu/shuf/src/shuf.rs index 6a3e325c7..5950fe4e4 100644 --- a/src/uu/shuf/src/shuf.rs +++ b/src/uu/shuf/src/shuf.rs @@ -75,17 +75,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; let options = Options { - head_count: match matches.value_of(options::HEAD_COUNT) { - Some(count) => match count.parse::() { + head_count: { + let headcounts = matches + .values_of(options::HEAD_COUNT) + .unwrap_or_default() + .collect(); + match parse_head_count(&headcounts) { Ok(val) => val, - Err(_) => { - return Err(USimpleError::new( - 1, - format!("invalid line count: {}", count.quote()), - )); - } - }, - None => std::usize::MAX, + Err(msg) => return Err(USimpleError::new(1, msg)), + } }, output: matches.value_of(options::OUTPUT).map(String::from), random_source: matches.value_of(options::RANDOM_SOURCE).map(String::from), @@ -152,6 +150,7 @@ pub fn uu_app<'a>() -> Command<'a> { .short('n') .long(options::HEAD_COUNT) .takes_value(true) + .multiple_occurrences(true) .value_name("COUNT") .help("output at most COUNT lines"), ) @@ -299,6 +298,17 @@ fn parse_range(input_range: &str) -> Result<(usize, usize), String> { } } +fn parse_head_count(headcounts: &Vec<&str>) -> Result { + let mut result = std::usize::MAX; + for count in headcounts { + match count.parse::() { + Ok(pv) => result = std::cmp::min(result, pv), + Err(_) => return Err(format!("invalid line count: {}", count.quote())), + } + } + Ok(result) +} + enum WrappedRng { RngFile(rand_read_adapter::ReadRng), RngDefault(rand::rngs::ThreadRng), diff --git a/tests/by-util/test_shuf.rs b/tests/by-util/test_shuf.rs index 86828dc45..dc3355dff 100644 --- a/tests/by-util/test_shuf.rs +++ b/tests/by-util/test_shuf.rs @@ -196,3 +196,17 @@ fn test_shuf_invalid_input_line_count() { .fails() .stderr_contains("invalid line count: 'a'"); } + +#[test] +fn test_shuf_multiple_input_line_count() { + let result = new_ucmd!() + .args(&["-i10-200", "-n", "10", "-n", "5"]) + .succeeds(); + result.no_stderr(); + let result_seq: Vec<&str> = result + .stdout_str() + .split('\n') + .filter(|x| !x.is_empty()) + .collect(); + assert_eq!(result_seq.len(), 5, "Output should have 5 items"); +} From 16ad4bc0693b8ed349e9ee968750913078914999 Mon Sep 17 00:00:00 2001 From: DevSabb Date: Mon, 28 Mar 2022 10:31:27 -0400 Subject: [PATCH 21/26] fix clippy warning --- src/uu/shuf/src/shuf.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/shuf/src/shuf.rs b/src/uu/shuf/src/shuf.rs index 5950fe4e4..64e56a198 100644 --- a/src/uu/shuf/src/shuf.rs +++ b/src/uu/shuf/src/shuf.rs @@ -76,7 +76,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let options = Options { head_count: { - let headcounts = matches + let headcounts: Vec<&str> = matches .values_of(options::HEAD_COUNT) .unwrap_or_default() .collect(); @@ -298,7 +298,7 @@ fn parse_range(input_range: &str) -> Result<(usize, usize), String> { } } -fn parse_head_count(headcounts: &Vec<&str>) -> Result { +fn parse_head_count(headcounts: &[&str]) -> Result { let mut result = std::usize::MAX; for count in headcounts { match count.parse::() { From 68b1f04f7dbbe731c51f3a097f6b22019e15b0ea Mon Sep 17 00:00:00 2001 From: DevSabb Date: Mon, 28 Mar 2022 11:09:26 -0400 Subject: [PATCH 22/26] fix more clippy warnings --- src/uu/shuf/src/shuf.rs | 12 +++++------- tests/by-util/test_shuf.rs | 10 +++++----- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/uu/shuf/src/shuf.rs b/src/uu/shuf/src/shuf.rs index 64e56a198..0b62ec84a 100644 --- a/src/uu/shuf/src/shuf.rs +++ b/src/uu/shuf/src/shuf.rs @@ -7,7 +7,7 @@ // spell-checker:ignore (ToDO) cmdline evec seps rvec fdata -use clap::{crate_version, Arg, Command}; +use clap::{crate_version, Arg, Command, Values}; use rand::prelude::SliceRandom; use rand::RngCore; use std::fs::File; @@ -76,11 +76,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let options = Options { head_count: { - let headcounts: Vec<&str> = matches - .values_of(options::HEAD_COUNT) - .unwrap_or_default() - .collect(); - match parse_head_count(&headcounts) { + let mut headcounts: Values<'_> = + matches.values_of(options::HEAD_COUNT).unwrap_or_default(); + match parse_head_count(&mut headcounts) { Ok(val) => val, Err(msg) => return Err(USimpleError::new(1, msg)), } @@ -298,7 +296,7 @@ fn parse_range(input_range: &str) -> Result<(usize, usize), String> { } } -fn parse_head_count(headcounts: &[&str]) -> Result { +fn parse_head_count(headcounts: &mut Values<'_>) -> Result { let mut result = std::usize::MAX; for count in headcounts { match count.parse::() { diff --git a/tests/by-util/test_shuf.rs b/tests/by-util/test_shuf.rs index dc3355dff..1f67416da 100644 --- a/tests/by-util/test_shuf.rs +++ b/tests/by-util/test_shuf.rs @@ -202,11 +202,11 @@ fn test_shuf_multiple_input_line_count() { let result = new_ucmd!() .args(&["-i10-200", "-n", "10", "-n", "5"]) .succeeds(); - result.no_stderr(); - let result_seq: Vec<&str> = result + result.no_stderr(); + + let result_seq = result .stdout_str() .split('\n') - .filter(|x| !x.is_empty()) - .collect(); - assert_eq!(result_seq.len(), 5, "Output should have 5 items"); + .filter(|x| !x.is_empty()); + assert_eq!(result_seq.count(), 5, "Output should have 5 items"); } From bb64e699ec9355021853ccc8d2cd64bb9fc0b445 Mon Sep 17 00:00:00 2001 From: DevSabb Date: Mon, 28 Mar 2022 11:33:38 -0400 Subject: [PATCH 23/26] fix lint errors --- tests/by-util/test_shuf.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/by-util/test_shuf.rs b/tests/by-util/test_shuf.rs index 1f67416da..defa2d394 100644 --- a/tests/by-util/test_shuf.rs +++ b/tests/by-util/test_shuf.rs @@ -202,11 +202,9 @@ fn test_shuf_multiple_input_line_count() { let result = new_ucmd!() .args(&["-i10-200", "-n", "10", "-n", "5"]) .succeeds(); + result.no_stderr(); - let result_seq = result - .stdout_str() - .split('\n') - .filter(|x| !x.is_empty()); - assert_eq!(result_seq.count(), 5, "Output should have 5 items"); + let result_count = result.stdout_str().split('\n').filter(|x| !x.is_empty()).count(); + assert_eq!(result_count, 5, "Output should have 5 items"); } From eeafdc7021a45a06bc6ed3d43956d3f6ab2c4074 Mon Sep 17 00:00:00 2001 From: DevSabb Date: Mon, 28 Mar 2022 11:36:50 -0400 Subject: [PATCH 24/26] fix lint errors attempt 2 --- tests/by-util/test_shuf.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/by-util/test_shuf.rs b/tests/by-util/test_shuf.rs index defa2d394..682b0dab6 100644 --- a/tests/by-util/test_shuf.rs +++ b/tests/by-util/test_shuf.rs @@ -205,6 +205,10 @@ fn test_shuf_multiple_input_line_count() { result.no_stderr(); - let result_count = result.stdout_str().split('\n').filter(|x| !x.is_empty()).count(); + let result_count = result + .stdout_str() + .split('\n') + .filter(|x| !x.is_empty()) + .count(); assert_eq!(result_count, 5, "Output should have 5 items"); } From e152ebaeadfd9f364c771a121d6a5bd97bc7b891 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Fri, 25 Mar 2022 10:46:13 +0100 Subject: [PATCH 25/26] df: fix calculation of Use% column Change formula from: "Used/Size * 100" to "Used/(Used + Avail) * 100". This formula also works if "Used" and "Avail" do not add up to "Size", which is the case if there are reserved disk blocks. --- src/uu/df/src/table.rs | 10 +++++++--- tests/by-util/test_df.rs | 25 ++++++++----------------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index a99adaa6c..39c762711 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -149,24 +149,28 @@ impl From for Row { ffree, .. } = fs.usage; + let bused = blocks - bfree; Self { file: fs.file, fs_device: dev_name, fs_type, fs_mount: mount_dir, bytes: blocksize * blocks, - bytes_used: blocksize * (blocks - bfree), + bytes_used: blocksize * bused, bytes_avail: blocksize * bavail, bytes_usage: if blocks == 0 { None } else { - Some(((blocks - bfree) as f64) / blocks as f64) + // We use "(bused + bavail)" instead of "blocks" because on some filesystems (e.g. + // ext4) "blocks" also includes reserved blocks we ignore for the usage calculation. + // https://www.gnu.org/software/coreutils/faq/coreutils-faq.html#df-Size-and-Used-and-Available-do-not-add-up + Some((bused as f64) / (bused + bavail) as f64) }, #[cfg(target_os = "macos")] bytes_capacity: if bavail == 0 { None } else { - Some(bavail as f64 / ((blocks - bfree + bavail) as f64)) + Some(bavail as f64 / ((bused + bavail) as f64)) }, inodes: files, inodes_used: files - ffree, diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index b178c40c0..87fd04312 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -1,4 +1,4 @@ -// spell-checker:ignore udev +// spell-checker:ignore udev pcent use crate::common::util::*; #[test] @@ -139,33 +139,24 @@ fn test_total() { #[test] fn test_use_percentage() { - // Example output: - // - // Filesystem 1K-blocks Used Available Use% Mounted on - // udev 3858016 0 3858016 0% /dev - // ... - // /dev/loop14 63488 63488 0 100% /snap/core20/1361 - let output = new_ucmd!().succeeds().stdout_move_str(); + let output = new_ucmd!() + .args(&["--output=used,avail,pcent"]) + .succeeds() + .stdout_move_str(); // Skip the header line. let lines: Vec<&str> = output.lines().skip(1).collect(); for line in lines { let mut iter = line.split_whitespace(); - iter.next(); - let reported_size = iter.next().unwrap().parse::().unwrap(); let reported_used = iter.next().unwrap().parse::().unwrap(); - // Skip "Available" column - iter.next(); - if cfg!(target_os = "macos") { - // Skip "Capacity" column - iter.next(); - } + let reported_avail = iter.next().unwrap().parse::().unwrap(); let reported_percentage = iter.next().unwrap(); let reported_percentage = reported_percentage[..reported_percentage.len() - 1] .parse::() .unwrap(); - let computed_percentage = (100.0 * (reported_used / reported_size)).ceil() as u8; + let computed_percentage = + (100.0 * (reported_used / (reported_used + reported_avail))).ceil() as u8; assert_eq!(computed_percentage, reported_percentage); } From 136803ef3dd824b7f4b14368fed5bc2962214c03 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 28 Mar 2022 23:10:33 +0200 Subject: [PATCH 26/26] makefile: also build basenc --- GNUmakefile | 1 + 1 file changed, 1 insertion(+) diff --git a/GNUmakefile b/GNUmakefile index b3278f9ee..281952736 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -54,6 +54,7 @@ endif PROGS := \ base32 \ base64 \ + basenc \ basename \ cat \ cksum \