diff --git a/crates/notfiles/src/linker.rs b/crates/notfiles/src/linker.rs index 2dc68ae..2e7605f 100644 --- a/crates/notfiles/src/linker.rs +++ b/crates/notfiles/src/linker.rs @@ -217,11 +217,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/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 + } }