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 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-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/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) { 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]