Merge pull request #1835 from cachix/safe-write-watched-files

direnv: fix excessive reloads

Author: domenkozar

Cargo.lock

@@ -1,14 +1,14 @@
 [workspace]
 resolver = "2"
 members = [
-    "devenv",
-    "devenv-generate",
-    "devenv-eval-cache",
-    "devenv-run-tests",
-    "devenv-tasks",
-    "http-client-tls",
-    "nix-conf-parser",
-    "xtask",
+  "devenv",
+  "devenv-generate",
+  "devenv-eval-cache",
+  "devenv-run-tests",
+  "devenv-tasks",
+  "http-client-tls",
+  "nix-conf-parser",
+  "xtask",
 ]
 
 [workspace.package]
@@ -35,6 +35,7 @@ cli-table = "0.4.9"
 console = "0.15.8"
 dotlock = "0.5.0"
 dialoguer = "0.11.0"
+fd-lock = "4"
 futures = "0.3.30"
 hex = "0.4.3"
 include_dir = "0.7.3"
@@ -51,10 +52,10 @@ pretty_assertions = { version = "1.4.0", features = ["unstable"] }
 regex = "1.10.3"
 schemars = "0.8.16"
 schematic = { version = "0.17.11", features = [
-    "schema",
-    "yaml",
-    "renderer_template",
-    "renderer_json_schema",
+  "schema",
+  "yaml",
+  "renderer_template",
+  "renderer_json_schema",
 ] }
 serde = { version = "1.0.197", features = ["derive"] }
 serde_json = "1.0.114"
@@ -69,13 +70,13 @@ tracing = "0.1.40"
 tracing-core = "0.1.32"
 tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
 tokio = { version = "1.39.3", features = [
-    "process",
-    "fs",
-    "io-util",
-    "macros",
-    "rt-multi-thread",
-    "sync",
-    "time",
+  "process",
+  "fs",
+  "io-util",
+  "macros",
+  "rt-multi-thread",
+  "sync",
+  "time",
 ] }
 tokio-util = { version = "0.7.12", features = ["io"] }
 which = "7.0.2"

Cargo.toml

@@ -45,6 +45,10 @@ impl<'a> CachedCommand<'a> {
     }
 
     /// Watch additional paths for changes.
+    ///
+    /// WARN: Be careful watching generated files.
+    /// External tools like direnv are triggered solely by the modification date and don't compare file contents.
+    /// Use [util::write_file_with_lock] to safely write such files.
     pub fn watch_path<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
         self.extra_paths.push(path.as_ref().to_path_buf());
         self

devenv-eval-cache/src/command.rs

@@ -20,6 +20,7 @@ cli-table.workspace = true
 console.workspace = true
 dialoguer.workspace = true
 dotlock.workspace = true
+fd-lock.workspace = true
 hex.workspace = true
 include_dir.workspace = true
 indoc.workspace = true

devenv/Cargo.toml

@@ -1,4 +1,4 @@
-use super::{cli, cnix, config, tasks};
+use super::{cli, cnix, config, tasks, util};
 use clap::crate_version;
 use cli_table::Table;
 use cli_table::{print_stderr, WithTitle};
@@ -866,11 +866,10 @@ impl Devenv {
                 }
             }
         }
-        fs::write(
+        util::write_file_with_lock(
             self.devenv_dotfile.join("flake.json"),
-            serde_json::to_string(&flake_inputs).unwrap(),
-        )
-        .expect("Failed to write flake.json");
+            &serde_json::to_string(&flake_inputs).unwrap(),
+        )?;
         fs::write(
             self.devenv_dotfile.join("devenv.json"),
             serde_json::to_string(&self.config).unwrap(),
@@ -916,13 +915,7 @@ impl Devenv {
         );
         let flake = FLAKE_TMPL.replace("__DEVENV_VARS__", &vars);
         let flake_path = self.devenv_root.join(DEVENV_FLAKE);
-
-        // Avoid writing the flake if it hasn't changed.
-        // direnv's watch_file triggers a reload based solely on mtime, which becomes annoying if we constantly touch this file.
-        let existing_flake = fs::read_to_string(&flake_path).unwrap_or_default();
-        if flake != existing_flake {
-            fs::write(flake_path, flake).expect("Failed to write flake.nix");
-        }
+        util::write_file_with_lock(&flake_path, &flake)?;
 
         self.assembled = true;
         Ok(())

devenv/src/devenv.rs

@@ -3,6 +3,7 @@ pub(crate) mod cnix;
 pub mod config;
 mod devenv;
 pub mod log;
+mod util;
 
 pub use cli::{default_system, GlobalOptions};
 pub use devenv::{Devenv, DevenvOptions, DIRENVRC, DIRENVRC_VERSION};

devenv/src/lib.rs

@@ -0,0 +1,59 @@
+use fd_lock::RwLock;
+use miette::{miette, IntoDiagnostic, Result};
+use std::fs;
+use std::io::Write;
+use std::path::Path;
+
+/// Safely write a file with locking, avoiding writing if the content hasn't changed.
+///
+/// Returns Ok(true) if the file was written, Ok(false) if no write was needed.
+pub fn write_file_with_lock<P: AsRef<Path>>(path: P, content: &str) -> Result<bool> {
+    let path = path.as_ref();
+
+    // Create parent directories if they don't exist
+    if let Some(parent) = path.parent() {
+        if !parent.exists() {
+            fs::create_dir_all(parent)
+                .into_diagnostic()
+                .map_err(|e| miette!("Failed to create directory {}: {}", parent.display(), e))?;
+        }
+    }
+
+    // Open or create the file with locking
+    let file = fs::OpenOptions::new()
+        .read(true)
+        .write(true)
+        .create(true)
+        .open(path)
+        .into_diagnostic()
+        .map_err(|e| miette!("Failed to open file {}: {}", path.display(), e))?;
+
+    // Acquire an exclusive lock on the file
+    let mut file_lock = RwLock::new(file);
+    let mut file_handle = file_lock
+        .write()
+        .into_diagnostic()
+        .map_err(|e| miette!("Failed to lock file {}: {}", path.display(), e))?;
+
+    // Read existing content
+    let existing_content = fs::read_to_string(path).unwrap_or_default();
+
+    // Compare and write only if different
+    if content != existing_content {
+        file_handle
+            .set_len(0)
+            .into_diagnostic()
+            .map_err(|e| miette!("Failed to truncate file {}: {}", path.display(), e))?;
+
+        file_handle
+            .write_all(content.as_bytes())
+            .into_diagnostic()
+            .map_err(|e| miette!("Failed to write to file {}: {}", path.display(), e))?;
+
+        // File was written
+        Ok(true)
+    } else {
+        // No write needed
+        Ok(false)
+    }
+}