Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

du: add support of --dereference-args & minor changes #4723

Merged
merged 8 commits into from
Apr 13, 2023
Merged

du: add support of --dereference-args & minor changes #4723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5a1d9ce
du: an error code when done on a non existing file
sylvestre Apr 9, 2023
eea8d40
du: add support of --dereference-args
sylvestre Apr 9, 2023
47cb40c
du: adjust test_du_basics_bad_name as we made it work
sylvestre Apr 9, 2023
f08b8da
du: directly get a vec of pathbuf to avoid several type changes
sylvestre Apr 11, 2023
f475a36
du: reduce the complexity by moving the time option mgmt into a function
sylvestre Apr 11, 2023
b2fd728
du: reduce the complexity by moving get_convert_size_fn option mgmt …
sylvestre Apr 11, 2023
e82e6a3
du: remove cognitive_complexity clippy allow
sylvestre Apr 11, 2023
00a866e
du: move to use an enum
sylvestre Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 94 additions & 55 deletions src/uu/du/src/du.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use std::time::{Duration, UNIX_EPOCH};
use std::{error::Error, fmt::Display};
use uucore::display::{print_verbatim, Quotable};
use uucore::error::FromIo;
use uucore::error::{UError, UResult};
use uucore::error::{set_exit_code, UError, UResult};
use uucore::parse_glob;
use uucore::parse_size::{parse_size, ParseSizeError};
use uucore::{
Expand Down Expand Up @@ -68,6 +68,7 @@ mod options {
pub const TIME_STYLE: &str = "time-style";
pub const ONE_FILE_SYSTEM: &str = "one-file-system";
pub const DEREFERENCE: &str = "dereference";
pub const DEREFERENCE_ARGS: &str = "dereference-args";
pub const INODES: &str = "inodes";
pub const EXCLUDE: &str = "exclude";
pub const EXCLUDE_FROM: &str = "exclude-from";
Expand All @@ -88,11 +89,18 @@ struct Options {
total: bool,
separate_dirs: bool,
one_file_system: bool,
dereference: bool,
dereference: Deref,
inodes: bool,
verbose: bool,
}

#[derive(PartialEq)]
enum Deref {
All,
Args(Vec<PathBuf>),
None,
}

#[derive(PartialEq, Eq, Hash, Clone, Copy)]
struct FileInfo {
file_id: u128,
Expand All @@ -112,21 +120,30 @@ struct Stat {
}

impl Stat {
fn new(path: PathBuf, options: &Options) -> Result<Self> {
let metadata = if options.dereference {
fs::metadata(&path)?
} else {
fs::symlink_metadata(&path)?
fn new(path: &Path, options: &Options) -> Result<Self> {
// Determine whether to dereference (follow) the symbolic link
let should_dereference = match &options.dereference {
Deref::All => true,
Deref::Args(paths) => paths.contains(&path.to_path_buf()),
Deref::None => false,
};

let metadata = if should_dereference {
// Get metadata, following symbolic links if necessary
fs::metadata(path)
} else {
// Get metadata without following symbolic links
fs::symlink_metadata(path)
}?;

#[cfg(not(windows))]
let file_info = FileInfo {
file_id: metadata.ino() as u128,
dev_id: metadata.dev(),
};
#[cfg(not(windows))]
return Ok(Self {
path,
path: path.to_path_buf(),
is_dir: metadata.is_dir(),
size: metadata.len(),
blocks: metadata.blocks(),
Expand All @@ -138,12 +155,12 @@ impl Stat {
});

#[cfg(windows)]
let size_on_disk = get_size_on_disk(&path);
let size_on_disk = get_size_on_disk(path);
#[cfg(windows)]
let file_info = get_file_info(&path);
let file_info = get_file_info(path);
#[cfg(windows)]
Ok(Self {
path,
path: path.to_path_buf(),
is_dir: metadata.is_dir(),
size: metadata.len(),
blocks: size_on_disk / 1024 * 2,
Expand Down Expand Up @@ -296,7 +313,7 @@ fn du(
'file_loop: for f in read {
match f {
Ok(entry) => {
match Stat::new(entry.path(), options) {
match Stat::new(&entry.path(), options) {
Ok(this_stat) => {
// We have an exclude list
for pattern in exclude {
Expand Down Expand Up @@ -397,6 +414,20 @@ fn convert_size_other(size: u64, _multiplier: u64, block_size: u64) -> String {
format!("{}", ((size as f64) / (block_size as f64)).ceil())
}

fn get_convert_size_fn(matches: &ArgMatches) -> Box<dyn Fn(u64, u64, u64) -> String> {
if matches.get_flag(options::HUMAN_READABLE) || matches.get_flag(options::SI) {
Box::new(convert_size_human)
} else if matches.get_flag(options::BYTES) {
Box::new(convert_size_b)
} else if matches.get_flag(options::BLOCK_SIZE_1K) {
Box::new(convert_size_k)
} else if matches.get_flag(options::BLOCK_SIZE_1M) {
Box::new(convert_size_m)
} else {
Box::new(convert_size_other)
}
}

#[derive(Debug)]
enum DuError {
InvalidMaxDepthArg(String),
Expand Down Expand Up @@ -490,7 +521,6 @@ fn build_exclude_patterns(matches: &ArgMatches) -> UResult<Vec<Pattern>> {
}

#[uucore::main]
#[allow(clippy::cognitive_complexity)]
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let args = args.collect_ignore();

Expand All @@ -505,26 +535,33 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
summarize,
)?;

let files = match matches.get_one::<String>(options::FILE) {
Some(_) => matches
.get_many::<String>(options::FILE)
.unwrap()
.map(PathBuf::from)
.collect(),
None => vec![PathBuf::from(".")],
};

let options = Options {
all: matches.get_flag(options::ALL),
max_depth,
total: matches.get_flag(options::TOTAL),
separate_dirs: matches.get_flag(options::SEPARATE_DIRS),
one_file_system: matches.get_flag(options::ONE_FILE_SYSTEM),
dereference: matches.get_flag(options::DEREFERENCE),
dereference: if matches.get_flag(options::DEREFERENCE) {
Deref::All
} else if matches.get_flag(options::DEREFERENCE_ARGS) {
// We don't care about the cost of cloning as it is rarely used
Deref::Args(files.clone())
} else {
Deref::None
},
inodes: matches.get_flag(options::INODES),
verbose: matches.get_flag(options::VERBOSE),
};

let files = match matches.get_one::<String>(options::FILE) {
Some(_) => matches
.get_many::<String>(options::FILE)
.unwrap()
.map(|s| s.as_str())
.collect(),
None => vec!["."],
};

if options.inodes
&& (matches.get_flag(options::APPARENT_SIZE) || matches.get_flag(options::BYTES))
{
Expand All @@ -547,19 +584,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
} else {
1024
};
let convert_size_fn = {
if matches.get_flag(options::HUMAN_READABLE) || matches.get_flag(options::SI) {
convert_size_human
} else if matches.get_flag(options::BYTES) {
convert_size_b
} else if matches.get_flag(options::BLOCK_SIZE_1K) {
convert_size_k
} else if matches.get_flag(options::BLOCK_SIZE_1M) {
convert_size_m
} else {
convert_size_other
}
};

let convert_size_fn = get_convert_size_fn(&matches);

let convert_size = |size: u64| {
if options.inodes {
size.to_string()
Expand All @@ -580,11 +607,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let excludes = build_exclude_patterns(&matches)?;

let mut grand_total = 0;
'loop_file: for path_string in files {
'loop_file: for path in files {
// Skip if we don't want to ignore anything
if !&excludes.is_empty() {
let path_string = path.to_string_lossy();
for pattern in &excludes {
if pattern.matches(path_string) {
if pattern.matches(&path_string) {
// if the directory is ignored, leave early
if options.verbose {
println!("{} ignored", path_string.quote());
Expand All @@ -594,9 +622,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
}
}

let path = PathBuf::from(&path_string);
// Check existence of path provided in argument
if let Ok(stat) = Stat::new(path, &options) {
if let Ok(stat) = Stat::new(&path, &options) {
// Kick off the computation of disk usage from the initial path
let mut inodes: HashSet<FileInfo> = HashSet::new();
if let Some(inode) = stat.inode {
Expand All @@ -616,20 +643,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {

if matches.contains_id(options::TIME) {
let tm = {
let secs = {
match matches.get_one::<String>(options::TIME) {
Some(s) => match s.as_str() {
"ctime" | "status" => stat.modified,
"access" | "atime" | "use" => stat.accessed,
"birth" | "creation" => stat
.created
.ok_or_else(|| DuError::InvalidTimeArg(s.into()))?,
// below should never happen as clap already restricts the values.
_ => unreachable!("Invalid field for --time"),
},
None => stat.modified,
}
};
let secs = matches
.get_one::<String>(options::TIME)
.map(|s| get_time_secs(s, &stat))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately this should probably happen outside of the loop. Could be a good-first-issue. So make some intermediate enum for in Options and then match on that instead of the string of the args.

.transpose()?
.unwrap_or(stat.modified);
DateTime::<Local>::from(UNIX_EPOCH + Duration::from_secs(secs))
};
if !summarize || index == len - 1 {
Expand All @@ -652,9 +670,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
} else {
show_error!(
"{}: {}",
path_string.maybe_quote(),
path.to_string_lossy().maybe_quote(),
"No such file or directory"
);
set_exit_code(1);
}
}

Expand All @@ -666,6 +685,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
Ok(())
}

fn get_time_secs(s: &str, stat: &Stat) -> std::result::Result<u64, DuError> {
let secs = match s {
"ctime" | "status" => stat.modified,
"access" | "atime" | "use" => stat.accessed,
"birth" | "creation" => stat
.created
.ok_or_else(|| DuError::InvalidTimeArg(s.into()))?,
// below should never happen as clap already restricts the values.
_ => unreachable!("Invalid field for --time"),
};
Ok(secs)
}

fn parse_time_style(s: Option<&str>) -> UResult<&str> {
match s {
Some(s) => match s {
Expand Down Expand Up @@ -788,6 +820,13 @@ pub fn uu_app() -> Command {
.help("dereference all symbolic links")
.action(ArgAction::SetTrue)
)
.arg(
Arg::new(options::DEREFERENCE_ARGS)
.short('D')
.long(options::DEREFERENCE_ARGS)
.help("dereference only symlinks that are listed on the command line")
.action(ArgAction::SetTrue)
)
// .arg(
// Arg::new("no-dereference")
// .short('P')
Expand Down
55 changes: 52 additions & 3 deletions tests/by-util/test_du.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
// * For the full copyright and license information, please view the LICENSE
// * file that was distributed with this source code.

// spell-checker:ignore (paths) sublink subwords azerty azeaze xcwww azeaz amaz azea qzerty tazerty
// spell-checker:ignore (paths) sublink subwords azerty azeaze xcwww azeaz amaz azea qzerty tazerty tsublink
#[cfg(not(windows))]
use regex::Regex;
#[cfg(not(windows))]
use std::io::Write;

#[cfg(any(target_os = "linux", target_os = "android"))]
Expand Down Expand Up @@ -122,7 +121,7 @@ fn test_du_invalid_size() {
fn test_du_basics_bad_name() {
new_ucmd!()
.arg("bad_name")
.succeeds() // TODO: replace with ".fails()" once `du` is fixed
.fails()
.stderr_only("du: bad_name: No such file or directory\n");
}

Expand Down Expand Up @@ -286,6 +285,30 @@ fn test_du_dereference() {
_du_dereference(result.stdout_str());
}

#[cfg(not(windows))]
#[test]
fn test_du_dereference_args() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;

at.mkdir_all("subdir");
let mut file1 = at.make_file("subdir/file-ignore1");
file1.write_all(b"azeaze").unwrap();
let mut file2 = at.make_file("subdir/file-ignore1");
file2.write_all(b"amaz?ng").unwrap();
at.symlink_dir("subdir", "sublink");

let result = ts.ucmd().arg("-D").arg("-s").arg("sublink").succeeds();
let stdout = result.stdout_str();

assert!(!stdout.starts_with("0"));
assert!(stdout.contains("sublink"));

// Without the option
let result = ts.ucmd().arg("-s").arg("sublink").succeeds();
result.stdout_contains("0\tsublink\n");
}

#[cfg(target_vendor = "apple")]
fn _du_dereference(s: &str) {
assert_eq!(s, "4\tsubdir/links/deeper_dir\n16\tsubdir/links\n");
Expand Down Expand Up @@ -851,3 +874,29 @@ fn test_du_exclude_invalid_syntax() {
.fails()
.stderr_contains("du: Invalid exclude syntax");
}

#[cfg(not(windows))]
#[test]
fn test_du_symlink_fail() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;

at.symlink_file("non-existing.txt", "target.txt");

ts.ucmd().arg("-L").arg("target.txt").fails().code_is(1);
}

#[cfg(not(windows))]
#[test]
fn test_du_symlink_multiple_fail() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;

at.symlink_file("non-existing.txt", "target.txt");
let mut file1 = at.make_file("file1");
file1.write_all(b"azeaze").unwrap();

let result = ts.ucmd().arg("-L").arg("target.txt").arg("file1").fails();
assert_eq!(result.code(), 1);
result.stdout_contains("4\tfile1\n");
}