From 199cf98b4fc9693c5a9627d84b209e12db072fc2 Mon Sep 17 00:00:00 2001 From: AP Date: Mon, 19 Oct 2026 03:19:07 +0530 Subject: [PATCH] fix(du): count a hard-linked file once, not once per link (#19283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- crates/nu-command/src/filesystem/du.rs | 58 +++++++++------ crates/nu-command/src/filesystem/ls.rs | 8 +- crates/nu-command/src/platform/dir_info.rs | 77 +++++++++++++++++-- crates/nu-command/src/platform/mod.rs | 2 +- crates/nu-command/tests/commands/du.rs | 86 ++++++++++++++++++++++ 5 files changed, 202 insertions(+), 29 deletions(-) diff --git a/crates/nu-command/src/filesystem/du.rs b/crates/nu-command/src/filesystem/du.rs index bfbb2d820d..f1e428ca69 100644 --- a/crates/nu-command/src/filesystem/du.rs +++ b/crates/nu-command/src/filesystem/du.rs @@ -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::>::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>, ) -> Result + 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, } } diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 0476fcff0a..88e316f6fb 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -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 { diff --git a/crates/nu-command/src/platform/dir_info.rs b/crates/nu-command/src/platform/dir_info.rs index cf7ee57c82..6f595d2e2d 100644 --- a/crates/nu-command/src/platform/dir_info.rs +++ b/crates/nu-command/src/platform/dir_info.rs @@ -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; +#[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, 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`'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, span: Span, signals: &Signals, + seen: &mut SeenSet, ) -> Result { 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 { 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, params: &DirBuilder) -> Self { + fn add_file(mut self, f: impl Into, 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; diff --git a/crates/nu-command/src/platform/mod.rs b/crates/nu-command/src/platform/mod.rs index d82600e777..60c4ef79a7 100644 --- a/crates/nu-command/src/platform/mod.rs +++ b/crates/nu-command/src/platform/mod.rs @@ -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; diff --git a/crates/nu-command/tests/commands/du.rs b/crates/nu-command/tests/commands/du.rs index be5264a458..c94f12f396 100644 --- a/crates/nu-command/tests/commands/du.rs +++ b/crates/nu-command/tests/commands/du.rs @@ -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 = 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 = 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(()) + }) +}