diff --git a/CHANGELOG.md b/CHANGELOG.md index 0caf95e395..ee0deeed93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug Fixes - Fixed authenticated Cachix pulls failing with HTTP 401 even when a valid auth token was configured. The token was resolved correctly but applied to Nix too late, after the store had already opened and made its first request, so private cache lookups went out unauthenticated. The netrc credentials are now in place before the store opens. +- Fixed local files and directories pulled into the Nix store by path (e.g. `scripts.foo.exec = ./foo.sh;` or `languages.rust.import ./.`) not being tracked as eval-cache dependencies, so editing them returned stale `devenv build`/shell results until the cache was manually refreshed. Such sources are now tracked, with directories hashed recursively over their contents so nested edits are detected ([#2886](https://github.com/cachix/devenv/issues/2886), [#2893](https://github.com/cachix/devenv/issues/2893)). - Fixed `devenv shell` lingering as a background process, often pinned at 100%+ CPU, after its terminal window or tab was closed. The shell now reacts to the SIGHUP/SIGINT/SIGTERM that already trigger devenv's graceful shutdown by killing the inner shell, instead of orphaning it ([#2845](https://github.com/cachix/devenv/issues/2845)). - Fixed `devenv shell`/`devenv update` failing with `authentication required but no callback set` when a `url."ssh://git@github.com/".insteadOf` git config rewrites GitHub HTTPS URLs to SSH. GitHub flake inputs now resolve over SSH using your ssh-agent ([#2842](https://github.com/cachix/devenv/issues/2842)). - Fixed the "N files" counter under "Evaluating shell" inflating from generic Nix log lines. Now only counts actual file read operations. diff --git a/devenv-cache-core/src/file.rs b/devenv-cache-core/src/file.rs index cd2d22aeee..34caa5090b 100644 --- a/devenv-cache-core/src/file.rs +++ b/devenv-cache-core/src/file.rs @@ -192,6 +192,57 @@ pub fn compute_directory_hash>(path: P) -> CacheResult>(path: P) -> CacheResult { + let path = path.as_ref(); + let mut entries = Vec::new(); + + // Skip the root directory itself, sort by file name for consistent ordering + for entry in WalkDir::new(path).min_depth(1).sort_by_file_name() { + match entry { + Ok(entry) => { + let rel = entry + .path() + .strip_prefix(path) + .unwrap_or_else(|_| entry.path()) + .to_string_lossy() + .into_owned(); + let file_type = entry.file_type(); + + if file_type.is_dir() { + entries.push(format!("dir {rel}")); + } else if file_type.is_symlink() { + // Record the link target, not its contents, matching Nix. + let target = std::fs::read_link(entry.path()) + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_default(); + entries.push(format!("symlink {rel} -> {target}")); + } else { + match compute_file_hash(entry.path()) { + Ok(hash) => entries.push(format!("file {rel} {hash}")), + Err(_) => entries.push(format!("file_error {rel}")), + } + } + } + Err(e) => { + // Include error entries as well to detect when errors change + entries.push(format!("error {e}")); + } + } + } + + Ok(compute_string_hash(&entries.join("\n"))) +} + /// Compute a hash of a string pub fn compute_string_hash(content: &str) -> String { let hash = blake3::hash(content.as_bytes()); @@ -233,6 +284,50 @@ mod tests { assert_ne!(hash, hash3); } + #[test] + fn test_directory_content_hash_detects_nested_changes() { + let temp_dir = TempDir::new().unwrap(); + let nested = temp_dir.path().join("a").join("b"); + std::fs::create_dir_all(&nested).unwrap(); + let file_path = nested.join("c.txt"); + std::fs::write(&file_path, b"hello").unwrap(); + + let hash = compute_directory_content_hash(temp_dir.path()).unwrap(); + assert!(!hash.is_empty()); + + // Same contents hash identically. + assert_eq!( + hash, + compute_directory_content_hash(temp_dir.path()).unwrap() + ); + + // Editing a deeply nested file changes the hash, even though the + // directory listing is unchanged. + std::fs::write(&file_path, b"world").unwrap(); + assert_ne!( + hash, + compute_directory_content_hash(temp_dir.path()).unwrap() + ); + } + + #[test] + fn test_directory_content_hash_is_location_independent() { + // The same tree at two different locations hashes identically, because + // entries are keyed by their path relative to the root. + let make_tree = || { + let dir = TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join("src")).unwrap(); + std::fs::write(dir.path().join("src").join("main.rs"), b"fn main() {}").unwrap(); + dir + }; + let a = make_tree(); + let b = make_tree(); + assert_eq!( + compute_directory_content_hash(a.path()).unwrap(), + compute_directory_content_hash(b.path()).unwrap(), + ); + } + #[test] fn test_tracked_file() { let temp_dir = TempDir::new().unwrap(); diff --git a/devenv-cache-core/src/lib.rs b/devenv-cache-core/src/lib.rs index 994f37b27a..aa92f3f954 100644 --- a/devenv-cache-core/src/lib.rs +++ b/devenv-cache-core/src/lib.rs @@ -18,4 +18,6 @@ pub mod time; // Re-export common types for convenience pub use db::Database; pub use error::{CacheError, CacheResult}; -pub use file::{TrackedFile, compute_file_hash, compute_string_hash}; +pub use file::{ + TrackedFile, compute_directory_content_hash, compute_file_hash, compute_string_hash, +}; diff --git a/devenv-core/src/eval_op.rs b/devenv-core/src/eval_op.rs index d8153101f7..60da3ab095 100644 --- a/devenv-core/src/eval_op.rs +++ b/devenv-core/src/eval_op.rs @@ -5,7 +5,7 @@ //! //! Note: `EvalOp` is duplicated in `devenv_activity::EvalOp`. -use crate::internal_log::InternalLog; +use crate::internal_log::{ActivityType, Field, InternalLog}; use regex::Regex; use std::path::PathBuf; use std::sync::{Arc, LazyLock}; @@ -107,6 +107,31 @@ impl EvalOp { _ => None, } } + + /// Extract an `EvalOp` from a structured Nix activity. + /// + /// Some operations are surfaced as structured activities rather than free-text + /// log messages. Unlike messages, activities are emitted regardless of the + /// configured verbosity, so this is the reliable way to observe them. + /// + /// `ActivityType::EvalCopySource` carries the source path in field 0 and the + /// destination store path in field 1. + pub fn from_activity(typ: ActivityType, fields: &[Field]) -> Option { + match typ { + ActivityType::EvalCopySource => { + let source = match fields.first() { + Some(Field::String(s)) => PathBuf::from(s), + _ => return None, + }; + let target = match fields.get(1) { + Some(Field::String(s)) => PathBuf::from(s), + _ => return None, + }; + Some(EvalOp::CopiedSource { source, target }) + } + _ => None, + } + } } /// Observer trait for receiving evaluation operations. @@ -158,6 +183,39 @@ mod tests { ); } + #[test] + fn test_copied_source_from_activity() { + // Nix emits source copies as a structured `EvalCopySource` activity: + // field 0 = source path, field 1 = destination store path. + let fields = vec![ + Field::String("/path/to/source".to_string()), + Field::String("/nix/store/abc-source".to_string()), + ]; + let op = EvalOp::from_activity(ActivityType::EvalCopySource, &fields); + assert_eq!( + op, + Some(EvalOp::CopiedSource { + source: PathBuf::from("/path/to/source"), + target: PathBuf::from("/nix/store/abc-source"), + }) + ); + } + + #[test] + fn test_from_activity_ignores_other_types() { + let fields = vec![Field::String("/some/path".to_string())]; + assert_eq!(EvalOp::from_activity(ActivityType::Build, &fields), None); + } + + #[test] + fn test_from_activity_requires_both_fields() { + let fields = vec![Field::String("/only/source".to_string())]; + assert_eq!( + EvalOp::from_activity(ActivityType::EvalCopySource, &fields), + None + ); + } + #[test] fn test_evaluated_file() { let log = create_log("evaluating file '/path/to/file'"); diff --git a/devenv-core/src/internal_log.rs b/devenv-core/src/internal_log.rs index a9a6e9e952..a9166bfb46 100644 --- a/devenv-core/src/internal_log.rs +++ b/devenv-core/src/internal_log.rs @@ -136,6 +136,9 @@ pub enum ActivityType { PostBuildHook = 110, BuildWaiting = 111, FetchTree = 112, + /// A local source path was copied into the store during evaluation. + /// Fields: [0] = source path, [1] = destination store path. + EvalCopySource = 113, } #[derive(Debug, thiserror::Error)] diff --git a/devenv-core/src/nix_log_bridge.rs b/devenv-core/src/nix_log_bridge.rs index 0604dee810..43db656de1 100644 --- a/devenv-core/src/nix_log_bridge.rs +++ b/devenv-core/src/nix_log_bridge.rs @@ -270,6 +270,17 @@ impl NixLogBridge { fields, .. } => { + // Some eval operations (e.g. a source copied into the store) are + // surfaced as structured activities rather than log messages. + // Activities are delivered regardless of verbosity, so record any + // input dependency they carry for cache invalidation. + if let Some(op) = EvalOp::from_activity(typ, &fields) { + if let Ok(guard) = self.observers.lock() { + for observer in guard.iter() { + observer.record(op.clone()); + } + } + } self.handle_activity_start(id, typ, text, fields); } InternalLog::Stop { id } => { @@ -674,6 +685,7 @@ pub fn activity_type_from_str(s: &str) -> ActivityType { "post-build-hook" => ActivityType::PostBuildHook, "build-waiting" => ActivityType::BuildWaiting, "fetch-tree" => ActivityType::FetchTree, + "eval-copy-source" => ActivityType::EvalCopySource, _ => ActivityType::Unknown, } } @@ -821,6 +833,10 @@ mod tests { ActivityType::Substitute ); assert_eq!(activity_type_from_str("copy-path"), ActivityType::CopyPath); + assert_eq!( + activity_type_from_str("eval-copy-source"), + ActivityType::EvalCopySource + ); assert_eq!( activity_type_from_str("unknown-type"), ActivityType::Unknown diff --git a/devenv-eval-cache/migrations/20260603000000_add-recursive-to-file-input.sql b/devenv-eval-cache/migrations/20260603000000_add-recursive-to-file-input.sql new file mode 100644 index 0000000000..f06edd5d67 --- /dev/null +++ b/devenv-eval-cache/migrations/20260603000000_add-recursive-to-file-input.sql @@ -0,0 +1,7 @@ +-- Track whether a directory input must be hashed recursively over its contents. +-- +-- Directories observed via `copied source` / tracked devenv paths end up in the +-- Nix store with all of their contents, so a change to any nested file must +-- invalidate the cache. Directories observed via `readDir` only depend on their +-- listing, so they keep the cheaper name-only hashing (recursive = 0). +ALTER TABLE file_input ADD COLUMN recursive BOOLEAN NOT NULL DEFAULT 0; diff --git a/devenv-eval-cache/src/caching_eval.rs b/devenv-eval-cache/src/caching_eval.rs index 80b84cf6c3..6053a9053a 100644 --- a/devenv-eval-cache/src/caching_eval.rs +++ b/devenv-eval-cache/src/caching_eval.rs @@ -242,7 +242,7 @@ impl CachingEvalService { // Add extra watch paths let fallback_time = SystemTime::now(); for path in &self.config.extra_watch_paths { - if let Ok(desc) = FileInputDesc::new(path.clone(), fallback_time) { + if let Ok(desc) = FileInputDesc::new(path.clone(), fallback_time, true) { all_inputs.push(Input::File(desc)); } } @@ -884,7 +884,7 @@ mod tests { // Store a result with the real file let json_output = r#"{"shell":"/nix/store/abc-shell"}"#; let inputs = vec![Input::File( - FileInputDesc::new(temp_file, SystemTime::now()).unwrap(), + FileInputDesc::new(temp_file, SystemTime::now(), false).unwrap(), )]; service.store(&key, json_output, inputs).await.unwrap(); @@ -957,7 +957,7 @@ mod tests { // Store with the file as input let json_output = r#"{"result":"original"}"#; let inputs = vec![Input::File( - FileInputDesc::new(temp_file.clone(), SystemTime::now()).unwrap(), + FileInputDesc::new(temp_file.clone(), SystemTime::now(), false).unwrap(), )]; service.store(&key, json_output, inputs).await.unwrap(); @@ -986,7 +986,7 @@ mod tests { // Store with the file as input let json_output = r#"{"content":"original content"}"#; let inputs = vec![Input::File( - FileInputDesc::new(temp_file.clone(), SystemTime::now()).unwrap(), + FileInputDesc::new(temp_file.clone(), SystemTime::now(), false).unwrap(), )]; service.store(&key, json_output, inputs).await.unwrap(); @@ -1022,7 +1022,7 @@ mod tests { // Store with the file as input let json_output = r#"{"content":"stable content"}"#; let inputs = vec![Input::File( - FileInputDesc::new(temp_file.clone(), SystemTime::now()).unwrap(), + FileInputDesc::new(temp_file.clone(), SystemTime::now(), false).unwrap(), )]; service.store(&key, json_output, inputs).await.unwrap(); @@ -1057,8 +1057,8 @@ mod tests { // Store with multiple file inputs let json_output = r#"{"config":{"version":"1.0"},"data":"important data"}"#; let inputs = vec![ - Input::File(FileInputDesc::new(file1.clone(), SystemTime::now()).unwrap()), - Input::File(FileInputDesc::new(file2.clone(), SystemTime::now()).unwrap()), + Input::File(FileInputDesc::new(file1.clone(), SystemTime::now(), false).unwrap()), + Input::File(FileInputDesc::new(file2.clone(), SystemTime::now(), false).unwrap()), ]; service.store(&key, json_output, inputs).await.unwrap(); @@ -1168,7 +1168,7 @@ mod tests { // Store with both file and env inputs let json_output = r#"{"file":"file content","env":"env_value"}"#; let inputs = vec![ - Input::File(FileInputDesc::new(temp_file.clone(), SystemTime::now()).unwrap()), + Input::File(FileInputDesc::new(temp_file.clone(), SystemTime::now(), false).unwrap()), Input::Env(EnvInputDesc::new(env_name.to_string()).unwrap()), ]; service.store(&key, json_output, inputs).await.unwrap(); @@ -1231,7 +1231,7 @@ mod tests { let service = CachingEvalService::new(pool.clone()); let json_output = r#"{"persistent":true}"#; let inputs = vec![Input::File( - FileInputDesc::new(temp_file.clone(), SystemTime::now()).unwrap(), + FileInputDesc::new(temp_file.clone(), SystemTime::now(), false).unwrap(), )]; service.store(&key, json_output, inputs).await.unwrap(); } @@ -1448,7 +1448,7 @@ mod tests { .unwrap(); let inputs = vec![Input::File( - FileInputDesc::new(file_path, SystemTime::now()).unwrap(), + FileInputDesc::new(file_path, SystemTime::now(), false).unwrap(), )]; assert!(any_input_modified_after(&inputs, threshold)); } @@ -1468,7 +1468,7 @@ mod tests { .unwrap(); let inputs = vec![Input::File( - FileInputDesc::new(file_path, SystemTime::now()).unwrap(), + FileInputDesc::new(file_path, SystemTime::now(), false).unwrap(), )]; assert!(!any_input_modified_after(&inputs, SystemTime::now())); } diff --git a/devenv-eval-cache/src/db.rs b/devenv-eval-cache/src/db.rs index cd9d8dd5f3..810400e1f2 100644 --- a/devenv-eval-cache/src/db.rs +++ b/devenv-eval-cache/src/db.rs @@ -22,6 +22,8 @@ pub struct FileInputRow { pub path: PathBuf, /// Whether the path is a directory pub is_directory: bool, + /// Whether a directory is hashed recursively over its contents + pub recursive: bool, /// The hash of the file's content pub content_hash: String, /// The last modified time of the file @@ -34,12 +36,14 @@ impl sqlx::FromRow<'_, SqliteRow> for FileInputRow { fn from_row(row: &SqliteRow) -> Result { let path: &[u8] = row.get("path"); let is_directory: bool = row.get("is_directory"); + let recursive: bool = row.get("recursive"); let content_hash: String = row.get("content_hash"); let modified_at: i64 = row.get("modified_at"); let updated_at: i64 = row.get("updated_at"); Ok(Self { path: PathBuf::from(OsStr::from_bytes(path)), is_directory, + recursive, content_hash, modified_at: time::system_time_from_unix_seconds(modified_at), updated_at: time::system_time_from_unix_seconds(updated_at), @@ -71,6 +75,7 @@ impl From for FileInputDesc { Self { path: row.path, is_directory: row.is_directory, + recursive: row.recursive, content_hash: empty_to_none(row.content_hash), modified_at: row.modified_at, } @@ -307,11 +312,12 @@ where // Reuse the same file_input table as command caching let insert_file_input = r#" - INSERT INTO file_input (path, is_directory, content_hash, modified_at) - VALUES (?, ?, ?, ?) + INSERT INTO file_input (path, is_directory, recursive, content_hash, modified_at) + VALUES (?, ?, ?, ?, ?) ON CONFLICT (path) DO UPDATE SET content_hash = excluded.content_hash, is_directory = excluded.is_directory, + recursive = excluded.recursive, modified_at = excluded.modified_at, updated_at = ? RETURNING id @@ -322,6 +328,7 @@ where for FileInputDesc { path, is_directory, + recursive, content_hash, modified_at, } in file_inputs @@ -330,6 +337,7 @@ where let id: i64 = sqlx::query(insert_file_input) .bind(path.to_path_buf().into_os_string().as_bytes()) .bind(is_directory) + .bind(recursive) .bind(content_hash.as_deref().unwrap_or("")) .bind(modified_at) .bind(now) @@ -399,7 +407,7 @@ pub async fn get_files_by_eval_id( ) -> Result, sqlx::Error> { let files = sqlx::query_as( r#" - SELECT f.path, f.is_directory, f.content_hash, f.modified_at, f.updated_at + SELECT f.path, f.is_directory, f.recursive, f.content_hash, f.modified_at, f.updated_at FROM file_input f JOIN eval_input_path eip ON f.id = eip.file_input_id WHERE eip.cached_eval_id = ? @@ -618,6 +626,7 @@ mod tests { Input::File(FileInputDesc { path: "/path/to/devenv.nix".into(), is_directory: false, + recursive: false, content_hash: Some("hash1".to_string()), modified_at, }), @@ -675,6 +684,7 @@ mod tests { let inputs1 = vec![Input::File(FileInputDesc { path: "/path/to/file1.nix".into(), is_directory: false, + recursive: false, content_hash: Some("old_hash".to_string()), modified_at, })]; @@ -696,6 +706,7 @@ mod tests { let inputs2 = vec![Input::File(FileInputDesc { path: "/path/to/file2.nix".into(), is_directory: false, + recursive: false, content_hash: Some("new_hash".to_string()), modified_at, })]; @@ -735,6 +746,7 @@ mod tests { let shared_file = Input::File(FileInputDesc { path: "/path/to/shared.nix".into(), is_directory: false, + recursive: false, content_hash: Some("shared_hash".to_string()), modified_at, }); @@ -772,6 +784,7 @@ mod tests { let inputs = vec![Input::File(FileInputDesc { path: "/path/to/a.nix".into(), is_directory: false, + recursive: false, content_hash: Some("h1".to_string()), modified_at, })]; diff --git a/devenv-eval-cache/src/eval_inputs.rs b/devenv-eval-cache/src/eval_inputs.rs index cbb7d5083a..7b8d3c02c4 100644 --- a/devenv-eval-cache/src/eval_inputs.rs +++ b/devenv-eval-cache/src/eval_inputs.rs @@ -3,7 +3,7 @@ //! This module contains types that describe file and environment variable //! dependencies tracked during Nix evaluation for caching purposes. -use devenv_cache_core::{compute_file_hash, compute_string_hash}; +use devenv_cache_core::{compute_directory_content_hash, compute_file_hash, compute_string_hash}; use std::io; use std::path::PathBuf; use std::time::{SystemTime, UNIX_EPOCH}; @@ -66,6 +66,13 @@ impl Input { pub struct FileInputDesc { pub path: PathBuf, pub is_directory: bool, + /// Whether a directory must be hashed recursively over its contents. + /// + /// `true` for sources copied into the Nix store (`copied source`, tracked + /// devenv paths), where a change to any nested file changes what ends up in + /// the store. `false` for directories that were only listed (`readDir`), + /// where only the set of immediate entry names matters. Ignored for files. + pub recursive: bool, pub content_hash: Option, pub modified_at: SystemTime, } @@ -94,15 +101,14 @@ impl FileInputDesc { /// the timestamp of when this function was called. /// /// All timestamps are truncated to second precision. - pub fn new(path: PathBuf, fallback_system_time: SystemTime) -> Result { + pub fn new( + path: PathBuf, + fallback_system_time: SystemTime, + recursive: bool, + ) -> Result { let is_directory = path.is_dir(); let content_hash = if is_directory { - let mut paths: Vec = std::fs::read_dir(&path)? - .filter_map(Result::ok) - .map(|entry| entry.path().to_string_lossy().to_string()) - .collect(); - paths.sort(); - Some(compute_string_hash(&paths.join("\n"))) + Some(hash_directory(&path, recursive)?) } else { compute_file_hash(&path) .map_err(|e| std::io::Error::other(format!("Failed to compute file hash: {e}"))) @@ -116,12 +122,34 @@ impl FileInputDesc { Ok(Self { path, is_directory, + recursive, content_hash, modified_at, }) } } +/// Hash a directory's contents. +/// +/// When `recursive` is set, the entire tree's file contents are hashed (used for +/// sources copied into the Nix store). Otherwise only the sorted list of +/// immediate entry names is hashed (used for `readDir`, where only the listing +/// is observed during evaluation). +fn hash_directory(path: &std::path::Path, recursive: bool) -> Result { + if recursive { + compute_directory_content_hash(path).map_err(|e| { + std::io::Error::other(format!("Failed to compute directory content hash: {e}")) + }) + } else { + let mut paths: Vec = std::fs::read_dir(path)? + .filter_map(Result::ok) + .map(|entry| entry.path().to_string_lossy().to_string()) + .collect(); + paths.sort(); + Ok(compute_string_hash(&paths.join("\n"))) + } +} + /// Description of an environment variable input dependency. #[derive(Clone, Debug, Eq, PartialEq)] pub struct EnvInputDesc { @@ -183,31 +211,36 @@ pub fn check_file_state(file: &FileInputDesc) -> io::Result { }; let modified_at = metadata.modified().and_then(truncate_to_seconds)?; - if modified_at == file.modified_at { - // File has not been modified + let mtime_unchanged = modified_at == file.modified_at; + + // Fast path: for files and non-recursive directories, an unchanged mtime + // means the input is unchanged. Recursively-hashed directories cannot use + // this, because editing a file nested inside the directory does not bump the + // directory's own mtime. + let needs_rehash = file.is_directory && file.recursive; + if mtime_unchanged && !needs_rehash { return Ok(FileState::Unchanged); } - // mtime has changed, check if content has changed + // Recompute the hash to see whether the content has actually changed. let new_hash = if file.is_directory { if !metadata.is_dir() { return Ok(FileState::Removed); } - - let mut paths: Vec = std::fs::read_dir(&file.path)? - .filter_map(Result::ok) - .map(|entry| entry.path().to_string_lossy().to_string()) - .collect(); - paths.sort(); - compute_string_hash(&paths.join("\n")) + hash_directory(&file.path, file.recursive)? } else { compute_file_hash(&file.path) .map_err(|e| std::io::Error::other(format!("Failed to compute file hash: {e}")))? }; if Some(&new_hash) == file.content_hash.as_ref() { - // File touched but hash unchanged - Ok(FileState::MetadataModified { modified_at }) + if mtime_unchanged { + // Re-hashed a recursive directory and nothing changed. + Ok(FileState::Unchanged) + } else { + // Touched but hash unchanged. + Ok(FileState::MetadataModified { modified_at }) + } } else { // Hash has changed, return new hash Ok(FileState::Modified { @@ -295,6 +328,7 @@ mod tests { FileInputDesc { path: file_path, is_directory: false, + recursive: false, content_hash, modified_at: truncated_modified_at, } @@ -368,12 +402,14 @@ mod tests { let file1 = Input::File(FileInputDesc { path: path.clone(), is_directory: false, + recursive: false, content_hash: content_hash.clone(), modified_at: UNIX_EPOCH, }); let file2 = Input::File(FileInputDesc { path: path.clone(), is_directory: false, + recursive: false, content_hash: content_hash.clone(), modified_at: UNIX_EPOCH + std::time::Duration::from_secs(1), }); @@ -385,6 +421,81 @@ mod tests { assert_eq!(inputs[0], file2); } + /// Reproduces https://github.com/cachix/devenv/issues/2886 + /// + /// When Nix copies a source tree into the store (`EvalOp::CopiedSource`, e.g. + /// `languages.rust.import ./.`), the whole tree content is what ends up in the + /// store. The eval cache must therefore detect changes to file *contents* + /// nested inside the directory, not just the top-level entry names. + /// + /// Currently a directory's content hash is derived from the sorted list of its + /// immediate child names only, so editing a nested file leaves the hash + /// unchanged and `devenv build` returns a stale output path. + #[test] + fn test_directory_hash_changes_on_nested_content() { + let temp_dir = TempDir::with_prefix("test_directory_hash_nested").unwrap(); + let src = temp_dir.path().join("src"); + std::fs::create_dir(&src).unwrap(); + let main_rs = src.join("main.rs"); + std::fs::write(&main_rs, b"fn main() { println!(\"Hello, world!\"); }").unwrap(); + + let before = FileInputDesc::new(temp_dir.path().to_path_buf(), UNIX_EPOCH, true).unwrap(); + assert!(before.is_directory); + + // Change a nested file's content without adding/removing any entries, so + // the top-level directory listing stays identical. + std::fs::write(&main_rs, b"fn main() { println!(\"Goodbye, world!\"); }").unwrap(); + + let after = FileInputDesc::new(temp_dir.path().to_path_buf(), UNIX_EPOCH, true).unwrap(); + + assert_ne!( + before.content_hash, after.content_hash, + "directory content hash must change when a nested file's content changes" + ); + } + + /// `check_file_state` must report `Modified` when a file nested inside a + /// recursively-tracked directory changes content, even though editing a + /// nested file does not bump the top-level directory's mtime. + #[test] + fn test_check_state_detects_nested_content_change() { + let temp_dir = TempDir::with_prefix("test_check_state_nested").unwrap(); + let src = temp_dir.path().join("src"); + std::fs::create_dir(&src).unwrap(); + let main_rs = src.join("main.rs"); + std::fs::write(&main_rs, b"fn main() { println!(\"Hello, world!\"); }").unwrap(); + + let desc = FileInputDesc::new(temp_dir.path().to_path_buf(), UNIX_EPOCH, true).unwrap(); + + // Edit a nested file. The top-level directory's mtime is unaffected. + std::fs::write(&main_rs, b"fn main() { println!(\"Goodbye, world!\"); }").unwrap(); + + assert!( + matches!(check_file_state(&desc), Ok(FileState::Modified { .. })), + "nested content change must invalidate a recursively-tracked directory" + ); + } + + /// A non-recursive directory (e.g. tracked via `readDir`) only depends on its + /// listing, so a nested content change must not invalidate it. + #[test] + fn test_check_state_ignores_nested_change_for_non_recursive_dir() { + let temp_dir = TempDir::with_prefix("test_check_state_non_recursive").unwrap(); + let src = temp_dir.path().join("src"); + std::fs::create_dir(&src).unwrap(); + let main_rs = src.join("main.rs"); + std::fs::write(&main_rs, b"fn main() { println!(\"Hello, world!\"); }").unwrap(); + + let desc = FileInputDesc::new(temp_dir.path().to_path_buf(), UNIX_EPOCH, false).unwrap(); + + std::fs::write(&main_rs, b"fn main() { println!(\"Goodbye, world!\"); }").unwrap(); + + assert!( + matches!(check_file_state(&desc), Ok(FileState::Unchanged)), + "nested content change must not invalidate a non-recursive directory" + ); + } + #[test] fn test_truncate_system_time_to_seconds() { let time = SystemTime::now(); diff --git a/devenv-eval-cache/src/ffi_cache.rs b/devenv-eval-cache/src/ffi_cache.rs index 4a73b7faf6..13cb3b148f 100644 --- a/devenv-eval-cache/src/ffi_cache.rs +++ b/devenv-eval-cache/src/ffi_cache.rs @@ -139,6 +139,11 @@ pub fn ops_to_inputs(ops: impl IntoIterator, config: &CachingConf let mut inputs: Vec = Vec::new(); for op in ops { + // A directory must be hashed recursively over its contents when its whole + // tree ends up in the Nix store (`copied source`, tracked devenv paths). + // `readDir` and friends only observe the listing, so name-only hashing is + // sufficient and avoids spurious invalidation. + let recursive = matches!(op, EvalOp::CopiedSource { .. } | EvalOp::TrackedPath { .. }); match op { EvalOp::ReadFile { source } | EvalOp::ReadDir { source } @@ -166,7 +171,7 @@ pub fn ops_to_inputs(ops: impl IntoIterator, config: &CachingConf } // Create file input descriptor - if let Ok(desc) = FileInputDesc::new(source, fallback_time) { + if let Ok(desc) = FileInputDesc::new(source, fallback_time, recursive) { inputs.push(Input::File(desc)); } } @@ -183,9 +188,10 @@ pub fn ops_to_inputs(ops: impl IntoIterator, config: &CachingConf } } - // Add extra watch paths + // Add extra watch paths. These are meant to trigger re-evaluation on any + // change, so watch directories recursively. for path in &config.extra_watch_paths { - if let Ok(desc) = FileInputDesc::new(path.clone(), fallback_time) { + if let Ok(desc) = FileInputDesc::new(path.clone(), fallback_time, true) { inputs.push(Input::File(desc)); } } diff --git a/devenv.lock b/devenv.lock index 7172dfde8e..1e17a3e337 100644 --- a/devenv.lock +++ b/devenv.lock @@ -599,10 +599,11 @@ "nixpkgs-regression": "nixpkgs-regression" }, "locked": { - "lastModified": 1779748925, + "lastModified": 1780513790, + "narHash": "sha256-VNbHa18JCiESsXKrY9jxeiAVYOPopx8aeF9WSu0z9nQ=", "owner": "cachix", "repo": "nix", - "rev": "0bc443c8ff235c3547d09327b48aaa2ab98b15f2", + "rev": "4bc81008c18fd4bc2c4981feae1b69eb15aaecb4", "type": "github" }, "original": { @@ -984,4 +985,4 @@ }, "root": "root", "version": 7 -} +} \ No newline at end of file diff --git a/flake.lock b/flake.lock index 12354b30cd..e0277a9102 100644 --- a/flake.lock +++ b/flake.lock @@ -160,11 +160,11 @@ "nixpkgs-regression": [] }, "locked": { - "lastModified": 1779748925, - "narHash": "sha256-meIhqGC04O5VXbKSFXSQoOKp+XCq5RMnwAk1Guo0VQo=", + "lastModified": 1780513790, + "narHash": "sha256-VNbHa18JCiESsXKrY9jxeiAVYOPopx8aeF9WSu0z9nQ=", "owner": "cachix", "repo": "nix", - "rev": "0bc443c8ff235c3547d09327b48aaa2ab98b15f2", + "rev": "4bc81008c18fd4bc2c4981feae1b69eb15aaecb4", "type": "github" }, "original": {