From be168ec3ca278097ae1dc65a936a99f44466b851 Mon Sep 17 00:00:00 2001 From: Damien Nguyen Date: Fri, 8 May 2026 13:20:28 +0200 Subject: [PATCH] reload: exclude devenv dotfile from eval inputs and watch set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lib.fileset.fromSource ./.` (and any other `readDir` on a parent of `.devenv/`) was dragging devenv's own churn into the Nix tracked-input set: the eval-cache SQLite db + WAL/SHM are rewritten on every evaluation, the tasks db on every task run, and shell-env.sh / imports.txt / generated bootstrap files on every build. Tracked inputs that change every run mean cache validity is poisoned and the reload watcher self-triggers in an infinite loop after the first reload. Exclude the whole devenv dotfile from both the eval cache's tracked inputs (CachingConfig.excluded_paths) and the reload watcher (is_watchable_input in reload::owner). To preserve the documented $DEVENV_STATE contract, carve out `/state/` via CachingConfig.excluded_path_exceptions and the equivalent predicate in the watcher: files users persist there still trigger reload and cache invalidation. The carve-out would otherwise re-admit devenv-managed leaves under `state/` — the tasks-cache sqlite db (rewritten on every task run) and the git-hooks state dir (rewritten on every reload) — and reintroduce the same loop scoped to state/. Switch `ops_to_inputs` to longest-prefix-match between `excluded_paths` and `excluded_path_exceptions`: the most specific rule wins, ties favor the exception. Callers can then list `/state/tasks.db`, `tasks.db-wal`, `tasks.db-shm`, and `git-hooks` back in `excluded_paths` and have them override the broader state exception. `is_watchable_input` mirrors the same leaf list as defense in depth in case a stale eval-cache row predates the filter. `Path::starts_with` matches on components, so each sqlite sibling (`-wal`, `-shm`) is its own component and is listed explicitly — a `tasks.db` prefix would not cover them. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + devenv-eval-cache/src/db.rs | 3 + devenv-eval-cache/src/ffi_cache.rs | 122 ++++++++++++++++++- devenv-eval-cache/src/lib.rs | 2 +- devenv-nix-backend/src/backend.rs | 34 +++++- devenv/src/devenv/mod.rs | 2 +- devenv/src/reload/owner.rs | 187 +++++++++++++++++++++++++++-- 7 files changed, 336 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dac571378..3ebc11c35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Fixed long lines in `devenv shell` getting a hard newline inserted at the wrap point when copying to clipboard. The shell now preserves the soft-wrap when flushing wrapped output into the terminal's scrollback, so clipboard copy keeps the original single line ([#2865](https://github.com/cachix/devenv/issues/2865)). - Fixed files declared with the `files` option not being regenerated when an auto-loaded (`devenv allow`) shell reloaded after `devenv update`. enterShell tasks (including `devenv:files`) now re-run on hot-reload, matching a fresh shell entry, instead of only updating environment variables ([#2864](https://github.com/cachix/devenv/issues/2864)). - Fixed `devenv test --no-tui` (and any other non-TUI invocation) silently discarding all output from the `enterTest` script, so the test runner's output, traces, and failure messages never reached the terminal or CI logs. Output from commands run in the shell is now printed in non-TUI mode. +- Fixed `devenv shell` self-triggering hot-reload in an infinite loop after the first reload, and `lib.fileset.fromSource ./.` (and similar `readDir` calls on a parent of `.devenv/`) dragging devenv's own churn (eval cache WAL/SHM, tasks DB, generated shell scripts, `imports.txt`, …) into the Nix tracked-input set and causing spurious rebuilds. The devenv dotfile dir is now excluded from both the reload watch set and the eval cache's tracked inputs, with a carve-out for `.devenv/state/` (`$DEVENV_STATE`) so files users persist there still trigger reload. ### Improvements diff --git a/devenv-eval-cache/src/db.rs b/devenv-eval-cache/src/db.rs index cd9d8dd5f..c74668719 100644 --- a/devenv-eval-cache/src/db.rs +++ b/devenv-eval-cache/src/db.rs @@ -15,6 +15,9 @@ pub(crate) fn empty_to_none(s: String) -> Option { // Create a constant for embedded migrations pub const MIGRATIONS: sqlx::migrate::Migrator = sqlx::migrate!(); +/// Filename of the SQLite eval cache database under the devenv dotfile dir. +pub const DB_FILENAME: &str = "nix-eval-cache.db"; + /// The row type for the `file_input` table. #[derive(Clone, Debug, PartialEq)] pub struct FileInputRow { diff --git a/devenv-eval-cache/src/ffi_cache.rs b/devenv-eval-cache/src/ffi_cache.rs index 4a73b7faf..1a5a054cc 100644 --- a/devenv-eval-cache/src/ffi_cache.rs +++ b/devenv-eval-cache/src/ffi_cache.rs @@ -67,7 +67,19 @@ pub struct CachingConfig { /// Additional paths to watch for changes beyond those detected during eval. pub extra_watch_paths: Vec, /// Paths to exclude from cache invalidation (e.g., generated files). + /// Prefix-matched: any source whose path starts with one of these is + /// dropped, unless `excluded_path_exceptions` carves it back in by a + /// longer (more specific) prefix. pub excluded_paths: Vec, + /// Carve-outs that override `excluded_paths`. Combined with + /// `excluded_paths` under longest-prefix-match: for each source, the + /// most specific matching entry wins. Ties favor the exception so a + /// broad exclude with an equal-depth exception is still tracked. This + /// lets callers exclude a parent broadly, carve out a subdirectory, + /// and then re-exclude a leaf inside it by adding a longer entry to + /// `excluded_paths` (e.g. exclude `.devenv/`, keep `.devenv/state/`, + /// re-exclude `.devenv/state/tasks.db`). + pub excluded_path_exceptions: Vec, /// Environment variable names to exclude from cache invalidation /// (e.g., vars already tracked via NixArgs). pub excluded_envs: Vec, @@ -156,12 +168,30 @@ pub fn ops_to_inputs(ops: impl IntoIterator, config: &CachingConf continue; } - // Skip excluded paths - if config + // Longest-prefix-match between `excluded_paths` and + // `excluded_path_exceptions`. The most specific matching + // rule wins; ties favor the exception. This lets callers + // re-exclude a leaf inside an otherwise-allowed carve-out + // (e.g. exclude `.devenv/`, allow `.devenv/state/`, + // re-exclude `.devenv/state/tasks.db`). + let best_excluded = config .excluded_paths .iter() - .any(|excluded| source.starts_with(excluded)) - { + .filter(|p| source.starts_with(p)) + .map(|p| p.components().count()) + .max(); + let best_allowed = config + .excluded_path_exceptions + .iter() + .filter(|p| source.starts_with(p)) + .map(|p| p.components().count()) + .max(); + let drop = match (best_excluded, best_allowed) { + (None, _) => false, + (Some(_), None) => true, + (Some(e), Some(a)) => e > a, + }; + if drop { continue; } @@ -282,6 +312,90 @@ mod tests { assert!(inputs.is_empty()); } + #[test] + fn test_ops_to_inputs_excluded_path_exceptions_kept() { + // Broad exclude with a narrow carve-out: anything under /excluded is + // dropped, except /excluded/keep — used to ignore devenv's own state + // dir while still tracking user files under $DEVENV_STATE. + let config = CachingConfig { + excluded_paths: vec![PathBuf::from("/excluded")], + excluded_path_exceptions: vec![PathBuf::from("/excluded/keep")], + ..Default::default() + }; + let ops = vec![ + EvalOp::ReadFile { + source: PathBuf::from("/excluded/internal.db"), + }, + EvalOp::ReadFile { + source: PathBuf::from("/excluded/keep/user-file.txt"), + }, + ]; + let inputs = ops_to_inputs(ops, &config); + assert_eq!(inputs.len(), 1); + match &inputs[0] { + Input::File(desc) => { + assert_eq!(desc.path, PathBuf::from("/excluded/keep/user-file.txt")) + } + _ => panic!("expected file input"), + } + } + + #[test] + fn test_ops_to_inputs_longest_prefix_re_excludes_leaf() { + // Re-exclude a leaf inside an exception: `excluded_paths` covers a + // broad parent, `excluded_path_exceptions` carves out a subdir, + // and a longer entry in `excluded_paths` re-excludes a leaf inside + // that subdir. Models the devenv layout: exclude `.devenv/`, keep + // `.devenv/state/`, but drop devenv-managed `state/tasks.db*`. + // `Path::starts_with` matches at component boundaries, so each + // sqlite sibling (`-wal`, `-shm`) is its own component and must be + // listed explicitly — they are *not* covered by a `tasks.db` + // prefix. + let config = CachingConfig { + excluded_paths: vec![ + PathBuf::from("/d"), + PathBuf::from("/d/state/tasks.db"), + PathBuf::from("/d/state/tasks.db-wal"), + PathBuf::from("/d/state/tasks.db-shm"), + PathBuf::from("/d/state/git-hooks"), + ], + excluded_path_exceptions: vec![PathBuf::from("/d/state")], + ..Default::default() + }; + let ops = vec![ + // Dropped by `/d`. + EvalOp::ReadFile { + source: PathBuf::from("/d/shell-env.sh"), + }, + // Kept by `/d/state` carve-out. + EvalOp::ReadFile { + source: PathBuf::from("/d/state/postgres/data"), + }, + // Dropped: leaf exclusions are deeper than the carve-out. + EvalOp::ReadFile { + source: PathBuf::from("/d/state/tasks.db"), + }, + EvalOp::ReadFile { + source: PathBuf::from("/d/state/tasks.db-wal"), + }, + EvalOp::ReadFile { + source: PathBuf::from("/d/state/tasks.db-shm"), + }, + EvalOp::ReadFile { + source: PathBuf::from("/d/state/git-hooks/config.json"), + }, + ]; + let inputs = ops_to_inputs(ops, &config); + let kept: Vec<_> = inputs + .iter() + .map(|i| match i { + Input::File(d) => d.path.clone(), + _ => panic!(), + }) + .collect(); + assert_eq!(kept, vec![PathBuf::from("/d/state/postgres/data")]); + } + #[test] fn test_ops_to_inputs_filters_excluded_envs() { let config = CachingConfig { diff --git a/devenv-eval-cache/src/lib.rs b/devenv-eval-cache/src/lib.rs index a3b8164c6..50d5a8137 100644 --- a/devenv-eval-cache/src/lib.rs +++ b/devenv-eval-cache/src/lib.rs @@ -20,4 +20,4 @@ pub use ffi_cache::{CachingConfig, EvalCacheKey, InputTracker, ops_to_inputs}; pub use resource_manager::{ResourceManager, ResourceSpec}; // Re-export database query functions for file tracking -pub use db::{get_all_tracked_file_paths, get_file_inputs_by_key_hash}; +pub use db::{DB_FILENAME, get_all_tracked_file_paths, get_file_inputs_by_key_hash}; diff --git a/devenv-nix-backend/src/backend.rs b/devenv-nix-backend/src/backend.rs index 59a924530..d7e5c37d2 100644 --- a/devenv-nix-backend/src/backend.rs +++ b/devenv-nix-backend/src/backend.rs @@ -325,7 +325,39 @@ impl NixCBackend { force_refresh: self.cache_settings.refresh_eval_cache, extra_watch_paths: core_config_watch_paths(&self.paths.root), excluded_envs: vec!["NIXPKGS_CONFIG".to_string()], - excluded_paths: vec![self.nixpkgs_config_path.clone()], + // Exclude devenv's own dotfile dir from the tracked input + // set. Files inside (eval cache + WAL/SHM, tasks DB, + // generated shell scripts, imports.txt, etc.) are + // rewritten on every evaluation or build; tracking them + // would let `lib.fileset.fromSource ./.` (or any other + // `readDir` on a parent) drag devenv's own churn into + // the cache key and trigger spurious rebuilds. + excluded_paths: { + let state = self.paths.dotfile.join("state"); + vec![ + self.nixpkgs_config_path.clone(), + self.paths.dotfile.clone(), + // Re-exclude devenv-managed leaves that the + // broader `state/` carve-out below would + // otherwise re-admit. The tasks DB is rewritten + // on every task run and the git-hooks state on + // every reload; tracking either would invalidate + // the eval cache and self-trigger reload loops. + // `tasks.db-wal`/`tasks.db-shm` are listed + // separately because path prefix matching is + // component-wise, not byte-wise. + state.join("tasks.db"), + state.join("tasks.db-wal"), + state.join("tasks.db-shm"), + state.join("git-hooks"), + ] + }, + // Carve-out: keep `.devenv/state/` tracked. That's the + // documented `$DEVENV_STATE` area where users persist + // files they want reload/eval to react to. The + // devenv-internal leaves above re-exclude themselves by + // longest-prefix-match. + excluded_path_exceptions: vec![self.paths.dotfile.join("state")], }; let service = CachingEvalService::with_config(pool.clone(), config.clone()); let invalidation_flag = self.devenv_value_invalidated.clone(); diff --git a/devenv/src/devenv/mod.rs b/devenv/src/devenv/mod.rs index 329fafaec..1251c7a6f 100644 --- a/devenv/src/devenv/mod.rs +++ b/devenv/src/devenv/mod.rs @@ -385,7 +385,7 @@ impl Devenv { if cache_settings.eval_cache { eval_cache_pool .get_or_try_init(|| async { - let db_path = devenv_dotfile.join("nix-eval-cache.db"); + let db_path = devenv_dotfile.join(devenv_eval_cache::DB_FILENAME); let db = devenv_cache_core::db::Database::new( db_path, &devenv_eval_cache::db::MIGRATIONS, diff --git a/devenv/src/reload/owner.rs b/devenv/src/reload/owner.rs index d9de46fa1..6a481ce49 100644 --- a/devenv/src/reload/owner.rs +++ b/devenv/src/reload/owner.rs @@ -116,6 +116,48 @@ pub fn spawn_owner( (DevenvClient { tx }, handle) } +/// Drop paths that should never end up in the reload watch set. +/// +/// Excludes: +/// - missing files (likely deleted between eval and reload setup) +/// - `/nix/store` paths (immutable) +/// - anything inside the devenv dotfile dir *except* the `state/` subdir +/// (`$DEVENV_STATE`). Devenv churns most of the dotfile on every +/// evaluation (eval cache + WAL/SHM, tasks DB, generated shell scripts, +/// `imports.txt`, …); watching those would self-trigger reloads in an +/// infinite loop and propagate spurious changes when `lib.fileset` +/// `readDir`s a parent of `.devenv/`. The `state/` subdir is the +/// documented user-write area and stays watchable, minus a handful of +/// devenv-managed leaves under it (tasks DB + WAL/SHM, git-hooks state) +/// that would also self-trigger reloads — these mirror the eval cache's +/// longest-prefix-match exclusions in `CachingConfig` and act as +/// defense-in-depth in case a stale eval-cache row predates the filter. +fn is_watchable_input(path: &Path, dotfile: &Path) -> bool { + if !path.exists() || path.starts_with("/nix/store") { + return false; + } + if !path.starts_with(dotfile) { + return true; + } + let state = dotfile.join("state"); + if !path.starts_with(&state) { + return false; + } + // `Path::starts_with` matches at component boundaries, so each sqlite + // sibling (`-wal`, `-shm`) needs its own entry. + for internal in [ + state.join("tasks.db"), + state.join("tasks.db-wal"), + state.join("tasks.db-shm"), + state.join("git-hooks"), + ] { + if path.starts_with(&internal) { + return false; + } + } + true +} + async fn handle_request(devenv: &Devenv, req: DevenvRequest, verbosity: VerbosityLevel) { match req { DevenvRequest::PrepareExec { cmd, args, reply } => { @@ -178,23 +220,29 @@ async fn add_watch_paths(devenv: &Devenv, watcher: &WatcherHandle) { let Some(pool) = devenv.eval_cache_pool() else { return; }; + let dotfile = devenv.dotfile().to_path_buf(); if let Some(cache_key) = devenv.shell_cache_key() - && watch_by_key(pool, &cache_key.key_hash, watcher).await + && watch_by_key(pool, &cache_key.key_hash, watcher, &dotfile).await { return; } - watch_all_tracked(pool, watcher).await; + watch_all_tracked(pool, watcher, &dotfile).await; } -async fn watch_by_key(pool: &sqlx::SqlitePool, key_hash: &str, watcher: &WatcherHandle) -> bool { +async fn watch_by_key( + pool: &sqlx::SqlitePool, + key_hash: &str, + watcher: &WatcherHandle, + dotfile: &Path, +) -> bool { match devenv_eval_cache::get_file_inputs_by_key_hash(pool, key_hash).await { Ok(inputs) if !inputs.is_empty() => { let paths: Vec = inputs .into_iter() .map(|i| i.path) - .filter(|p| watchable(p)) + .filter(|p| is_watchable_input(p, dotfile)) .collect(); watcher.watch_many(paths).await; true @@ -207,10 +255,13 @@ async fn watch_by_key(pool: &sqlx::SqlitePool, key_hash: &str, watcher: &Watcher } } -async fn watch_all_tracked(pool: &sqlx::SqlitePool, watcher: &WatcherHandle) { +async fn watch_all_tracked(pool: &sqlx::SqlitePool, watcher: &WatcherHandle, dotfile: &Path) { match devenv_eval_cache::get_all_tracked_file_paths(pool).await { Ok(paths) => { - let filtered: Vec = paths.into_iter().filter(|p| watchable(p)).collect(); + let filtered: Vec = paths + .into_iter() + .filter(|p| is_watchable_input(p, dotfile)) + .collect(); watcher.watch_many(filtered).await; } Err(e) => { @@ -219,6 +270,126 @@ async fn watch_all_tracked(pool: &sqlx::SqlitePool, watcher: &WatcherHandle) { } } -fn watchable(path: &Path) -> bool { - path.exists() && !path.starts_with("/nix/store") +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn rejects_devenv_internal_files() { + // Everything devenv writes under the dotfile (eval-cache SQLite + + // WAL/SHM, tasks DB, generated shell scripts, imports.txt, etc.) + // would otherwise drag devenv churn into the reload watcher and + // into Nix tracked inputs whenever `lib.fileset` `readDir`s a + // parent of `.devenv/`. + let temp = TempDir::new().unwrap(); + let dotfile = temp.path().join(".devenv"); + std::fs::create_dir_all(&dotfile).unwrap(); + + for internal in [ + "nix-eval-cache.db", + "nix-eval-cache.db-wal", + "nix-eval-cache.db-shm", + "tasks.db", + "tasks.db-wal", + "imports.txt", + "shell-env.sh", + "input-paths.txt", + ] { + let path = dotfile.join(internal); + std::fs::write(&path, b"").unwrap(); + assert!( + !is_watchable_input(&path, &dotfile), + "{} must be excluded from watch set", + internal + ); + } + } + + #[test] + fn rejects_devenv_internal_subdirs() { + // Subdirs devenv generates (bootstrap/, gc/) must also be filtered. + let temp = TempDir::new().unwrap(); + let dotfile = temp.path().join(".devenv"); + let bootstrap = dotfile.join("bootstrap"); + std::fs::create_dir_all(&bootstrap).unwrap(); + let lib = bootstrap.join("bootstrapLib.nix"); + std::fs::write(&lib, b"{}").unwrap(); + + assert!(!is_watchable_input(&lib, &dotfile)); + } + + #[test] + fn rejects_nix_store_and_missing_paths() { + let temp = TempDir::new().unwrap(); + let dotfile = temp.path().join(".devenv"); + std::fs::create_dir_all(&dotfile).unwrap(); + + assert!(!is_watchable_input( + Path::new("/nix/store/abc-foo/x.nix"), + &dotfile + )); + assert!(!is_watchable_input( + &temp.path().join("does-not-exist"), + &dotfile + )); + } + + #[test] + fn accepts_real_user_input_files() { + let temp = TempDir::new().unwrap(); + let dotfile = temp.path().join(".devenv"); + std::fs::create_dir_all(&dotfile).unwrap(); + + let user_file = temp.path().join("devenv.nix"); + std::fs::write(&user_file, b"{}").unwrap(); + + assert!(is_watchable_input(&user_file, &dotfile)); + } + + #[test] + fn accepts_files_under_devenv_state() { + // `$DEVENV_STATE` lives at `/state/`. User-owned files + // there (service data, mkcert root, etc.) stay watchable — that's + // the carve-out from the broader dotfile exclusion. + let temp = TempDir::new().unwrap(); + let dotfile = temp.path().join(".devenv"); + let state_dir = dotfile.join("state"); + std::fs::create_dir_all(state_dir.join("mkcert")).unwrap(); + + let state_file = state_dir.join("mkcert/rootCA.pem"); + std::fs::write(&state_file, b"").unwrap(); + + assert!(is_watchable_input(&state_file, &dotfile)); + } + + #[test] + fn rejects_devenv_managed_state_leaves() { + // The `state/` carve-out re-admits everything under it by default. + // devenv-managed leaves churn on every task run / reload and would + // self-trigger reloads if left in the watch set — they must be + // dropped despite living under the exception. + let temp = TempDir::new().unwrap(); + let dotfile = temp.path().join(".devenv"); + let state_dir = dotfile.join("state"); + std::fs::create_dir_all(state_dir.join("git-hooks")).unwrap(); + + for internal in [ + "tasks.db", + "tasks.db-wal", + "tasks.db-shm", + "git-hooks/config.json", + ] { + let path = state_dir.join(internal); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&path, b"").unwrap(); + assert!( + !is_watchable_input(&path, &dotfile), + "{} must be excluded from watch set", + internal + ); + } + } }