From 35675fdfe7639acdb81c0bdc97a3f59780a52a04 Mon Sep 17 00:00:00 2001 From: Antonio Gurgel Date: Sat, 27 Mar 2021 01:18:47 -0700 Subject: [PATCH] install: implement `-C` / `--compare` (#1811) * install: implement `-C` / `--compare` GNU coreutils [1] checks the following: whether - either file is nonexistent, - there's a sticky bit or set[ug]id bit in play, - either file isn't a regular file, - the sizes of both files mismatch, - the destination file's owner differs from intended, or - the contents of both files mismatch. [1] https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/install.c?h=v8.32#n174 * Add test: non-regular files * Forgot a #[test] * Give up on non-regular file test * `cargo fmt` install.rs --- Cargo.lock | 20 ++++++++ Cargo.toml | 1 + src/uu/install/Cargo.toml | 1 + src/uu/install/src/install.rs | 95 +++++++++++++++++++++++++++++++++-- tests/by-util/test_install.rs | 85 +++++++++++++++++++++++++++++++ 5 files changed, 197 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4851a6e13..12cfe7ed6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -187,6 +187,7 @@ dependencies = [ "glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.85 (registry+https://github.com/rust-lang/crates.io-index)", + "nix 0.20.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)", "regex 1.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "sha1 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -517,6 +518,11 @@ name = "fake-simd" version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "file_diff" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "filetime" version = "0.2.14" @@ -730,6 +736,17 @@ dependencies = [ "void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "nix" +version = "0.20.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "cc 1.0.61 (registry+https://github.com/rust-lang/crates.io-index)", + "cfg-if 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.85 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "nodrop" version = "0.1.14" @@ -1658,6 +1675,7 @@ name = "uu_install" version = "0.0.4" dependencies = [ "clap 2.33.3 (registry+https://github.com/rust-lang/crates.io-index)", + "file_diff 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "filetime 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.85 (registry+https://github.com/rust-lang/crates.io-index)", "time 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2501,6 +2519,7 @@ dependencies = [ "checksum either 1.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457" "checksum env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "44533bbbb3bb3c1fa17d9f2e4e38bbbaf8396ba82193c4cb1b6445d711445d36" "checksum fake-simd 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed" +"checksum file_diff 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "31a7a908b8f32538a2143e59a6e4e2508988832d5d4d6f7c156b3cbc762643a5" "checksum filetime 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)" = "1d34cfa13a63ae058bfa601fe9e313bbdb3746427c1459185464ce0fcf62e1e8" "checksum fnv 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)" = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" "checksum fs_extra 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" @@ -2532,6 +2551,7 @@ dependencies = [ "checksum memchr 2.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" "checksum memoffset 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "157b4208e3059a8f9e78d559edc658e13df41410cb3ae03979c83130067fdd87" "checksum nix 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)" = "4dbdc256eaac2e3bd236d93ad999d3479ef775c863dbda3068c4006a92eec51b" +"checksum nix 0.20.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fa9b4819da1bc61c0ea48b63b7bc8604064dd43013e7cc325df098d49cd7c18a" "checksum nodrop 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)" = "72ef4a56884ca558e5ddb05a1d1e7e1bfd9a68d9ed024c21704cc98872dae1bb" "checksum num-integer 0.1.44 (registry+https://github.com/rust-lang/crates.io-index)" = "d2cc698a63b549a70bc047073d2949cce27cd1c7b0a4a862d08a8031bc2801db" "checksum num-traits 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)" = "9a64b1ec5cda2586e284722486d802acf1f7dbdc623e2bfc57e65ca1cd099290" diff --git a/Cargo.toml b/Cargo.toml index 208fd5d9c..08e9a3bb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -334,6 +334,7 @@ conv = "0.3" filetime = "0.2" glob = "0.3.0" libc = "0.2" +nix = "0.20.0" rand = "0.7" regex = "1.0" sha1 = { version="0.6", features=["std"] } diff --git a/src/uu/install/Cargo.toml b/src/uu/install/Cargo.toml index 9841ac64a..5aec0b07c 100644 --- a/src/uu/install/Cargo.toml +++ b/src/uu/install/Cargo.toml @@ -20,6 +20,7 @@ path = "src/install.rs" [dependencies] clap = "2.33" filetime = "0.2" +file_diff = "1.0.0" libc = ">= 0.2" uucore = { version=">=0.0.7", package="uucore", path="../../uucore", features=["mode", "perms", "entries"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 1ac9bc743..db54ee22d 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -13,10 +13,12 @@ mod mode; extern crate uucore; use clap::{App, Arg, ArgMatches}; +use file_diff::diff; use filetime::{set_file_times, FileTime}; use uucore::entries::{grp2gid, usr2uid}; use uucore::perms::{wrap_chgrp, wrap_chown, Verbosity}; +use libc::{getegid, geteuid}; use std::fs; use std::fs::File; use std::os::unix::fs::MetadataExt; @@ -34,6 +36,7 @@ pub struct Behavior { group: String, verbose: bool, preserve_timestamps: bool, + compare: bool, } #[derive(Clone, Eq, PartialEq)] @@ -112,11 +115,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("ignored") ) .arg( - // TODO implement flag Arg::with_name(OPT_COMPARE) .short("C") .long(OPT_COMPARE) - .help("(unimplemented) compare each pair of source and destination files, and in some cases, do not modify the destination at all") + .help("compare each pair of source and destination files, and in some cases, do not modify the destination at all") ) .arg( Arg::with_name(OPT_DIRECTORY) @@ -262,8 +264,6 @@ fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> { Err("--backup") } else if matches.is_present(OPT_BACKUP_2) { Err("-b") - } else if matches.is_present(OPT_COMPARE) { - Err("--compare, -C") } else if matches.is_present(OPT_CREATED) { Err("-D") } else if matches.is_present(OPT_STRIP) { @@ -338,6 +338,7 @@ fn behavior(matches: &ArgMatches) -> Result { group: matches.value_of(OPT_GROUP).unwrap_or("").to_string(), verbose: matches.is_present(OPT_VERBOSE), preserve_timestamps: matches.is_present(OPT_PRESERVE_TIMESTAMPS), + compare: matches.is_present(OPT_COMPARE), }) } @@ -500,7 +501,13 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> { ); return Err(()); } - } else if let Err(err) = fs::copy(from, to) { + } + + if b.compare && !need_copy(from, to, b) { + return Ok(()); + } + + if let Err(err) = fs::copy(from, to) { show_error!( "cannot install '{}' to '{}': {}", from.display(), @@ -583,3 +590,81 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> { Ok(()) } + +/// Return true if a file is necessary to copy. This is the case when: +/// - _from_ or _to_ is nonexistent; +/// - either file has a sticky bit or set[ug]id bit, or the user specified one; +/// - either file isn't a regular file; +/// - the sizes of _from_ and _to_ differ; +/// - _to_'s owner differs from intended; or +/// - the contents of _from_ and _to_ differ. +/// +/// # Parameters +/// +/// _from_ and _to_, if existent, must be non-directories. +/// +/// # Errors +/// +/// Crashes the program if a nonexistent owner or group is specified in _b_. +/// +fn need_copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> bool { + let from_meta = match fs::metadata(from) { + Ok(meta) => meta, + Err(_) => return true, + }; + let to_meta = match fs::metadata(to) { + Ok(meta) => meta, + Err(_) => return true, + }; + + // setuid || setgid || sticky + let extra_mode: u32 = 0o7000; + + if b.specified_mode.unwrap_or(0) & extra_mode != 0 + || from_meta.mode() & extra_mode != 0 + || to_meta.mode() & extra_mode != 0 + { + return true; + } + + if !from_meta.is_file() || !to_meta.is_file() { + return true; + } + + if from_meta.len() != to_meta.len() { + return true; + } + + // TODO: if -P (#1809) and from/to contexts mismatch, return true. + + if !b.owner.is_empty() { + let owner_id = match usr2uid(&b.owner) { + Ok(id) => id, + _ => crash!(1, "no such user: {}", b.owner), + }; + if owner_id != to_meta.uid() { + return true; + } + } else if !b.group.is_empty() { + let group_id = match grp2gid(&b.group) { + Ok(id) => id, + _ => crash!(1, "no such group: {}", b.group), + }; + if group_id != to_meta.gid() { + return true; + } + } else { + #[cfg(not(target_os = "windows"))] + unsafe { + if to_meta.uid() != geteuid() || to_meta.gid() != getegid() { + return true; + } + } + } + + if !diff(from.to_str().unwrap(), to.to_str().unwrap()) { + return true; + } + + false +} diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 89dfb0e56..dfd5c1c8d 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -1,4 +1,5 @@ use crate::common::util::*; +use filetime::FileTime; use rust_users::*; use std::os::unix::fs::PermissionsExt; @@ -407,3 +408,87 @@ fn test_install_failing_no_such_file() { assert!(r.code == Some(1)); assert!(r.stderr.contains("No such file or directory")); } + +#[test] +fn test_install_copy_then_compare_file() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let file1 = "test_install_copy_then_compare_file_a1"; + let file2 = "test_install_copy_then_compare_file_a2"; + + at.touch(file1); + scene + .ucmd() + .arg("-C") + .arg(file1) + .arg(file2) + .succeeds() + .no_stderr(); + + let mut file2_meta = at.metadata(file2); + let before = FileTime::from_last_modification_time(&file2_meta); + + scene + .ucmd() + .arg("-C") + .arg(file1) + .arg(file2) + .succeeds() + .no_stderr(); + + file2_meta = at.metadata(file2); + let after = FileTime::from_last_modification_time(&file2_meta); + + assert!(before == after); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_install_copy_then_compare_file_with_extra_mode() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + // XXX: can't tests introspect on their own names? + let file1 = "test_install_copy_then_compare_file_with_extra_mode_a1"; + let file2 = "test_install_copy_then_compare_file_with_extra_mode_a2"; + + at.touch(file1); + scene + .ucmd() + .arg("-C") + .arg(file1) + .arg(file2) + .succeeds() + .no_stderr(); + + let mut file2_meta = at.metadata(file2); + let before = FileTime::from_last_modification_time(&file2_meta); + + scene + .ucmd() + .arg("-C") + .arg(file1) + .arg(file2) + .arg("-m") + .arg("1644") + .succeeds() + .no_stderr(); + + file2_meta = at.metadata(file2); + let after_install_sticky = FileTime::from_last_modification_time(&file2_meta); + + assert!(before != after_install_sticky); + + // dest file still 1644, so need_copy ought to return `true` + scene + .ucmd() + .arg("-C") + .arg(file1) + .arg(file2) + .succeeds() + .no_stderr(); + + file2_meta = at.metadata(file2); + let after_install_sticky_again = FileTime::from_last_modification_time(&file2_meta); + + assert!(after_install_sticky != after_install_sticky_again); +}