diff --git a/crates/notfiles/src/linker.rs b/crates/notfiles/src/linker.rs index a08b6d1..426b6cf 100644 --- a/crates/notfiles/src/linker.rs +++ b/crates/notfiles/src/linker.rs @@ -196,11 +196,21 @@ pub fn link_package( } pub fn unlink_package( - _dotfiles_dir: &Path, + dotfiles_dir: &Path, state: &mut State, package: &str, opts: &LinkOptions, ) -> Result<(), NotfilesError> { + // Validate the package: it must either exist as a directory in dotfiles_dir + // or have entries in state. A name that satisfies neither is a user error. + let package_dir = dotfiles_dir.join(package); + let has_state_entries = !state.entries_for_package(package).is_empty(); + if !package_dir.is_dir() && !has_state_entries { + return Err(NotfilesError::PackageNotFound { + name: package.to_string(), + }); + } + let entries: Vec = state .entries_for_package(package) .into_iter() diff --git a/crates/notgraph/src/emit.rs b/crates/notgraph/src/emit.rs index 75bc88d..3ec0446 100644 --- a/crates/notgraph/src/emit.rs +++ b/crates/notgraph/src/emit.rs @@ -172,19 +172,27 @@ fn write_html( .map(|fs| fs.name.as_str()) .collect(); - let mut leaves: Vec<&str> = crate_graph.nodes.iter() + let mut leaves: Vec<&str> = crate_graph + .nodes + .iter() .map(|n| n.as_str()) .filter(|n| leaf_nodes.contains(n) && !root_nodes.contains(n)) .collect(); - let mut roots: Vec<&str> = crate_graph.nodes.iter() + let mut roots: Vec<&str> = crate_graph + .nodes + .iter() .map(|n| n.as_str()) .filter(|n| root_nodes.contains(n) && !leaf_nodes.contains(n)) .collect(); - let mut features: Vec<&str> = crate_graph.nodes.iter() + let mut features: Vec<&str> = crate_graph + .nodes + .iter() .map(|n| n.as_str()) .filter(|n| !leaf_nodes.contains(n) && !root_nodes.contains(n)) .collect(); - let mut isolated: Vec<&str> = crate_graph.nodes.iter() + let mut isolated: Vec<&str> = crate_graph + .nodes + .iter() .map(|n| n.as_str()) .filter(|n| leaf_nodes.contains(n) && root_nodes.contains(n)) .collect(); @@ -202,7 +210,11 @@ fn write_html( let syms = sym_count.get(n).copied().unwrap_or(0); format!( "{}[\"{}
in:{} out:{} | {} pub\"]", - mermaid_id(n), n, fi, fo, syms + mermaid_id(n), + n, + fi, + fo, + syms ) }; @@ -211,17 +223,23 @@ fn write_html( if !leaves.is_empty() { mermaid.push_str(" subgraph Core\n direction TB\n"); - for n in &leaves { mermaid.push_str(&format!(" {}\n", node_label(n))); } + for n in &leaves { + mermaid.push_str(&format!(" {}\n", node_label(n))); + } mermaid.push_str(" end\n"); } if !features.is_empty() { mermaid.push_str(" subgraph Features\n direction TB\n"); - for n in &features { mermaid.push_str(&format!(" {}\n", node_label(n))); } + for n in &features { + mermaid.push_str(&format!(" {}\n", node_label(n))); + } mermaid.push_str(" end\n"); } if !roots.is_empty() { mermaid.push_str(" subgraph Tools\n direction TB\n"); - for n in &roots { mermaid.push_str(&format!(" {}\n", node_label(n))); } + for n in &roots { + mermaid.push_str(&format!(" {}\n", node_label(n))); + } mermaid.push_str(" end\n"); } @@ -234,7 +252,8 @@ fn write_html( let color = heatmap_color(fs.fan_in); mermaid.push_str(&format!( " style {} fill:{},stroke:#4a9eff,color:#e0e0e0\n", - mermaid_id(&fs.name), color + mermaid_id(&fs.name), + color )); } @@ -316,10 +335,12 @@ fn write_html( let hotspot_rows: String = stats .hotspots .iter() - .map(|h| format!( - "{}{}{}", - h.name, h.kind, h.score - )) + .map(|h| { + format!( + "{}{}{}", + h.name, h.kind, h.score + ) + }) .collect::>() .join("\n"); diff --git a/crates/notgraph/src/main.rs b/crates/notgraph/src/main.rs index a47472a..5ba3b49 100644 --- a/crates/notgraph/src/main.rs +++ b/crates/notgraph/src/main.rs @@ -63,8 +63,14 @@ fn main() -> Result<()> { workspace_root.join(&cli.output) }; - emit::write_all(&output_dir, &crate_g, &module_graphs, &stats, &symbol_tables) - .context("failed to write reports")?; + emit::write_all( + &output_dir, + &crate_g, + &module_graphs, + &stats, + &symbol_tables, + ) + .context("failed to write reports")?; println!("notgraph: reports written to {}", output_dir.display()); diff --git a/crates/nothooks/src/lib.rs b/crates/nothooks/src/lib.rs index b1de2b3..7dd807a 100644 --- a/crates/nothooks/src/lib.rs +++ b/crates/nothooks/src/lib.rs @@ -1,7 +1,7 @@ pub mod runner; pub mod state; -use notcore::{HookPhase, HookSpec, Report, StepStatus}; +use notcore::{HookPhase, HookSpec, Report}; pub use runner::HookRunner; #[derive(Debug, PartialEq)] @@ -12,16 +12,8 @@ pub enum HookResult { } /// Run all hooks matching `phase` and collect into a `Report`. +/// +/// State is loaded once and saved once per call — not once per hook. pub fn run_phase(hooks: &[HookSpec], phase: &HookPhase, runner: &HookRunner) -> Report { - let mut report = Report::default(); - for hook in hooks.iter().filter(|h| &h.phase == phase) { - let result = runner.run_hook(hook); - let status = match &result { - HookResult::Ok => StepStatus::Ok, - HookResult::Skipped => StepStatus::Skipped, - HookResult::Failed(msg) => StepStatus::Failed(msg.clone()), - }; - report.add(&hook.name, status); - } - report + runner.run_phase(hooks, phase) } diff --git a/crates/nothooks/src/runner.rs b/crates/nothooks/src/runner.rs index 5212f0c..0be1c47 100644 --- a/crates/nothooks/src/runner.rs +++ b/crates/nothooks/src/runner.rs @@ -1,6 +1,6 @@ use crate::HookResult; use crate::state::HookState; -use notcore::{HookPhase, HookSpec}; +use notcore::{HookPhase, HookSpec, Report, StepStatus}; use std::path::PathBuf; use std::process::Command; @@ -46,9 +46,41 @@ impl HookRunner { } } - pub fn run_hook(&self, spec: &HookSpec) -> HookResult { + /// Run all hooks in `hooks` that match `phase`. + /// + /// State is loaded once before iterating and saved once at the end (only if + /// at least one Setup hook completed successfully), avoiding N file reads + /// and writes per phase. + pub fn run_phase(&self, hooks: &[HookSpec], phase: &HookPhase) -> Report { let mut state = HookState::load(&self.state_dir).unwrap_or_default(); + let mut state_dirty = false; + let mut report = Report::default(); + for spec in hooks.iter().filter(|h| &h.phase == phase) { + let result = self.run_hook_with_state(spec, &mut state, &mut state_dirty); + let status = match &result { + HookResult::Ok => StepStatus::Ok, + HookResult::Skipped => StepStatus::Skipped, + HookResult::Failed(msg) => StepStatus::Failed(msg.clone()), + }; + report.add(&spec.name, status); + } + + if state_dirty { + let _ = state.save(&self.state_dir); + } + + report + } + + /// Execute a single hook, using the caller-owned `state` and `dirty` flag + /// instead of loading/saving per invocation. + fn run_hook_with_state( + &self, + spec: &HookSpec, + state: &mut HookState, + state_dirty: &mut bool, + ) -> HookResult { if spec.phase == HookPhase::Setup && !self.force && state.is_done(&spec.name) { return HookResult::Skipped; } @@ -63,7 +95,7 @@ impl HookRunner { Ok(status) if status.success() => { if spec.phase == HookPhase::Setup { state.mark_done(&spec.name); - let _ = state.save(&self.state_dir); + *state_dirty = true; } HookResult::Ok } @@ -71,4 +103,18 @@ impl HookRunner { Err(e) => HookResult::Failed(e.to_string()), } } + + /// Run a single hook with its own load/save cycle. + /// + /// Prefer [`HookRunner::run_phase`] when running multiple hooks to avoid + /// repeated state I/O. + pub fn run_hook(&self, spec: &HookSpec) -> HookResult { + let mut state = HookState::load(&self.state_dir).unwrap_or_default(); + let mut state_dirty = false; + let result = self.run_hook_with_state(spec, &mut state, &mut state_dirty); + if state_dirty { + let _ = state.save(&self.state_dir); + } + result + } } diff --git a/crates/notsecrets/src/sources/bitwarden.rs b/crates/notsecrets/src/sources/bitwarden.rs index 7006e96..ec6f4e9 100644 --- a/crates/notsecrets/src/sources/bitwarden.rs +++ b/crates/notsecrets/src/sources/bitwarden.rs @@ -50,15 +50,19 @@ impl BitwardenSource { source: anyhow::anyhow!("bw unlock spawn: {e}"), })?; if let Some(mut stdin) = child.stdin.take() { - stdin.write_all(password.as_bytes()).map_err(|e| AgeError::SourceError { - name: self.name().to_string(), - source: anyhow::anyhow!("bw unlock stdin write: {e}"), - })?; + stdin + .write_all(password.as_bytes()) + .map_err(|e| AgeError::SourceError { + name: self.name().to_string(), + source: anyhow::anyhow!("bw unlock stdin write: {e}"), + })?; } - let output = child.wait_with_output().map_err(|e| AgeError::SourceError { - name: self.name().to_string(), - source: anyhow::anyhow!("bw unlock wait: {e}"), - })?; + let output = child + .wait_with_output() + .map_err(|e| AgeError::SourceError { + name: self.name().to_string(), + source: anyhow::anyhow!("bw unlock wait: {e}"), + })?; if !output.status.success() { return Err(AgeError::SourceError { name: self.name().to_string(),