diff --git a/crates/bashkit/src/builtins/ls/list.rs b/crates/bashkit/src/builtins/ls/list.rs index 1692b861a..99c1a67e7 100644 --- a/crates/bashkit/src/builtins/ls/list.rs +++ b/crates/bashkit/src/builtins/ls/list.rs @@ -1,4 +1,9 @@ //! ls builtin - list directory contents. +//! +//! Decisions: +//! - `-R` lists directories through bounded `walk` batches (`WalkCache`), +//! one query per batch on bulk backends; without `-a` dot directories are +//! not descended, so the default walk reads exactly what `ls -R` prints. // Uses unwrap() for validated single-char strings. #![allow(clippy::unwrap_used)] @@ -10,6 +15,7 @@ use std::path::Path; use crate::builtins::clap_env::apply_env_defaults; use crate::builtins::generated::ls_args::LS_ENV_DEFAULTS; +use crate::builtins::walk_cache::WalkCache; use crate::builtins::{Builtin, Context, resolve_path}; use crate::error::Result; use crate::fs::FileType; @@ -273,9 +279,11 @@ impl Builtin for Ls { } // Then output directory listings + let mut listings = WalkCache::new(opts.recursive).skip_hidden(!opts.all); for (i, path_str, path) in &dir_args { if let Err(e) = list_directory( &ctx, + &mut listings, path, path_str, &mut output, @@ -293,8 +301,10 @@ impl Builtin for Ls { } } +#[allow(clippy::too_many_arguments)] async fn list_directory( ctx: &Context<'_>, + listings: &mut WalkCache, path: &Path, display_path: &str, output: &mut String, @@ -316,9 +326,8 @@ async fn list_directory( output.push_str(&format!("{}:\n", header)); } - let entries = ctx - .fs - .read_dir(path) + let entries = listings + .list(&*ctx.fs, path, None) .await .map_err(|e| format!("cannot open directory '{}': {}", display_path, e))?; @@ -402,7 +411,7 @@ async fn list_directory( for (subpath, display) in subdirs { // Box the future to avoid infinite recursion type size Box::pin(list_directory( - ctx, &subpath, &display, output, opts, true, true, + ctx, listings, &subpath, &display, output, opts, true, true, )) .await?; } diff --git a/crates/bashkit/src/builtins/tree.rs b/crates/bashkit/src/builtins/tree.rs index b83ff421e..9ae71e072 100644 --- a/crates/bashkit/src/builtins/tree.rs +++ b/crates/bashkit/src/builtins/tree.rs @@ -1,8 +1,15 @@ //! tree builtin command - display directory tree +//! +//! Decisions: +//! - Directories are listed through bounded `walk` batches (`WalkCache`): +//! hidden directories are not descended without `-a`, batches never exceed +//! the remaining entry budget, and `-I` falls back to one `read_dir` per +//! directory (the walk cannot prune by pattern). use async_trait::async_trait; use std::path::Path; +use super::walk_cache::WalkCache; use super::{Builtin, Context, resolve_path}; use crate::error::Result; use crate::fs::vfs_join; @@ -64,6 +71,8 @@ struct TreeState { counts: TreeCounts, budget: TreeBudget, output: String, + /// Listings in bounded `walk` batches (one query each on bulk backends). + listings: WalkCache, } #[async_trait] @@ -159,6 +168,8 @@ impl Builtin for Tree { counts: TreeCounts { dirs: 0, files: 0 }, budget, output: String::new(), + // `-I` prunes directories by name, which a walk cannot know. + listings: WalkCache::new(opts.exclude_pattern.is_none()).skip_hidden(!opts.show_hidden), }; for path_str in &paths { @@ -225,12 +236,15 @@ async fn build_tree( return Ok(()); } - let entries = match ctx.fs.read_dir(dir).await { + let remaining = state.budget.remaining_entries(); + // Never prefetch past the entry budget: tree stops there. + state.listings.set_max_entries(remaining.saturating_add(1)); + let levels = opts.max_depth.map(|m| m.saturating_sub(depth)); + let entries = match state.listings.list(&*ctx.fs, dir, levels).await { Ok(e) => e, Err(_) => return Ok(()), }; - let remaining = state.budget.remaining_entries(); let mut filtered: Vec<_> = entries .into_iter() .filter(|e| { diff --git a/crates/bashkit/src/builtins/walk_cache.rs b/crates/bashkit/src/builtins/walk_cache.rs index 2c873382c..4675c550f 100644 --- a/crates/bashkit/src/builtins/walk_cache.rs +++ b/crates/bashkit/src/builtins/walk_cache.rs @@ -1,4 +1,5 @@ -//! Bounded `FileSystem::walk` prefetch for tree walkers (`find`, `du`). +//! Bounded `FileSystem::walk` prefetch for tree walkers (`find`, `du`, +//! `ls -R`, `tree`). //! THREAT[TM-DOS-140]: walks over millions of files hold one batch. //! //! Decisions: @@ -10,6 +11,9 @@ //! requested directory (path spelling mismatch), bulk walking is turned off //! for the run and plain `read_dir` takes over, so a mismatch can never turn //! into one batch-sized walk per directory. +//! - Callers that skip hidden directories pass `skip_hidden`, and callers +//! with an entry budget cap `max_entries` at what is left of it, so the +//! default `read_dir` walk never reads a directory the caller would not. use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -22,6 +26,7 @@ pub(crate) struct WalkCache { listings: HashMap>, enabled: bool, max_entries: usize, + skip_hidden: bool, } impl WalkCache { @@ -31,9 +36,21 @@ impl WalkCache { listings: HashMap::new(), enabled, max_entries: WalkOptions::DEFAULT_MAX_ENTRIES, + skip_hidden: false, } } + /// Never descend into dot directories below the listed one. + pub(crate) fn skip_hidden(mut self, skip: bool) -> Self { + self.skip_hidden = skip; + self + } + + /// Cap each batch below the default (e.g. at an entry budget's remainder). + pub(crate) fn set_max_entries(&mut self, entries: usize) { + self.max_entries = entries.clamp(1, WalkOptions::DEFAULT_MAX_ENTRIES); + } + /// Stop prefetching (an action may change or cut the traversal). pub(crate) fn disable(&mut self) { self.enabled = false; @@ -52,7 +69,9 @@ impl WalkCache { return Ok(list); } if self.enabled && max_depth != Some(0) { - let opts = WalkOptions::new(max_depth).max_entries(self.max_entries); + let opts = WalkOptions::new(max_depth) + .max_entries(self.max_entries) + .skip_hidden(self.skip_hidden); if let Ok(batch) = fs.walk(dir, opts).await { let mut own = None; for listing in batch { diff --git a/crates/bashkit/src/fs/traits.rs b/crates/bashkit/src/fs/traits.rs index 6c4b35cec..8826588b7 100644 --- a/crates/bashkit/src/fs/traits.rs +++ b/crates/bashkit/src/fs/traits.rs @@ -459,6 +459,11 @@ pub struct WalkOptions { /// Stop once the answer holds this many entries (at most one listing /// more). Callers list the rest on demand. pub max_entries: usize, + /// Do not descend into directories below the root whose name starts + /// with `.` (they are still listed as entries). A hint for callers that + /// skip hidden directories (`ls -R`, `tree` without `-a`): an + /// implementation may ignore it and return their listings anyway. + pub skip_hidden: bool, } impl WalkOptions { @@ -471,6 +476,7 @@ impl WalkOptions { Self { max_depth, max_entries: Self::DEFAULT_MAX_ENTRIES, + skip_hidden: false, } } @@ -479,6 +485,12 @@ impl WalkOptions { self.max_entries = entries; self } + + /// Set [`WalkOptions::skip_hidden`]. + pub fn skip_hidden(mut self, skip: bool) -> Self { + self.skip_hidden = skip; + self + } } impl Default for WalkOptions { @@ -528,7 +540,9 @@ pub async fn walk_with_read_dir( let child_depth = depth + 1; if opts.max_depth.is_none_or(|m| child_depth < m) { for entry in &entries { - if entry.metadata.file_type.is_dir() { + if entry.metadata.file_type.is_dir() + && !(opts.skip_hidden && entry.name.starts_with('.')) + { pending.push((super::vfs_join(&path, &entry.name), child_depth, None)); } } diff --git a/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs b/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs index d2768433b..00fa6cf5c 100644 --- a/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs +++ b/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs @@ -406,6 +406,103 @@ async fn ls_recursive_and_tree_list_each_directory_once() { } } +#[tokio::test] +async fn ls_recursive_and_tree_use_one_bulk_walk() { + let (fs, mut bash) = setup(CountingFs::new().with_native_walk()).await; + for script in ["ls -R tree", "ls -lR tree", "tree tree", "tree -L 2 tree"] { + let (c, _) = run(&fs, &mut bash, script).await; + assert_eq!(c.walk, 1, "{script}: {c}"); + assert_eq!(c.read_dir, 0, "{script}: {c}"); + assert!(c.total() <= 3, "{script}: {c}"); + } +} + +/// Hidden directories (a `.git` with many subdirectories) are not read by +/// `ls -R`/`tree` without `-a`, bulk walk or not. +#[tokio::test] +async fn ls_recursive_and_tree_skip_hidden_directories() { + async fn add_hidden(fs: &dyn FileSystem) { + for i in 0..20 { + fs.mkdir( + Path::new(&format!("/workspace/tree/.git/objects/{i}")), + true, + ) + .await + .unwrap(); + } + } + let (fs, mut bash) = setup(CountingFs::new()).await; + add_hidden(fs.as_ref()).await; + for script in ["ls -R tree", "tree tree"] { + let (c, _) = run(&fs, &mut bash, script).await; + assert_eq!(c.read_dir, DIRS, "{script}: {c}"); + } + let (c, _) = run(&fs, &mut bash, "ls -aR tree").await; + assert_eq!(c.read_dir, DIRS + 2 + 20, "{c}"); +} + +/// The bulk walk must not change what `ls -R` and `tree` print. +#[tokio::test] +async fn ls_recursive_and_tree_output_is_the_same_with_bulk_walk() { + async fn layout(fs: &dyn FileSystem) { + seed(fs).await; + fs.mkdir(Path::new("/workspace/tree/.hidden/inner"), true) + .await + .unwrap(); + fs.write_file(Path::new("/workspace/tree/.hidden/inner/x"), b"x") + .await + .unwrap(); + fs.symlink(Path::new("d1"), Path::new("/workspace/tree/link-d1")) + .await + .unwrap(); + } + let plain = Arc::new(CountingFs::new()); + layout(plain.as_ref()).await; + let bulk = Arc::new(CountingFs::new().with_native_walk().with_walk_page(PAGE)); + layout(bulk.as_ref()).await; + let mut plain_bash = Bash::builder().fs(plain.clone()).cwd("/workspace").build(); + let mut bulk_bash = Bash::builder().fs(bulk.clone()).cwd("/workspace").build(); + for script in [ + "ls -R tree", + "ls -aR tree", + "ls -lR tree/d3", + "tree tree", + "tree -a tree", + "tree -L 2 tree", + "tree -d tree", + "tree -I s3 tree", + "cd tree && ls -R", + ] { + let (_, expected) = run(&plain, &mut plain_bash, script).await; + let (c, got) = run(&bulk, &mut bulk_bash, script).await; + assert_eq!(got, expected, "{script}"); + assert!(!expected.is_empty(), "{script}: vacuous"); + assert!( + c.walk_peak_entries <= (PAGE + SUB.max(FILES) + 3) as u64, + "{script}: {c}" + ); + } +} + +/// tree stops at its entry budget; prefetch must not read far past it. +#[tokio::test] +async fn tree_prefetch_stays_within_entry_budget() { + let fs = Arc::new(CountingFs::new()); + seed(fs.as_ref()).await; + let limits = bashkit::ExecutionLimits::new().max_commands(50); + let mut bash = Bash::builder() + .fs(fs.clone()) + .cwd("/workspace") + .limits(limits) + .build(); + fs.reset(); + let r = bash.exec("tree tree").await.unwrap(); + assert_ne!(r.exit_code, 0, "expected the entry budget to stop tree"); + let c = fs.counts(); + // 50 entries is a handful of directories; never the whole tree. + assert!(c.read_dir < 15, "{c}"); +} + #[tokio::test] async fn tar_reads_member_metadata_from_listings() { let (fs, mut bash) = setup(CountingFs::new()).await; diff --git a/docs/filesystem.md b/docs/filesystem.md index a25ab8c37..a289dc4dc 100644 --- a/docs/filesystem.md +++ b/docs/filesystem.md @@ -105,8 +105,10 @@ methods cut the count further: - `walk(root, WalkOptions)`: the directory listings below `root` in one call (for example one path-prefix query), up to `max_entries` entries per answer (10,000 by default) so huge trees are fetched in bounded batches. The - default issues one `read_dir` per directory. `find` and `du` use it and ask - for the next batch as they descend. + default issues one `read_dir` per directory. `find`, `du`, `ls -R` and + `tree` use it and ask for the next batch as they descend. + `WalkOptions::skip_hidden` tells it the caller will not enter dot + directories; a backend may ignore it. `cargo bench --bench remote_fs -- --counts` prints the calls each command makes over a copy of the bashkit repository. diff --git a/knowledge/foundations/vfs.md b/knowledge/foundations/vfs.md index 0361eb2e7..200e59182 100644 --- a/knowledge/foundations/vfs.md +++ b/knowledge/foundations/vfs.md @@ -424,13 +424,15 @@ and measured by `benches/remote_fs.rs` over the bashkit checkout: root, `RootFs` outside system trees, `ReadOnlyFs`, per-execution wrappers); otherwise they fall back to the default. Callers use it only when they would list every directory anyway: `find` without `-exec*`/`-delete`/`-prune`/ - `-quit`, and `du`, through `builtins/walk_cache.rs`: a directory missing + `-quit`, `du`, `ls -R` and `tree` (not with `-I`), through + `builtins/walk_cache.rs`: a directory missing from the cache triggers another bounded `walk` rooted there, listings are dropped as they are consumed, and an answer without the requested directory switches the run to plain `read_dir`. Memory stays at about one - batch for trees of millions of files (TM-DOS-140). Not `rg` (ignore rules prune), `ls -R` (skips dot - directories), `tree` (entry budget stops early), where an eager default - walk would read more than today. + batch for trees of millions of files (TM-DOS-140). `ls -R`/`tree` without + `-a` pass `WalkOptions::skip_hidden` so the default walk never enters dot + directories (a `.git`), and `tree` caps each batch at its remaining entry + budget. Not `rg`: ignore rules prune by file contents a walk cannot see. - **`SearchCapable` reaches builtins.** `MountableFs` forwards the provider of the backend that serves the whole requested subtree (none if another mount sits below it) and maps result paths back under the mount point. A mount @@ -459,7 +461,7 @@ and measured by `benches/remote_fs.rs` over the bashkit checkout: - `FollowFs` `copy`/`chmod`/`touch -d` take a fast path (one inner `stat` per path) instead of resolving every path component. - Known remaining cost: `**` globs list each directory twice (walk, then the - next component); `ls -R`/`tree` are one `read_dir` per directory. + next component). ## Host Mount Table (`HostMounts`) diff --git a/knowledge/log.md b/knowledge/log.md index 9828103d2..d02ac5a61 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -1,5 +1,9 @@ # Bashkit Knowledge Update Log +## 2026-11-03 + +* **ls -R / tree bulk walk**: `ls -R` and `tree` list through bounded `walk` batches (`WalkCache`) like `find`/`du`. New `WalkOptions::skip_hidden` keeps the default walk out of dot directories they do not print; `tree` caps batches at its remaining entry budget and keeps `read_dir` under `-I`. See [Virtual Filesystem](foundations/vfs.md#remote-backends-round-trips). + ## 2026-11-02 * **Per-call execution timeout**: `ExecOptions::timeout` caps one `exec_with_options` call at the smaller of it and `ExecutionLimits::timeout`; Python `execute*` methods expose it as `timeout_seconds`. See [Architecture](foundations/architecture.md#per-invocation-exec-state).