mirror of
https://github.com/nushell/nushell.git
synced 2026-10-28 17:05:03 +08:00
fix(du): count a hard-linked file once, not once per link (#19283)
## Description `du` walked every path and added each file's size independently, so a file with N hard links was counted N times and both size columns over-reported. The walk now tracks the identities of files it has already counted and skips the repeats. The set holds `uucore::fs::FileInformation` (device+inode on Unix, volume serial+file index on Windows) instead of a Unix-only `(dev, ino)` pair, so the dedup applies on Linux, macOS and Windows; the gate is `not(wasm32)`. One set covers the whole `du` invocation following the POSIX/GNU rule: a file counts once even across different path operands, so `du` and `du d1 d2` agree (first occurrence wins). Only files with more than one link enter the set; single-link files never touch it. `DirBuilder` stays settings-only — the set is threaded through as `&mut` rather than living in a cloned struct. `ls --du` uses the new signature; within each listed directory a hard-linked file now counts once, matching `du`. Adds regression tests: a file with multiple hard links contributes its size exactly once, including when the links sit under different path operands. ## User-facing changes (Release notes) - Fixed `du` over-reporting sizes for hard-linked files: a file with multiple hard links is now counted once instead of once per link, on all platforms (Linux, macOS, Windows). - `du --long` lists the first-seen link only, matching GNU `du -a`. - `ls --du` now counts a hard-linked file once inside each listed directory, like `du` does. - First link wins: when the same file is reachable through multiple hard links, only the first occurrence is counted and listed — e.g. `du c b a` on three links to one file shows a single row for `c`. This matches POSIX/BSD/GNU `du`, but scripts that expect one row per operand will see fewer rows. ## Additional notes - Fixes #19019
This commit is contained in:
5 files changed
+202
-29
No files matched your search
@@ -1,9 +1,10 @@
|
||||
use crate::{DirBuilder, DirInfo, ExcludeGlob, FileInfo};
|
||||
use crate::{DirBuilder, DirInfo, ExcludeGlob, FileInfo, SeenSet};
|
||||
use nu_engine::command_prelude::*;
|
||||
use nu_glob::MatchOptions;
|
||||
use nu_protocol::{NuGlob, PipelineMetadata, Signals};
|
||||
use serde::Deserialize;
|
||||
use std::path::Path;
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct Du;
|
||||
@@ -112,6 +113,11 @@ impl Command for Du {
|
||||
Some(paths)
|
||||
};
|
||||
|
||||
// Hard links are counted once per invocation, not once per path operand,
|
||||
// so a link shared between two operands is only reported under the first
|
||||
// one. The lock is only there to keep the lazy iterators `Send`.
|
||||
let seen = Arc::<Mutex<SeenSet>>::default();
|
||||
|
||||
match paths {
|
||||
None => {
|
||||
let args = DuArgs {
|
||||
@@ -123,17 +129,21 @@ impl Command for Du {
|
||||
max_depth,
|
||||
min_size,
|
||||
};
|
||||
Ok(
|
||||
du_for_one_pattern(args, ¤t_dir, tag, engine_state.signals().clone())?
|
||||
.into_pipeline_data_with_metadata(
|
||||
tag,
|
||||
engine_state.signals().clone(),
|
||||
PipelineMetadata {
|
||||
path_columns: vec![String::from("path")],
|
||||
..Default::default()
|
||||
},
|
||||
),
|
||||
)
|
||||
Ok(du_for_one_pattern(
|
||||
args,
|
||||
¤t_dir,
|
||||
tag,
|
||||
engine_state.signals().clone(),
|
||||
seen,
|
||||
)?
|
||||
.into_pipeline_data_with_metadata(
|
||||
tag,
|
||||
engine_state.signals().clone(),
|
||||
PipelineMetadata {
|
||||
path_columns: vec![String::from("path")],
|
||||
..Default::default()
|
||||
},
|
||||
))
|
||||
}
|
||||
Some(paths) => {
|
||||
let mut result_iters = vec![];
|
||||
@@ -152,6 +162,7 @@ impl Command for Du {
|
||||
¤t_dir,
|
||||
tag,
|
||||
engine_state.signals().clone(),
|
||||
seen.clone(),
|
||||
)?)
|
||||
}
|
||||
|
||||
@@ -185,6 +196,7 @@ fn du_for_one_pattern(
|
||||
current_dir: &Path,
|
||||
span: Span,
|
||||
signals: Signals,
|
||||
seen: Arc<Mutex<SeenSet>>,
|
||||
) -> Result<impl Iterator<Item = Value> + Send + use<>, ShellError> {
|
||||
let exclude = args
|
||||
.exclude
|
||||
@@ -221,24 +233,28 @@ fn du_for_one_pattern(
|
||||
let max_depth = args.max_depth.map(|f| f.item as u64);
|
||||
let min_size = args.min_size.map(|f| f.item as u64);
|
||||
|
||||
let params = DirBuilder {
|
||||
tag: span,
|
||||
min: min_size,
|
||||
deref,
|
||||
exclude,
|
||||
long,
|
||||
};
|
||||
let params = DirBuilder::new(span, min_size, deref, exclude, long);
|
||||
|
||||
Ok(paths.filter_map(move |p| match p {
|
||||
Ok(a) => {
|
||||
let mut seen = seen.lock().unwrap_or_else(|e| e.into_inner());
|
||||
if a.is_dir() {
|
||||
match DirInfo::new(a, ¶ms, max_depth, span, &signals) {
|
||||
match DirInfo::new(a, ¶ms, max_depth, span, &signals, &mut seen) {
|
||||
Ok(v) => Some(Value::from(v)),
|
||||
Err(_) => None,
|
||||
}
|
||||
} else {
|
||||
match FileInfo::new(a, deref, span, params.long) {
|
||||
Ok(v) => Some(Value::from(v)),
|
||||
// A file operand goes through the same check as a file
|
||||
// inside a directory: another link to an already counted
|
||||
// file gets no row.
|
||||
Ok(v) => {
|
||||
if v.count_once(&mut seen) {
|
||||
Some(Value::from(v))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
Err(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
use crate::{DirBuilder, DirInfo};
|
||||
use crate::{DirBuilder, DirInfo, SeenSet};
|
||||
use chrono::{DateTime, Local, LocalResult, TimeZone, Utc};
|
||||
use nu_engine::{command_prelude::*, glob_from};
|
||||
use nu_glob::MatchOptions;
|
||||
@@ -833,7 +833,11 @@ pub(crate) fn dir_entry_dict(
|
||||
if md.is_dir() {
|
||||
if du {
|
||||
let params = DirBuilder::new(Span::new(0, 2), None, false, None, false);
|
||||
let dir_size = DirInfo::new(filename, ¶ms, None, span, signals)?.get_size();
|
||||
// Fresh per directory: a link is counted once inside each
|
||||
// directory whose size is reported here.
|
||||
let mut seen = SeenSet::default();
|
||||
let dir_size =
|
||||
DirInfo::new(filename, ¶ms, None, span, signals, &mut seen)?.get_size();
|
||||
|
||||
Value::filesize(dir_size as i64, span)
|
||||
} else {
|
||||
|
||||
@@ -19,6 +19,14 @@ impl ExcludeGlob {
|
||||
}
|
||||
}
|
||||
|
||||
/// Files already counted while walking, so that a file with several hard links
|
||||
/// is only counted once. On `wasm32` no file identity is available, so the set
|
||||
/// is a unit type and the walking signatures stay the same everywhere.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub type SeenSet = std::collections::HashSet<uucore::fs::FileInformation>;
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
pub type SeenSet = ();
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct DirBuilder {
|
||||
pub tag: Span,
|
||||
@@ -58,13 +66,31 @@ pub struct DirInfo {
|
||||
long: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
#[derive(Clone)]
|
||||
pub struct FileInfo {
|
||||
path: PathBuf,
|
||||
size: u64,
|
||||
blocks: Option<u64>,
|
||||
tag: Span,
|
||||
long: bool,
|
||||
/// Identity of the file, used to count hard-linked files only once while
|
||||
/// walking. `None` for files with a single link, which never reach the set.
|
||||
/// Not available on `wasm32`.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
file_id: Option<uucore::fs::FileInformation>,
|
||||
}
|
||||
|
||||
// `uucore`'s file identity has no `Debug`, so this one cannot be derived.
|
||||
impl std::fmt::Debug for FileInfo {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("FileInfo")
|
||||
.field("path", &self.path)
|
||||
.field("size", &self.size)
|
||||
.field("blocks", &self.blocks)
|
||||
.field("tag", &self.tag)
|
||||
.field("long", &self.long)
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
|
||||
impl FileInfo {
|
||||
@@ -84,6 +110,23 @@ impl FileInfo {
|
||||
match m {
|
||||
Ok(d) => {
|
||||
let block_size = file_real_size_fast(&path, &d).ok();
|
||||
// `FileInformation` asks the filesystem again, so on Unix it is
|
||||
// only built for files the metadata already reported as linked.
|
||||
#[cfg(unix)]
|
||||
let file_id = {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
if d.nlink() > 1 {
|
||||
uucore::fs::FileInformation::from_path(&path, deref).ok()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
};
|
||||
// Off Unix the metadata has neither a link count nor a file id,
|
||||
// so the identity needs its own look at the file.
|
||||
#[cfg(not(any(unix, target_arch = "wasm32")))]
|
||||
let file_id = uucore::fs::FileInformation::from_path(&path, deref)
|
||||
.ok()
|
||||
.filter(|info| info.number_of_links() > 1);
|
||||
|
||||
Ok(FileInfo {
|
||||
path,
|
||||
@@ -91,11 +134,28 @@ impl FileInfo {
|
||||
size: d.len(),
|
||||
tag,
|
||||
long,
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
file_id,
|
||||
})
|
||||
}
|
||||
Err(e) => Err(IoError::new(e, tag, path).into()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Counts this file, unless another link to it was counted already, in
|
||||
/// which case `false` is returned and the caller must skip it.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub fn count_once(&self, seen: &mut SeenSet) -> bool {
|
||||
self.file_id
|
||||
.as_ref()
|
||||
.is_none_or(|id| seen.insert(id.clone()))
|
||||
}
|
||||
|
||||
/// Nothing is tracked on `wasm32`, so every file counts.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
pub fn count_once(&self, _seen: &mut SeenSet) -> bool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
impl DirInfo {
|
||||
@@ -105,6 +165,7 @@ impl DirInfo {
|
||||
depth: Option<u64>,
|
||||
span: Span,
|
||||
signals: &Signals,
|
||||
seen: &mut SeenSet,
|
||||
) -> Result<Self, ShellError> {
|
||||
let path = path.into();
|
||||
let from_io_error = IoError::factory(span, path.as_path());
|
||||
@@ -136,9 +197,9 @@ impl DirInfo {
|
||||
match f {
|
||||
Ok(i) => match i.file_type() {
|
||||
Ok(t) if t.is_dir() => {
|
||||
s = s.add_dir(i.path(), depth, params, span, signals)?
|
||||
s = s.add_dir(i.path(), depth, params, span, signals, seen)?
|
||||
}
|
||||
Ok(_t) => s = s.add_file(i.path(), params),
|
||||
Ok(_t) => s = s.add_file(i.path(), params, seen),
|
||||
Err(e) => s = s.add_error(from_io_error(e).into()),
|
||||
},
|
||||
Err(e) => s = s.add_error(from_io_error(e).into()),
|
||||
@@ -157,6 +218,7 @@ impl DirInfo {
|
||||
params: &DirBuilder,
|
||||
span: Span,
|
||||
signals: &Signals,
|
||||
seen: &mut SeenSet,
|
||||
) -> Result<Self, ShellError> {
|
||||
if let Some(current) = depth {
|
||||
if let Some(new) = current.checked_sub(1) {
|
||||
@@ -166,19 +228,24 @@ impl DirInfo {
|
||||
}
|
||||
}
|
||||
|
||||
let d = DirInfo::new(path, params, depth, span, signals)?;
|
||||
let d = DirInfo::new(path, params, depth, span, signals, seen)?;
|
||||
self.size += d.size;
|
||||
self.blocks += d.blocks;
|
||||
self.dirs.push(d);
|
||||
Ok(self)
|
||||
}
|
||||
|
||||
fn add_file(mut self, f: impl Into<PathBuf>, params: &DirBuilder) -> Self {
|
||||
fn add_file(mut self, f: impl Into<PathBuf>, params: &DirBuilder, seen: &mut SeenSet) -> Self {
|
||||
let f = f.into();
|
||||
let include = params.exclude.as_ref().is_none_or(|x| !x.matches_path(&f));
|
||||
if include {
|
||||
match FileInfo::new(f, params.deref, self.tag, self.long) {
|
||||
Ok(file) => {
|
||||
// A file reached through multiple hard links must only be
|
||||
// counted once, like POSIX and GNU/BSD du do.
|
||||
if !file.count_once(seen) {
|
||||
return self;
|
||||
}
|
||||
let inc = params.min.is_none_or(|s| file.size >= s);
|
||||
if inc {
|
||||
self.size += file.size;
|
||||
|
||||
@@ -18,7 +18,7 @@ mod whoami;
|
||||
pub use clear::Clear;
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub use clip::{ClipCommand, ClipCopy, ClipPaste};
|
||||
pub use dir_info::{DirBuilder, DirInfo, ExcludeGlob, FileInfo};
|
||||
pub use dir_info::{DirBuilder, DirInfo, ExcludeGlob, FileInfo, SeenSet};
|
||||
pub use input::Input;
|
||||
pub use input::InputList;
|
||||
pub use input::InputListen;
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use nu_test_support::fs::Stub::EmptyFile;
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
use nu_test_support::fs::Stub::FileWithContent;
|
||||
use nu_test_support::prelude::*;
|
||||
use rstest::rstest;
|
||||
|
||||
@@ -101,3 +103,87 @@ fn du_wildcards() -> Result {
|
||||
.expect_value_eq(1)
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn du_counts_hard_links_once() -> Result {
|
||||
Playground::setup("du_hard_links", |dirs, sandbox| {
|
||||
// 1 MiB of real (allocated) bytes; the directory entry overhead stays
|
||||
// orders of magnitude below the assertion bound either way.
|
||||
let body = "x".repeat(1024 * 1024);
|
||||
sandbox
|
||||
.within("links")
|
||||
.with_files(&[FileWithContent("a", &body)]);
|
||||
drop(body);
|
||||
|
||||
let links = dirs.test().join("links");
|
||||
std::fs::hard_link(links.join("a"), links.join("b")).expect("link b");
|
||||
std::fs::hard_link(links.join("a"), links.join("c")).expect("link c");
|
||||
|
||||
// `a`, `b` and `c` are the same file: both columns must report ~1 MiB,
|
||||
// not ~3 MiB. The lower bound catches a walk that counted nothing.
|
||||
let sizes: Vec<i64> = test()
|
||||
.cwd(dirs.test())
|
||||
.run("du links | first | select apparent physical | values | into int")?;
|
||||
|
||||
assert!(
|
||||
sizes[0] >= 1024 * 1024,
|
||||
"hard-linked file not counted: apparent = {}",
|
||||
sizes[0]
|
||||
);
|
||||
assert!(
|
||||
sizes[0] < 2 * 1024 * 1024,
|
||||
"hard-linked file counted more than once: apparent = {}",
|
||||
sizes[0]
|
||||
);
|
||||
// No lower bound for `physical`: file systems with transparent
|
||||
// compression report less than the allocated bytes.
|
||||
assert!(
|
||||
sizes[1] < 2 * 1024 * 1024,
|
||||
"hard-linked file counted more than once: physical = {}",
|
||||
sizes[1]
|
||||
);
|
||||
Ok(())
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn du_counts_hard_links_once_across_paths() -> Result {
|
||||
Playground::setup("du_hard_links_across_paths", |dirs, sandbox| {
|
||||
let body = "x".repeat(1024 * 1024);
|
||||
sandbox
|
||||
.within("d1")
|
||||
.with_files(&[FileWithContent("a", &body)]);
|
||||
drop(body);
|
||||
sandbox.mkdir("d2");
|
||||
|
||||
std::fs::hard_link(
|
||||
dirs.test().join("d1").join("a"),
|
||||
dirs.test().join("d2").join("b"),
|
||||
)
|
||||
.expect("link b");
|
||||
|
||||
// The link lives under both operands, so only the first row carries its
|
||||
// size; the second one is directory entry overhead.
|
||||
let sizes: Vec<i64> = test()
|
||||
.cwd(dirs.test())
|
||||
.run("du d1 d2 | get apparent | into int")?;
|
||||
|
||||
assert!(
|
||||
sizes.len() == 2,
|
||||
"expected one row per path operand, got {sizes:?}"
|
||||
);
|
||||
assert!(
|
||||
sizes[0] >= 1024 * 1024 && sizes[0] < 2 * 1024 * 1024,
|
||||
"hard-linked file not counted exactly once: apparent = {}",
|
||||
sizes[0]
|
||||
);
|
||||
assert!(
|
||||
sizes[1] <= 4096,
|
||||
"hard-linked file counted under the second path too: apparent = {}",
|
||||
sizes[1]
|
||||
);
|
||||
Ok(())
|
||||
})
|
||||
}
|
||||
Reference in new issue
Block a user