From cec7ac94092dfa647644f7e3f44e67734cc7d318 Mon Sep 17 00:00:00 2001 From: armada45-pixel Date: Tue, 13 Oct 2026 06:56:38 +0700 Subject: [PATCH 1/3] Cut per-frame and per-keystroke work in the explore screen Switching the metric re-ranks the tree in place instead of cloning it, which on a whole disk is tens of millions of nodes copied to change an order. A frame builds label text only for the tiles whose labels it will draw, and reuses the set of marked crumbs until the marks or the tree change. The sibling menu names only the rows it shows, layout picks its kept children with a partial sort, and `Node::find` walks a real queue. The free-space ticker asks the volume off the UI thread, since a network or sleeping drive can take seconds to answer, and shift-scroll panning is clamped like every other way of moving the view. Co-Authored-By: Claude Opus 5.5 --- crates/disktree-app/src/marks.rs | 12 ++ crates/disktree-app/src/state.rs | 241 ++++++++++++++++++++-------- crates/disktree-core/src/tree.rs | 9 +- crates/disktree-core/src/treemap.rs | 15 +- 4 files changed, 203 insertions(+), 74 deletions(-) diff --git a/crates/disktree-app/src/marks.rs b/crates/disktree-app/src/marks.rs index 89141ae3..d8bbcd2b 100644 --- a/crates/disktree-app/src/marks.rs +++ b/crates/disktree-app/src/marks.rs @@ -15,6 +15,9 @@ use rustc_hash::FxHashSet; pub struct Marks { items: Vec, index: FxHashSet, + /// Bumped whenever the marks, or the tree they were last read from, + /// change: what a cache derived from them is keyed by. + generation: u64, } impl Marks { @@ -34,10 +37,16 @@ impl Marks { self.items.is_empty() } + /// See the field: differs whenever the marks may have. + pub const fn generation(&self) -> u64 { + self.generation + } + /// Mark `target`, or unmark it when it is already marked. /// /// Returns `true` when the path ended up marked. pub fn toggle(&mut self, target: Target) -> bool { + self.generation += 1; if self.index.remove(&target.path) { self.items.retain(|item| item.path != target.path); false @@ -49,12 +58,14 @@ impl Marks { } pub fn remove(&mut self, path: &Path) { + self.generation += 1; if self.index.remove(path) { self.items.retain(|item| item.path != path); } } pub fn clear(&mut self) { + self.generation += 1; self.items.clear(); self.index.clear(); } @@ -78,6 +89,7 @@ impl Marks { } } self.items = resolved; + self.generation += 1; } } diff --git a/crates/disktree-app/src/state.rs b/crates/disktree-app/src/state.rs index ee351f3a..fbde302c 100644 --- a/crates/disktree-app/src/state.rs +++ b/crates/disktree-app/src/state.rs @@ -282,6 +282,18 @@ impl LayoutTransition { } } +/// The marks as crumbs, for the tree and the marks they were resolved +/// against. +struct MarkedCrumbs { + /// [`Marks::generation`] when resolved. + generation: u64, + /// The tree then, by address and only ever compared. A tree re-ranked + /// in place keeps its address, and refreshes the marks, which moves + /// on `generation`. + tree: Option<*const Node>, + crumbs: Rc>>, +} + /// Layout for one (crumbs, area, options) combination. struct LayoutCache { key: LayoutKey, @@ -360,6 +372,7 @@ pub struct Disktree { pub transition: Option, pub layout_options: LayoutOptions, cache: Option, + marked: Option, /// Mouse position in treemap-local pixels, for hit-testing and the tooltip. pub pointer: Option>, @@ -495,6 +508,7 @@ impl Disktree { ..LayoutOptions::default() }, cache: None, + marked: None, pointer: None, treemap_origin: Rc::new(Cell::new(Point::new(px(0.), px(0.)))), treemap_size: Rc::new(Cell::new(size(px(0.), px(0.)))), @@ -1055,11 +1069,24 @@ impl Disktree { }) .unwrap_or(Duration::from_millis(1200)); cx.background_executor().timer(interval).await; + let Ok(path) = + this.update(cx, |this, _| this.root_path.clone()) + else { + break; + }; + // Asked off the UI thread: a network share or a drive + // spinning up can take seconds to answer, and the window, + // Escape included, must not wait with it. + let asked = path.clone(); + let fresh = cx + .background_executor() + .spawn(async move { space_info(&asked).ok() }) + .await; let ok = this .update(cx, |this, cx| { - let path = this.root_path.clone(); - let fresh = space_info(&path).ok(); - if fresh != this.space { + // A root chosen while the old one answered has + // been measured already; this is not its number. + if this.root_path == path && fresh != this.space { this.space = fresh; cx.notify(); } @@ -1138,21 +1165,31 @@ impl Disktree { let Some(node) = self.node_at(parent) else { return (Vec::new(), 0); }; - let mut rows: Vec = node + // Ranked as plain numbers, with names copied only for the rows + // shown: a directory can hold a million entries, and the menu asks + // again on every key. + let mut ranked: Vec<(usize, u64)> = node .children .iter() .enumerate() - .map(|(index, child)| Sibling { - index, - name: child.name.to_string(), - value: child.value(metric), - category: child.category, - is_dir: child.is_dir(), + .map(|(index, child)| (index, child.value(metric))) + .collect(); + ranked.sort_by_key(|&(_, value)| std::cmp::Reverse(value)); + let more = ranked.len().saturating_sub(SIBLING_ROWS); + ranked.truncate(SIBLING_ROWS); + let rows = ranked + .into_iter() + .map(|(index, value)| { + let child = &node.children[index]; + Sibling { + index, + name: child.name.to_string(), + value, + category: child.category, + is_dir: child.is_dir(), + } }) .collect(); - rows.sort_by_key(|row| std::cmp::Reverse(row.value)); - let more = rows.len().saturating_sub(SIBLING_ROWS); - rows.truncate(SIBLING_ROWS); (rows, more) } @@ -1783,29 +1820,33 @@ impl Disktree { let hovered = self.hovered.clone(); let selected = self.selected.clone(); - // Marks are paths; the mosaic thinks in crumbs. Resolve once per frame - // rather than building a path for every tile. - let mut marked: FxHashSet> = FxHashSet::default(); - for item in self.marks.items() { - if let Some(crumbs) = self.crumbs_for_path(&item.path) { - marked.insert(crumbs); - } - } + // Marks are paths; the mosaic thinks in crumbs. + let marked = self.marked_crumbs(); - let Some(tiles) = self.layout().map(<[Tile]>::to_vec) else { + // Bring the cache up to date, then take it out for the pass and put + // it back after: the tiles are read beside the tree and the marks, + // and copying thousands of them, crumbs and all, ran on every + // pointer move. + let cache = if self.layout().is_some() { + self.cache.take() + } else { + None + }; + let Some(cache) = cache else { return Mosaic { view, ..Mosaic::default() }; }; + let tiles = &cache.tiles; // Hue comes from the node's kind; lightness from its depth in this // view, so the first level always reads as the first level. let age = self.color_mode == ColorMode::Age; let now = self.scanned_at; let mut decorations = Vec::with_capacity(tiles.len()); - let mut labels = Vec::new(); - for tile in &tiles { + let mut slots: Vec> = Vec::new(); + for tile in tiles { let crumbs = tile.crumbs(); let node = match &tile.kind { TileKind::Node { crumbs } => self.node_at(crumbs), @@ -1830,8 +1871,9 @@ impl Disktree { let is_covered = !marked.is_empty() && (1..crumbs.len()) .any(|length| marked.contains(&crumbs[..length])); + let rect = self.animated_rect(tile.rect); decorations.push(TileDeco { - rect: self.animated_rect(tile.rect), + rect, depth: tile.depth, category, age_bucket, @@ -1846,51 +1888,64 @@ impl Disktree { // Labels are chosen in screen space: zooming in makes room for more // of them, which is the point of zooming in. - let drawn = self.animated_rect(tile.rect); - let screen = view.project( - tile.header - .map_or(drawn, |header| self.animated_rect(header)), - ); + let header = tile.header.map(|header| self.animated_rect(header)); + let screen = view.project(header.unwrap_or(rect)); if screen.w < LABEL_MIN_W_REMS * self.rem || screen.h < LABEL_MIN_H_REMS * self.rem { continue; } - match &tile.kind { - TileKind::Node { crumbs } => { - let Some(node) = self.node_at(crumbs) else { + let (others, dim) = match &tile.kind { + TileKind::Node { .. } => { + if node.is_none() { continue; - }; - labels.push(Label { - text: node.name.to_string(), - rect: self.animated_rect(tile.rect), - header: tile - .header - .map(|header| self.animated_rect(header)), - depth: tile.depth, - dim: filtered == Filtered::Out, - marked: is_marked || is_covered, - size_text: crate::widgets::short_value(node, metric), - }); + } + (0, filtered == Filtered::Out) } - TileKind::Others { count, .. } => labels.push(Label { - text: format!("+{count} more"), - rect: self.animated_rect(tile.rect), - header: None, - depth: tile.depth, - dim: self.matches.is_some(), - marked: false, - size_text: String::new(), - }), - } + TileKind::Others { count, .. } => { + (*count, self.matches.is_some()) + } + }; + slots.push(LabelSlot { + node, + others, + rect, + header, + depth: tile.depth, + dim, + // The merged tail is never marked: its crumbs are its + // parent's, and a mark there is drawn on the parent. + marked: node.is_some() && (is_marked || is_covered), + }); } - labels.sort_by(|left, right| { + slots.sort_by(|left, right| { let left_area = left.rect.w * left.rect.h; let right_area = right.rect.w * right.rect.h; right_area.total_cmp(&left_area) }); - labels.truncate(MAX_LABELS); + slots.truncate(MAX_LABELS); + // Text only for the labels that will be shaped: a deep view has + // thousands of tiles big enough for one, and a name and a size + // formatted for each was thrown away for all but `MAX_LABELS`. + let labels = slots + .into_iter() + .map(|slot| Label { + text: slot.node.map_or_else( + || format!("+{} more", slot.others), + |node| node.name.to_string(), + ), + rect: slot.rect, + header: slot.header, + depth: slot.depth, + dim: slot.dim, + marked: slot.marked, + size_text: slot.node.map_or_else(String::new, |node| { + crate::widgets::short_value(node, metric) + }), + }) + .collect(); + self.cache = Some(cache); Mosaic { tiles: decorations, @@ -1899,6 +1954,34 @@ impl Disktree { } } + /// The marks as crumbs. Resolved when the marks or the tree change + /// rather than on every frame: each mark is a walk down the tree with + /// a search through a directory's entries at every level, and a frame + /// is drawn for every pointer move. + fn marked_crumbs(&mut self) -> Rc>> { + let generation = self.marks.generation(); + let tree = self.tree.as_ref().map(Arc::as_ptr); + if let Some(marked) = &self.marked + && marked.generation == generation + && marked.tree == tree + { + return Rc::clone(&marked.crumbs); + } + let crumbs: Rc>> = Rc::new( + self.marks + .items() + .iter() + .filter_map(|item| self.crumbs_for_path(&item.path)) + .collect(), + ); + self.marked = Some(MarkedCrumbs { + generation, + tree, + crumbs: Rc::clone(&crumbs), + }); + crumbs + } + /// Crumbs for an absolute path, if the tree still contains it. #[allow( clippy::unnecessary_wraps, @@ -2060,17 +2143,20 @@ impl Disktree { let directory = self.current_path(); let selected = self.selected.as_deref().and_then(|c| self.path_at(c)); self.options.metric = self.options.metric.toggled(); - if let Some(tree) = &self.tree { - let mut tree = (**tree).clone(); - disktree_core::tree::aggregate(&mut tree, self.options.metric); - discard(self.tree.replace(Arc::new(tree))); - let metric = self.options.metric; - self.marks.refresh( - &self.root_path, - self.tree.as_ref().unwrap(), - metric, + if let Some(tree) = &mut self.tree { + // Re-ranked in place: copying a disk's millions of nodes took + // seconds and held two trees at once. Only a search or a + // widening scan still reading the tree makes this a copy, and + // then whichever of them finishes last frees the old one. + disktree_core::tree::aggregate( + Arc::make_mut(tree), + self.options.metric, ); } + if let Some(tree) = &self.tree { + self.marks + .refresh(&self.root_path, tree, self.options.metric); + } self.crumbs = self.crumbs_for_path(&directory).unwrap_or_default(); self.selected = selected.and_then(|path| self.crumbs_for_path(&path)); self.forget_hover(); @@ -2254,7 +2340,8 @@ impl Disktree { } => { if outcome.is_ok() { self.run_summary.removed += 1; - self.run_summary.bytes += bytes; + self.run_summary.bytes = + self.run_summary.bytes.saturating_add(bytes); } else { self.run_summary.failed += 1; } @@ -2918,8 +3005,10 @@ impl Disktree { } if event.modifiers.shift { // Pan instead of zoom, for looking around a magnified view. - self.view.origin_y = - (self.view.origin_y - lines * 40.0 / self.view.scale).max(0.0); + // Clamped at both ends, as zooming is: stopping only at the + // top let the mosaic be scrolled off the bottom of the view. + self.view.origin_y -= lines * 40.0 / self.view.scale; + self.view = self.view.clamped(self.treemap_size.get()); cx.notify(); return; } @@ -3025,6 +3114,20 @@ pub struct Label { pub size_text: String, } +/// A label a tile has room for, before its text is made: only the +/// [`MAX_LABELS`] largest get any. +struct LabelSlot<'a> { + /// The tile's node; `None` for the merged tail. + node: Option<&'a Node>, + /// How many entries a merged tail stands for. + others: usize, + rect: Rect, + header: Option, + depth: u32, + dim: bool, + marked: bool, +} + /// How many labels one frame will shape. const MAX_LABELS: usize = 150; diff --git a/crates/disktree-core/src/tree.rs b/crates/disktree-core/src/tree.rs index 6991a86a..b4a2c2d1 100644 --- a/crates/disktree-core/src/tree.rs +++ b/crates/disktree-core/src/tree.rs @@ -1,5 +1,6 @@ //! The scanned tree. +use std::collections::VecDeque; use std::num::NonZeroU64; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; @@ -233,8 +234,10 @@ impl Node { if needle.is_empty() { return None; } - let mut queue = vec![(Vec::new(), self)]; - while let Some((crumbs, node)) = queue.pop() { + // Taken from the front: popping the back would search depth-first, + // and find a deep match in a late branch before a shallow one. + let mut queue = VecDeque::from([(Vec::new(), self)]); + while let Some((crumbs, node)) = queue.pop_front() { for (index, child) in node.children.iter().enumerate() { if child.name.to_lowercase().contains(&needle) { let found: Vec = crumbs @@ -247,7 +250,7 @@ impl Node { if child.is_dir() && !child.children.is_empty() { let mut next = crumbs.clone(); next.push(index); - queue.push((next, child)); + queue.push_back((next, child)); } } } diff --git a/crates/disktree-core/src/treemap.rs b/crates/disktree-core/src/treemap.rs index 598b5590..3bd572b4 100644 --- a/crates/disktree-core/src/treemap.rs +++ b/crates/disktree-core/src/treemap.rs @@ -220,9 +220,20 @@ fn place_children( if ranked.is_empty() { return; } - ranked.sort_by(|left, right| right.1.total_cmp(&left.1)); - + // Largest first, and of two equals the earlier child, which is the + // order a stable sort of the children would give. + let by_value = |left: &(usize, f64), right: &(usize, f64)| { + right.1.total_cmp(&left.1).then(left.0.cmp(&right.0)) + }; let kept = ranked.len().min(options.max_children); + // Only the children that get a tile need ordering. The rest are + // summed, so a directory of a million files is partitioned around + // the cut rather than sorted, on every resize. + if kept < ranked.len() { + ranked.select_nth_unstable_by(kept, by_value); + } + ranked[..kept].sort_unstable_by(by_value); + let mut values: Vec = ranked[..kept].iter().map(|(_, value)| *value).collect(); let mut sources: Vec> = ranked[..kept] From 05612e2d70e73748f3ae5366f619c21157d82c3a Mon Sep 17 00:00:00 2001 From: armada45-pixel Date: Tue, 13 Oct 2026 06:56:45 +0700 Subject: [PATCH 2/3] Harden the git probe, the link guard and the file-table reader The git probe sets GIT_ALLOW_PROTOCOL=none: a scanned checkout's own configuration could otherwise name a transport for git to run while disktree only meant to read its status. The removal guard no longer passes a path whose real location cannot be read while its parent is there to read; a link leading out of the scanned root could hide behind that failure. The NTFS file-table reader bounds what a crafted volume can make it do: attribute-list records are capped, a table that claims more than its runs hold is refused, and a directory graph with a cycle stops the walk so the scan falls back to the ordinary one. Byte totals saturate instead of overflowing; release builds check overflow, so a wrapped sum was a panic. Co-Authored-By: Claude Opus 5.5 --- crates/disktree-app/src/git.rs | 57 ++++++++++++++++++++++++++++ crates/disktree-core/src/filter.rs | 6 ++- crates/disktree-core/src/insights.rs | 3 +- crates/disktree-core/src/mft.rs | 46 +++++++++++++++++++++- crates/disktree-core/src/removal.rs | 26 +++++++++++-- crates/disktree-core/src/scan.rs | 3 +- 6 files changed, 131 insertions(+), 10 deletions(-) diff --git a/crates/disktree-app/src/git.rs b/crates/disktree-app/src/git.rs index b4a69e31..cb3e5af1 100644 --- a/crates/disktree-app/src/git.rs +++ b/crates/disktree-app/src/git.rs @@ -200,6 +200,12 @@ fn git(path: &Path, args: &[&str]) -> Option { .env("GIT_TERMINAL_PROMPT", "0") .env("GIT_OPTIONAL_LOCKS", "0") .env("GIT_NO_LAZY_FETCH", "1") + // `protocol.allow=never` above is only the default: a checkout's + // own `protocol.ext.allow=always` is more specific and wins, and a + // git older than 2.44 does not know `GIT_NO_LAZY_FETCH`. With this + // set git asks no configuration at all, only this list, and no + // transport is called `none`. + .env("GIT_ALLOW_PROTOCOL", "none") .stdin(Stdio::null()) .stderr(Stdio::null()) .output() @@ -293,6 +299,57 @@ mod tests { assert!(!marker.exists(), "the checkout's fsmonitor ran"); } + /// A partial clone whose promisor remote is a command, allowed by the + /// checkout's own configuration, and a branch whose commits are + /// missing: fetching them would run it. Whatever git makes of such a + /// checkout, the command must not run. + #[cfg(unix)] + #[test] + fn a_checkout_cannot_make_git_fetch_through_its_own_transport() { + let dir = tempfile::tempdir().expect("tempdir"); + let marker = dir.path().join("ran"); + let git = |args: &[&str]| { + Command::new("git") + .arg("-C") + .arg(dir.path()) + .args(args) + .output() + .is_ok_and(|output| output.status.success()) + }; + if !git_installed() || !git(&["init", "-q"]) { + return; // no git on this machine + } + let url = format!("ext::sh -c 'touch \"{}\"'", marker.display()); + for (key, value) in [ + ("core.repositoryformatversion", "1"), + ("extensions.partialClone", "origin"), + ("remote.origin.url", url.as_str()), + ("remote.origin.promisor", "true"), + ("remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*"), + ("protocol.ext.allow", "always"), + ("branch.main.remote", "origin"), + ("branch.main.merge", "refs/heads/main"), + ] { + assert!(git(&["config", key, value]), "{key}"); + } + // Two commits nobody has: the branch, and what it tracks. + let store = dir.path().join(".git"); + for (name, id) in [ + ("refs/heads/main", "1".repeat(40)), + ("refs/remotes/origin/main", "2".repeat(40)), + ] { + let path = store.join(name); + std::fs::create_dir_all(path.parent().expect("a parent")) + .expect("mkdir"); + std::fs::write(path, format!("{id}\n")).expect("write ref"); + } + std::fs::write(store.join("HEAD"), "ref: refs/heads/main\n") + .expect("write HEAD"); + + let _ = state(dir.path()); + assert!(!marker.exists(), "the checkout's transport ran"); + } + /// A signed stash and `log.showSignature`: listing stashes the way /// `git stash list` does would run the checkout's `gpg.program`. #[cfg(unix)] diff --git a/crates/disktree-core/src/filter.rs b/crates/disktree-core/src/filter.rs index 24c1a7a9..fb717a9a 100644 --- a/crates/disktree-core/src/filter.rs +++ b/crates/disktree-core/src/filter.rs @@ -104,7 +104,9 @@ fn visit( if contains_ignoring_case(&child.name, &matches.needle) { matches.keep.insert(crumbs.clone(), Keep::Whole); matches.count += 1; - total.0 += child.bytes; + // Saturating, as `tree::aggregate` is: a corrupt volume's + // file table can claim any size. + total.0 = child.bytes.saturating_add(total.0); total.1 += child.files; } else if !child.children.is_empty() { let (bytes, files) = visit(child, crumbs, matches); @@ -112,7 +114,7 @@ fn visit( matches .keep .insert(crumbs.clone(), Keep::Partial { bytes, files }); - total.0 += bytes; + total.0 = bytes.saturating_add(total.0); total.1 += files; } } diff --git a/crates/disktree-core/src/insights.rs b/crates/disktree-core/src/insights.rs index 589029c5..7b2b3396 100644 --- a/crates/disktree-core/src/insights.rs +++ b/crates/disktree-core/src/insights.rs @@ -111,7 +111,8 @@ fn visit( && now - child.modified > STALE_DAYS * DAY; if is_stale { stale.0 += 1; - stale.1 += child.bytes; + // Saturating: see `tree::aggregate`. + stale.1 = child.bytes.saturating_add(stale.1); // A stale experiment is judged whole; its caches go with it. continue; } diff --git a/crates/disktree-core/src/mft.rs b/crates/disktree-core/src/mft.rs index 0a200440..4b236b54 100644 --- a/crates/disktree-core/src/mft.rs +++ b/crates/disktree-core/src/mft.rs @@ -23,6 +23,7 @@ use std::io; use std::num::NonZeroU64; use std::os::windows::fs::{FileExt as _, OpenOptionsExt as _}; use std::path::Path; +use std::sync::atomic::{AtomicUsize, Ordering}; use rayon::prelude::*; use windows_sys::Win32::Storage::FileSystem::{ @@ -211,6 +212,7 @@ pub fn scan( starts, options, progress, + directories: AtomicUsize::new(0), }; let tree = table.directory(ROOT, crate::scan::file_name(root), 0); // Hundreds of megabytes, whose freeing the caller would otherwise @@ -333,9 +335,18 @@ fn table_layout(volume: &File, geometry: &Geometry) -> Option<(Runs, Vec)> { { let number = reference & 0xFFFF_FFFF_FFFF; if number != 0 && !numbers.contains(&number) { + // A real table spreads over a handful of records. A list + // naming thousands is a crafted one, and each is searched + // for here and read below. + if numbers.len() >= MOST_LIST_RECORDS { + return None; + } numbers.push(number); } - if length == 0 { + // An entry is never shorter than its own fixed part; one that + // says so would have this loop step through 64 MiB a byte at + // a time. + if usize::from(length) < LIST_ENTRY_BYTES { break; } at += usize::from(length); @@ -364,10 +375,31 @@ fn table_layout(volume: &File, geometry: &Geometry) -> Option<(Runs, Vec)> { { return None; } + // The volume's size above is the boot sector's own claim, and a crafted + // one claims anything. What can be read is not: the table's last + // cluster has to be there, and runs that do not overlap cannot hold + // more than lies before their end. Without this, one bit set far into + // an imaginary table sized `table`'s slots in the tens of gigabytes. + let end = runs.iter().map(|&(offset, length)| offset + length).max()?; + if listed > end { + return None; + } + read_at( + volume, + end.checked_sub(geometry.cluster)?, + usize::try_from(geometry.cluster).ok()?, + )?; let bitmap = contents(volume, geometry, &records, BITMAP)?; Some((runs, bitmap)) } +/// Records an `$ATTRIBUTE_LIST` may name before the table is taken for a +/// crafted one: a table's own attributes fit in a few. +const MOST_LIST_RECORDS: usize = 4096; + +/// The fixed part of an `$ATTRIBUTE_LIST` entry, before its name. +const LIST_ENTRY_BYTES: usize = 0x18; + /// The unnamed attribute `kind` across `records`: the piece that starts /// it, which carries its sizes, and the runs of every piece in order. fn stream( @@ -1032,6 +1064,11 @@ struct Table<'a> { starts: Vec, options: &'a ScanOptions, progress: &'a ScanProgress, + /// Directories built so far. Each is built from one entry, so more of + /// them than there are entries means one was reached twice: a crafted + /// table that gives directories several names doubles the tree at + /// every level, and forty levels of that is all the memory there is. + directories: AtomicUsize, } impl Table<'_> { @@ -1060,7 +1097,11 @@ impl Table<'_> { name: Box, depth: usize, ) -> Result { - if depth >= MOST_LEVELS || self.progress.is_cancelled() { + if depth >= MOST_LEVELS + || self.progress.is_cancelled() + || self.directories.fetch_add(1, Ordering::Relaxed) + > self.names.len() + { return Err(Stop); } let descend = self.options.max_depth.is_none_or(|max| depth < max); @@ -1566,6 +1607,7 @@ mod tests { starts, options, progress, + directories: AtomicUsize::new(0), } } diff --git a/crates/disktree-core/src/removal.rs b/crates/disktree-core/src/removal.rs index 60277529..491630a3 100644 --- a/crates/disktree-core/src/removal.rs +++ b/crates/disktree-core/src/removal.rs @@ -530,8 +530,8 @@ fn misread_by_win32(name: &str) -> bool { /// user's own choice of root, so the real path is compared with the real /// root. The home directory is looked for again by real path, too. /// -/// Anything that cannot be resolved passes: the removal of a path that is -/// gone fails on its own. +/// A path that is gone passes: its removal fails on its own. One that is +/// still there and cannot be resolved, under a root that can, is refused. fn linked( path: &Path, root: &Path, @@ -539,7 +539,25 @@ fn linked( home: Option<&Home>, ) -> Option { let (parent, name) = (path.parent()?, path.file_name()?); - let real = guard_key(&fs::canonicalize(parent).ok()?.join(name)); + let real_parent = match fs::canonicalize(parent) { + Ok(real_parent) => real_parent, + // Gone, or on a volume where nothing resolves, the root included: + // there is no real path to compare with. + Err(_) + if real_root.is_none() || fs::symlink_metadata(parent).is_err() => + { + return None; + } + // The root resolved and this, inside it and still there, did not: + // whether a link leads out can no longer be told, so it stays. + Err(error) => { + return Some(format!( + "its real location could not be read ({error}), so a link \ + leading out of the scanned root cannot be ruled out" + )); + } + }; + let real = guard_key(&real_parent.join(name)); if let (Some(real_root), Ok(rest)) = (real_root, path.strip_prefix(root)) && real != guard_key(&real_root.join(rest)) { @@ -1102,7 +1120,7 @@ fn run( }; if outcome.is_ok() { removed += 1; - bytes += target.bytes; + bytes = bytes.saturating_add(target.bytes); } else { failed += 1; } diff --git a/crates/disktree-core/src/scan.rs b/crates/disktree-core/src/scan.rs index 693253a7..5115accb 100644 --- a/crates/disktree-core/src/scan.rs +++ b/crates/disktree-core/src/scan.rs @@ -1016,7 +1016,8 @@ impl Tally { // though the tree does not count it. self.files += 1; } - self.bytes += node.bytes; + // Saturating: see `tree::aggregate`. + self.bytes = node.bytes.saturating_add(self.bytes); } fn flush(&mut self, progress: &ScanProgress) { From cc8a21c83df53097c60846f5c9f267f943e09ffc Mon Sep 17 00:00:00 2001 From: armada45-pixel Date: Tue, 13 Oct 2026 06:56:45 +0700 Subject: [PATCH 3/3] Pin release actions and scope the signing secrets Actions are pinned to commit SHAs, checkouts keep no credentials, and the macOS signing secrets reach only the two steps that use them instead of every step of the job, the build's own scripts included. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 50 ++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 04712f50..9075a12f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,6 +11,10 @@ on: description: "Existing tag to (re)build, e.g. v0.9.0" required: true +# Every job uploads to the release, so every job needs to write. The token +# reaches only the steps that name it: no checkout keeps it, and every +# action is pinned to a commit, since a tag can be moved under a release +# that signs with the secrets below. permissions: contents: write @@ -25,9 +29,10 @@ jobs: tag: ${{ steps.version.outputs.tag }} version: ${{ steps.version.outputs.version }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: ref: ${{ inputs.tag || github.ref }} + persist-credentials: false - name: Version id: version @@ -69,9 +74,10 @@ jobs: - { runner: ubuntu-22.04-arm, arch: aarch64 } runs-on: ${{ matrix.runner }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: ref: ${{ inputs.tag || github.ref }} + persist-credentials: false - name: System libraries run: | @@ -81,11 +87,11 @@ jobs: libx11-xcb-dev libxcb1-dev libfontconfig-dev libfreetype-dev \ libvulkan-dev libasound2-dev libssl-dev libzstd-dev - - uses: dtolnay/rust-toolchain@master + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # master with: toolchain: "1.97" - - uses: Swatinem/rust-cache@v2 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 - name: Test run: cargo xtask test @@ -137,29 +143,31 @@ jobs: # MACOS_SIGN_IDENTITY "Developer ID Application: Name (TEAMID)" # NOTARY_KEY_P8 base64 of an App Store Connect API key # NOTARY_KEY_ID, NOTARY_ISSUER that key's ID and issuer - MACOS_CERTIFICATE: ${{ secrets.MACOS_CERTIFICATE }} - MACOS_CERTIFICATE_PASSWORD: ${{ secrets.MACOS_CERTIFICATE_PASSWORD }} - MACOS_SIGN_IDENTITY: ${{ secrets.MACOS_SIGN_IDENTITY }} - NOTARY_KEY_P8: ${{ secrets.NOTARY_KEY_P8 }} - NOTARY_KEY_ID: ${{ secrets.NOTARY_KEY_ID }} - NOTARY_ISSUER: ${{ secrets.NOTARY_ISSUER }} + # Only whether there is a certificate is known to the whole job. The + # secrets themselves go to the two steps that use them, not to the + # tests and every dependency's build script before those. + HAS_CERTIFICATE: ${{ secrets.MACOS_CERTIFICATE != '' }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: ref: ${{ inputs.tag || github.ref }} + persist-credentials: false - - uses: dtolnay/rust-toolchain@master + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # master with: toolchain: "1.97" - - uses: Swatinem/rust-cache@v2 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 - name: Test run: cargo xtask test # A keychain of its own for the certificate, deleted with the runner. - name: Signing certificate - if: env.MACOS_CERTIFICATE != '' + if: env.HAS_CERTIFICATE == 'true' + env: + MACOS_CERTIFICATE: ${{ secrets.MACOS_CERTIFICATE }} + MACOS_CERTIFICATE_PASSWORD: ${{ secrets.MACOS_CERTIFICATE_PASSWORD }} run: | keychain="$RUNNER_TEMP/signing.keychain-db" password=$(uuidgen) @@ -176,9 +184,14 @@ jobs: rm "$RUNNER_TEMP/cert.p12" - name: Bundle + env: + MACOS_SIGN_IDENTITY: ${{ secrets.MACOS_SIGN_IDENTITY }} + NOTARY_KEY_P8: ${{ secrets.NOTARY_KEY_P8 }} + NOTARY_KEY_ID: ${{ secrets.NOTARY_KEY_ID }} + NOTARY_ISSUER: ${{ secrets.NOTARY_ISSUER }} run: | args=() - if [ -n "$MACOS_CERTIFICATE" ]; then + if [ "$HAS_CERTIFICATE" = true ]; then args+=(--sign "$MACOS_SIGN_IDENTITY") if [ -n "$NOTARY_KEY_P8" ]; then echo "$NOTARY_KEY_P8" | base64 --decode \ @@ -218,15 +231,16 @@ jobs: steps: - run: git config --global core.autocrlf false - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: ref: ${{ inputs.tag || github.ref }} + persist-credentials: false - - uses: dtolnay/rust-toolchain@master + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # master with: toolchain: "1.97" - - uses: Swatinem/rust-cache@v2 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 - name: Test run: cargo xtask test