From f1d317147b3c8f132164f560a20ec157e7eb5e68 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Sat, 19 Jun 2021 20:26:28 +0200 Subject: [PATCH 01/43] id: add support for showing SELinux context (--context/-Z) --- .github/workflows/CICD.yml | 5 +- .github/workflows/GNU.yml | 9 +- Cargo.lock | 164 ++++++++++++++++++++++++++++++++++++- Cargo.toml | 1 + src/uu/id/Cargo.toml | 1 + src/uu/id/src/id.rs | 57 +++++++++++-- tests/by-util/test_id.rs | 101 +++++++++++++++++++++-- 7 files changed, 315 insertions(+), 23 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index fcaddd310..6626c2e1e 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -11,7 +11,7 @@ env: PROJECT_NAME: coreutils PROJECT_DESC: "Core universal (cross-platform) utilities" PROJECT_AUTH: "uutils" - RUST_MIN_SRV: "1.43.1" ## v1.43.0 + RUST_MIN_SRV: "1.51.0" ## v1.43.0 RUST_COV_SRV: "2020-08-01" ## (~v1.47.0) supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel on: [push, pull_request] @@ -222,7 +222,7 @@ jobs: - name: "Run make build" shell: bash run: | - sudo apt-get -y update ; sudo apt-get -y install python3-sphinx; + sudo apt-get -y update ; sudo apt-get -y install python3-sphinx libselinux1 libselinux1-dev ; make build build: @@ -260,6 +260,7 @@ jobs: esac case '${{ matrix.job.os }}' in macos-latest) brew install coreutils ;; # needed for testing + ubuntu-latest) sudo apt-get -y update ; sudo apt-get -y install libselinux1 libselinux1-dev ;; # TODO: probably redundant here esac - name: Initialize workflow variables id: vars diff --git a/.github/workflows/GNU.yml b/.github/workflows/GNU.yml index 1f9250900..570217865 100644 --- a/.github/workflows/GNU.yml +++ b/.github/workflows/GNU.yml @@ -45,7 +45,14 @@ jobs: - name: Run GNU tests shell: bash run: | - bash uutils/util/run-gnu-test.sh + # bash uutils/util/run-gnu-test.sh # TODO: revert after testing + bash uutils/util/run-gnu-test.sh tests/id/context.sh + bash uutils/util/run-gnu-test.sh tests/id/no-context.sh + bash uutils/util/run-gnu-test.sh tests/id/smack.sh + bash uutils/util/run-gnu-test.sh tests/id/uid.sh + bash uutils/util/run-gnu-test.sh tests/id/setgid.sh + bash uutils/util/run-gnu-test.sh tests/id/zero.sh + bash uutils/util/run-gnu-test.sh tests/id/gnu-zero-uids.sh - name: Extract tests info shell: bash run: | diff --git a/Cargo.lock b/Cargo.lock index a059c1cd5..d70b94de6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -71,6 +71,29 @@ dependencies = [ "compare", ] +[[package]] +name = "bindgen" +version = "0.58.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f8523b410d7187a43085e7e064416ea32ded16bd0a4e6fc025e21616d01258f" +dependencies = [ + "bitflags", + "cexpr", + "clang-sys", + "clap", + "env_logger 0.8.4", + "lazy_static", + "lazycell", + "log", + "peeking_take_while", + "proc-macro2", + "quote 1.0.9", + "regex", + "rustc-hash", + "shlex", + "which", +] + [[package]] name = "bit-set" version = "0.5.2" @@ -142,6 +165,15 @@ version = "1.0.68" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a72c244c1ff497a746a7e1fb3d14bd08420ecda70c8f25c7112f2781652d787" +[[package]] +name = "cexpr" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4aedb84272dbe89af497cf81375129abda4fc0a9e7c5d317498c15cc30c0d27" +dependencies = [ + "nom", +] + [[package]] name = "cfg-if" version = "0.1.10" @@ -167,6 +199,17 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "clang-sys" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "853eda514c284c2287f4bf20ae614f8781f40a81d32ecda6e91449304dfe077c" +dependencies = [ + "glob 0.3.0", + "libc", + "libloading", +] + [[package]] name = "clap" version = "2.33.3" @@ -228,6 +271,7 @@ dependencies = [ "rand 0.7.3", "regex", "rlimit", + "selinux", "sha1", "tempfile", "textwrap", @@ -578,6 +622,19 @@ dependencies = [ "regex", ] +[[package]] +name = "env_logger" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3" +dependencies = [ + "atty", + "humantime", + "log", + "regex", + "termcolor", +] + [[package]] name = "fake-simd" version = "0.1.2" @@ -727,6 +784,12 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "humantime" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a3a5bfb195931eeb336b2a7b4d761daec841b97f947d34394601737a7bba5e4" + [[package]] name = "if_rust_version" version = "1.0.0" @@ -782,12 +845,28 @@ version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" +[[package]] +name = "lazycell" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" + [[package]] name = "libc" version = "0.2.85" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ccac4b00700875e6a07c6cde370d44d32fa01c5a65cdd2fca6858c479d28bb3" +[[package]] +name = "libloading" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f84d96438c15fcd6c3f244c8fce01d1e2b9c6b5623e9c711dc9286d8fc92d6a" +dependencies = [ + "cfg-if 1.0.0", + "winapi 0.3.9", +] + [[package]] name = "locale" version = "0.2.2" @@ -919,6 +998,16 @@ version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72ef4a56884ca558e5ddb05a1d1e7e1bfd9a68d9ed024c21704cc98872dae1bb" +[[package]] +name = "nom" +version = "5.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ffb4262d26ed83a1c0a33a38fe2bb15797329c85770da05e6b828ddb782627af" +dependencies = [ + "memchr 2.4.0", + "version_check", +] + [[package]] name = "ntapi" version = "0.3.6" @@ -982,9 +1071,9 @@ checksum = "b8f8bdf33df195859076e54ab11ee78a1b208382d3a26ec40d142ffc1ecc49ef" [[package]] name = "once_cell" -version = "1.7.2" +version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3" +checksum = "692fcb63b64b1758029e0a96ee63e049ce8c5948587f2f7208df04625e5f6b56" [[package]] name = "onig" @@ -1084,6 +1173,12 @@ dependencies = [ "proc-macro-hack", ] +[[package]] +name = "peeking_take_while" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "19b17cddbe7ec3f8bc800887bab5e717348c95ea2ca0b1bf0837fb964dc67099" + [[package]] name = "pkg-config" version = "0.3.19" @@ -1175,7 +1270,7 @@ version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a44883e74aa97ad63db83c4bf8ca490f02b2fc02f92575e720c8551e843c945f" dependencies = [ - "env_logger", + "env_logger 0.7.1", "log", "rand 0.7.3", "rand_core 0.5.1", @@ -1364,6 +1459,12 @@ dependencies = [ "redox_syscall 0.2.8", ] +[[package]] +name = "reference-counted-singleton" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e03aec30be874d0786379a6ee483c653c58dd6863ad4ed873e697ed968f9358" + [[package]] name = "regex" version = "1.5.4" @@ -1418,6 +1519,12 @@ version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3e52c148ef37f8c375d49d5a73aa70713125b7f19095948a923f80afdeb22ec2" +[[package]] +name = "rustc-hash" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" + [[package]] name = "same-file" version = "1.0.6" @@ -1433,6 +1540,32 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" +[[package]] +name = "selinux" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5e36fd4d2d189b51696bcff8d45a8defe6d9c3a353a6b1449ccc46d22a81d8e" +dependencies = [ + "bitflags", + "libc", + "once_cell", + "reference-counted-singleton", + "selinux-sys", + "thiserror", +] + +[[package]] +name = "selinux-sys" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9dc2e3e97d611bf255de85a72a385565d1f6340bc6fc6fd6402555b54382a9f" +dependencies = [ + "bindgen", + "cc", + "dunce", + "walkdir", +] + [[package]] name = "semver" version = "0.9.0" @@ -1479,6 +1612,12 @@ dependencies = [ "generic-array", ] +[[package]] +name = "shlex" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42a568c8f2cd051a4d283bd6eb0343ac214c1b0f1ac19f93e1175b2dee38c73d" + [[package]] name = "signal-hook" version = "0.1.17" @@ -1599,6 +1738,15 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "termcolor" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" +dependencies = [ + "winapi-util", +] + [[package]] name = "termion" version = "1.5.6" @@ -2073,6 +2221,7 @@ name = "uu_id" version = "0.0.6" dependencies = [ "clap", + "selinux", "uucore", "uucore_procs", ] @@ -2830,6 +2979,15 @@ version = "0.10.2+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" +[[package]] +name = "which" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d011071ae14a2f6671d0b74080ae0cd8ebf3a6f8c9589a2cd45f23126fe29724" +dependencies = [ + "libc", +] + [[package]] name = "wild" version = "2.0.4" diff --git a/Cargo.toml b/Cargo.toml index 804c5f978..06e6ed505 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -352,6 +352,7 @@ unindent = "0.1" uucore = { version=">=0.0.8", package="uucore", path="src/uucore", features=["entries", "process"] } walkdir = "2.2" atty = "0.2" +selinux = "0.1.1" [target.'cfg(unix)'.dev-dependencies] rlimit = "0.4.0" diff --git a/src/uu/id/Cargo.toml b/src/uu/id/Cargo.toml index 308d6089d..a14422924 100644 --- a/src/uu/id/Cargo.toml +++ b/src/uu/id/Cargo.toml @@ -18,6 +18,7 @@ path = "src/id.rs" clap = "2.33" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "process"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } +selinux = "0.1.1" [[bin]] name = "id" diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index 9037745eb..0e29536e3 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -26,7 +26,7 @@ // * Help text based on BSD's `id` manpage and GNU's `id` manpage. // -// spell-checker:ignore (ToDO) asid auditid auditinfo auid cstr egid emod euid getaudit getlogin gflag nflag pline rflag termid uflag gsflag zflag testsuite +// spell-checker:ignore (ToDO) asid auditid auditinfo auid cstr egid emod euid getaudit getlogin gflag nflag pline rflag termid uflag gsflag zflag cflag testsuite #![allow(non_camel_case_types)] #![allow(dead_code)] @@ -35,6 +35,8 @@ extern crate uucore; use clap::{crate_version, App, Arg}; +#[cfg(target_os = "linux")] +use selinux::{self, KernelSupport, SecurityContext}; use std::ffi::CStr; use uucore::entries::{self, Group, Locate, Passwd}; pub use uucore::libc; @@ -93,6 +95,8 @@ struct State { gsflag: bool, // --groups rflag: bool, // --real zflag: bool, // --zero + cflag: bool, // --context + selinux_supported: bool, ids: Option, // The behavior for calling GNU's `id` and calling GNU's `id $USER` is similar but different. // * The SELinux context is only displayed without a specified user. @@ -147,6 +151,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::OPT_GROUP) .short("g") .long(options::OPT_GROUP) + .conflicts_with(options::OPT_EFFECTIVE_USER) .help("Display only the effective group ID as a number"), ) .arg( @@ -156,6 +161,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .conflicts_with_all(&[ options::OPT_GROUP, options::OPT_EFFECTIVE_USER, + options::OPT_CONTEXT, options::OPT_HUMAN_READABLE, options::OPT_PASSWORD, options::OPT_AUDIT, @@ -207,7 +213,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::OPT_CONTEXT) .short("Z") .long(options::OPT_CONTEXT) - .help("NotImplemented: print only the security context of the process"), + .conflicts_with_all(&[options::OPT_GROUP, options::OPT_EFFECTIVE_USER]) + .help("print only the security context of the process"), ) .arg( Arg::with_name(options::ARG_USERS) @@ -229,6 +236,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { gsflag: matches.is_present(options::OPT_GROUPS), rflag: matches.is_present(options::OPT_REAL_ID), zflag: matches.is_present(options::OPT_ZERO), + cflag: matches.is_present(options::OPT_CONTEXT), + + #[cfg(not(target_os = "linux"))] + selinux_supported: false, + #[cfg(target_os = "linux")] + selinux_supported: selinux::kernel_support() != KernelSupport::Unsupported, user_specified: !users.is_empty(), ids: None, }; @@ -238,13 +251,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { !(state.uflag || state.gflag || state.gsflag) }; - if (state.nflag || state.rflag) && default_format { + if (state.nflag || state.rflag) && default_format && !state.cflag { crash!(1, "cannot print only names or real IDs in default format"); } - if (state.zflag) && default_format { + if state.zflag && default_format && !state.cflag { // NOTE: GNU testsuite "id/zero.sh" needs this stderr output: crash!(1, "option --zero not permitted in default format"); } + if state.user_specified && state.cflag { + crash!(1, "cannot print security context when user specified"); + } let delimiter = { if state.zflag { @@ -262,6 +278,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; let mut exit_code = 0; + if state.cflag { + if state.selinux_supported { + // print SElinux context and exit + #[cfg(target_os = "linux")] + if let Ok(context) = SecurityContext::current(false) { + let bytes = context.as_bytes(); + print!("{}{}", String::from_utf8_lossy(bytes), line_ending); + } else { + // print error because `cflag` was explicitly requested + crash!(1, "can't get process context"); + } + return exit_code; + } else { + crash!(1, "--context (-Z) works only on an SELinux-enabled kernel"); + } + } + for i in 0..=users.len() { let possible_pw = if !state.user_specified { None @@ -518,11 +551,17 @@ fn id_print(state: &State, groups: Vec) { .join(",") ); - // NOTE: (SELinux NotImplemented) placeholder: - // if !state.user_specified { - // // print SElinux context (does not depend on "-Z") - // print!(" context={}", get_selinux_contexts().join(":")); - // } + #[cfg(target_os = "linux")] + if state.selinux_supported + && !state.user_specified + && std::env::var_os("POSIXLY_CORRECT").is_none() + { + // print SElinux context (does not depend on "-Z") + if let Ok(context) = SecurityContext::current(false) { + let bytes = context.as_bytes(); + print!(" context={}", String::from_utf8_lossy(bytes)); + } + } } #[cfg(not(target_os = "linux"))] diff --git a/tests/by-util/test_id.rs b/tests/by-util/test_id.rs index b4b929a2c..bbb9533af 100644 --- a/tests/by-util/test_id.rs +++ b/tests/by-util/test_id.rs @@ -52,14 +52,6 @@ fn test_id_no_specified_user() { let exp_result = unwrap_or_return!(expected_result(&[])); let mut _exp_stdout = exp_result.stdout_str().to_string(); - #[cfg(target_os = "linux")] - { - // NOTE: (SELinux NotImplemented) strip 'context' part from exp_stdout: - if let Some(context_offset) = exp_result.stdout_str().find(" context=") { - _exp_stdout.replace_range(context_offset.._exp_stdout.len() - 1, ""); - } - } - result .stdout_is(_exp_stdout) .stderr_is(exp_result.stderr_str()) @@ -425,6 +417,99 @@ fn test_id_zero() { } } +#[test] +#[cfg(target_os = "linux")] +fn test_id_context() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!( + "{}: test skipped: Kernel has no support for SElinux context", + UUTILS_INFO + ); + return; + } + let scene = TestScenario::new(util_name!()); + for c_flag in &["-Z", "--context"] { + scene + .ucmd() + .args(&[c_flag]) + .succeeds() + .stdout_only(unwrap_or_return!(expected_result(&[c_flag])).stdout_str()); + for &z_flag in &["-z", "--zero"] { + let args = [c_flag, z_flag]; + scene + .ucmd() + .args(&args) + .succeeds() + .stdout_only(unwrap_or_return!(expected_result(&args)).stdout_str()); + for &opt1 in &["--name", "--real"] { + // id: cannot print only names or real IDs in default format + let args = [opt1, c_flag]; + scene + .ucmd() + .args(&args) + .succeeds() + .stdout_only(unwrap_or_return!(expected_result(&args)).stdout_str()); + let args = [opt1, c_flag, z_flag]; + scene + .ucmd() + .args(&args) + .succeeds() + .stdout_only(unwrap_or_return!(expected_result(&args)).stdout_str()); + for &opt2 in &["--user", "--group", "--groups"] { + // u/g/G n/r z Z + // for now, we print clap's standard response for "conflicts_with" instead of: + // id: cannot print "only" of more than one choice + let args = [opt2, c_flag, opt1]; + let _result = scene.ucmd().args(&args).fails(); + // let exp_result = unwrap_or_return!(expected_result(&args)); + // result + // .stdout_is(exp_result.stdout_str()) + // .stderr_is(exp_result.stderr_str()) + // .code_is(exp_result.code()); + } + } + for &opt2 in &["--user", "--group", "--groups"] { + // u/g/G z Z + // for now, we print clap's standard response for "conflicts_with" instead of: + // id: cannot print "only" of more than one choice + let args = [opt2, c_flag]; + let _result = scene.ucmd().args(&args).fails(); + // let exp_result = unwrap_or_return!(expected_result(&args)); + // result + // .stdout_is(exp_result.stdout_str()) + // .stderr_is(exp_result.stderr_str()) + // .code_is(exp_result.code()); + } + } + } +} + +#[test] +#[cfg(unix)] +fn test_id_no_specified_user_posixly() { + // gnu/tests/id/no-context.sh + + let scene = TestScenario::new(util_name!()); + let result = scene.ucmd().env("POSIXLY_CORRECT", "1").succeeds(); + assert!(!result.stdout_str().contains("context=")); + + #[cfg(target_os = "linux")] + { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!( + "{}: test skipped: Kernel has no support for SElinux context", + UUTILS_INFO + ); + return; + } else { + let result = scene.ucmd().succeeds(); + assert!(result.stdout_str().contains("context=")); + } + } +} + fn check_coreutil_version(util_name: &str, version_expected: &str) -> String { // example: // $ id --version | head -n 1 From bd0ca4513e82fa263d4a4f0a91ac3c049d9b456a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 27 Jun 2021 07:52:07 +0200 Subject: [PATCH 02/43] update min rust to 1.51 Co-authored-by: Roy Ivy III --- .github/workflows/CICD.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 6626c2e1e..0b245da74 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -11,8 +11,8 @@ env: PROJECT_NAME: coreutils PROJECT_DESC: "Core universal (cross-platform) utilities" PROJECT_AUTH: "uutils" - RUST_MIN_SRV: "1.51.0" ## v1.43.0 - RUST_COV_SRV: "2020-08-01" ## (~v1.47.0) supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel + RUST_MIN_SRV: "1.51.0" ## v1.51.0 + RUST_COV_SRV: "1.51.0" ## supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel on: [push, pull_request] From 7abc6a665ef8d8be53ed5ef9e2678fcea040cd0c Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 12 Jul 2021 17:33:24 +0200 Subject: [PATCH 03/43] id: add conditional compilation for selinux --- .github/workflows/CICD.yml | 10 +++++++--- Cargo.toml | 3 ++- build.rs | 4 ++-- src/uu/id/Cargo.toml | 5 ++++- src/uu/id/src/id.rs | 33 ++++++++++++++++++++++----------- tests/by-util/test_id.rs | 15 +++++++++++++-- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 80d03e257..9abfcaa81 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -13,8 +13,8 @@ env: PROJECT_NAME: coreutils PROJECT_DESC: "Core universal (cross-platform) utilities" PROJECT_AUTH: "uutils" - RUST_MIN_SRV: "1.51.0" ## v1.51.0 - RUST_COV_SRV: "1.51.0" ## supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel + RUST_MIN_SRV: "1.51.0" ## MSRV v1.51.0 + RUST_COV_SRV: "2020-07-01" ## (~v1.47.0) supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel on: [push, pull_request] @@ -249,6 +249,8 @@ jobs: # { os, target, cargo-options, features, use-cross, toolchain } - { os: ubuntu-latest , target: arm-unknown-linux-gnueabihf , features: feat_os_unix_gnueabihf , use-cross: use-cross } - { os: ubuntu-latest , target: aarch64-unknown-linux-gnu , features: feat_os_unix_gnueabihf , use-cross: use-cross } + - { os: ubuntu-latest , target: x86_64-unknown-linux-gnu , features: feat_os_unix , use-cross: use-cross } + # - { os: ubuntu-latest , target: x86_64-unknown-linux-gnu , features: feat_selinux , use-cross: use-cross } # - { os: ubuntu-18.04 , target: i586-unknown-linux-gnu , features: feat_os_unix , use-cross: use-cross } ## note: older windows platform; not required, dev-FYI only # - { os: ubuntu-18.04 , target: i586-unknown-linux-gnu , features: feat_os_unix , use-cross: use-cross } ## note: older windows platform; not required, dev-FYI only - { os: ubuntu-18.04 , target: i686-unknown-linux-gnu , features: feat_os_unix , use-cross: use-cross } @@ -272,7 +274,9 @@ jobs: esac case '${{ matrix.job.os }}' in macos-latest) brew install coreutils ;; # needed for testing - ubuntu-latest) sudo apt-get -y update ; sudo apt-get -y install libselinux1 libselinux1-dev ;; # TODO: probably redundant here + esac + case '${{ matrix.job.features }}' in + feat_selinux) sudo apt-get -y update ; sudo apt-get -y install libselinux1-dev ;; # TODO: is here the right place for this? esac - name: Initialize workflow variables id: vars diff --git a/Cargo.toml b/Cargo.toml index d11b57bfe..c588f735e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -146,6 +146,7 @@ feat_os_unix_musl = [ feat_require_crate_cpp = [ "stdbuf", ] +feat_selinux = ["id/selinux", "selinux"] # "feat_require_unix" == set of utilities requiring support which is only available on unix platforms (as of 2020-04-23) feat_require_unix = [ "chgrp", @@ -229,6 +230,7 @@ clap = { version = "2.33", features = ["wrap_help"] } lazy_static = { version="1.3" } textwrap = { version="=0.11.0", features=["term_size"] } # !maint: [2020-05-10; rivy] unstable crate using undocumented features; pinned currently, will review uucore = { version=">=0.0.9", package="uucore", path="src/uucore" } +selinux = { version="0.1.1", optional = true } # * uutils uu_test = { optional=true, version="0.0.7", package="uu_test", path="src/uu/test" } # @@ -353,7 +355,6 @@ unindent = "0.1" uucore = { version=">=0.0.9", package="uucore", path="src/uucore", features=["entries", "process"] } walkdir = "2.2" atty = "0.2" -selinux = "0.1.1" [target.'cfg(unix)'.dev-dependencies] rlimit = "0.4.0" diff --git a/build.rs b/build.rs index e9fe129eb..4fbb27cce 100644 --- a/build.rs +++ b/build.rs @@ -28,8 +28,8 @@ pub fn main() { if val == "1" && key.starts_with(env_feature_prefix) { let krate = key[env_feature_prefix.len()..].to_lowercase(); match krate.as_ref() { - "default" | "macos" | "unix" | "windows" => continue, // common/standard feature names - "nightly" | "test_unimplemented" => continue, // crate-local custom features + "default" | "macos" | "unix" | "windows" | "selinux" => continue, // common/standard feature names + "nightly" | "test_unimplemented" => continue, // crate-local custom features "test" => continue, // over-ridden with 'uu_test' to avoid collision with rust core crate 'test' s if s.starts_with(feature_prefix) => continue, // crate feature sets _ => {} // util feature name diff --git a/src/uu/id/Cargo.toml b/src/uu/id/Cargo.toml index 5a99c8431..b82c0e0f0 100644 --- a/src/uu/id/Cargo.toml +++ b/src/uu/id/Cargo.toml @@ -18,8 +18,11 @@ path = "src/id.rs" clap = { version = "2.33", features = ["wrap_help"] } uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["entries", "process"] } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } -selinux = "0.1.1" +selinux = { version="0.1.1", optional = true } [[bin]] name = "id" path = "src/main.rs" + +[features] +feat_selinux = ["selinux"] diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index b220bfdba..ce44b513d 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -35,8 +35,8 @@ extern crate uucore; use clap::{crate_version, App, Arg}; -#[cfg(target_os = "linux")] -use selinux::{self, KernelSupport, SecurityContext}; +#[cfg(all(target_os = "linux", feature = "selinux"))] +use selinux; use std::ffi::CStr; use uucore::entries::{self, Group, Locate, Passwd}; pub use uucore::libc; @@ -52,6 +52,11 @@ macro_rules! cstr2cow { static ABOUT: &str = "Print user and group information for each specified USER, or (when USER omitted) for the current user."; +#[cfg(not(feature = "selinux"))] +static CONTEXT_HELP_TEXT: &str = "print only the security context of the process (not enabled)"; +#[cfg(feature = "selinux")] +static CONTEXT_HELP_TEXT: &str = "print only the security context of the process"; + mod options { pub const OPT_AUDIT: &str = "audit"; // GNU's id does not have this pub const OPT_CONTEXT: &str = "context"; @@ -138,10 +143,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { zflag: matches.is_present(options::OPT_ZERO), cflag: matches.is_present(options::OPT_CONTEXT), - #[cfg(not(target_os = "linux"))] - selinux_supported: false, - #[cfg(target_os = "linux")] - selinux_supported: selinux::kernel_support() != KernelSupport::Unsupported, + selinux_supported: { + #[cfg(feature = "selinux")] + { + selinux::kernel_support() != selinux::KernelSupport::Unsupported + } + #[cfg(not(feature = "selinux"))] + { + false + } + }, user_specified: !users.is_empty(), ids: None, }; @@ -181,8 +192,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if state.cflag { if state.selinux_supported { // print SElinux context and exit - #[cfg(target_os = "linux")] - if let Ok(context) = SecurityContext::current(false) { + #[cfg(all(target_os = "linux", feature = "selinux"))] + if let Ok(context) = selinux::SecurityContext::current(false) { let bytes = context.as_bytes(); print!("{}{}", String::from_utf8_lossy(bytes), line_ending); } else { @@ -412,7 +423,7 @@ pub fn uu_app() -> App<'static, 'static> { .short("Z") .long(options::OPT_CONTEXT) .conflicts_with_all(&[options::OPT_GROUP, options::OPT_EFFECTIVE_USER]) - .help("print only the security context of the process"), + .help(CONTEXT_HELP_TEXT), ) .arg( Arg::with_name(options::ARG_USERS) @@ -555,13 +566,13 @@ fn id_print(state: &State, groups: Vec) { .join(",") ); - #[cfg(target_os = "linux")] + #[cfg(all(target_os = "linux", feature = "selinux"))] if state.selinux_supported && !state.user_specified && std::env::var_os("POSIXLY_CORRECT").is_none() { // print SElinux context (does not depend on "-Z") - if let Ok(context) = SecurityContext::current(false) { + if let Ok(context) = selinux::SecurityContext::current(false) { let bytes = context.as_bytes(); print!(" context={}", String::from_utf8_lossy(bytes)); } diff --git a/tests/by-util/test_id.rs b/tests/by-util/test_id.rs index eeb12f28b..c6e67a923 100644 --- a/tests/by-util/test_id.rs +++ b/tests/by-util/test_id.rs @@ -17,6 +17,17 @@ fn test_id_no_specified_user() { let exp_result = unwrap_or_return!(expected_result(&ts, &[])); let mut _exp_stdout = exp_result.stdout_str().to_string(); + #[cfg(not(feature = "feat_selinux"))] + { + // NOTE: strip 'context' part from exp_stdout if selinux not enabled: + // example: + // uid=1001(runner) gid=121(docker) groups=121(docker),4(adm),101(systemd-journal) \ + // context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 + if let Some(context_offset) = exp_result.stdout_str().find(" context=") { + _exp_stdout.replace_range(context_offset.._exp_stdout.len() - 1, ""); + } + } + result .stdout_is(_exp_stdout) .stderr_is(exp_result.stderr_str()) @@ -354,7 +365,7 @@ fn test_id_zero() { } #[test] -#[cfg(target_os = "linux")] +#[cfg(feature = "feat_selinux")] fn test_id_context() { use selinux::{self, KernelSupport}; if selinux::kernel_support() == KernelSupport::Unsupported { @@ -423,7 +434,7 @@ fn test_id_no_specified_user_posixly() { let result = ts.ucmd().env("POSIXLY_CORRECT", "1").succeeds(); assert!(!result.stdout_str().contains("context=")); - #[cfg(target_os = "linux")] + #[cfg(all(target_os = "linux", feature = "feat_selinux"))] { use selinux::{self, KernelSupport}; if selinux::kernel_support() == KernelSupport::Unsupported { From 36a192c5f62f27ea6fc07e8b63e27bd3320de044 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Tue, 13 Jul 2021 10:40:07 +0200 Subject: [PATCH 04/43] id: fix error handling for id_print() --- src/uu/id/src/id.rs | 75 +++++++++++++++++++++++++++++++--------- tests/by-util/test_id.rs | 5 ++- 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index ce44b513d..929b00dba 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -118,6 +118,7 @@ struct State { // 1000 10 968 975 // +++ exited with 0 +++ user_specified: bool, + exit_code: i32, } pub fn uumain(args: impl uucore::Args) -> i32 { @@ -155,6 +156,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }, user_specified: !users.is_empty(), ids: None, + exit_code: 0, }; let default_format = { @@ -187,7 +189,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { '\n' } }; - let mut exit_code = 0; if state.cflag { if state.selinux_supported { @@ -200,7 +201,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // print error because `cflag` was explicitly requested crash!(1, "can't get process context"); } - return exit_code; + return state.exit_code; } else { crash!(1, "--context (-Z) works only on an SELinux-enabled kernel"); } @@ -214,7 +215,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Ok(p) => Some(p), Err(_) => { show_error!("'{}': no such user", users[i]); - exit_code = 1; + state.exit_code = 1; if i + 1 >= users.len() { break; } else { @@ -228,17 +229,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if matches.is_present(options::OPT_PASSWORD) { // BSD's `id` ignores all but the first specified user pline(possible_pw.map(|v| v.uid())); - return exit_code; + return state.exit_code; }; if matches.is_present(options::OPT_HUMAN_READABLE) { // BSD's `id` ignores all but the first specified user pretty(possible_pw); - return exit_code; + return state.exit_code; } if matches.is_present(options::OPT_AUDIT) { // BSD's `id` ignores specified users auditid(); - return exit_code; + return state.exit_code; } let (uid, gid) = possible_pw.map(|p| (p.uid(), p.gid())).unwrap_or(( @@ -258,7 +259,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if state.nflag { entries::gid2grp(gid).unwrap_or_else(|_| { show_error!("cannot find name for group ID {}", gid); - exit_code = 1; + state.exit_code = 1; gid.to_string() }) } else { @@ -273,7 +274,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if state.nflag { entries::uid2usr(uid).unwrap_or_else(|_| { show_error!("cannot find name for user ID {}", uid); - exit_code = 1; + state.exit_code = 1; uid.to_string() }) } else { @@ -298,7 +299,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if state.nflag { entries::gid2grp(id).unwrap_or_else(|_| { show_error!("cannot find name for group ID {}", id); - exit_code = 1; + state.exit_code = 1; id.to_string() }) } else { @@ -317,7 +318,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } if default_format { - id_print(&state, groups); + id_print(&mut state, groups); } print!("{}", line_ending); @@ -326,7 +327,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } - exit_code + state.exit_code } pub fn uu_app() -> App<'static, 'static> { @@ -543,25 +544,65 @@ fn auditid() { println!("asid={}", auditinfo.ai_asid); } -fn id_print(state: &State, groups: Vec) { +fn id_print(state: &mut State, groups: Vec) { let uid = state.ids.as_ref().unwrap().uid; let gid = state.ids.as_ref().unwrap().gid; let euid = state.ids.as_ref().unwrap().euid; let egid = state.ids.as_ref().unwrap().egid; - print!("uid={}({})", uid, entries::uid2usr(uid).unwrap()); - print!(" gid={}({})", gid, entries::gid2grp(gid).unwrap()); + print!( + "uid={}({})", + uid, + entries::uid2usr(uid).unwrap_or_else(|_| { + show_error!("cannot find name for user ID {}", uid); + state.exit_code = 1; + uid.to_string() + }) + ); + print!( + " gid={}({})", + gid, + entries::gid2grp(gid).unwrap_or_else(|_| { + show_error!("cannot find name for group ID {}", gid); + state.exit_code = 1; + gid.to_string() + }) + ); if !state.user_specified && (euid != uid) { - print!(" euid={}({})", euid, entries::uid2usr(euid).unwrap()); + print!( + " euid={}({})", + euid, + entries::uid2usr(euid).unwrap_or_else(|_| { + show_error!("cannot find name for user ID {}", euid); + state.exit_code = 1; + euid.to_string() + }) + ); } if !state.user_specified && (egid != gid) { - print!(" egid={}({})", euid, entries::gid2grp(egid).unwrap()); + print!( + " egid={}({})", + euid, + entries::gid2grp(egid).unwrap_or_else(|_| { + show_error!("cannot find name for group ID {}", egid); + state.exit_code = 1; + egid.to_string() + }) + ); } print!( " groups={}", groups .iter() - .map(|&gr| format!("{}({})", gr, entries::gid2grp(gr).unwrap())) + .map(|&gr| format!( + "{}({})", + gr, + entries::gid2grp(gr).unwrap_or_else(|_| { + show_error!("cannot find name for group ID {}", gr); + state.exit_code = 1; + gr.to_string() + }) + )) .collect::>() .join(",") ); diff --git a/tests/by-util/test_id.rs b/tests/by-util/test_id.rs index c6e67a923..db918aa33 100644 --- a/tests/by-util/test_id.rs +++ b/tests/by-util/test_id.rs @@ -431,8 +431,11 @@ fn test_id_no_specified_user_posixly() { // gnu/tests/id/no-context.sh let ts = TestScenario::new(util_name!()); - let result = ts.ucmd().env("POSIXLY_CORRECT", "1").succeeds(); + let result = ts.ucmd().env("POSIXLY_CORRECT", "1").run(); assert!(!result.stdout_str().contains("context=")); + if !is_ci() { + result.success(); + } #[cfg(all(target_os = "linux", feature = "feat_selinux"))] { From 6111cd6e1b4ce7060769143c029cb93d55b25a39 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Tue, 13 Jul 2021 12:24:03 +0200 Subject: [PATCH 05/43] id: add note about conditional compiling to README section --- .github/workflows/CICD.yml | 5 +---- .github/workflows/GnuTests.yml | 9 +-------- Cargo.toml | 6 +++++- src/uu/id/src/id.rs | 11 ++++++++--- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 9abfcaa81..9fd08b26f 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -14,7 +14,7 @@ env: PROJECT_DESC: "Core universal (cross-platform) utilities" PROJECT_AUTH: "uutils" RUST_MIN_SRV: "1.51.0" ## MSRV v1.51.0 - RUST_COV_SRV: "2020-07-01" ## (~v1.47.0) supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel + RUST_COV_SRV: "2020-08-01" ## (~v1.47.0) supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel on: [push, pull_request] @@ -275,9 +275,6 @@ jobs: case '${{ matrix.job.os }}' in macos-latest) brew install coreutils ;; # needed for testing esac - case '${{ matrix.job.features }}' in - feat_selinux) sudo apt-get -y update ; sudo apt-get -y install libselinux1-dev ;; # TODO: is here the right place for this? - esac - name: Initialize workflow variables id: vars shell: bash diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index fb6ffc0ac..8bf6c091b 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -48,14 +48,7 @@ jobs: - name: Run GNU tests shell: bash run: | - # bash uutils/util/run-gnu-test.sh # TODO: revert after testing - bash uutils/util/run-gnu-test.sh tests/id/context.sh - bash uutils/util/run-gnu-test.sh tests/id/no-context.sh - bash uutils/util/run-gnu-test.sh tests/id/smack.sh - bash uutils/util/run-gnu-test.sh tests/id/uid.sh - bash uutils/util/run-gnu-test.sh tests/id/setgid.sh - bash uutils/util/run-gnu-test.sh tests/id/zero.sh - bash uutils/util/run-gnu-test.sh tests/id/gnu-zero-uids.sh + bash uutils/util/run-gnu-test.sh - name: Extract testing info shell: bash run: | diff --git a/Cargo.toml b/Cargo.toml index c588f735e..052e32c09 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -138,6 +138,11 @@ feat_os_unix_musl = [ # "feat_require_unix", ] +# "feat_selinux" == set of utilities providing support for SELinux Security Context if enabled with `--features feat_selinux`. +# NOTE: +# The selinux(-sys) crate requires `libselinux` headers and shared library to be accessible in the C toolchain at compile time. +# Running a uutils compiled with `feat_selinux` requires an SELinux enabled Kernel at run time. +feat_selinux = ["id/selinux", "selinux"] ## feature sets with requirements (restricting cross-platform availability) # # ** NOTE: these `feat_require_...` sets should be minimized as much as possible to encourage cross-platform availability of utilities @@ -146,7 +151,6 @@ feat_os_unix_musl = [ feat_require_crate_cpp = [ "stdbuf", ] -feat_selinux = ["id/selinux", "selinux"] # "feat_require_unix" == set of utilities requiring support which is only available on unix platforms (as of 2020-04-23) feat_require_unix = [ "chgrp", diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index 929b00dba..9f92a4561 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -5,7 +5,10 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// + +// spell-checker:ignore (ToDO) asid auditid auditinfo auid cstr egid emod euid getaudit getlogin gflag nflag pline rflag termid uflag gsflag zflag cflag + +// README: // This was originally based on BSD's `id` // (noticeable in functionality, usage text, options text, etc.) // and synced with: @@ -25,8 +28,10 @@ // // * Help text based on BSD's `id` manpage and GNU's `id` manpage. // - -// spell-checker:ignore (ToDO) asid auditid auditinfo auid cstr egid emod euid getaudit getlogin gflag nflag pline rflag termid uflag gsflag zflag cflag +// * This passes GNU's coreutils Test suite (8.32) for "tests/id/context.sh" if compiled with +// `--features feat_selinux`. It should also pass "tests/id/no-context.sh", but that depends on +// `uu_ls -Z` being implemented and therefore fails at the moment +// #![allow(non_camel_case_types)] #![allow(dead_code)] From 46a2491727bda53241698b0da153c726459d6c70 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Fri, 23 Jul 2021 19:04:18 +0300 Subject: [PATCH 06/43] stat: Support \t in --printf format --- src/uu/stat/src/stat.rs | 1 + tests/by-util/test_stat.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index 70c06bdf6..c56971f6b 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -436,6 +436,7 @@ impl Stater { 'f' => tokens.push(Token::Char('\x0C')), 'n' => tokens.push(Token::Char('\n')), 'r' => tokens.push(Token::Char('\r')), + 't' => tokens.push(Token::Char('\t')), 'v' => tokens.push(Token::Char('\x0B')), c => { show_warning!("unrecognized escape '\\{}'", c); diff --git a/tests/by-util/test_stat.rs b/tests/by-util/test_stat.rs index 7cff0d89c..af9e3de45 100644 --- a/tests/by-util/test_stat.rs +++ b/tests/by-util/test_stat.rs @@ -64,7 +64,7 @@ mod test_generate_tokens { #[test] fn printf_format() { - let s = "%-# 15a\\r\\\"\\\\\\a\\b\\e\\f\\v%+020.-23w\\x12\\167\\132\\112\\n"; + let s = "%-# 15a\\t\\r\\\"\\\\\\a\\b\\e\\f\\v%+020.-23w\\x12\\167\\132\\112\\n"; let expected = vec![ Token::Directive { flag: F_LEFT | F_ALTER | F_SPACE, @@ -72,6 +72,7 @@ mod test_generate_tokens { precision: -1, format: 'a', }, + Token::Char('\t'), Token::Char('\r'), Token::Char('"'), Token::Char('\\'), From 5c7afe7a6b2cb3b062e6ea64de102ea09736027b Mon Sep 17 00:00:00 2001 From: Syukron Rifail M Date: Sat, 24 Jul 2021 18:17:28 +0700 Subject: [PATCH 07/43] df: add UResult --- src/uu/df/src/df.rs | 59 +++++++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 1092938df..baa5fe292 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -8,6 +8,8 @@ #[macro_use] extern crate uucore; +use uucore::error::UCustomError; +use uucore::error::UResult; #[cfg(unix)] use uucore::fsext::statfs_fn; use uucore::fsext::{read_fs_list, FsUsage, MountInfo}; @@ -19,8 +21,10 @@ use std::cell::Cell; use std::collections::HashMap; use std::collections::HashSet; +use std::error::Error; #[cfg(unix)] use std::ffi::CString; +use std::fmt::Display; #[cfg(unix)] use std::mem; @@ -33,9 +37,6 @@ use std::path::Path; static ABOUT: &str = "Show information about the file system on which each FILE resides,\n\ or all file systems by default."; -static EXIT_OK: i32 = 0; -static EXIT_ERR: i32 = 1; - static OPT_ALL: &str = "all"; static OPT_BLOCKSIZE: &str = "blocksize"; static OPT_DIRECT: &str = "direct"; @@ -226,8 +227,8 @@ fn filter_mount_list(vmi: Vec, paths: &[String], opt: &Options) -> Ve /// Convert `value` to a human readable string based on `base`. /// e.g. It returns 1G when value is 1 * 1024 * 1024 * 1024 and base is 1024. /// Note: It returns `value` if `base` isn't positive. -fn human_readable(value: u64, base: i64) -> String { - match base { +fn human_readable(value: u64, base: i64) -> UResult { + let base_str = match base { d if d < 0 => value.to_string(), // ref: [Binary prefix](https://en.wikipedia.org/wiki/Binary_prefix) @@ @@ -242,8 +243,10 @@ fn human_readable(value: u64, base: i64) -> String { NumberPrefix::Prefixed(prefix, bytes) => format!("{:.1}{}", bytes, prefix.symbol()), }, - _ => crash!(EXIT_ERR, "Internal error: Unknown base value {}", base), - } + _ => return Err(DfError::InvalidBaseValue(base.to_string()).into()), + }; + + Ok(base_str) } fn use_size(free_size: u64, total_size: u64) -> String { @@ -256,7 +259,31 @@ fn use_size(free_size: u64, total_size: u64) -> String { ); } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[derive(Debug)] +enum DfError { + InvalidBaseValue(String), +} + +impl Display for DfError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DfError::InvalidBaseValue(s) => write!(f, "Internal error: Unknown base value {}", s), + } + } +} + +impl Error for DfError {} + +impl UCustomError for DfError { + fn code(&self) -> i32 { + match self { + DfError::InvalidBaseValue(_) => 1, + } + } +} + +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = get_usage(); let matches = uu_app().usage(&usage[..]).get_matches_from(args); @@ -269,7 +296,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { { if matches.is_present(OPT_INODES) { println!("{}: doesn't support -i option", executable!()); - return EXIT_OK; + return Ok(()); } } @@ -353,15 +380,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if opt.show_inode_instead { print!( "{0: >12} ", - human_readable(fs.usage.files, opt.human_readable_base) + human_readable(fs.usage.files, opt.human_readable_base)? ); print!( "{0: >12} ", - human_readable(fs.usage.files - fs.usage.ffree, opt.human_readable_base) + human_readable(fs.usage.files - fs.usage.ffree, opt.human_readable_base)? ); print!( "{0: >12} ", - human_readable(fs.usage.ffree, opt.human_readable_base) + human_readable(fs.usage.ffree, opt.human_readable_base)? ); print!( "{0: >5} ", @@ -375,15 +402,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let free_size = fs.usage.blocksize * fs.usage.bfree; print!( "{0: >12} ", - human_readable(total_size, opt.human_readable_base) + human_readable(total_size, opt.human_readable_base)? ); print!( "{0: >12} ", - human_readable(total_size - free_size, opt.human_readable_base) + human_readable(total_size - free_size, opt.human_readable_base)? ); print!( "{0: >12} ", - human_readable(free_size, opt.human_readable_base) + human_readable(free_size, opt.human_readable_base)? ); if cfg!(target_os = "macos") { let used = fs.usage.blocks - fs.usage.bfree; @@ -396,7 +423,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { println!(); } - EXIT_OK + Ok(()) } pub fn uu_app() -> App<'static, 'static> { From abf4c69b281dfd7f275c2a0101de09c0df41c947 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 25 Jul 2021 13:55:24 -0400 Subject: [PATCH 08/43] tac: correct error message when reading from dir. Correct the error message produced by `tac` when trying to read from a directory. Previously if the path 'a' referred to a directory, then running `tac a` would produce the error message dir: read error: Invalid argument after this commit it produces a: read error: Invalid argument which matches GNU `tac`. --- src/uu/tac/src/tac.rs | 2 +- tests/by-util/test_tac.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index ae1fd9bc5..ad3badff4 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -97,7 +97,7 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { let path = Path::new(filename); if path.is_dir() || path.metadata().is_err() { if path.is_dir() { - show_error!("dir: read error: Invalid argument"); + show_error!("{}: read error: Invalid argument", filename); } else { show_error!( "failed to open '{}' for reading: No such file or directory", diff --git a/tests/by-util/test_tac.rs b/tests/by-util/test_tac.rs index a8adbb28e..a16988acf 100644 --- a/tests/by-util/test_tac.rs +++ b/tests/by-util/test_tac.rs @@ -66,5 +66,5 @@ fn test_invalid_input() { .ucmd() .arg("a") .fails() - .stderr_contains("dir: read error: Invalid argument"); + .stderr_contains("a: read error: Invalid argument"); } From 2648f330e4403734c5cce4795f2b700c6374d269 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 25 Jul 2021 15:19:29 -0400 Subject: [PATCH 09/43] tac: handle no line separators in file Change the behavior of `tac` when there are no line separators in the input. Previously, it was appending an extra line separator; this commit prevents that from happening. The input is now writted directly to stdout. --- src/uu/tac/src/tac.rs | 9 ++++++++- tests/by-util/test_tac.rs | 5 +++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index ae1fd9bc5..2622333e1 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -139,9 +139,16 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { i += 1; } } + // If the file contains no line separators, then simply write + // the contents of the file directly to stdout. + if offsets.is_empty() { + out.write_all(&data) + .unwrap_or_else(|e| crash!(1, "failed to write to stdout: {}", e)); + return exit_code; + } // if there isn't a separator at the end of the file, fake it - if offsets.is_empty() || *offsets.last().unwrap() < data.len() - slen { + if *offsets.last().unwrap() < data.len() - slen { offsets.push(data.len()); } diff --git a/tests/by-util/test_tac.rs b/tests/by-util/test_tac.rs index a8adbb28e..54054db53 100644 --- a/tests/by-util/test_tac.rs +++ b/tests/by-util/test_tac.rs @@ -68,3 +68,8 @@ fn test_invalid_input() { .fails() .stderr_contains("dir: read error: Invalid argument"); } + +#[test] +fn test_no_line_separators() { + new_ucmd!().pipe_in("a").succeeds().stdout_is("a"); +} From bd7d8fdde7cbcb5bf19279a6151515c764332145 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 27 Jul 2021 23:54:29 +0200 Subject: [PATCH 10/43] sort: remove duplication from usage string The custom usage string does not have to include the "sort\nUsage:" part, because this part is already printed by clap. It prevents the following duplication: USAGE: sort Usage: sort [OPTION]... [FILE].. Now, only the following is printed: USAGE: sort [OPTION]... [FILE]... --- src/uu/sort/src/sort.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 55bcdb77b..91365b603 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -916,9 +916,7 @@ impl FieldSelector { fn get_usage() -> String { format!( - "{0} -Usage: - {0} [OPTION]... [FILE]... + "{0} [OPTION]... [FILE]... Write the sorted concatenation of all FILE(s) to standard output. Mandatory arguments for long options are mandatory for short options too. With no FILE, or when FILE is -, read standard input.", From f34505df54acf93ab617bbd1a283b01101b57e38 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 30 Jul 2021 14:41:47 +0200 Subject: [PATCH 11/43] bump the minimal version for coverage to 1.52 Drivers: https://github.com/rust-lang/rust/issues/71395 https://github.com/rust-lang/rust/pull/80470 needed by grcov --- .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 b2d77a5a4..012c14264 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -14,7 +14,7 @@ env: PROJECT_DESC: "Core universal (cross-platform) utilities" PROJECT_AUTH: "uutils" RUST_MIN_SRV: "1.43.1" ## v1.43.0 - RUST_COV_SRV: "2020-08-01" ## (~v1.47.0) supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel + RUST_COV_SRV: "2021-05-06" ## (~v1.52.0) supported rust version for code coverage; (date required/used by 'coverage') ## !maint: refactor when code coverage support is included in the stable channel on: [push, pull_request] From 3e096491f3694ae420a43e50d63388c81e543b85 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 30 Jul 2021 15:22:20 +0200 Subject: [PATCH 12/43] Remove clippy allow was causing unused attribute `allow` --- src/uu/cp/src/cp.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 4deaefa98..031890b5a 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -55,7 +55,6 @@ use walkdir::WalkDir; use std::os::unix::fs::PermissionsExt; #[cfg(target_os = "linux")] -#[allow(clippy::missing_safety_doc)] ioctl!(write ficlone with 0x94, 9; std::os::raw::c_int); quick_error! { From a214ca60bdf03f450b342f24187e080b81ce340c Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 27 Jul 2021 15:49:38 +0200 Subject: [PATCH 13/43] sort: allow null bytes for -t --- src/uu/sort/src/sort.rs | 5 ++++- tests/by-util/test_sort.rs | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 91365b603..f779bca38 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -1065,7 +1065,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if let Some(arg) = matches.args.get(options::SEPARATOR) { let separator = arg.vals[0].to_string_lossy(); - let separator = separator; + let mut separator = separator.as_ref(); + if separator == "\\0" { + separator = "\0"; + } if separator.len() != 1 { crash!(1, "separator must be exactly one character long"); } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 4a1cc3fa9..a9519e153 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -959,3 +959,12 @@ fn test_key_takes_one_arg() { .succeeds() .stdout_is_fixture("keys_open_ended.expected"); } + +#[test] +fn test_separator_null() { + new_ucmd!() + .args(&["-k1,1", "-k3,3", "-t", "\\0"]) + .pipe_in("z\0a\0b\nz\0b\0a\na\0z\0z\n") + .succeeds() + .stdout_only("a\0z\0z\nz\0b\0a\nz\0a\0b\n"); +} From 891d25cebd7e888540ba2bfbfea018f00d1cc434 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 25 Jul 2021 22:24:37 +0200 Subject: [PATCH 14/43] sort: fix exit codes and error messages --- src/uu/sort/src/chunks.rs | 4 ++-- src/uu/sort/src/ext_sort.rs | 2 +- src/uu/sort/src/sort.rs | 29 +++++++++++++++++++++-------- tests/by-util/test_sort.rs | 4 ++-- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/uu/sort/src/chunks.rs b/src/uu/sort/src/chunks.rs index 9e9d212c2..5ab98392d 100644 --- a/src/uu/sort/src/chunks.rs +++ b/src/uu/sort/src/chunks.rs @@ -175,7 +175,7 @@ pub fn read( // because it was only temporarily transmuted to a Vec> to make recycling possible. std::mem::transmute::>, Vec>>(lines) }; - let read = crash_if_err!(1, std::str::from_utf8(&buffer[..read])); + let read = crash_if_err!(2, std::str::from_utf8(&buffer[..read])); let mut line_data = LineData { selections, num_infos, @@ -313,7 +313,7 @@ fn read_to_buffer( Err(e) if e.kind() == ErrorKind::Interrupted => { // retry } - Err(e) => crash!(1, "{}", e), + Err(e) => crash!(2, "{}", e), } } } diff --git a/src/uu/sort/src/ext_sort.rs b/src/uu/sort/src/ext_sort.rs index e0814b7a2..201dda8bb 100644 --- a/src/uu/sort/src/ext_sort.rs +++ b/src/uu/sort/src/ext_sort.rs @@ -211,7 +211,7 @@ fn read_write_loop( } let tmp_dir = crash_if_err!( - 1, + 2, tempfile::Builder::new() .prefix("uutils_sort") .tempdir_in(tmp_dir_parent) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 91365b603..3d7e890b9 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -215,7 +215,7 @@ impl GlobalSettings { Ok(f) => BufWriter::new(Box::new(f) as Box), Err(e) => { show_error!("{0}: {1}", filename, e.to_string()); - panic!("Could not open output file"); + crash!(2, "Could not open output file"); } }, None => BufWriter::new(Box::new(stdout()) as Box), @@ -942,7 +942,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let usage = get_usage(); let mut settings: GlobalSettings = Default::default(); - let matches = uu_app().usage(&usage[..]).get_matches_from(args); + let matches = uu_app().usage(&usage[..]).get_matches_from_safe(args); + + let matches = crash_if_err!(2, matches); settings.debug = matches.is_present(options::DEBUG); @@ -1060,14 +1062,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { /* if no file, default to stdin */ files.push("-".to_owned()); } else if settings.check && files.len() != 1 { - crash!(1, "extra operand `{}' not allowed with -c", files[1]) + crash!(2, "extra operand `{}' not allowed with -c", files[1]) } if let Some(arg) = matches.args.get(options::SEPARATOR) { let separator = arg.vals[0].to_string_lossy(); let separator = separator; if separator.len() != 1 { - crash!(1, "separator must be exactly one character long"); + crash!(2, "separator must be exactly one character long"); } settings.separator = Some(separator.chars().next().unwrap()) } @@ -1338,7 +1340,7 @@ fn exec(files: &[String], settings: &GlobalSettings) -> i32 { file_merger.write_all(settings); } else if settings.check { if files.len() > 1 { - crash!(1, "only one file allowed with -c"); + crash!(2, "only one file allowed with -c"); } return check::check(files.first().unwrap(), settings); } else { @@ -1623,7 +1625,11 @@ fn print_sorted<'a, T: Iterator>>(iter: T, settings: &Global } } -// from cat.rs +/// Strips the trailing " (os error XX)" from io error strings. +fn strip_errno(err: &str) -> &str { + &err[..err.find(" (os error ").unwrap_or(err.len())] +} + fn open(path: impl AsRef) -> Box { let path = path.as_ref(); if path == "-" { @@ -1631,10 +1637,17 @@ fn open(path: impl AsRef) -> Box { return Box::new(stdin) as Box; } - match File::open(Path::new(path)) { + let path = Path::new(path); + + match File::open(path) { Ok(f) => Box::new(f) as Box, Err(e) => { - crash!(2, "cannot read: {0:?}: {1}", path, e); + crash!( + 2, + "cannot read: {0}: {1}", + path.to_string_lossy(), + strip_errno(&e.to_string()) + ); } } } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 4a1cc3fa9..e594de0a7 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -839,9 +839,9 @@ fn test_nonexistent_file() { .status_code(2) .stderr_only( #[cfg(not(windows))] - "sort: cannot read: \"nonexistent.txt\": No such file or directory (os error 2)", + "sort: cannot read: nonexistent.txt: No such file or directory", #[cfg(windows)] - "sort: cannot read: \"nonexistent.txt\": The system cannot find the file specified. (os error 2)", + "sort: cannot read: nonexistent.txt: The system cannot find the file specified.", ); } From c6e044927c95fbeab5364349e9e1e7d1e09158e9 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 25 Jul 2021 22:25:54 +0200 Subject: [PATCH 15/43] sort: print check messages to stderr, not stdout --- src/uu/sort/src/check.rs | 4 ++-- tests/by-util/test_sort.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/uu/sort/src/check.rs b/src/uu/sort/src/check.rs index f1cd22686..44c71674e 100644 --- a/src/uu/sort/src/check.rs +++ b/src/uu/sort/src/check.rs @@ -56,7 +56,7 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { ) == Ordering::Greater { if !settings.check_silent { - println!("sort: {}:{}: disorder: {}", path, line_idx, new_first.line); + eprintln!("sort: {}:{}: disorder: {}", path, line_idx, new_first.line); } return 1; } @@ -68,7 +68,7 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { if compare_by(a, b, settings, chunk.line_data(), chunk.line_data()) == Ordering::Greater { if !settings.check_silent { - println!("sort: {}:{}: disorder: {}", path, line_idx, b.line); + eprintln!("sort: {}:{}: disorder: {}", path, line_idx, b.line); } return 1; } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index e594de0a7..1add0bb54 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -181,7 +181,7 @@ fn test_check_zero_terminated_failure() { .arg("-c") .arg("zero-terminated.txt") .fails() - .stdout_is("sort: zero-terminated.txt:2: disorder: ../../fixtures/du\n"); + .stderr_only("sort: zero-terminated.txt:2: disorder: ../../fixtures/du\n"); } #[test] @@ -775,13 +775,13 @@ fn test_check() { .arg(diagnose_arg) .arg("check_fail.txt") .fails() - .stdout_is("sort: check_fail.txt:6: disorder: 5\n"); + .stderr_only("sort: check_fail.txt:6: disorder: 5\n"); new_ucmd!() .arg(diagnose_arg) .arg("multiple_files.expected") .succeeds() - .stdout_is(""); + .stderr_is(""); } } From a33b6d87b578990893257c2b063f8579a3f960c4 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 30 Jul 2021 20:48:07 +0200 Subject: [PATCH 16/43] sort: open the output file at the beginning This causes us to print an error message about an invalid output file right at the start of the invocation, but *after* verifying other arguments. --- src/uu/sort/src/ext_sort.rs | 18 ++++++++--- src/uu/sort/src/merge.rs | 6 ++-- src/uu/sort/src/sort.rs | 59 ++++++++++++++++++++++--------------- tests/by-util/test_sort.rs | 27 +++++++++++++++++ 4 files changed, 80 insertions(+), 30 deletions(-) diff --git a/src/uu/sort/src/ext_sort.rs b/src/uu/sort/src/ext_sort.rs index 201dda8bb..ea53900e2 100644 --- a/src/uu/sort/src/ext_sort.rs +++ b/src/uu/sort/src/ext_sort.rs @@ -28,6 +28,7 @@ use crate::merge::ClosedTmpFile; use crate::merge::WriteableCompressedTmpFile; use crate::merge::WriteablePlainTmpFile; use crate::merge::WriteableTmpFile; +use crate::Output; use crate::{ chunks::{self, Chunk}, compare_by, merge, sort_by, GlobalSettings, @@ -38,7 +39,11 @@ use tempfile::TempDir; const START_BUFFER_SIZE: usize = 8_000; /// Sort files by using auxiliary files for storing intermediate chunks (if needed), and output the result. -pub fn ext_sort(files: &mut impl Iterator>, settings: &GlobalSettings) { +pub fn ext_sort( + files: &mut impl Iterator>, + settings: &GlobalSettings, + output: Output, +) { let (sorted_sender, sorted_receiver) = std::sync::mpsc::sync_channel(1); let (recycled_sender, recycled_receiver) = std::sync::mpsc::sync_channel(1); thread::spawn({ @@ -51,6 +56,7 @@ pub fn ext_sort(files: &mut impl Iterator>, settings settings, sorted_receiver, recycled_sender, + output, ); } else { reader_writer::<_, WriteablePlainTmpFile>( @@ -58,6 +64,7 @@ pub fn ext_sort(files: &mut impl Iterator>, settings settings, sorted_receiver, recycled_sender, + output, ); } } @@ -67,6 +74,7 @@ fn reader_writer>, Tmp: WriteableTmpFile settings: &GlobalSettings, receiver: Receiver, sender: SyncSender, + output: Output, ) { let separator = if settings.zero_terminated { b'\0' @@ -96,7 +104,7 @@ fn reader_writer>, Tmp: WriteableTmpFile settings, Some((tmp_dir, tmp_dir_size)), ); - merger.write_all(settings); + merger.write_all(settings, output); } ReadResult::SortedSingleChunk(chunk) => { if settings.unique { @@ -106,9 +114,10 @@ fn reader_writer>, Tmp: WriteableTmpFile == Ordering::Equal }), settings, + output, ); } else { - print_sorted(chunk.lines().iter(), settings); + print_sorted(chunk.lines().iter(), settings, output); } } ReadResult::SortedTwoChunks([a, b]) => { @@ -128,9 +137,10 @@ fn reader_writer>, Tmp: WriteableTmpFile }) .map(|(line, _)| line), settings, + output, ); } else { - print_sorted(merged_iter.map(|(line, _)| line), settings); + print_sorted(merged_iter.map(|(line, _)| line), settings, output); } } ReadResult::EmptyInput => { diff --git a/src/uu/sort/src/merge.rs b/src/uu/sort/src/merge.rs index 12d7a9b9b..b8d69fb14 100644 --- a/src/uu/sort/src/merge.rs +++ b/src/uu/sort/src/merge.rs @@ -25,7 +25,7 @@ use tempfile::TempDir; use crate::{ chunks::{self, Chunk, RecycledChunk}, - compare_by, GlobalSettings, + compare_by, GlobalSettings, Output, }; /// Merge pre-sorted `Box`s. @@ -238,8 +238,8 @@ pub struct FileMerger<'a> { impl<'a> FileMerger<'a> { /// Write the merged contents to the output file. - pub fn write_all(&mut self, settings: &GlobalSettings) { - let mut out = settings.out_writer(); + pub fn write_all(&mut self, settings: &GlobalSettings, output: Output) { + let mut out = output.into_write(); self.write_all_to(settings, &mut out); } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 3d7e890b9..6e4cd8a23 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -37,7 +37,7 @@ use std::env; use std::ffi::OsStr; use std::fs::File; use std::hash::{Hash, Hasher}; -use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; +use std::io::{stdin, stdout, BufRead, BufReader, Read, Write}; use std::ops::Range; use std::path::Path; use std::path::PathBuf; @@ -146,6 +146,29 @@ impl SortMode { } } +pub struct Output { + file: Option, +} + +impl Output { + fn new(name: Option<&str>) -> Self { + Self { + file: name.map(|name| { + File::create(name).unwrap_or_else(|e| { + crash!(2, "open failed: {}: {}", name, strip_errno(&e.to_string())) + }) + }), + } + } + + fn into_write(self) -> Box { + match self.file { + Some(file) => Box::new(file), + None => Box::new(stdout()), + } + } +} + #[derive(Clone)] pub struct GlobalSettings { mode: SortMode, @@ -156,7 +179,6 @@ pub struct GlobalSettings { ignore_non_printing: bool, merge: bool, reverse: bool, - output_file: Option, stable: bool, unique: bool, check: bool, @@ -209,19 +231,6 @@ impl GlobalSettings { } } - fn out_writer(&self) -> BufWriter> { - match self.output_file { - Some(ref filename) => match File::create(Path::new(&filename)) { - Ok(f) => BufWriter::new(Box::new(f) as Box), - Err(e) => { - show_error!("{0}: {1}", filename, e.to_string()); - crash!(2, "Could not open output file"); - } - }, - None => BufWriter::new(Box::new(stdout()) as Box), - } - } - /// Precompute some data needed for sorting. /// This function **must** be called before starting to sort, and `GlobalSettings` may not be altered /// afterwards. @@ -253,7 +262,6 @@ impl Default for GlobalSettings { ignore_non_printing: false, merge: false, reverse: false, - output_file: None, stable: false, unique: false, check: false, @@ -1053,7 +1061,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.ignore_leading_blanks = matches.is_present(options::IGNORE_LEADING_BLANKS); - settings.output_file = matches.value_of(options::OUTPUT).map(String::from); settings.reverse = matches.is_present(options::REVERSE); settings.stable = matches.is_present(options::STABLE); settings.unique = matches.is_present(options::UNIQUE); @@ -1099,9 +1106,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ); } + let output = Output::new(matches.value_of(options::OUTPUT)); + settings.init_precomputed(); - exec(&files, &settings) + exec(&files, &settings, output) } pub fn uu_app() -> App<'static, 'static> { @@ -1334,10 +1343,10 @@ pub fn uu_app() -> App<'static, 'static> { .arg(Arg::with_name(options::FILES).multiple(true).takes_value(true)) } -fn exec(files: &[String], settings: &GlobalSettings) -> i32 { +fn exec(files: &[String], settings: &GlobalSettings, output: Output) -> i32 { if settings.merge { let mut file_merger = merge::merge(files.iter().map(open), settings); - file_merger.write_all(settings); + file_merger.write_all(settings, output); } else if settings.check { if files.len() > 1 { crash!(2, "only one file allowed with -c"); @@ -1346,7 +1355,7 @@ fn exec(files: &[String], settings: &GlobalSettings) -> i32 { } else { let mut lines = files.iter().map(open); - ext_sort(&mut lines, settings); + ext_sort(&mut lines, settings, output); } 0 } @@ -1618,8 +1627,12 @@ fn month_compare(a: &str, b: &str) -> Ordering { } } -fn print_sorted<'a, T: Iterator>>(iter: T, settings: &GlobalSettings) { - let mut writer = settings.out_writer(); +fn print_sorted<'a, T: Iterator>>( + iter: T, + settings: &GlobalSettings, + output: Output, +) { + let mut writer = output.into_write(); for line in iter { line.print(&mut writer, settings); } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 1add0bb54..b4b4ba41c 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -959,3 +959,30 @@ fn test_key_takes_one_arg() { .succeeds() .stdout_is_fixture("keys_open_ended.expected"); } + +#[test] +fn test_verifies_out_file() { + let inputs = ["" /* no input */, "some input"]; + for &input in &inputs { + new_ucmd!() + .args(&["-o", "nonexistent_dir/nonexistent_file"]) + .pipe_in(input) + .fails() + .status_code(2) + .stderr_only( + #[cfg(not(windows))] + "sort: open failed: nonexistent_dir/nonexistent_file: No such file or directory", + #[cfg(windows)] + "sort: open failed: nonexistent_dir/nonexistent_file: The system cannot find the path specified.", + ); + } +} + +#[test] +fn test_verifies_out_file_after_keys() { + new_ucmd!() + .args(&["-o", "nonexistent_dir/nonexistent_file", "-k", "0"]) + .fails() + .status_code(2) + .stderr_contains("failed to parse key"); +} From 1bb530eebb84b5ea8ef60f40671d46f15a91df4f Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 30 Jul 2021 21:02:49 +0200 Subject: [PATCH 17/43] sort: validate input files at startup --- src/uu/sort/src/sort.rs | 8 ++++++++ tests/by-util/test_sort.rs | 20 ++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 6e4cd8a23..186b7dc67 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -1106,6 +1106,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ); } + // Verify that we can open all input files. + // It is the correct behavior to close all files afterwards, + // and to reopen them at a later point. This is different from how the output file is handled, + // probably to prevent running out of file descriptors. + for file in &files { + open(file); + } + let output = Output::new(matches.value_of(options::OUTPUT)); settings.init_precomputed(); diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index b4b4ba41c..74241ef93 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -979,10 +979,26 @@ fn test_verifies_out_file() { } #[test] -fn test_verifies_out_file_after_keys() { +fn test_verifies_files_after_keys() { new_ucmd!() - .args(&["-o", "nonexistent_dir/nonexistent_file", "-k", "0"]) + .args(&[ + "-o", + "nonexistent_dir/nonexistent_file", + "-k", + "0", + "nonexistent_dir/input_file", + ]) .fails() .status_code(2) .stderr_contains("failed to parse key"); } + +#[test] +#[cfg(unix)] +fn test_verifies_input_files() { + new_ucmd!() + .args(&["/dev/random", "nonexistent_file"]) + .fails() + .status_code(2) + .stderr_is("sort: cannot read: nonexistent_file: No such file or directory"); +} From 47595050245d79cd5e54c4b5966e1be3fb4405b0 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 30 Jul 2021 21:03:50 +0200 Subject: [PATCH 18/43] sort: split a line to make rustfmt work again --- src/uu/sort/src/sort.rs | 138 +++++++++++++++++++--------------------- 1 file changed, 64 insertions(+), 74 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 186b7dc67..3ed253ba4 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -1129,73 +1129,57 @@ pub fn uu_app() -> App<'static, 'static> { Arg::with_name(options::modes::SORT) .long(options::modes::SORT) .takes_value(true) - .possible_values( - &[ - "general-numeric", - "human-numeric", - "month", - "numeric", - "version", - "random", - ] - ) - .conflicts_with_all(&options::modes::ALL_SORT_MODES) - ) - .arg( - make_sort_mode_arg( - options::modes::HUMAN_NUMERIC, - "h", - "compare according to human readable sizes, eg 1M > 100k" - ), - ) - .arg( - make_sort_mode_arg( - options::modes::MONTH, - "M", - "compare according to month name abbreviation" - ), - ) - .arg( - make_sort_mode_arg( - options::modes::NUMERIC, - "n", - "compare according to string numerical value" - ), - ) - .arg( - make_sort_mode_arg( - options::modes::GENERAL_NUMERIC, - "g", - "compare according to string general numerical value" - ), - ) - .arg( - make_sort_mode_arg( - options::modes::VERSION, - "V", - "Sort by SemVer version number, eg 1.12.2 > 1.1.2", - ), - ) - .arg( - make_sort_mode_arg( - options::modes::RANDOM, - "R", - "shuffle in random order", - ), + .possible_values(&[ + "general-numeric", + "human-numeric", + "month", + "numeric", + "version", + "random", + ]) + .conflicts_with_all(&options::modes::ALL_SORT_MODES), ) + .arg(make_sort_mode_arg( + options::modes::HUMAN_NUMERIC, + "h", + "compare according to human readable sizes, eg 1M > 100k", + )) + .arg(make_sort_mode_arg( + options::modes::MONTH, + "M", + "compare according to month name abbreviation", + )) + .arg(make_sort_mode_arg( + options::modes::NUMERIC, + "n", + "compare according to string numerical value", + )) + .arg(make_sort_mode_arg( + options::modes::GENERAL_NUMERIC, + "g", + "compare according to string general numerical value", + )) + .arg(make_sort_mode_arg( + options::modes::VERSION, + "V", + "Sort by SemVer version number, eg 1.12.2 > 1.1.2", + )) + .arg(make_sort_mode_arg( + options::modes::RANDOM, + "R", + "shuffle in random order", + )) .arg( Arg::with_name(options::DICTIONARY_ORDER) .short("d") .long(options::DICTIONARY_ORDER) .help("consider only blanks and alphanumeric characters") - .conflicts_with_all( - &[ - options::modes::NUMERIC, - options::modes::GENERAL_NUMERIC, - options::modes::HUMAN_NUMERIC, - options::modes::MONTH, - ] - ), + .conflicts_with_all(&[ + options::modes::NUMERIC, + options::modes::GENERAL_NUMERIC, + options::modes::HUMAN_NUMERIC, + options::modes::MONTH, + ]), ) .arg( Arg::with_name(options::MERGE) @@ -1223,7 +1207,10 @@ pub fn uu_app() -> App<'static, 'static> { .short("C") .long(options::check::CHECK_SILENT) .conflicts_with(options::OUTPUT) - .help("exit successfully if the given file is already sorted, and exit with status 1 otherwise."), + .help( + "exit successfully if the given file is already sorted,\ + and exit with status 1 otherwise.", + ), ) .arg( Arg::with_name(options::IGNORE_CASE) @@ -1236,14 +1223,12 @@ pub fn uu_app() -> App<'static, 'static> { .short("i") .long(options::IGNORE_NONPRINTING) .help("ignore nonprinting characters") - .conflicts_with_all( - &[ - options::modes::NUMERIC, - options::modes::GENERAL_NUMERIC, - options::modes::HUMAN_NUMERIC, - options::modes::MONTH - ] - ), + .conflicts_with_all(&[ + options::modes::NUMERIC, + options::modes::GENERAL_NUMERIC, + options::modes::HUMAN_NUMERIC, + options::modes::MONTH, + ]), ) .arg( Arg::with_name(options::IGNORE_LEADING_BLANKS) @@ -1292,7 +1277,8 @@ pub fn uu_app() -> App<'static, 'static> { .short("t") .long(options::SEPARATOR) .help("custom separator for -k") - .takes_value(true)) + .takes_value(true), + ) .arg( Arg::with_name(options::ZERO_TERMINATED) .short("z") @@ -1327,13 +1313,13 @@ pub fn uu_app() -> App<'static, 'static> { .long(options::COMPRESS_PROG) .help("compress temporary files with PROG, decompress with PROG -d") .long_help("PROG has to take input from stdin and output to stdout") - .value_name("PROG") + .value_name("PROG"), ) .arg( Arg::with_name(options::BATCH_SIZE) .long(options::BATCH_SIZE) .help("Merge at most N_MERGE inputs at once.") - .value_name("N_MERGE") + .value_name("N_MERGE"), ) .arg( Arg::with_name(options::FILES0_FROM) @@ -1348,7 +1334,11 @@ pub fn uu_app() -> App<'static, 'static> { .long(options::DEBUG) .help("underline the parts of the line that are actually used for sorting"), ) - .arg(Arg::with_name(options::FILES).multiple(true).takes_value(true)) + .arg( + Arg::with_name(options::FILES) + .multiple(true) + .takes_value(true), + ) } fn exec(files: &[String], settings: &GlobalSettings, output: Output) -> i32 { From fb477360b29a2ef6f962e1f57b9c0ddc0804b13f Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 30 Jul 2021 23:23:58 -0400 Subject: [PATCH 19/43] expand: expand support for --tabs arguments Add support for * space-separated list of tab stops, like `--tabs="2 4 6"`, * mixed comma- and space-separated lists, like `--tabs="2,4 6"`, * the slash specifier in the last tab stop, like `--tabs=1,/5`, * the plus specifier in the last tab stop, like `--tabs=1,+5`. --- src/uu/expand/src/expand.rs | 176 +++++++++++++++++++++++++++++------ tests/by-util/test_expand.rs | 137 +++++++++++++++++++++++++++ 2 files changed, 286 insertions(+), 27 deletions(-) diff --git a/src/uu/expand/src/expand.rs b/src/uu/expand/src/expand.rs index 66c3eb259..d5c37ce21 100644 --- a/src/uu/expand/src/expand.rs +++ b/src/uu/expand/src/expand.rs @@ -36,28 +36,86 @@ fn get_usage() -> String { format!("{0} [OPTION]... [FILE]...", executable!()) } -fn tabstops_parse(s: String) -> Vec { - let words = s.split(','); +/// The mode to use when replacing tabs beyond the last one specified in +/// the `--tabs` argument. +enum RemainingMode { + None, + Slash, + Plus, +} - let nums = words - .map(|sn| { - sn.parse::() - .unwrap_or_else(|_| crash!(1, "{}\n", "tab size contains invalid character(s)")) - }) - .collect::>(); +/// Decide whether the character is either a space or a comma. +/// +/// # Examples +/// +/// ```rust,ignore +/// assert!(is_space_or_comma(' ')) +/// assert!(is_space_or_comma(',')) +/// assert!(!is_space_or_comma('a')) +/// ``` +fn is_space_or_comma(c: char) -> bool { + c == ' ' || c == ',' +} - if nums.iter().any(|&n| n == 0) { - crash!(1, "{}\n", "tab size cannot be 0"); +/// Parse a list of tabstops from a `--tabs` argument. +/// +/// This function returns both the vector of numbers appearing in the +/// comma- or space-separated list, and also an optional mode, specified +/// by either a "/" or a "+" character appearing before the final number +/// 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: String) -> (RemainingMode, Vec) { + // 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]); } - if let (false, _) = nums - .iter() - .fold((true, 0), |(acc, last), &n| (acc && last <= n, n)) - { - crash!(1, "{}\n", "tab sizes must be ascending"); - } + let mut nums = vec![]; + let mut remaining_mode = RemainingMode::None; + for word in s.split(is_space_or_comma) { + let bytes = word.as_bytes(); + for i in 0..bytes.len() { + match bytes[i] { + b'+' => { + remaining_mode = RemainingMode::Plus; + } + b'/' => { + remaining_mode = RemainingMode::Slash; + } + _ => { + // Parse a number from the byte sequence. + let num = from_utf8(&bytes[i..]).unwrap().parse::().unwrap(); - nums + // Tab size must be positive. + if num == 0 { + crash!(1, "{}\n", "tab size cannot be 0"); + } + + // 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; + } + } + } + } + // If no numbers could be parsed (for example, if `s` were "+,+,+"), + // then just use the default tabstops. + if nums.is_empty() { + nums = vec![DEFAULT_TABSTOP]; + } + (remaining_mode, nums) } struct Options { @@ -66,13 +124,17 @@ struct Options { tspaces: String, iflag: bool, uflag: bool, + + /// Strategy for expanding tabs for columns beyond those specified + /// in `tabstops`. + remaining_mode: RemainingMode, } impl Options { fn new(matches: &ArgMatches) -> Options { - let tabstops = match matches.value_of(options::TABS) { + let (remaining_mode, tabstops) = match matches.value_of(options::TABS) { Some(s) => tabstops_parse(s.to_string()), - None => vec![DEFAULT_TABSTOP], + None => (RemainingMode::None, vec![DEFAULT_TABSTOP]), }; let iflag = matches.is_present(options::INITIAL); @@ -102,6 +164,7 @@ impl Options { tspaces, iflag, uflag, + remaining_mode, } } } @@ -159,13 +222,41 @@ fn open(path: String) -> BufReader> { } } -fn next_tabstop(tabstops: &[usize], col: usize) -> usize { - if tabstops.len() == 1 { - tabstops[0] - col % tabstops[0] - } else { - match tabstops.iter().find(|&&t| t > col) { +/// Compute the number of spaces to the next tabstop. +/// +/// `tabstops` is the sequence of tabstop locations. +/// +/// `col` is the index of the current cursor in the line being written. +/// +/// If `remaining_mode` is [`RemainingMode::Plus`], then the last entry +/// in the `tabstops` slice is interpreted as a relative number of +/// spaces, which this function will return for every input value of +/// `col` beyond the end of the second-to-last element of `tabstops`. +/// +/// If `remaining_mode` is [`RemainingMode::Plus`], then the last entry +/// in the `tabstops` slice is interpreted as a relative number of +/// spaces, which this function will return for every input value of +/// `col` beyond the end of the second-to-last element of `tabstops`. +fn next_tabstop(tabstops: &[usize], col: usize, remaining_mode: &RemainingMode) -> usize { + let num_tabstops = tabstops.len(); + match remaining_mode { + RemainingMode::Plus => match tabstops[0..num_tabstops - 1].iter().find(|&&t| t > col) { Some(t) => t - col, - None => 1, + None => tabstops[num_tabstops - 1] - 1, + }, + RemainingMode::Slash => match tabstops[0..num_tabstops - 1].iter().find(|&&t| t > col) { + Some(t) => t - col, + None => tabstops[num_tabstops - 1] - col % tabstops[num_tabstops - 1], + }, + RemainingMode::None => { + if num_tabstops == 1 { + tabstops[0] - col % tabstops[0] + } else { + match tabstops.iter().find(|&&t| t > col) { + Some(t) => t - col, + None => 1, + } + } } } } @@ -232,12 +323,16 @@ fn expand(options: Options) { match ctype { Tab => { // figure out how many spaces to the next tabstop - let nts = next_tabstop(ts, col); + let nts = next_tabstop(ts, col, &options.remaining_mode); col += nts; // now dump out either spaces if we're expanding, or a literal tab if we're not if init || !options.iflag { - safe_unwrap!(output.write_all(options.tspaces[..nts].as_bytes())); + if nts <= options.tspaces.len() { + safe_unwrap!(output.write_all(options.tspaces[..nts].as_bytes())); + } else { + safe_unwrap!(output.write_all(" ".repeat(nts).as_bytes())); + }; } else { safe_unwrap!(output.write_all(&buf[byte..byte + nbytes])); } @@ -269,3 +364,30 @@ fn expand(options: Options) { } } } + +#[cfg(test)] +mod tests { + use super::next_tabstop; + use super::RemainingMode; + + #[test] + fn test_next_tabstop_remaining_mode_none() { + assert_eq!(next_tabstop(&[1, 5], 0, &RemainingMode::None), 1); + assert_eq!(next_tabstop(&[1, 5], 3, &RemainingMode::None), 2); + assert_eq!(next_tabstop(&[1, 5], 6, &RemainingMode::None), 1); + } + + #[test] + fn test_next_tabstop_remaining_mode_plus() { + assert_eq!(next_tabstop(&[1, 5], 0, &RemainingMode::Plus), 1); + assert_eq!(next_tabstop(&[1, 5], 3, &RemainingMode::Plus), 4); + assert_eq!(next_tabstop(&[1, 5], 6, &RemainingMode::Plus), 4); + } + + #[test] + fn test_next_tabstop_remaining_mode_slash() { + assert_eq!(next_tabstop(&[1, 5], 0, &RemainingMode::Slash), 1); + assert_eq!(next_tabstop(&[1, 5], 3, &RemainingMode::Slash), 2); + assert_eq!(next_tabstop(&[1, 5], 6, &RemainingMode::Slash), 4); + } +} diff --git a/tests/by-util/test_expand.rs b/tests/by-util/test_expand.rs index 834a09736..3d4ac71f3 100644 --- a/tests/by-util/test_expand.rs +++ b/tests/by-util/test_expand.rs @@ -53,3 +53,140 @@ fn test_with_multiple_files() { .stdout_contains(" return") .stdout_contains(" "); } + +#[test] +fn test_tabs_space_separated_list() { + new_ucmd!() + .args(&["--tabs", "3 6 9"]) + .pipe_in("a\tb\tc\td\te") + .succeeds() + .stdout_is("a b c d e"); +} + +#[test] +fn test_tabs_mixed_style_list() { + new_ucmd!() + .args(&["--tabs", ", 3,6 9"]) + .pipe_in("a\tb\tc\td\te") + .succeeds() + .stdout_is("a b c d e"); +} + +#[test] +fn test_tabs_empty_string() { + new_ucmd!() + .args(&["--tabs", ""]) + .pipe_in("a\tb\tc") + .succeeds() + .stdout_is("a b c"); +} + +#[test] +fn test_tabs_comma_only() { + new_ucmd!() + .args(&["--tabs", ","]) + .pipe_in("a\tb\tc") + .succeeds() + .stdout_is("a b c"); +} + +#[test] +fn test_tabs_space_only() { + new_ucmd!() + .args(&["--tabs", " "]) + .pipe_in("a\tb\tc") + .succeeds() + .stdout_is("a b c"); +} + +#[test] +fn test_tabs_slash() { + new_ucmd!() + .args(&["--tabs", "/"]) + .pipe_in("a\tb\tc") + .succeeds() + .stdout_is("a b c"); +} + +#[test] +fn test_tabs_plus() { + new_ucmd!() + .args(&["--tabs", "+"]) + .pipe_in("a\tb\tc") + .succeeds() + .stdout_is("a b c"); +} + +#[test] +fn test_tabs_trailing_slash() { + new_ucmd!() + .arg("--tabs=1,/5") + .pipe_in("\ta\tb\tc") + .succeeds() + // 0 1 + // 01234567890 + .stdout_is(" a b c"); +} + +#[test] +fn test_tabs_trailing_slash_long_columns() { + new_ucmd!() + .arg("--tabs=1,/3") + .pipe_in("\taaaa\tbbbb\tcccc") + .succeeds() + // 0 1 + // 01234567890123456 + .stdout_is(" aaaa bbbb cccc"); +} + +#[test] +fn test_tabs_trailing_plus() { + new_ucmd!() + .arg("--tabs=1,+5") + .pipe_in("\ta\tb\tc") + .succeeds() + // 0 1 + // 012345678901 + .stdout_is(" a b c"); +} + +#[test] +fn test_tabs_trailing_plus_long_columns() { + new_ucmd!() + .arg("--tabs=1,+3") + .pipe_in("\taaaa\tbbbb\tcccc") + .succeeds() + // 0 1 + // 012345678901234567 + .stdout_is(" aaaa bbbb cccc"); +} + +#[test] +fn test_tabs_must_be_ascending() { + new_ucmd!() + .arg("--tabs=1,1") + .fails() + .stderr_contains("tab sizes must be ascending"); +} + +#[test] +fn test_tabs_keep_last_trailing_specifier() { + // If there are multiple trailing specifiers, use only the last one + // before the number. + new_ucmd!() + .arg("--tabs=1,+/+/5") + .pipe_in("\ta\tb\tc") + .succeeds() + // 0 1 + // 01234567890 + .stdout_is(" a b c"); +} + +#[test] +fn test_tabs_comma_separated_no_numbers() { + new_ucmd!() + .arg("--tabs=+,/,+,/") + .pipe_in("\ta\tb\tc") + .succeeds() + .stdout_is(" a b c"); +} From 6b73ddcd12bef3f1c911654900454d6ef84826f7 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 31 Jul 2021 10:38:32 +0200 Subject: [PATCH 20/43] Silent the spell checker on a test examples --- tests/by-util/test_expand.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/by-util/test_expand.rs b/tests/by-util/test_expand.rs index 3d4ac71f3..9c06fe904 100644 --- a/tests/by-util/test_expand.rs +++ b/tests/by-util/test_expand.rs @@ -1,4 +1,5 @@ use crate::common::util::*; +// spell-checker:ignore (ToDO) taaaa tbbbb tcccc #[test] fn test_with_tab() { From 095e53aed2ae5b116da857eb382888686234abbf Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 26 Jul 2021 17:38:53 +0200 Subject: [PATCH 21/43] sort: disallow equal lines for --check with --unique --- src/uu/sort/src/check.rs | 12 +++++++++--- tests/by-util/test_sort.rs | 12 ++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/uu/sort/src/check.rs b/src/uu/sort/src/check.rs index 44c71674e..de320ef77 100644 --- a/src/uu/sort/src/check.rs +++ b/src/uu/sort/src/check.rs @@ -26,6 +26,13 @@ use std::{ /// /// The code we should exit with. pub fn check(path: &str, settings: &GlobalSettings) -> i32 { + let max_allowed_cmp = if settings.unique { + // If `unique` is enabled, the previous line must compare _less_ to the next one. + Ordering::Less + } else { + // Otherwise, the line previous line must compare _less or equal_ to the next one. + Ordering::Equal + }; let file = open(path); let (recycled_sender, recycled_receiver) = sync_channel(2); let (loaded_sender, loaded_receiver) = sync_channel(2); @@ -53,7 +60,7 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { settings, prev_chunk.line_data(), chunk.line_data(), - ) == Ordering::Greater + ) > max_allowed_cmp { if !settings.check_silent { eprintln!("sort: {}:{}: disorder: {}", path, line_idx, new_first.line); @@ -65,8 +72,7 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { for (a, b) in chunk.lines().iter().tuple_windows() { line_idx += 1; - if compare_by(a, b, settings, chunk.line_data(), chunk.line_data()) == Ordering::Greater - { + if compare_by(a, b, settings, chunk.line_data(), chunk.line_data()) > max_allowed_cmp { if !settings.check_silent { eprintln!("sort: {}:{}: disorder: {}", path, line_idx, b.line); } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 5f44ce35f..a5ca80de9 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -796,6 +796,18 @@ fn test_check_silent() { } } +#[test] +fn test_check_unique() { + // Due to a clap bug the combination "-cu" does not work. "-c -u" works. + // See https://github.com/clap-rs/clap/issues/2624 + new_ucmd!() + .args(&["-c", "-u"]) + .pipe_in("A\nA\n") + .fails() + .code_is(1) + .stderr_only("sort: -:2: disorder: A"); +} + #[test] fn test_dictionary_and_nonprinting_conflicts() { let conflicting_args = ["n", "h", "g", "M"]; From f851fb64544b83b24d00d1826252c35dedd6c819 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 26 Jul 2021 12:28:29 +0200 Subject: [PATCH 22/43] sort: initialize the salt when the random setting is on a key as well Additionall, change the type of `salt` from `String` to `Option<[u8; 16]>`. --- src/uu/sort/src/sort.rs | 32 ++++++++++++++------------------ tests/by-util/test_sort.rs | 37 ++++++++++++------------------------- 2 files changed, 26 insertions(+), 43 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 4362863d5..06bcc7ff8 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -29,7 +29,6 @@ use custom_str_cmp::custom_str_cmp; use ext_sort::ext_sort; use fnv::FnvHasher; use numeric_str_cmp::{human_numeric_str_cmp, numeric_str_cmp, NumInfo, NumInfoParseSettings}; -use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use rayon::prelude::*; use std::cmp::Ordering; @@ -183,7 +182,7 @@ pub struct GlobalSettings { unique: bool, check: bool, check_silent: bool, - salt: String, + salt: Option<[u8; 16]>, selectors: Vec, separator: Option, threads: String, @@ -266,7 +265,7 @@ impl Default for GlobalSettings { unique: false, check: false, check_silent: false, - salt: String::new(), + salt: None, selectors: vec![], separator: None, threads: String::new(), @@ -1006,7 +1005,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } else if matches.is_present(options::modes::RANDOM) || matches.value_of(options::modes::SORT) == Some("random") { - settings.salt = get_rand_string(); + settings.salt = Some(get_rand_string()); SortMode::Random } else { SortMode::Default @@ -1086,9 +1085,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if let Some(values) = matches.values_of(options::KEY) { for value in values { - settings - .selectors - .push(FieldSelector::parse(value, &settings)); + let selector = FieldSelector::parse(value, &settings); + if selector.settings.mode == SortMode::Random && settings.salt.is_none() { + settings.salt = Some(get_rand_string()); + } + settings.selectors.push(selector); } } @@ -1397,7 +1398,7 @@ fn compare_by<'a>( let settings = &selector.settings; let cmp: Ordering = match settings.mode { - SortMode::Random => random_shuffle(a_str, b_str, &global_settings.salt), + SortMode::Random => random_shuffle(a_str, b_str, &global_settings.salt.unwrap()), SortMode::Numeric => { let a_num_info = &a_line_data.num_infos [a.index * global_settings.precomputed.num_infos_per_line + num_info_index]; @@ -1546,12 +1547,8 @@ fn general_numeric_compare(a: &GeneralF64ParseResult, b: &GeneralF64ParseResult) a.partial_cmp(b).unwrap() } -fn get_rand_string() -> String { - thread_rng() - .sample_iter(&Alphanumeric) - .take(16) - .map(char::from) - .collect::() +fn get_rand_string() -> [u8; 16] { + thread_rng().sample(rand::distributions::Standard) } fn get_hash(t: &T) -> u64 { @@ -1560,10 +1557,9 @@ fn get_hash(t: &T) -> u64 { s.finish() } -fn random_shuffle(a: &str, b: &str, salt: &str) -> Ordering { - let da = get_hash(&[a, salt].concat()); - let db = get_hash(&[b, salt].concat()); - +fn random_shuffle(a: &str, b: &str, salt: &[u8]) -> Ordering { + let da = get_hash(&(a, salt)); + let db = get_hash(&(b, salt)); da.cmp(&db) } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 5f44ce35f..16437d526 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -220,32 +220,19 @@ fn test_random_shuffle_contains_all_lines() { #[test] fn test_random_shuffle_two_runs_not_the_same() { - // check to verify that two random shuffles are not equal; this has the - // potential to fail in the very unlikely event that the random order is the same - // as the starting order, or if both random sorts end up having the same order. - const FILE: &str = "default_unsorted_ints.expected"; - let (at, _ucmd) = at_and_ucmd!(); - let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); - let expected = at.read(FILE); - let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); + for arg in &["-R", "-k1,1R"] { + // check to verify that two random shuffles are not equal; this has the + // potential to fail in the very unlikely event that the random order is the same + // as the starting order, or if both random sorts end up having the same order. + const FILE: &str = "default_unsorted_ints.expected"; + let (at, _ucmd) = at_and_ucmd!(); + let result = new_ucmd!().arg(arg).arg(FILE).run().stdout_move_str(); + let expected = at.read(FILE); + let unexpected = new_ucmd!().arg(arg).arg(FILE).run().stdout_move_str(); - assert_ne!(result, expected); - assert_ne!(result, unexpected); -} - -#[test] -fn test_random_shuffle_contains_two_runs_not_the_same() { - // check to verify that two random shuffles are not equal; this has the - // potential to fail in the unlikely event that random order is the same - // as the starting order, or if both random sorts end up having the same order. - const FILE: &str = "default_unsorted_ints.expected"; - let (at, _ucmd) = at_and_ucmd!(); - let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); - let expected = at.read(FILE); - let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); - - assert_ne!(result, expected); - assert_ne!(result, unexpected); + assert_ne!(result, expected); + assert_ne!(result, unexpected); + } } #[test] From 3564dc57924acd958a77085836dd34f9fed0fd76 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 26 Jul 2021 16:27:24 +0200 Subject: [PATCH 23/43] sort: compare strings before comparing hashes Since lines that compare equal should be sorted together, we need to first compare the lines (taking settings into account). Only if they do not compare equal we should compare the hashes. --- src/uu/sort/src/sort.rs | 17 ++++++++++++++++- tests/by-util/test_sort.rs | 10 ++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 06bcc7ff8..aa4d483c0 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -1398,7 +1398,22 @@ fn compare_by<'a>( let settings = &selector.settings; let cmp: Ordering = match settings.mode { - SortMode::Random => random_shuffle(a_str, b_str, &global_settings.salt.unwrap()), + SortMode::Random => { + // check if the two strings are equal + if custom_str_cmp( + a_str, + b_str, + settings.ignore_non_printing, + settings.dictionary_order, + settings.ignore_case, + ) == Ordering::Equal + { + Ordering::Equal + } else { + // Only if they are not equal compare by the hash + random_shuffle(a_str, b_str, &global_settings.salt.unwrap()) + } + } SortMode::Numeric => { let a_num_info = &a_line_data.num_infos [a.index * global_settings.precomputed.num_infos_per_line + num_info_index]; diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 16437d526..8573ce03f 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -235,6 +235,16 @@ fn test_random_shuffle_two_runs_not_the_same() { } } +#[test] +fn test_random_ignore_case() { + let input = "ABC\nABc\nAbC\nAbc\naBC\naBc\nabC\nabc\n"; + new_ucmd!() + .args(&["-fR"]) + .pipe_in(input) + .succeeds() + .stdout_is(input); +} + #[test] fn test_numeric_floats_and_ints() { test_helper( From 849086e9c5c2dd24ca956e89a255cafa7c494cb2 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 31 Jul 2021 20:12:38 +0200 Subject: [PATCH 24/43] sort: handle cases where the output file is also an input file In such cases we have to create a temporary copy of the input file to prevent overwriting the input with the output. This only affects merge sort, because it is the only mode where we start writing to the output before having read all inputs. --- src/uu/sort/src/check.rs | 17 ++++++++-- src/uu/sort/src/merge.rs | 59 ++++++++++++++++++++++++++------ src/uu/sort/src/sort.rs | 69 +++++++++++++++++++++++++------------- tests/by-util/test_sort.rs | 10 ++++++ 4 files changed, 119 insertions(+), 36 deletions(-) diff --git a/src/uu/sort/src/check.rs b/src/uu/sort/src/check.rs index de320ef77..350a98e23 100644 --- a/src/uu/sort/src/check.rs +++ b/src/uu/sort/src/check.rs @@ -14,6 +14,7 @@ use crate::{ use itertools::Itertools; use std::{ cmp::Ordering, + ffi::OsStr, io::Read, iter, sync::mpsc::{sync_channel, Receiver, SyncSender}, @@ -25,7 +26,7 @@ use std::{ /// # Returns /// /// The code we should exit with. -pub fn check(path: &str, settings: &GlobalSettings) -> i32 { +pub fn check(path: &OsStr, settings: &GlobalSettings) -> i32 { let max_allowed_cmp = if settings.unique { // If `unique` is enabled, the previous line must compare _less_ to the next one. Ordering::Less @@ -63,7 +64,12 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { ) > max_allowed_cmp { if !settings.check_silent { - eprintln!("sort: {}:{}: disorder: {}", path, line_idx, new_first.line); + eprintln!( + "sort: {}:{}: disorder: {}", + path.to_string_lossy(), + line_idx, + new_first.line + ); } return 1; } @@ -74,7 +80,12 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { line_idx += 1; if compare_by(a, b, settings, chunk.line_data(), chunk.line_data()) > max_allowed_cmp { if !settings.check_silent { - eprintln!("sort: {}:{}: disorder: {}", path, line_idx, b.line); + eprintln!( + "sort: {}:{}: disorder: {}", + path.to_string_lossy(), + line_idx, + b.line + ); } return 1; } diff --git a/src/uu/sort/src/merge.rs b/src/uu/sort/src/merge.rs index b8d69fb14..4a45028f7 100644 --- a/src/uu/sort/src/merge.rs +++ b/src/uu/sort/src/merge.rs @@ -9,10 +9,11 @@ use std::{ cmp::Ordering, + ffi::OsString, fs::{self, File}, io::{BufWriter, Read, Write}, iter, - path::PathBuf, + path::{Path, PathBuf}, process::{Child, ChildStdin, ChildStdout, Command, Stdio}, rc::Rc, sync::mpsc::{channel, sync_channel, Receiver, Sender, SyncSender}, @@ -25,28 +26,66 @@ use tempfile::TempDir; use crate::{ chunks::{self, Chunk, RecycledChunk}, - compare_by, GlobalSettings, Output, + compare_by, open, GlobalSettings, Output, }; +/// If the output file occurs in the input files as well, copy the contents of the output file +/// and replace its occurrences in the inputs with that copy. +fn replace_output_file_in_input_files( + files: &mut [OsString], + settings: &GlobalSettings, + output: Option<&str>, +) -> Option<(TempDir, usize)> { + let mut copy: Option<(TempDir, PathBuf)> = None; + if let Some(Ok(output_path)) = output.map(|path| Path::new(path).canonicalize()) { + for file in files { + if let Ok(file_path) = Path::new(file).canonicalize() { + if file_path == output_path { + if let Some((_dir, copy)) = © { + *file = copy.clone().into_os_string(); + } else { + let tmp_dir = tempfile::Builder::new() + .prefix("uutils_sort") + .tempdir_in(&settings.tmp_dir) + .unwrap(); + let copy_path = tmp_dir.path().join("0"); + std::fs::copy(file_path, ©_path).unwrap(); + *file = copy_path.clone().into_os_string(); + copy = Some((tmp_dir, copy_path)) + } + } + } + } + } + // if we created a TempDir its size must be one. + copy.map(|(dir, _copy)| (dir, 1)) +} + /// Merge pre-sorted `Box`s. /// /// If `settings.merge_batch_size` is greater than the length of `files`, intermediate files will be used. /// If `settings.compress_prog` is `Some`, intermediate files will be compressed with it. -pub fn merge>>( - files: Files, - settings: &GlobalSettings, -) -> FileMerger { +pub fn merge<'a>( + files: &mut [OsString], + settings: &'a GlobalSettings, + output: Option<&str>, +) -> FileMerger<'a> { + let tmp_dir = replace_output_file_in_input_files(files, settings, output); if settings.compress_prog.is_none() { merge_with_file_limit::<_, _, WriteablePlainTmpFile>( - files.map(|file| PlainMergeInput { inner: file }), + files + .iter() + .map(|file| PlainMergeInput { inner: open(file) }), settings, - None, + tmp_dir, ) } else { merge_with_file_limit::<_, _, WriteableCompressedTmpFile>( - files.map(|file| PlainMergeInput { inner: file }), + files + .iter() + .map(|file| PlainMergeInput { inner: open(file) }), settings, - None, + tmp_dir, ) } } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index aa4d483c0..1f674e549 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -33,8 +33,8 @@ use rand::{thread_rng, Rng}; use rayon::prelude::*; use std::cmp::Ordering; use std::env; -use std::ffi::OsStr; -use std::fs::File; +use std::ffi::{OsStr, OsString}; +use std::fs::{File, OpenOptions}; use std::hash::{Hash, Hasher}; use std::io::{stdin, stdout, BufRead, BufReader, Read, Write}; use std::ops::Range; @@ -146,26 +146,46 @@ impl SortMode { } pub struct Output { - file: Option, + file: Option<(String, File)>, } impl Output { fn new(name: Option<&str>) -> Self { Self { file: name.map(|name| { - File::create(name).unwrap_or_else(|e| { - crash!(2, "open failed: {}: {}", name, strip_errno(&e.to_string())) - }) + // This is different from `File::create()` because we don't truncate the output yet. + // This allows using the output file as an input file. + ( + name.to_owned(), + OpenOptions::new() + .write(true) + .create(true) + .open(name) + .unwrap_or_else(|e| { + crash!(2, "open failed: {}: {}", name, strip_errno(&e.to_string())) + }), + ) }), } } fn into_write(self) -> Box { match self.file { - Some(file) => Box::new(file), + Some((_name, file)) => { + // truncate the file + file.set_len(0).unwrap(); + Box::new(file) + } None => Box::new(stdout()), } } + + fn as_output_name(&self) -> Option<&str> { + match &self.file { + Some((name, _file)) => Some(name), + None => None, + } + } } #[derive(Clone)] @@ -956,29 +976,28 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.debug = matches.is_present(options::DEBUG); // check whether user specified a zero terminated list of files for input, otherwise read files from args - let mut files: Vec = if matches.is_present(options::FILES0_FROM) { - let files0_from: Vec = matches - .values_of(options::FILES0_FROM) - .map(|v| v.map(ToString::to_string).collect()) + let mut files: Vec = if matches.is_present(options::FILES0_FROM) { + let files0_from: Vec = matches + .values_of_os(options::FILES0_FROM) + .map(|v| v.map(ToOwned::to_owned).collect()) .unwrap_or_default(); let mut files = Vec::new(); for path in &files0_from { - let reader = open(path.as_str()); + let reader = open(&path); let buf_reader = BufReader::new(reader); for line in buf_reader.split(b'\0').flatten() { - files.push( + files.push(OsString::from( std::str::from_utf8(&line) - .expect("Could not parse string from zero terminated input.") - .to_string(), - ); + .expect("Could not parse string from zero terminated input."), + )); } } files } else { matches - .values_of(options::FILES) - .map(|v| v.map(ToString::to_string).collect()) + .values_of_os(options::FILES) + .map(|v| v.map(ToOwned::to_owned).collect()) .unwrap_or_default() }; @@ -1066,9 +1085,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if files.is_empty() { /* if no file, default to stdin */ - files.push("-".to_owned()); + files.push("-".to_string().into()); } else if settings.check && files.len() != 1 { - crash!(2, "extra operand `{}' not allowed with -c", files[1]) + crash!( + 2, + "extra operand `{}' not allowed with -c", + files[1].to_string_lossy() + ) } if let Some(arg) = matches.args.get(options::SEPARATOR) { @@ -1122,7 +1145,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.init_precomputed(); - exec(&files, &settings, output) + exec(&mut files, &settings, output) } pub fn uu_app() -> App<'static, 'static> { @@ -1345,9 +1368,9 @@ pub fn uu_app() -> App<'static, 'static> { ) } -fn exec(files: &[String], settings: &GlobalSettings, output: Output) -> i32 { +fn exec(files: &mut [OsString], settings: &GlobalSettings, output: Output) -> i32 { if settings.merge { - let mut file_merger = merge::merge(files.iter().map(open), settings); + let mut file_merger = merge::merge(files, settings, output.as_output_name()); file_merger.write_all(settings, output); } else if settings.check { if files.len() > 1 { diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index b9cdb2b80..4c4a7a697 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1020,3 +1020,13 @@ fn test_separator_null() { .succeeds() .stdout_only("a\0z\0z\nz\0b\0a\nz\0a\0b\n"); } + +#[test] +fn test_output_is_input() { + let input = "a\nb\nc\n"; + let (at, mut cmd) = at_and_ucmd!(); + at.touch("file"); + at.append("file", input); + cmd.args(&["-m", "-o", "file", "file"]).succeeds(); + assert_eq!(at.read("file"), input); +} From f29239beecacadd085ef2f8d4e8bb96a47f20466 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 31 Jul 2021 16:15:22 +0200 Subject: [PATCH 25/43] sort: buffer writes to the output This fixes a regression from a33b6d87b578990893257c2b063f8579a3f960c4 --- src/uu/sort/src/sort.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 1f674e549..13c2c3cfa 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -36,7 +36,7 @@ use std::env; use std::ffi::{OsStr, OsString}; use std::fs::{File, OpenOptions}; use std::hash::{Hash, Hasher}; -use std::io::{stdin, stdout, BufRead, BufReader, Read, Write}; +use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; use std::ops::Range; use std::path::Path; use std::path::PathBuf; @@ -169,15 +169,15 @@ impl Output { } } - fn into_write(self) -> Box { - match self.file { + fn into_write(self) -> BufWriter> { + BufWriter::new(match self.file { Some((_name, file)) => { // truncate the file file.set_len(0).unwrap(); Box::new(file) } None => Box::new(stdout()), - } + }) } fn as_output_name(&self) -> Option<&str> { From 5bf4536bdde00f1268e3172d6aad3ef86c56f8e2 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 31 Jul 2021 19:40:38 +0200 Subject: [PATCH 26/43] sort: ignore failure to truncate the output file --- src/uu/sort/src/sort.rs | 2 +- tests/by-util/test_sort.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 13c2c3cfa..7c855ab19 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -173,7 +173,7 @@ impl Output { BufWriter::new(match self.file { Some((_name, file)) => { // truncate the file - file.set_len(0).unwrap(); + let _ = file.set_len(0); Box::new(file) } None => Box::new(stdout()), diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 4c4a7a697..36bed4b94 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1030,3 +1030,12 @@ fn test_output_is_input() { cmd.args(&["-m", "-o", "file", "file"]).succeeds(); assert_eq!(at.read("file"), input); } + +#[test] +#[cfg(unix)] +fn test_output_device() { + new_ucmd!() + .args(&["-o", "/dev/null"]) + .pipe_in("input") + .succeeds(); +} From 418f5b7692f3e487c875a4b6252e04e8b00a3130 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 31 Jul 2021 20:19:11 +0200 Subject: [PATCH 27/43] sort: handle empty merge inputs --- src/uu/sort/src/merge.rs | 14 ++++++++------ tests/by-util/test_sort.rs | 9 +++++++++ tests/fixtures/sort/empty.txt | 0 3 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/sort/empty.txt diff --git a/src/uu/sort/src/merge.rs b/src/uu/sort/src/merge.rs index 4a45028f7..a5ac9411b 100644 --- a/src/uu/sort/src/merge.rs +++ b/src/uu/sort/src/merge.rs @@ -194,12 +194,14 @@ fn merge_without_limit>( let mut mergeable_files = vec![]; for (file_number, receiver) in loaded_receivers.into_iter().enumerate() { - mergeable_files.push(MergeableFile { - current_chunk: Rc::new(receiver.recv().unwrap()), - file_number, - line_idx: 0, - receiver, - }) + if let Ok(chunk) = receiver.recv() { + mergeable_files.push(MergeableFile { + current_chunk: Rc::new(chunk), + file_number, + line_idx: 0, + receiver, + }) + } } FileMerger { diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 36bed4b94..8e9861a39 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1039,3 +1039,12 @@ fn test_output_device() { .pipe_in("input") .succeeds(); } + +#[test] +fn test_merge_empty_input() { + new_ucmd!() + .args(&["-m", "empty.txt"]) + .succeeds() + .no_stderr() + .no_stdout(); +} diff --git a/tests/fixtures/sort/empty.txt b/tests/fixtures/sort/empty.txt new file mode 100644 index 000000000..e69de29bb From 663c9751a164659222859a96a2ff695d3bcced72 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 31 Jul 2021 23:58:33 +0200 Subject: [PATCH 28/43] sort: do not exit with failure for "--version" or "--help" --- src/uu/sort/src/sort.rs | 20 +++++++++++++++++--- tests/by-util/test_sort.rs | 17 +++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index aa4d483c0..3bb5eea4e 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -949,9 +949,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let usage = get_usage(); let mut settings: GlobalSettings = Default::default(); - let matches = uu_app().usage(&usage[..]).get_matches_from_safe(args); - - let matches = crash_if_err!(2, matches); + let matches = match uu_app().usage(&usage[..]).get_matches_from_safe(args) { + Ok(t) => t, + Err(e) => { + // not all clap "Errors" are because of a failure to parse arguments. + // "--version" also causes an Error to be returned, but we should not print to stderr + // nor return with a non-zero exit code in this case (we should print to stdout and return 0). + // This logic is similar to the code in clap, but we return 2 as the exit code in case of real failure + // (clap returns 1). + if e.use_stderr() { + eprintln!("{}", e.message); + return 2; + } else { + println!("{}", e.message); + return 0; + } + } + }; settings.debug = matches.is_present(options::DEBUG); diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index b9cdb2b80..0b7436861 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1020,3 +1020,20 @@ fn test_separator_null() { .succeeds() .stdout_only("a\0z\0z\nz\0b\0a\nz\0a\0b\n"); } + +#[test] +fn test_no_error_for_version() { + new_ucmd!() + .arg("--version") + .succeeds() + .stdout_contains("sort"); +} + +#[test] +fn test_wrong_args_exit_code() { + new_ucmd!() + .arg("--misspelled") + .fails() + .status_code(2) + .stderr_contains("--misspelled"); +} From 55d1dc78b0ed4b84cdb6a2e4f35f52ea89726e52 Mon Sep 17 00:00:00 2001 From: Yagiz Degirmenci Date: Sun, 1 Aug 2021 01:25:21 +0300 Subject: [PATCH 29/43] feat(ln): use UResult Signed-off-by: Yagiz Degirmenci --- src/uu/ln/src/ln.rs | 84 ++++++++++++++++++++++++++++++++------------- 1 file changed, 61 insertions(+), 23 deletions(-) diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index d354acce9..e5226b97d 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -11,9 +11,12 @@ extern crate uucore; use clap::{crate_version, App, Arg}; +use uucore::error::{UCustomError, UResult}; use std::borrow::Cow; +use std::error::Error; use std::ffi::OsStr; +use std::fmt::Display; use std::fs; use std::io::{stdin, Result}; @@ -44,6 +47,51 @@ pub enum OverwriteMode { Force, } +#[derive(Debug)] +enum LnError { + TargetIsDirectory(String), + SomeLinksFailed, + FailedToLink(String), + MissingDestination(String), + ExtraOperand(String), + InvalidBackupMode(String), +} + +impl Display for LnError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::TargetIsDirectory(s) => write!(f, "target '{}' is not a directory", s), + Self::FailedToLink(s) => write!(f, "failed to link '{}'", s), + Self::SomeLinksFailed => write!(f, "some links failed to create"), + Self::MissingDestination(s) => { + write!(f, "missing destination file operand after '{}'", s) + } + Self::ExtraOperand(s) => write!( + f, + "extra operand '{}'\nTry '{} --help' for more information.", + s, + executable!() + ), + Self::InvalidBackupMode(s) => write!(f, "{}", s), + } + } +} + +impl Error for LnError {} + +impl UCustomError for LnError { + fn code(&self) -> i32 { + match self { + Self::TargetIsDirectory(_) => 1, + Self::SomeLinksFailed => 1, + Self::FailedToLink(_) => 1, + Self::MissingDestination(_) => 1, + Self::ExtraOperand(_) => 1, + Self::InvalidBackupMode(_) => 1, + } + } +} + fn get_usage() -> String { format!( "{0} [OPTION]... [-T] TARGET LINK_NAME (1st form) @@ -86,7 +134,8 @@ mod options { static ARG_FILES: &str = "files"; -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = get_usage(); let long_usage = get_long_usage(); @@ -122,8 +171,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ); let backup_mode = match backup_mode { Err(err) => { - show_usage_error!("{}", err); - return 1; + return Err(LnError::InvalidBackupMode(err.to_string().into()).into()); } Ok(mode) => mode, }; @@ -246,7 +294,7 @@ pub fn uu_app() -> App<'static, 'static> { ) } -fn exec(files: &[PathBuf], settings: &Settings) -> i32 { +fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> { // Handle cases where we create links in a directory first. if let Some(ref name) = settings.target_dir { // 4th form: a directory is specified by -t. @@ -267,35 +315,25 @@ fn exec(files: &[PathBuf], settings: &Settings) -> i32 { // 1st form. Now there should be only two operands, but if -T is // specified we may have a wrong number of operands. if files.len() == 1 { - show_error!( - "missing destination file operand after '{}'", - files[0].to_string_lossy() - ); - return 1; + return Err(LnError::MissingDestination(files[0].to_string_lossy().into()).into()); } if files.len() > 2 { - show_error!( - "extra operand '{}'\nTry '{} --help' for more information.", - files[2].display(), - executable!() - ); - return 1; + return Err(LnError::ExtraOperand(files[2].display().to_string().into()).into()); } assert!(!files.is_empty()); match link(&files[0], &files[1], settings) { - Ok(_) => 0, + Ok(_) => Ok(()), Err(e) => { - show_error!("{}", e); - 1 + // show_error!("{}", e); + Err(LnError::FailedToLink(e.to_string().into()).into()) } } } -fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> i32 { +fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> UResult<()> { if !target_dir.is_dir() { - show_error!("target '{}' is not a directory", target_dir.display()); - return 1; + return Err(LnError::TargetIsDirectory(target_dir.display().to_string()).into()); } let mut all_successful = true; @@ -354,9 +392,9 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) } } if all_successful { - 0 + Ok(()) } else { - 1 + Err(LnError::SomeLinksFailed.into()) } } From 65dd6afa133a33ced1e777240e80018c26495f41 Mon Sep 17 00:00:00 2001 From: Yagiz Degirmenci Date: Sun, 1 Aug 2021 01:27:25 +0300 Subject: [PATCH 30/43] chore(ln): delete comment Signed-off-by: Yagiz Degirmenci --- src/uu/ln/src/ln.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index e5226b97d..c7489ade7 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -324,10 +324,7 @@ fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> { match link(&files[0], &files[1], settings) { Ok(_) => Ok(()), - Err(e) => { - // show_error!("{}", e); - Err(LnError::FailedToLink(e.to_string().into()).into()) - } + Err(e) => Err(LnError::FailedToLink(e.to_string().into()).into()), } } From 95a890e5f876a382e8d6fe0633465bcda30e5079 Mon Sep 17 00:00:00 2001 From: Yagiz Degirmenci Date: Sun, 1 Aug 2021 01:42:12 +0300 Subject: [PATCH 31/43] chore(ln): fix clippy errors Signed-off-by: Yagiz Degirmenci --- src/uu/ln/src/ln.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index c7489ade7..65bdcf36c 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -171,7 +171,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { ); let backup_mode = match backup_mode { Err(err) => { - return Err(LnError::InvalidBackupMode(err.to_string().into()).into()); + return Err(LnError::InvalidBackupMode(err).into()); } Ok(mode) => mode, }; @@ -318,13 +318,13 @@ fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> { return Err(LnError::MissingDestination(files[0].to_string_lossy().into()).into()); } if files.len() > 2 { - return Err(LnError::ExtraOperand(files[2].display().to_string().into()).into()); + return Err(LnError::ExtraOperand(files[2].display().to_string()).into()); } assert!(!files.is_empty()); match link(&files[0], &files[1], settings) { Ok(_) => Ok(()), - Err(e) => Err(LnError::FailedToLink(e.to_string().into()).into()), + Err(e) => Err(LnError::FailedToLink(e.to_string()).into()), } } From 3d23ace9b8c02d0e06311db68ecec434c9ed32ac Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 1 Aug 2021 17:12:06 +0200 Subject: [PATCH 32/43] sort: remove redundant comment --- src/uu/sort/src/ext_sort.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/uu/sort/src/ext_sort.rs b/src/uu/sort/src/ext_sort.rs index ea53900e2..816bf1e1d 100644 --- a/src/uu/sort/src/ext_sort.rs +++ b/src/uu/sort/src/ext_sort.rs @@ -89,8 +89,6 @@ fn reader_writer>, Tmp: WriteableTmpFile files, &settings.tmp_dir, separator, - // Heuristically chosen: Dividing by 10 seems to keep our memory usage roughly - // around settings.buffer_size as a whole. buffer_size, settings, receiver, From af47c66a00c0912e58d346cd3a26896a8db075c1 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 1 Aug 2021 17:12:35 +0200 Subject: [PATCH 33/43] sort: improve tests --- src/uu/sort/src/check.rs | 8 +++++++- tests/by-util/test_sort.rs | 27 ++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/uu/sort/src/check.rs b/src/uu/sort/src/check.rs index 350a98e23..d82565c3d 100644 --- a/src/uu/sort/src/check.rs +++ b/src/uu/sort/src/check.rs @@ -42,7 +42,13 @@ pub fn check(path: &OsStr, settings: &GlobalSettings) -> i32 { move || reader(file, recycled_receiver, loaded_sender, &settings) }); for _ in 0..2 { - let _ = recycled_sender.send(RecycledChunk::new(100 * 1024)); + let _ = recycled_sender.send(RecycledChunk::new(if settings.buffer_size < 100 * 1024 { + // when the buffer size is smaller than 100KiB we choose it instead of the default. + // this improves testability. + settings.buffer_size + } else { + 100 * 1024 + })); } let mut prev_chunk: Option = None; diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 838eb4f98..cac9a5a5d 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -771,6 +771,7 @@ fn test_check() { new_ucmd!() .arg(diagnose_arg) .arg("check_fail.txt") + .arg("--buffer-size=10b") .fails() .stderr_only("sort: check_fail.txt:6: disorder: 5\n"); @@ -892,6 +893,29 @@ fn test_compress() { .stdout_only_fixture("ext_sort.expected"); } +#[test] +#[cfg(target_os = "linux")] +fn test_compress_merge() { + new_ucmd!() + .args(&[ + "--compress-program", + "gzip", + "-S", + "10", + "--batch-size=2", + "-m", + "--unique", + "merge_ints_interleaved_1.txt", + "merge_ints_interleaved_2.txt", + "merge_ints_interleaved_3.txt", + "merge_ints_interleaved_3.txt", + "merge_ints_interleaved_2.txt", + "merge_ints_interleaved_1.txt", + ]) + .succeeds() + .stdout_only_fixture("merge_ints_interleaved.expected"); +} + #[test] fn test_compress_fail() { TestScenario::new(util_name!()) @@ -1027,7 +1051,8 @@ fn test_output_is_input() { let (at, mut cmd) = at_and_ucmd!(); at.touch("file"); at.append("file", input); - cmd.args(&["-m", "-o", "file", "file"]).succeeds(); + cmd.args(&["-m", "-u", "-o", "file", "file", "file", "file"]) + .succeeds(); assert_eq!(at.read("file"), input); } From 450a487d763b299856a1d8ff4dfb2a753c6ec711 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 1 Aug 2021 18:02:52 +0200 Subject: [PATCH 34/43] sort: ignore broken pipes in a test Since sort exits early due to the nonexistent file, it might no longer be around when we try to send it the input. This is "by design" and can be ignored. --- tests/by-util/test_sort.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index cac9a5a5d..1f21722ed 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1000,6 +1000,7 @@ fn test_verifies_out_file() { new_ucmd!() .args(&["-o", "nonexistent_dir/nonexistent_file"]) .pipe_in(input) + .ignore_stdin_write_error() .fails() .status_code(2) .stderr_only( From e8eb15f05e5c5ba4de5989f5eb0a9eb390102738 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 1 Aug 2021 16:50:33 +0200 Subject: [PATCH 35/43] core/error: require UCustomError to be Send For multi-threaded programs like sort it is necessary to be able to send errors between threads. --- src/uucore/src/lib/mods/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs index c64e7df66..d82da9feb 100644 --- a/src/uucore/src/lib/mods/error.rs +++ b/src/uucore/src/lib/mods/error.rs @@ -251,7 +251,7 @@ impl Display for UError { /// /// A crate like [`quick_error`](https://crates.io/crates/quick-error) might /// also be used, but will still require an `impl` for the `code` method. -pub trait UCustomError: Error { +pub trait UCustomError: Error + Send { /// Error code of a custom error. /// /// Set a return value for each variant of an enum-type to associate an From 940559f0e1b98c02e470734ac6d9d4b050d0c522 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 29 Jun 2021 19:20:01 -0300 Subject: [PATCH 36/43] tail: handle `-` as an alias for stdin --- Cargo.lock | 1 + src/uu/tail/Cargo.toml | 4 ++ src/uu/tail/README.md | 2 +- src/uu/tail/src/platform/mod.rs | 3 +- src/uu/tail/src/platform/unix.rs | 22 ++++++++- src/uu/tail/src/tail.rs | 83 +++++++++++++++++++++++--------- 6 files changed, 90 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 51424332d..f411c5920 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2570,6 +2570,7 @@ version = "0.0.6" dependencies = [ "clap", "libc", + "nix 0.20.0", "redox_syscall 0.1.57", "uucore", "uucore_procs", diff --git a/src/uu/tail/Cargo.toml b/src/uu/tail/Cargo.toml index 273c67bb3..47db8dc6b 100644 --- a/src/uu/tail/Cargo.toml +++ b/src/uu/tail/Cargo.toml @@ -24,6 +24,10 @@ winapi = { version="0.3", features=["fileapi", "handleapi", "processthreadsapi", [target.'cfg(target_os = "redox")'.dependencies] redox_syscall = "0.1" +[target.'cfg(unix)'.dependencies] +nix = "0.20" +libc = "0.2" + [[bin]] name = "tail" path = "src/main.rs" diff --git a/src/uu/tail/README.md b/src/uu/tail/README.md index b7f92f8e4..94b6816af 100644 --- a/src/uu/tail/README.md +++ b/src/uu/tail/README.md @@ -11,7 +11,7 @@ ### Others -- [ ] The current implementation does not handle `-` as an alias for stdin. +- [ ] The current implementation doesn't follow stdin in non-unix platforms ## Possible optimizations diff --git a/src/uu/tail/src/platform/mod.rs b/src/uu/tail/src/platform/mod.rs index 010c5c4ac..4a8982713 100644 --- a/src/uu/tail/src/platform/mod.rs +++ b/src/uu/tail/src/platform/mod.rs @@ -2,13 +2,14 @@ * This file is part of the uutils coreutils package. * * (c) Alexander Batischev + * (c) Thomas Queiroz * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ #[cfg(unix)] -pub use self::unix::{supports_pid_checks, Pid, ProcessChecker}; +pub use self::unix::{stdin_is_pipe_or_fifo, supports_pid_checks, Pid, ProcessChecker}; #[cfg(windows)] pub use self::windows::{supports_pid_checks, Pid, ProcessChecker}; diff --git a/src/uu/tail/src/platform/unix.rs b/src/uu/tail/src/platform/unix.rs index 167f693e6..580a40135 100644 --- a/src/uu/tail/src/platform/unix.rs +++ b/src/uu/tail/src/platform/unix.rs @@ -2,6 +2,7 @@ * This file is part of the uutils coreutils package. * * (c) Alexander Batischev + * (c) Thomas Queiroz * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. @@ -9,7 +10,13 @@ // spell-checker:ignore (ToDO) errno EPERM ENOSYS -use std::io::Error; +use std::io::{stdin, Error}; + +use std::os::unix::prelude::AsRawFd; + +use nix::sys::stat::fstat; + +use libc::{S_IFIFO, S_IFSOCK}; pub type Pid = libc::pid_t; @@ -40,3 +47,16 @@ pub fn supports_pid_checks(pid: self::Pid) -> bool { fn get_errno() -> i32 { Error::last_os_error().raw_os_error().unwrap() } + +pub fn stdin_is_pipe_or_fifo() -> bool { + let fd = stdin().lock().as_raw_fd(); + fd >= 0 // GNU tail checks fd >= 0 + && match fstat(fd) { + Ok(stat) => { + let mode = stat.st_mode; + // NOTE: This is probably not the most correct way to check this + (mode & S_IFIFO != 0) || (mode & S_IFSOCK != 0) + } + Err(err) => panic!("{}", err), + } +} diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index 4970cdcc2..471c1a404 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -2,6 +2,7 @@ // * // * (c) Morten Olsen Lysgaard // * (c) Alexander Batischev +// * (c) Thomas Queiroz // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. @@ -29,6 +30,9 @@ use std::time::Duration; use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::ringbuffer::RingBuffer; +#[cfg(unix)] +use crate::platform::stdin_is_pipe_or_fifo; + pub mod options { pub mod verbosity { pub static QUIET: &str = "quiet"; @@ -130,25 +134,56 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let files: Vec = matches .values_of(options::ARG_FILES) .map(|v| v.map(ToString::to_string).collect()) - .unwrap_or_default(); + .unwrap_or_else(|| vec![String::from("-")]); - if files.is_empty() { - let mut buffer = BufReader::new(stdin()); - unbounded_tail(&mut buffer, &settings); - } else { - let multiple = files.len() > 1; - let mut first_header = true; - let mut readers = Vec::new(); + let multiple = files.len() > 1; + let mut first_header = true; + let mut readers: Vec<(Box, &String)> = Vec::new(); - for filename in &files { - if (multiple || verbose) && !quiet { - if !first_header { - println!(); - } + #[cfg(unix)] + let stdin_string = String::from("standard input"); + + for filename in &files { + let use_stdin = filename.as_str() == "-"; + if (multiple || verbose) && !quiet { + if !first_header { + println!(); + } + if use_stdin { + println!("==> standard input <=="); + } else { println!("==> {} <==", filename); } - first_header = false; + } + first_header = false; + if use_stdin { + let mut reader = BufReader::new(stdin()); + unbounded_tail(&mut reader, &settings); + + // Don't follow stdin since there are no checks for pipes/FIFOs + // + // FIXME windows has GetFileType which can determine if the file is a pipe/FIFO + // so this check can also be performed + + #[cfg(unix)] + { + /* + POSIX specification regarding tail -f + + If the input file is a regular file or if the file operand specifies a FIFO, do not + terminate after the last line of the input file has been copied, but read and copy + further bytes from the input file when they become available. If no file operand is + specified and standard input is a pipe or FIFO, the -f option shall be ignored. If + the input file is not a FIFO, pipe, or regular file, it is unspecified whether or + not the -f option shall be ignored. + */ + + if settings.follow && !stdin_is_pipe_or_fifo() { + readers.push((Box::new(reader), &stdin_string)); + } + } + } else { let path = Path::new(filename); if path.is_dir() { continue; @@ -158,20 +193,20 @@ pub fn uumain(args: impl uucore::Args) -> i32 { bounded_tail(&mut file, &settings); if settings.follow { let reader = BufReader::new(file); - readers.push(reader); + readers.push((Box::new(reader), filename)); } } else { let mut reader = BufReader::new(file); unbounded_tail(&mut reader, &settings); if settings.follow { - readers.push(reader); + readers.push((Box::new(reader), filename)); } } } + } - if settings.follow { - follow(&mut readers[..], &files[..], &settings); - } + if settings.follow { + follow(&mut readers[..], &settings); } 0 @@ -248,8 +283,12 @@ pub fn uu_app() -> App<'static, 'static> { ) } -fn follow(readers: &mut [BufReader], filenames: &[String], settings: &Settings) { +fn follow(readers: &mut [(T, &String)], settings: &Settings) { assert!(settings.follow); + if readers.is_empty() { + return; + } + let mut last = readers.len() - 1; let mut read_some = false; let mut process = platform::ProcessChecker::new(settings.pid); @@ -260,7 +299,7 @@ fn follow(readers: &mut [BufReader], filenames: &[String], settings: let pid_is_dead = !read_some && settings.pid != 0 && process.is_dead(); read_some = false; - for (i, reader) in readers.iter_mut().enumerate() { + for (i, (reader, filename)) in readers.iter_mut().enumerate() { // Print all new content since the last pass loop { let mut datum = String::new(); @@ -269,7 +308,7 @@ fn follow(readers: &mut [BufReader], filenames: &[String], settings: Ok(_) => { read_some = true; if i != last { - println!("\n==> {} <==", filenames[i]); + println!("\n==> {} <==", filename); last = i; } print!("{}", datum); From 6127ea9642854d3c596292a448ed6d99515452d1 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Mon, 12 Jul 2021 19:46:43 -0400 Subject: [PATCH 37/43] tests/tail: add test for as an alias for stdin --- tests/by-util/test_tail.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index e8dd63317..28c3580bb 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -23,6 +23,15 @@ fn test_stdin_default() { .stdout_is_fixture("foobar_stdin_default.expected"); } +#[test] +fn test_stdin_explicit() { + new_ucmd!() + .pipe_in_fixture(FOOBAR_TXT) + .arg("-") + .run() + .stdout_is_fixture("foobar_stdin_default.expected"); +} + #[test] fn test_single_default() { new_ucmd!() From 53b16e9b4387da551fbc1a43ee8b981abcb83b45 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 13 Jul 2021 19:17:25 -0400 Subject: [PATCH 38/43] docs/spell: add 'Thomas Queiroz' to spell-checker exceptions word list --- .vscode/cspell.dictionaries/people.wordlist.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.vscode/cspell.dictionaries/people.wordlist.txt b/.vscode/cspell.dictionaries/people.wordlist.txt index d7665585b..405733836 100644 --- a/.vscode/cspell.dictionaries/people.wordlist.txt +++ b/.vscode/cspell.dictionaries/people.wordlist.txt @@ -151,6 +151,9 @@ Sylvestre Ledru T Jameson Little Jameson Little +Thomas Queiroz + Thomas + Queiroz Tobias Bohumir Schottdorf Tobias Bohumir From a4709c805c8468cc0c4f3d45d98d7548a8142148 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 1 Aug 2021 16:51:56 +0200 Subject: [PATCH 39/43] sort: use UResult --- src/uu/sort/src/check.rs | 42 +++--- src/uu/sort/src/chunks.rs | 32 +++-- src/uu/sort/src/ext_sort.rs | 64 +++++---- src/uu/sort/src/merge.rs | 186 +++++++++++++----------- src/uu/sort/src/sort.rs | 273 +++++++++++++++++++++++++----------- 5 files changed, 370 insertions(+), 227 deletions(-) diff --git a/src/uu/sort/src/check.rs b/src/uu/sort/src/check.rs index d82565c3d..5be752be0 100644 --- a/src/uu/sort/src/check.rs +++ b/src/uu/sort/src/check.rs @@ -9,7 +9,7 @@ use crate::{ chunks::{self, Chunk, RecycledChunk}, - compare_by, open, GlobalSettings, + compare_by, open, GlobalSettings, SortError, }; use itertools::Itertools; use std::{ @@ -20,13 +20,14 @@ use std::{ sync::mpsc::{sync_channel, Receiver, SyncSender}, thread, }; +use uucore::error::UResult; /// Check if the file at `path` is ordered. /// /// # Returns /// /// The code we should exit with. -pub fn check(path: &OsStr, settings: &GlobalSettings) -> i32 { +pub fn check(path: &OsStr, settings: &GlobalSettings) -> UResult<()> { let max_allowed_cmp = if settings.unique { // If `unique` is enabled, the previous line must compare _less_ to the next one. Ordering::Less @@ -34,7 +35,7 @@ pub fn check(path: &OsStr, settings: &GlobalSettings) -> i32 { // Otherwise, the line previous line must compare _less or equal_ to the next one. Ordering::Equal }; - let file = open(path); + let file = open(path)?; let (recycled_sender, recycled_receiver) = sync_channel(2); let (loaded_sender, loaded_receiver) = sync_channel(2); thread::spawn({ @@ -69,15 +70,13 @@ pub fn check(path: &OsStr, settings: &GlobalSettings) -> i32 { chunk.line_data(), ) > max_allowed_cmp { - if !settings.check_silent { - eprintln!( - "sort: {}:{}: disorder: {}", - path.to_string_lossy(), - line_idx, - new_first.line - ); + return Err(SortError::Disorder { + file: path.to_owned(), + line_number: line_idx, + line: new_first.line.to_owned(), + silent: settings.check_silent, } - return 1; + .into()); } let _ = recycled_sender.send(prev_chunk.recycle()); } @@ -85,21 +84,19 @@ pub fn check(path: &OsStr, settings: &GlobalSettings) -> i32 { for (a, b) in chunk.lines().iter().tuple_windows() { line_idx += 1; if compare_by(a, b, settings, chunk.line_data(), chunk.line_data()) > max_allowed_cmp { - if !settings.check_silent { - eprintln!( - "sort: {}:{}: disorder: {}", - path.to_string_lossy(), - line_idx, - b.line - ); + return Err(SortError::Disorder { + file: path.to_owned(), + line_number: line_idx, + line: b.line.to_owned(), + silent: settings.check_silent, } - return 1; + .into()); } } prev_chunk = Some(chunk); } - 0 + Ok(()) } /// The function running on the reader thread. @@ -108,7 +105,7 @@ fn reader( receiver: Receiver, sender: SyncSender, settings: &GlobalSettings, -) { +) -> UResult<()> { let mut carry_over = vec![]; for recycled_chunk in receiver.iter() { let should_continue = chunks::read( @@ -124,9 +121,10 @@ fn reader( b'\n' }, settings, - ); + )?; if !should_continue { break; } } + Ok(()) } diff --git a/src/uu/sort/src/chunks.rs b/src/uu/sort/src/chunks.rs index 5ab98392d..80d6060d4 100644 --- a/src/uu/sort/src/chunks.rs +++ b/src/uu/sort/src/chunks.rs @@ -14,8 +14,9 @@ use std::{ use memchr::memchr_iter; use ouroboros::self_referencing; +use uucore::error::{UResult, USimpleError}; -use crate::{numeric_str_cmp::NumInfo, GeneralF64ParseResult, GlobalSettings, Line}; +use crate::{numeric_str_cmp::NumInfo, GeneralF64ParseResult, GlobalSettings, Line, SortError}; /// The chunk that is passed around between threads. /// `lines` consist of slices into `buffer`. @@ -137,10 +138,10 @@ pub fn read( max_buffer_size: Option, carry_over: &mut Vec, file: &mut T, - next_files: &mut impl Iterator, + next_files: &mut impl Iterator>, separator: u8, settings: &GlobalSettings, -) -> bool { +) -> UResult { let RecycledChunk { lines, selections, @@ -159,12 +160,12 @@ pub fn read( max_buffer_size, carry_over.len(), separator, - ); + )?; carry_over.clear(); carry_over.extend_from_slice(&buffer[read..]); if read != 0 { - let payload = Chunk::new(buffer, |buffer| { + let payload: UResult = Chunk::try_new(buffer, |buffer| { let selections = unsafe { // SAFETY: It is safe to transmute to an empty vector of selections with shorter lifetime. // It was only temporarily transmuted to a Vec> to make recycling possible. @@ -175,18 +176,19 @@ pub fn read( // because it was only temporarily transmuted to a Vec> to make recycling possible. std::mem::transmute::>, Vec>>(lines) }; - let read = crash_if_err!(2, std::str::from_utf8(&buffer[..read])); + let read = std::str::from_utf8(&buffer[..read]) + .map_err(|error| SortError::Uft8Error { error })?; let mut line_data = LineData { selections, num_infos, parsed_floats, }; parse_lines(read, &mut lines, &mut line_data, separator, settings); - ChunkContents { lines, line_data } + Ok(ChunkContents { lines, line_data }) }); - sender.send(payload).unwrap(); + sender.send(payload?).unwrap(); } - should_continue + Ok(should_continue) } /// Split `read` into `Line`s, and add them to `lines`. @@ -242,12 +244,12 @@ fn parse_lines<'a>( /// * Whether this function should be called again. fn read_to_buffer( file: &mut T, - next_files: &mut impl Iterator, + next_files: &mut impl Iterator>, buffer: &mut Vec, max_buffer_size: Option, start_offset: usize, separator: u8, -) -> (usize, bool) { +) -> UResult<(usize, bool)> { let mut read_target = &mut buffer[start_offset..]; let mut last_file_target_size = read_target.len(); loop { @@ -274,7 +276,7 @@ fn read_to_buffer( // We read enough lines. let end = last_line_end.unwrap(); // We want to include the separator here, because it shouldn't be carried over. - return (end + 1, true); + return Ok((end + 1, true)); } else { // We need to read more lines let len = buffer.len(); @@ -299,11 +301,11 @@ fn read_to_buffer( if let Some(next_file) = next_files.next() { // There is another file. last_file_target_size = leftover_len; - *file = next_file; + *file = next_file?; } else { // This was the last file. let read_len = buffer.len() - leftover_len; - return (read_len, false); + return Ok((read_len, false)); } } } @@ -313,7 +315,7 @@ fn read_to_buffer( Err(e) if e.kind() == ErrorKind::Interrupted => { // retry } - Err(e) => crash!(2, "{}", e), + Err(e) => return Err(USimpleError::new(2, e.to_string())), } } } diff --git a/src/uu/sort/src/ext_sort.rs b/src/uu/sort/src/ext_sort.rs index 816bf1e1d..8ff5665fd 100644 --- a/src/uu/sort/src/ext_sort.rs +++ b/src/uu/sort/src/ext_sort.rs @@ -22,6 +22,7 @@ use std::{ }; use itertools::Itertools; +use uucore::error::UResult; use crate::chunks::RecycledChunk; use crate::merge::ClosedTmpFile; @@ -29,6 +30,7 @@ use crate::merge::WriteableCompressedTmpFile; use crate::merge::WriteablePlainTmpFile; use crate::merge::WriteableTmpFile; use crate::Output; +use crate::SortError; use crate::{ chunks::{self, Chunk}, compare_by, merge, sort_by, GlobalSettings, @@ -40,10 +42,10 @@ const START_BUFFER_SIZE: usize = 8_000; /// Sort files by using auxiliary files for storing intermediate chunks (if needed), and output the result. pub fn ext_sort( - files: &mut impl Iterator>, + files: &mut impl Iterator>>, settings: &GlobalSettings, output: Output, -) { +) -> UResult<()> { let (sorted_sender, sorted_receiver) = std::sync::mpsc::sync_channel(1); let (recycled_sender, recycled_receiver) = std::sync::mpsc::sync_channel(1); thread::spawn({ @@ -57,7 +59,7 @@ pub fn ext_sort( sorted_receiver, recycled_sender, output, - ); + ) } else { reader_writer::<_, WriteablePlainTmpFile>( files, @@ -65,17 +67,20 @@ pub fn ext_sort( sorted_receiver, recycled_sender, output, - ); + ) } } -fn reader_writer>, Tmp: WriteableTmpFile + 'static>( +fn reader_writer< + F: Iterator>>, + Tmp: WriteableTmpFile + 'static, +>( files: F, settings: &GlobalSettings, receiver: Receiver, sender: SyncSender, output: Output, -) { +) -> UResult<()> { let separator = if settings.zero_terminated { b'\0' } else { @@ -93,16 +98,16 @@ fn reader_writer>, Tmp: WriteableTmpFile settings, receiver, sender, - ); + )?; match read_result { ReadResult::WroteChunksToFile { tmp_files, tmp_dir } => { let tmp_dir_size = tmp_files.len(); - let mut merger = merge::merge_with_file_limit::<_, _, Tmp>( + let merger = merge::merge_with_file_limit::<_, _, Tmp>( tmp_files.into_iter().map(|c| c.reopen()), settings, Some((tmp_dir, tmp_dir_size)), - ); - merger.write_all(settings, output); + )?; + merger.write_all(settings, output)?; } ReadResult::SortedSingleChunk(chunk) => { if settings.unique { @@ -145,6 +150,7 @@ fn reader_writer>, Tmp: WriteableTmpFile // don't output anything } } + Ok(()) } /// The function that is executed on the sorter thread. @@ -153,7 +159,11 @@ fn sorter(receiver: Receiver, sender: SyncSender, settings: Global payload.with_contents_mut(|contents| { sort_by(&mut contents.lines, &settings, &contents.line_data) }); - sender.send(payload).unwrap(); + if sender.send(payload).is_err() { + // The receiver has gone away, likely because the other thread hit an error. + // We stop silently because the actual error is printed by the other thread. + return; + } } } @@ -173,15 +183,15 @@ enum ReadResult { } /// The function that is executed on the reader/writer thread. fn read_write_loop( - mut files: impl Iterator>, + mut files: impl Iterator>>, tmp_dir_parent: &Path, separator: u8, buffer_size: usize, settings: &GlobalSettings, receiver: Receiver, sender: SyncSender, -) -> ReadResult { - let mut file = files.next().unwrap(); +) -> UResult> { + let mut file = files.next().unwrap()?; let mut carry_over = vec![]; // kick things off with two reads @@ -199,14 +209,14 @@ fn read_write_loop( &mut files, separator, settings, - ); + )?; if !should_continue { drop(sender); // We have already read the whole input. Since we are in our first two reads, // this means that we can fit the whole input into memory. Bypass writing below and // handle this case in a more straightforward way. - return if let Ok(first_chunk) = receiver.recv() { + return Ok(if let Ok(first_chunk) = receiver.recv() { if let Ok(second_chunk) = receiver.recv() { ReadResult::SortedTwoChunks([first_chunk, second_chunk]) } else { @@ -214,16 +224,14 @@ fn read_write_loop( } } else { ReadResult::EmptyInput - }; + }); } } - let tmp_dir = crash_if_err!( - 2, - tempfile::Builder::new() - .prefix("uutils_sort") - .tempdir_in(tmp_dir_parent) - ); + let tmp_dir = tempfile::Builder::new() + .prefix("uutils_sort") + .tempdir_in(tmp_dir_parent) + .map_err(|_| SortError::TmpDirCreationFailed)?; let mut sender_option = Some(sender); let mut file_number = 0; @@ -232,7 +240,7 @@ fn read_write_loop( let mut chunk = match receiver.recv() { Ok(it) => it, _ => { - return ReadResult::WroteChunksToFile { tmp_files, tmp_dir }; + return Ok(ReadResult::WroteChunksToFile { tmp_files, tmp_dir }); } }; @@ -241,7 +249,7 @@ fn read_write_loop( tmp_dir.path().join(file_number.to_string()), settings.compress_prog.as_deref(), separator, - ); + )?; tmp_files.push(tmp_file); file_number += 1; @@ -258,7 +266,7 @@ fn read_write_loop( &mut files, separator, settings, - ); + )?; if !should_continue { sender_option = None; } @@ -273,8 +281,8 @@ fn write( file: PathBuf, compress_prog: Option<&str>, separator: u8, -) -> I::Closed { - let mut tmp_file = I::create(file, compress_prog); +) -> UResult { + let mut tmp_file = I::create(file, compress_prog)?; write_lines(chunk.lines(), tmp_file.as_write(), separator); tmp_file.finished_writing() } diff --git a/src/uu/sort/src/merge.rs b/src/uu/sort/src/merge.rs index a5ac9411b..fad966f64 100644 --- a/src/uu/sort/src/merge.rs +++ b/src/uu/sort/src/merge.rs @@ -17,16 +17,17 @@ use std::{ process::{Child, ChildStdin, ChildStdout, Command, Stdio}, rc::Rc, sync::mpsc::{channel, sync_channel, Receiver, Sender, SyncSender}, - thread, + thread::{self, JoinHandle}, }; use compare::Compare; use itertools::Itertools; use tempfile::TempDir; +use uucore::error::UResult; use crate::{ chunks::{self, Chunk, RecycledChunk}, - compare_by, open, GlobalSettings, Output, + compare_by, open, GlobalSettings, Output, SortError, }; /// If the output file occurs in the input files as well, copy the contents of the output file @@ -35,7 +36,7 @@ fn replace_output_file_in_input_files( files: &mut [OsString], settings: &GlobalSettings, output: Option<&str>, -) -> Option<(TempDir, usize)> { +) -> UResult> { let mut copy: Option<(TempDir, PathBuf)> = None; if let Some(Ok(output_path)) = output.map(|path| Path::new(path).canonicalize()) { for file in files { @@ -47,9 +48,10 @@ fn replace_output_file_in_input_files( let tmp_dir = tempfile::Builder::new() .prefix("uutils_sort") .tempdir_in(&settings.tmp_dir) - .unwrap(); + .map_err(|_| SortError::TmpDirCreationFailed)?; let copy_path = tmp_dir.path().join("0"); - std::fs::copy(file_path, ©_path).unwrap(); + std::fs::copy(file_path, ©_path) + .map_err(|error| SortError::OpenTmpFileFailed { error })?; *file = copy_path.clone().into_os_string(); copy = Some((tmp_dir, copy_path)) } @@ -58,7 +60,7 @@ fn replace_output_file_in_input_files( } } // if we created a TempDir its size must be one. - copy.map(|(dir, _copy)| (dir, 1)) + Ok(copy.map(|(dir, _copy)| (dir, 1))) } /// Merge pre-sorted `Box`s. @@ -69,13 +71,13 @@ pub fn merge<'a>( files: &mut [OsString], settings: &'a GlobalSettings, output: Option<&str>, -) -> FileMerger<'a> { - let tmp_dir = replace_output_file_in_input_files(files, settings, output); +) -> UResult> { + let tmp_dir = replace_output_file_in_input_files(files, settings, output)?; if settings.compress_prog.is_none() { merge_with_file_limit::<_, _, WriteablePlainTmpFile>( files .iter() - .map(|file| PlainMergeInput { inner: open(file) }), + .map(|file| open(file).map(|file| PlainMergeInput { inner: file })), settings, tmp_dir, ) @@ -83,7 +85,7 @@ pub fn merge<'a>( merge_with_file_limit::<_, _, WriteableCompressedTmpFile>( files .iter() - .map(|file| PlainMergeInput { inner: open(file) }), + .map(|file| open(file).map(|file| PlainMergeInput { inner: file })), settings, tmp_dir, ) @@ -93,24 +95,25 @@ pub fn merge<'a>( // Merge already sorted `MergeInput`s. pub fn merge_with_file_limit< M: MergeInput + 'static, - F: ExactSizeIterator, + F: ExactSizeIterator>, Tmp: WriteableTmpFile + 'static, >( files: F, settings: &GlobalSettings, tmp_dir: Option<(TempDir, usize)>, -) -> FileMerger { +) -> UResult { if files.len() > settings.merge_batch_size { // If we did not get a tmp_dir, create one. - let (tmp_dir, mut tmp_dir_size) = tmp_dir.unwrap_or_else(|| { - ( + let (tmp_dir, mut tmp_dir_size) = match tmp_dir { + Some(x) => x, + None => ( tempfile::Builder::new() .prefix("uutils_sort") .tempdir_in(&settings.tmp_dir) - .unwrap(), + .map_err(|_| SortError::TmpDirCreationFailed)?, 0, - ) - }); + ), + }; let mut remaining_files = files.len(); let batches = files.chunks(settings.merge_batch_size); let mut batches = batches.into_iter(); @@ -118,14 +121,14 @@ pub fn merge_with_file_limit< while remaining_files != 0 { // Work around the fact that `Chunks` is not an `ExactSizeIterator`. remaining_files = remaining_files.saturating_sub(settings.merge_batch_size); - let mut merger = merge_without_limit(batches.next().unwrap(), settings); + let merger = merge_without_limit(batches.next().unwrap(), settings)?; let mut tmp_file = Tmp::create( tmp_dir.path().join(tmp_dir_size.to_string()), settings.compress_prog.as_deref(), - ); + )?; tmp_dir_size += 1; - merger.write_all_to(settings, tmp_file.as_write()); - temporary_files.push(tmp_file.finished_writing()); + merger.write_all_to(settings, tmp_file.as_write())?; + temporary_files.push(tmp_file.finished_writing()?); } assert!(batches.next().is_none()); merge_with_file_limit::<_, _, Tmp>( @@ -133,7 +136,7 @@ pub fn merge_with_file_limit< .into_iter() .map(Box::new(|c: Tmp::Closed| c.reopen()) as Box< - dyn FnMut(Tmp::Closed) -> ::Reopened, + dyn FnMut(Tmp::Closed) -> UResult<::Reopened>, >), settings, Some((tmp_dir, tmp_dir_size)), @@ -147,10 +150,10 @@ pub fn merge_with_file_limit< /// /// It is the responsibility of the caller to ensure that `files` yields only /// as many files as we are allowed to open concurrently. -fn merge_without_limit>( +fn merge_without_limit>>( files: F, settings: &GlobalSettings, -) -> FileMerger { +) -> UResult { let (request_sender, request_receiver) = channel(); let mut reader_files = Vec::with_capacity(files.size_hint().0); let mut loaded_receivers = Vec::with_capacity(files.size_hint().0); @@ -158,7 +161,7 @@ fn merge_without_limit>( let (sender, receiver) = sync_channel(2); loaded_receivers.push(receiver); reader_files.push(Some(ReaderFile { - file, + file: file?, sender, carry_over: vec![], })); @@ -175,7 +178,7 @@ fn merge_without_limit>( .unwrap(); } - thread::spawn({ + let reader_join_handle = thread::spawn({ let settings = settings.clone(); move || { reader( @@ -204,14 +207,15 @@ fn merge_without_limit>( } } - FileMerger { + Ok(FileMerger { heap: binary_heap_plus::BinaryHeap::from_vec_cmp( mergeable_files, FileComparator { settings }, ), request_sender, prev: None, - } + reader_join_handle, + }) } /// The struct on the reader thread representing an input file struct ReaderFile { @@ -226,7 +230,7 @@ fn reader( files: &mut [Option>], settings: &GlobalSettings, separator: u8, -) { +) -> UResult<()> { for (file_idx, recycled_chunk) in recycled_receiver.iter() { if let Some(ReaderFile { file, @@ -243,15 +247,16 @@ fn reader( &mut iter::empty(), separator, settings, - ); + )?; if !should_continue { // Remove the file from the list by replacing it with `None`. let ReaderFile { file, .. } = files[file_idx].take().unwrap(); // Depending on the kind of the `MergeInput`, this may delete the file: - file.finished_reading(); + file.finished_reading()?; } } } + Ok(()) } /// The struct on the main thread representing an input file pub struct MergeableFile { @@ -275,17 +280,20 @@ pub struct FileMerger<'a> { heap: binary_heap_plus::BinaryHeap>, request_sender: Sender<(usize, RecycledChunk)>, prev: Option, + reader_join_handle: JoinHandle>, } impl<'a> FileMerger<'a> { /// Write the merged contents to the output file. - pub fn write_all(&mut self, settings: &GlobalSettings, output: Output) { + pub fn write_all(self, settings: &GlobalSettings, output: Output) -> UResult<()> { let mut out = output.into_write(); - self.write_all_to(settings, &mut out); + self.write_all_to(settings, &mut out) } - pub fn write_all_to(&mut self, settings: &GlobalSettings, out: &mut impl Write) { + pub fn write_all_to(mut self, settings: &GlobalSettings, out: &mut impl Write) -> UResult<()> { while self.write_next(settings, out) {} + drop(self.request_sender); + self.reader_join_handle.join().unwrap() } fn write_next(&mut self, settings: &GlobalSettings, out: &mut impl Write) -> bool { @@ -369,36 +377,41 @@ impl<'a> Compare for FileComparator<'a> { } // Wait for the child to exit and check its exit code. -fn assert_child_success(mut child: Child, program: &str) { +fn check_child_success(mut child: Child, program: &str) -> UResult<()> { if !matches!( child.wait().map(|e| e.code()), Ok(Some(0)) | Ok(None) | Err(_) ) { - crash!(2, "'{}' terminated abnormally", program) + Err(SortError::CompressProgTerminatedAbnormally { + prog: program.to_owned(), + } + .into()) + } else { + Ok(()) } } /// A temporary file that can be written to. -pub trait WriteableTmpFile { +pub trait WriteableTmpFile: Sized { type Closed: ClosedTmpFile; type InnerWrite: Write; - fn create(path: PathBuf, compress_prog: Option<&str>) -> Self; + fn create(path: PathBuf, compress_prog: Option<&str>) -> UResult; /// Closes the temporary file. - fn finished_writing(self) -> Self::Closed; + fn finished_writing(self) -> UResult; fn as_write(&mut self) -> &mut Self::InnerWrite; } /// A temporary file that is (temporarily) closed, but can be reopened. pub trait ClosedTmpFile { type Reopened: MergeInput; /// Reopens the temporary file. - fn reopen(self) -> Self::Reopened; + fn reopen(self) -> UResult; } /// A pre-sorted input for merging. pub trait MergeInput: Send { type InnerRead: Read; /// Cleans this `MergeInput` up. /// Implementations may delete the backing file. - fn finished_reading(self); + fn finished_reading(self) -> UResult<()>; fn as_read(&mut self) -> &mut Self::InnerRead; } @@ -417,15 +430,17 @@ impl WriteableTmpFile for WriteablePlainTmpFile { type Closed = ClosedPlainTmpFile; type InnerWrite = BufWriter; - fn create(path: PathBuf, _: Option<&str>) -> Self { - WriteablePlainTmpFile { - file: BufWriter::new(File::create(&path).unwrap()), + fn create(path: PathBuf, _: Option<&str>) -> UResult { + Ok(WriteablePlainTmpFile { + file: BufWriter::new( + File::create(&path).map_err(|error| SortError::OpenTmpFileFailed { error })?, + ), path, - } + }) } - fn finished_writing(self) -> Self::Closed { - ClosedPlainTmpFile { path: self.path } + fn finished_writing(self) -> UResult { + Ok(ClosedPlainTmpFile { path: self.path }) } fn as_write(&mut self) -> &mut Self::InnerWrite { @@ -434,18 +449,22 @@ impl WriteableTmpFile for WriteablePlainTmpFile { } impl ClosedTmpFile for ClosedPlainTmpFile { type Reopened = PlainTmpMergeInput; - fn reopen(self) -> Self::Reopened { - PlainTmpMergeInput { - file: File::open(&self.path).unwrap(), + fn reopen(self) -> UResult { + Ok(PlainTmpMergeInput { + file: File::open(&self.path).map_err(|error| SortError::OpenTmpFileFailed { error })?, path: self.path, - } + }) } } impl MergeInput for PlainTmpMergeInput { type InnerRead = File; - fn finished_reading(self) { - fs::remove_file(self.path).ok(); + fn finished_reading(self) -> UResult<()> { + // we ignore failures to delete the temporary file, + // because there is a race at the end of the execution and the whole + // temporary directory might already be gone. + let _ = fs::remove_file(self.path); + Ok(()) } fn as_read(&mut self) -> &mut Self::InnerRead { @@ -473,35 +492,33 @@ impl WriteableTmpFile for WriteableCompressedTmpFile { type Closed = ClosedCompressedTmpFile; type InnerWrite = BufWriter; - fn create(path: PathBuf, compress_prog: Option<&str>) -> Self { + fn create(path: PathBuf, compress_prog: Option<&str>) -> UResult { let compress_prog = compress_prog.unwrap(); let mut command = Command::new(compress_prog); - command - .stdin(Stdio::piped()) - .stdout(File::create(&path).unwrap()); - let mut child = crash_if_err!( - 2, - command.spawn().map_err(|err| format!( - "couldn't execute compress program: errno {}", - err.raw_os_error().unwrap() - )) - ); + let tmp_file = + File::create(&path).map_err(|error| SortError::OpenTmpFileFailed { error })?; + command.stdin(Stdio::piped()).stdout(tmp_file); + let mut child = command + .spawn() + .map_err(|err| SortError::CompressProgExecutionFailed { + code: err.raw_os_error().unwrap(), + })?; let child_stdin = child.stdin.take().unwrap(); - WriteableCompressedTmpFile { + Ok(WriteableCompressedTmpFile { path, compress_prog: compress_prog.to_owned(), child, child_stdin: BufWriter::new(child_stdin), - } + }) } - fn finished_writing(self) -> Self::Closed { + fn finished_writing(self) -> UResult { drop(self.child_stdin); - assert_child_success(self.child, &self.compress_prog); - ClosedCompressedTmpFile { + check_child_success(self.child, &self.compress_prog)?; + Ok(ClosedCompressedTmpFile { path: self.path, compress_prog: self.compress_prog, - } + }) } fn as_write(&mut self) -> &mut Self::InnerWrite { @@ -511,33 +528,32 @@ impl WriteableTmpFile for WriteableCompressedTmpFile { impl ClosedTmpFile for ClosedCompressedTmpFile { type Reopened = CompressedTmpMergeInput; - fn reopen(self) -> Self::Reopened { + fn reopen(self) -> UResult { let mut command = Command::new(&self.compress_prog); let file = File::open(&self.path).unwrap(); command.stdin(file).stdout(Stdio::piped()).arg("-d"); - let mut child = crash_if_err!( - 2, - command.spawn().map_err(|err| format!( - "couldn't execute compress program: errno {}", - err.raw_os_error().unwrap() - )) - ); + let mut child = command + .spawn() + .map_err(|err| SortError::CompressProgExecutionFailed { + code: err.raw_os_error().unwrap(), + })?; let child_stdout = child.stdout.take().unwrap(); - CompressedTmpMergeInput { + Ok(CompressedTmpMergeInput { path: self.path, compress_prog: self.compress_prog, child, child_stdout, - } + }) } } impl MergeInput for CompressedTmpMergeInput { type InnerRead = ChildStdout; - fn finished_reading(self) { + fn finished_reading(self) -> UResult<()> { drop(self.child_stdout); - assert_child_success(self.child, &self.compress_prog); - fs::remove_file(self.path).ok(); + check_child_success(self.child, &self.compress_prog)?; + let _ = fs::remove_file(self.path); + Ok(()) } fn as_read(&mut self) -> &mut Self::InnerRead { @@ -550,7 +566,9 @@ pub struct PlainMergeInput { } impl MergeInput for PlainMergeInput { type InnerRead = R; - fn finished_reading(self) {} + fn finished_reading(self) -> UResult<()> { + Ok(()) + } fn as_read(&mut self) -> &mut Self::InnerRead { &mut self.inner } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 77cc4e9e9..be9510aef 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -33,14 +33,18 @@ use rand::{thread_rng, Rng}; use rayon::prelude::*; use std::cmp::Ordering; use std::env; +use std::error::Error; use std::ffi::{OsStr, OsString}; +use std::fmt::Display; use std::fs::{File, OpenOptions}; use std::hash::{Hash, Hasher}; use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; use std::ops::Range; use std::path::Path; use std::path::PathBuf; +use std::str::Utf8Error; use unicode_width::UnicodeWidthStr; +use uucore::error::{set_exit_code, UCustomError, UResult, USimpleError, UUsageError}; use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::version_cmp::version_cmp; use uucore::InvalidEncodingHandling; @@ -120,6 +124,111 @@ const POSITIVE: char = '+'; // available memory into consideration, instead of relying on this constant only. const DEFAULT_BUF_SIZE: usize = 1_000_000_000; // 1 GB +#[derive(Debug)] +enum SortError { + Disorder { + file: OsString, + line_number: usize, + line: String, + silent: bool, + }, + OpenFailed { + path: String, + error: std::io::Error, + }, + ReadFailed { + path: String, + error: std::io::Error, + }, + ParseKeyError { + key: String, + msg: String, + }, + OpenTmpFileFailed { + error: std::io::Error, + }, + CompressProgExecutionFailed { + code: i32, + }, + CompressProgTerminatedAbnormally { + prog: String, + }, + TmpDirCreationFailed, + Uft8Error { + error: Utf8Error, + }, +} + +impl Error for SortError {} + +impl UCustomError for SortError { + fn code(&self) -> i32 { + match self { + SortError::Disorder { .. } => 1, + _ => 2, + } + } + + fn usage(&self) -> bool { + false + } +} + +impl Display for SortError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SortError::Disorder { + file, + line_number, + line, + silent, + } => { + if !silent { + write!( + f, + "{}:{}: disorder: {}", + file.to_string_lossy(), + line_number, + line + ) + } else { + Ok(()) + } + } + SortError::OpenFailed { path, error } => write!( + f, + "open failed: {}: {}", + path, + strip_errno(&error.to_string()) + ), + SortError::ParseKeyError { key, msg } => { + write!(f, "failed to parse key `{}`: {}", key, msg) + } + SortError::ReadFailed { path, error } => write!( + f, + "cannot read: {}: {}", + path, + strip_errno(&error.to_string()) + ), + SortError::OpenTmpFileFailed { error } => { + write!( + f, + "failed to open temporary file: {}", + strip_errno(&error.to_string()) + ) + } + SortError::CompressProgExecutionFailed { code } => { + write!(f, "couldn't execute compress program: errno {}", code) + } + SortError::CompressProgTerminatedAbnormally { prog } => { + write!(f, "'{}' terminated abnormally", prog) + } + SortError::TmpDirCreationFailed => write!(f, "could not create temporary directory"), + SortError::Uft8Error { error } => write!(f, "{}", error), + } + } +} + #[derive(Eq, Ord, PartialEq, PartialOrd, Clone, Copy, Debug)] enum SortMode { Numeric, @@ -150,23 +259,23 @@ pub struct Output { } impl Output { - fn new(name: Option<&str>) -> Self { - Self { - file: name.map(|name| { - // This is different from `File::create()` because we don't truncate the output yet. - // This allows using the output file as an input file. - ( - name.to_owned(), - OpenOptions::new() - .write(true) - .create(true) - .open(name) - .unwrap_or_else(|e| { - crash!(2, "open failed: {}: {}", name, strip_errno(&e.to_string())) - }), - ) - }), - } + fn new(name: Option<&str>) -> UResult { + let file = if let Some(name) = name { + // This is different from `File::create()` because we don't truncate the output yet. + // This allows using the output file as an input file. + let file = OpenOptions::new() + .write(true) + .create(true) + .open(name) + .map_err(|e| SortError::OpenFailed { + path: name.to_owned(), + error: e, + })?; + Some((name.to_owned(), file)) + } else { + None + }; + Ok(Self { file }) } fn into_write(self) -> BufWriter> { @@ -724,33 +833,37 @@ impl FieldSelector { } } - fn parse(key: &str, global_settings: &GlobalSettings) -> Self { + fn parse(key: &str, global_settings: &GlobalSettings) -> UResult { let mut from_to = key.split(','); let (from, from_options) = Self::split_key_options(from_to.next().unwrap()); let to = from_to.next().map(|to| Self::split_key_options(to)); let options_are_empty = from_options.is_empty() && matches!(to, None | Some((_, ""))); - crash_if_err!( - 2, - if options_are_empty { - // Inherit the global settings if there are no options attached to this key. - (|| { - // This would be ideal for a try block, I think. In the meantime this closure allows - // to use the `?` operator here. - Self::new( - KeyPosition::new(from, 1, global_settings.ignore_leading_blanks)?, - to.map(|(to, _)| { - KeyPosition::new(to, 0, global_settings.ignore_leading_blanks) - }) - .transpose()?, - KeySettings::from(global_settings), - ) - })() - } else { - // Do not inherit from `global_settings`, as there are options attached to this key. - Self::parse_with_options((from, from_options), to) + + if options_are_empty { + // Inherit the global settings if there are no options attached to this key. + (|| { + // This would be ideal for a try block, I think. In the meantime this closure allows + // to use the `?` operator here. + Self::new( + KeyPosition::new(from, 1, global_settings.ignore_leading_blanks)?, + to.map(|(to, _)| { + KeyPosition::new(to, 0, global_settings.ignore_leading_blanks) + }) + .transpose()?, + KeySettings::from(global_settings), + ) + })() + } else { + // Do not inherit from `global_settings`, as there are options attached to this key. + Self::parse_with_options((from, from_options), to) + } + .map_err(|msg| { + SortError::ParseKeyError { + key: key.to_owned(), + msg, } - .map_err(|e| format!("failed to parse key `{}`: {}", key, e)) - ) + .into() + }) } fn parse_with_options( @@ -962,7 +1075,8 @@ fn make_sort_mode_arg<'a, 'b>(mode: &'a str, short: &'b str, help: &'b str) -> A arg } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args .collect_str(InvalidEncodingHandling::Ignore) .accept_any(); @@ -979,11 +1093,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // (clap returns 1). if e.use_stderr() { eprintln!("{}", e.message); - return 2; + set_exit_code(2); } else { println!("{}", e.message); - return 0; } + return Ok(()); } }; @@ -998,7 +1112,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let mut files = Vec::new(); for path in &files0_from { - let reader = open(&path); + let reader = open(&path)?; let buf_reader = BufReader::new(reader); for line in buf_reader.split(b'\0').flatten() { files.push(OsString::from( @@ -1055,12 +1169,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { env::set_var("RAYON_NUM_THREADS", &settings.threads); } - settings.buffer_size = matches - .value_of(options::BUF_SIZE) - .map_or(DEFAULT_BUF_SIZE, |s| { - GlobalSettings::parse_byte_count(s) - .unwrap_or_else(|e| crash!(2, "{}", format_error_message(e, s, options::BUF_SIZE))) - }); + settings.buffer_size = + matches + .value_of(options::BUF_SIZE) + .map_or(Ok(DEFAULT_BUF_SIZE), |s| { + GlobalSettings::parse_byte_count(s).map_err(|e| { + USimpleError::new(2, format_error_message(e, s, options::BUF_SIZE)) + }) + })?; settings.tmp_dir = matches .value_of(options::TMP_DIR) @@ -1070,9 +1186,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.compress_prog = matches.value_of(options::COMPRESS_PROG).map(String::from); if let Some(n_merge) = matches.value_of(options::BATCH_SIZE) { - settings.merge_batch_size = n_merge - .parse() - .unwrap_or_else(|_| crash!(2, "invalid --batch-size argument '{}'", n_merge)); + settings.merge_batch_size = n_merge.parse().map_err(|_| { + UUsageError::new(2, format!("invalid --batch-size argument '{}'", n_merge)) + })?; } settings.zero_terminated = matches.is_present(options::ZERO_TERMINATED); @@ -1101,11 +1217,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 { /* if no file, default to stdin */ files.push("-".to_string().into()); } else if settings.check && files.len() != 1 { - crash!( + return Err(UUsageError::new( 2, - "extra operand `{}' not allowed with -c", - files[1].to_string_lossy() - ) + format!( + "extra operand `{}' not allowed with -c", + files[1].to_string_lossy() + ), + )); } if let Some(arg) = matches.args.get(options::SEPARATOR) { @@ -1115,14 +1233,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { separator = "\0"; } if separator.len() != 1 { - crash!(2, "separator must be exactly one character long"); + return Err(UUsageError::new( + 2, + "separator must be exactly one character long".into(), + )); } settings.separator = Some(separator.chars().next().unwrap()) } if let Some(values) = matches.values_of(options::KEY) { for value in values { - let selector = FieldSelector::parse(value, &settings); + let selector = FieldSelector::parse(value, &settings)?; if selector.settings.mode == SortMode::Random && settings.salt.is_none() { settings.salt = Some(get_rand_string()); } @@ -1152,10 +1273,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // and to reopen them at a later point. This is different from how the output file is handled, // probably to prevent running out of file descriptors. for file in &files { - open(file); + open(file)?; } - let output = Output::new(matches.value_of(options::OUTPUT)); + let output = Output::new(matches.value_of(options::OUTPUT))?; settings.init_precomputed(); @@ -1382,21 +1503,20 @@ pub fn uu_app() -> App<'static, 'static> { ) } -fn exec(files: &mut [OsString], settings: &GlobalSettings, output: Output) -> i32 { +fn exec(files: &mut [OsString], settings: &GlobalSettings, output: Output) -> UResult<()> { if settings.merge { - let mut file_merger = merge::merge(files, settings, output.as_output_name()); - file_merger.write_all(settings, output); + let file_merger = merge::merge(files, settings, output.as_output_name())?; + file_merger.write_all(settings, output) } else if settings.check { if files.len() > 1 { - crash!(2, "only one file allowed with -c"); + Err(UUsageError::new(2, "only one file allowed with -c".into())) + } else { + check::check(files.first().unwrap(), settings) } - return check::check(files.first().unwrap(), settings); } else { let mut lines = files.iter().map(open); - - ext_sort(&mut lines, settings, output); + ext_sort(&mut lines, settings, output) } - 0 } fn sort_by<'a>(unsorted: &mut Vec>, settings: &GlobalSettings, line_data: &LineData<'a>) { @@ -1692,25 +1812,22 @@ fn strip_errno(err: &str) -> &str { &err[..err.find(" (os error ").unwrap_or(err.len())] } -fn open(path: impl AsRef) -> Box { +fn open(path: impl AsRef) -> UResult> { let path = path.as_ref(); if path == "-" { let stdin = stdin(); - return Box::new(stdin) as Box; + return Ok(Box::new(stdin) as Box); } let path = Path::new(path); match File::open(path) { - Ok(f) => Box::new(f) as Box, - Err(e) => { - crash!( - 2, - "cannot read: {0}: {1}", - path.to_string_lossy(), - strip_errno(&e.to_string()) - ); + Ok(f) => Ok(Box::new(f) as Box), + Err(error) => Err(SortError::ReadFailed { + path: path.to_string_lossy().to_string(), + error, } + .into()), } } From d74fb62df7d6b1dfc151f738687e936275adc214 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 25 Jul 2021 14:25:41 -0400 Subject: [PATCH 40/43] tac: support null separator --- src/uu/tac/src/tac.rs | 16 ++++++---------- tests/by-util/test_tac.rs | 9 +++++++++ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 407228e36..01cf09215 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -35,15 +35,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let before = matches.is_present(options::BEFORE); let regex = matches.is_present(options::REGEX); - let separator = match matches.value_of(options::SEPARATOR) { - Some(m) => { - if m.is_empty() { - crash!(1, "separator cannot be empty") - } else { - m.to_owned() - } - } - None => "\n".to_owned(), + let raw_separator = matches.value_of(options::SEPARATOR).unwrap_or("\n"); + let separator = if raw_separator.is_empty() { + "\0" + } else { + raw_separator }; let files: Vec = match matches.values_of(options::FILE) { @@ -51,7 +47,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { None => vec!["-".to_owned()], }; - tac(files, before, regex, &separator[..]) + tac(files, before, regex, separator) } pub fn uu_app() -> App<'static, 'static> { diff --git a/tests/by-util/test_tac.rs b/tests/by-util/test_tac.rs index 5fc88770f..599bc19c7 100644 --- a/tests/by-util/test_tac.rs +++ b/tests/by-util/test_tac.rs @@ -73,3 +73,12 @@ fn test_invalid_input() { fn test_no_line_separators() { new_ucmd!().pipe_in("a").succeeds().stdout_is("a"); } + +#[test] +fn test_null_separator() { + new_ucmd!() + .args(&["-s", ""]) + .pipe_in("a\0b\0") + .succeeds() + .stdout_is("b\0a\0"); +} From 2a4c0a867aa360d99350646b2e9a904e522b3a17 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Tue, 27 Jul 2021 22:43:04 -0400 Subject: [PATCH 41/43] tests: better arg names in link helper functions Change the argument names in * `AtPath::hard_link()`, * `AtPath::symlink_file()`, and * `AtPath::symlink_dir()`, from `src` and `dest` to `original` and `link` to match the arguments of the corresponding functions from the Rust standard library. For example, see `std::os::unix::fs::symlink()`. --- tests/common/util.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/common/util.rs b/tests/common/util.rs index 68b7caa9b..41389d567 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -591,28 +591,40 @@ impl AtPath { } } - pub fn hard_link(&self, src: &str, dst: &str) { + pub fn hard_link(&self, original: &str, link: &str) { log_info( "hard_link", - &format!("{},{}", self.plus_as_string(src), self.plus_as_string(dst)), + &format!( + "{},{}", + self.plus_as_string(original), + self.plus_as_string(link) + ), ); - hard_link(&self.plus(src), &self.plus(dst)).unwrap(); + hard_link(&self.plus(original), &self.plus(link)).unwrap(); } - pub fn symlink_file(&self, src: &str, dst: &str) { + pub fn symlink_file(&self, original: &str, link: &str) { log_info( "symlink", - &format!("{},{}", self.plus_as_string(src), self.plus_as_string(dst)), + &format!( + "{},{}", + self.plus_as_string(original), + self.plus_as_string(link) + ), ); - symlink_file(&self.plus(src), &self.plus(dst)).unwrap(); + symlink_file(&self.plus(original), &self.plus(link)).unwrap(); } - pub fn symlink_dir(&self, src: &str, dst: &str) { + pub fn symlink_dir(&self, original: &str, link: &str) { log_info( "symlink", - &format!("{},{}", self.plus_as_string(src), self.plus_as_string(dst)), + &format!( + "{},{}", + self.plus_as_string(original), + self.plus_as_string(link) + ), ); - symlink_dir(&self.plus(src), &self.plus(dst)).unwrap(); + symlink_dir(&self.plus(original), &self.plus(link)).unwrap(); } pub fn is_symlink(&self, path: &str) -> bool { From 8232c1c64d437fac280e24d3a83306ec265679c5 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 2 Aug 2021 20:36:06 +0200 Subject: [PATCH 42/43] id: increase MSRV in order to compile selinux crate * Bump MSRV from 1.43 to 1.47 in order to be able to compile `selinux` crate 0.1.3 * Bump MSRV for clippy to 1.47 * Add "selinuxlib" to spell-checker --- Cargo.lock | 58 ++++++++++++++++++++++++++++++++++++-------- Cargo.toml | 4 ++- clippy.toml | 2 +- src/uu/id/Cargo.toml | 2 +- 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1803a9c33..912955deb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -75,9 +75,9 @@ dependencies = [ [[package]] name = "bindgen" -version = "0.58.1" +version = "0.59.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f8523b410d7187a43085e7e064416ea32ded16bd0a4e6fc025e21616d01258f" +checksum = "453c49e5950bb0eb63bb3df640e31618846c89d5b7faa54040d76e98e0134375" dependencies = [ "bitflags", "cexpr", @@ -117,6 +117,18 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" +[[package]] +name = "bitvec" +version = "0.19.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8942c8d352ae1838c9dda0b0ca2ab657696ef2232a20147cf1b30ae1a9cb4321" +dependencies = [ + "funty", + "radium", + "tap", + "wyz", +] + [[package]] name = "blake2b_simd" version = "0.5.11" @@ -169,9 +181,9 @@ checksum = "4a72c244c1ff497a746a7e1fb3d14bd08420ecda70c8f25c7112f2781652d787" [[package]] name = "cexpr" -version = "0.4.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4aedb84272dbe89af497cf81375129abda4fc0a9e7c5d317498c15cc30c0d27" +checksum = "db507a7679252d2276ed0dd8113c6875ec56d3089f9225b2b42c30cc1f8e5c89" dependencies = [ "nom", ] @@ -681,6 +693,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba" +[[package]] +name = "funty" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fed34cd105917e91daa4da6b3728c47b068749d6a62c59811f06ed2ac71d9da7" + [[package]] name = "generic-array" version = "0.8.4" @@ -1004,10 +1022,12 @@ checksum = "72ef4a56884ca558e5ddb05a1d1e7e1bfd9a68d9ed024c21704cc98872dae1bb" [[package]] name = "nom" -version = "5.1.2" +version = "6.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ffb4262d26ed83a1c0a33a38fe2bb15797329c85770da05e6b828ddb782627af" +checksum = "e7413f999671bd4745a7b624bd370a569fb6bc574b23c83a3c5ed2e453f3d5e2" dependencies = [ + "bitvec", + "funty", "memchr 2.4.0", "version_check", ] @@ -1295,6 +1315,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "radium" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "941ba9d78d8e2f7ce474c015eea4d9c6d25b6a3327f9832ee29a4de27f91bbb8" + [[package]] name = "rand" version = "0.5.6" @@ -1546,9 +1572,9 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "selinux" -version = "0.1.1" +version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a5e36fd4d2d189b51696bcff8d45a8defe6d9c3a353a6b1449ccc46d22a81d8e" +checksum = "bd525eeb189eb26c8471463186bba87644e3d8a9c7ae392adaf9ec45ede574bc" dependencies = [ "bitflags", "libc", @@ -1560,9 +1586,9 @@ dependencies = [ [[package]] name = "selinux-sys" -version = "0.4.2" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9dc2e3e97d611bf255de85a72a385565d1f6340bc6fc6fd6402555b54382a9f" +checksum = "5d842d177120716580c4c6cb56dfe3c5f3a3e3dcec635091f1b2034b6c0be4c6" dependencies = [ "bindgen", "cc", @@ -1704,6 +1730,12 @@ dependencies = [ "unicode-xid 0.2.2", ] +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "tempfile" version = "3.2.0" @@ -3047,6 +3079,12 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "wyz" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85e60b0d1b5f99db2556934e21937020776a5d31520bf169e851ac44e6420214" + [[package]] name = "xattr" version = "0.2.2" diff --git a/Cargo.toml b/Cargo.toml index 052e32c09..04bfef500 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,8 @@ # coreutils (uutils) # * see the repository LICENSE, README, and CONTRIBUTING files for more information +# spell-checker:ignore (libs) libselinux + [package] name = "coreutils" version = "0.0.7" @@ -234,7 +236,7 @@ clap = { version = "2.33", features = ["wrap_help"] } lazy_static = { version="1.3" } textwrap = { version="=0.11.0", features=["term_size"] } # !maint: [2020-05-10; rivy] unstable crate using undocumented features; pinned currently, will review uucore = { version=">=0.0.9", package="uucore", path="src/uucore" } -selinux = { version="0.1.1", optional = true } +selinux = { version="0.1.3", optional = true } # * uutils uu_test = { optional=true, version="0.0.7", package="uu_test", path="src/uu/test" } # diff --git a/clippy.toml b/clippy.toml index 0a0a69a41..dbabdab50 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1 @@ -msrv = "1.43.1" +msrv = "1.47.0" diff --git a/src/uu/id/Cargo.toml b/src/uu/id/Cargo.toml index b82c0e0f0..518de2729 100644 --- a/src/uu/id/Cargo.toml +++ b/src/uu/id/Cargo.toml @@ -18,7 +18,7 @@ path = "src/id.rs" clap = { version = "2.33", features = ["wrap_help"] } uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["entries", "process"] } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } -selinux = { version="0.1.1", optional = true } +selinux = { version="0.1.3", optional = true } [[bin]] name = "id" From 91d48d137dbc1c8db22baafca3299d8bd9acefcd Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 2 Aug 2021 23:38:41 +0200 Subject: [PATCH 43/43] readme: update of the min rust version in the doc --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 083320ac0..43beafdba 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ to compile anywhere, and this is as good a way as any to try and learn it. ### Rust Version uutils follows Rust's release channels and is tested against stable, beta and nightly. -The current oldest supported version of the Rust compiler is `1.43.1`. +The current oldest supported version of the Rust compiler is `1.47`. On both Windows and Redox, only the nightly version is tested currently.