joe/notfiles
1
0
Fork 0
Code Issues 6 Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor: Formalize ports and update related files

Browse Source
This commit is contained in:
Joseph O'Brien
2026-04-15 21:27:25 -04:00
parent ac0672f64f
commit 67f20eef2d
23 changed files with 979 additions and 258 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
crates/notfiles/src/adapters/fs.rs
View File

@@ -5,6 +5,10 @@ use std::path::{Path, PathBuf};
pub struct FileStoreImpl;
impl FileStore for FileStoreImpl {
fn read(&self, path: &Path) -> Result<Vec<u8>, std::io::Error> {
std::fs::read(path)
}
fn read_to_string(&self, path: &Path) -> Result<String, std::io::Error> {
std::fs::read_to_string(path)
}
@@ -25,6 +29,10 @@ impl FileStore for FileStoreImpl {
std::fs::remove_dir_all(path)
}
fn remove_dir(&self, path: &Path) -> Result<(), std::io::Error> {
std::fs::remove_dir(path)
}
fn read_link(&self, path: &Path) -> Result<PathBuf, std::io::Error> {
std::fs::read_link(path)
}
@@ -41,6 +49,12 @@ impl FileStore for FileStoreImpl {
std::fs::create_dir_all(path)
}
fn read_dir(&self, path: &Path) -> Result<Vec<PathBuf>, std::io::Error> {
std::fs::read_dir(path)?
.map(|entry| entry.map(|entry| entry.path()))
.collect()
}
#[cfg(unix)]
fn symlink(&self, target: &Path, link: &Path) -> Result<(), std::io::Error> {
std::os::unix::fs::symlink(target, link)

24
crates/notfiles/src/lib.rs
View File

@@ -11,14 +11,22 @@ use std::path::Path;
pub use adapters::FileStoreImpl;
pub use linker::{LinkOptions, State};
pub use package::resolve_packages;
pub use package::{resolve_packages, resolve_packages_with_store};
pub use ports::FileStore;
pub fn link(dotfiles_dir: &Path, packages: &[String], opts: &LinkOptions) -> Result<State> {
let fs = &adapters::FileStoreImpl;
link_with_store(dotfiles_dir, packages, opts, &adapters::FileStoreImpl)
}
pub fn link_with_store(
dotfiles_dir: &Path,
packages: &[String],
opts: &LinkOptions,
fs: &dyn FileStore,
) -> Result<State> {
let config = notcore::Config::load(dotfiles_dir)?;
let mut state = linker::State::load(dotfiles_dir, fs)?;
let pkgs = resolve_packages(dotfiles_dir, packages)?;
let pkgs = resolve_packages_with_store(dotfiles_dir, packages, fs)?;
for pkg in &pkgs {
linker::link_package(dotfiles_dir, &config, &mut state, pkg, opts, fs)?;
}
@@ -27,7 +35,15 @@ pub fn link(dotfiles_dir: &Path, packages: &[String], opts: &LinkOptions) -> Res
}
pub fn unlink(dotfiles_dir: &Path, packages: &[String], opts: &LinkOptions) -> Result<()> {
let fs = &adapters::FileStoreImpl;
unlink_with_store(dotfiles_dir, packages, opts, &adapters::FileStoreImpl)
}
pub fn unlink_with_store(
dotfiles_dir: &Path,
packages: &[String],
opts: &LinkOptions,
fs: &dyn FileStore,
) -> Result<()> {
let mut state = linker::State::load(dotfiles_dir, fs)?;
let pkgs = if packages.is_empty() {
state

78
crates/notfiles/src/linker.rs
View File

@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use chrono::Utc;
use serde::{Deserialize, Serialize};
use crate::package::collect_files;
use crate::package::collect_files_with_store;
use crate::ports::FileStore;
use notcore::{Config, Method, NotfilesError, expand_tilde};
@@ -61,6 +61,11 @@ impl State {
self.entries.retain(|e| e.package != package);
}
pub fn remove_entry(&mut self, package: &str, source: &str, target: &str) {
self.entries
.retain(|e| !(e.package == package && e.source == source && e.target == target));
}
pub fn add_entry(&mut self, entry: StateEntry) {
// Remove existing entry for same source+target, then add new
self.entries
@@ -87,7 +92,7 @@ pub fn link_package(
let package_dir = dotfiles_dir.join(package);
let method = config.method_for(package);
let target_base = expand_tilde(config.target_for(package))?;
let files = collect_files(&package_dir, config, package)?;
let files = collect_files_with_store(&package_dir, config, package, fs)?;
if files.is_empty() {
if opts.verbose {
@@ -201,7 +206,7 @@ pub fn link_package(
}
}
Method::Copy => {
let content = std::fs::read(&source)?;
let content = fs.read(&source)?;
fs.write(&target, &content).map(|_| content.len() as u64)
}
};
@@ -261,13 +266,19 @@ pub fn unlink_package(
return Ok(());
}
let mut removed_entries = Vec::new();
for entry in &entries {
let target = PathBuf::from(&entry.target);
let source = PathBuf::from(&entry.source);
if !fs.exists(&target) && fs.symlink_metadata(&target).is_err() {
if opts.verbose {
println!(" \x1b[90mskip\x1b[0m {} (already gone)", target.display());
}
if !opts.dry_run {
removed_entries.push((entry.source.clone(), entry.target.clone()));
}
continue;
}
@@ -275,7 +286,6 @@ pub fn unlink_package(
Method::Symlink => {
// Verify it's a symlink pointing to our source
if let Ok(link_target) = fs.read_link(&target) {
let source = PathBuf::from(&entry.source);
if link_target != source {
if opts.verbose {
println!(
@@ -292,9 +302,36 @@ pub fn unlink_package(
continue;
}
}
Method::Copy => {
// For copies, trust the state file
}
Method::Copy => match (fs.read(&source), fs.read(&target)) {
(Ok(source_bytes), Ok(target_bytes)) if source_bytes == target_bytes => {}
(Ok(_), Ok(_)) => {
if opts.verbose {
println!(
" \x1b[33mskip\x1b[0m {} (copied file diverged from source)",
target.display()
);
}
continue;
}
(Err(_), _) => {
if opts.verbose {
println!(
" \x1b[33mskip\x1b[0m {} (source missing for copied file)",
target.display()
);
}
continue;
}
(_, Err(_)) => {
if opts.verbose {
println!(
" \x1b[33mskip\x1b[0m {} (cannot read copied target)",
target.display()
);
}
continue;
}
},
}
if opts.dry_run {
@@ -311,11 +348,14 @@ pub fn unlink_package(
// Clean up empty parent dirs
cleanup_empty_parents(&target, fs);
removed_entries.push((entry.source.clone(), entry.target.clone()));
}
}
if !opts.dry_run {
state.remove_package(package);
for (source, target) in removed_entries {
state.remove_entry(package, &source, &target);
}
state.save(dotfiles_dir, fs)?;
}
@@ -331,15 +371,10 @@ fn is_already_linked(source: &Path, target: &Path, method: Method, fs: &dyn File
false
}
}
Method::Copy => {
// Skip re-copy if target exists and has the same size and mtime as source.
match (fs.metadata(source), fs.metadata(target)) {
(Ok(sm), Ok(tm)) => {
sm.len() == tm.len() && sm.modified().ok() == tm.modified().ok()
}
_ => false,
}
}
Method::Copy => match (fs.read(source), fs.read(target)) {
(Ok(source_bytes), Ok(target_bytes)) => source_bytes == target_bytes,
_ => false,
},
}
}
@@ -349,18 +384,19 @@ fn backup_path(path: &Path) -> PathBuf {
PathBuf::from(format!("{name}.notfiles-backup-{timestamp}"))
}
fn cleanup_empty_parents(path: &Path, _fs: &dyn FileStore) {
fn cleanup_empty_parents(path: &Path, fs: &dyn FileStore) {
let mut dir = path.parent();
while let Some(parent) = dir {
// Stop at home dir or root
if Some(parent.to_path_buf()) == dirs::home_dir() || parent == Path::new("/") {
break;
}
if std::fs::read_dir(parent)
.map(|mut d| d.next().is_none())
if fs
.read_dir(parent)
.map(|children| children.is_empty())
.unwrap_or(false)
{
let _ = std::fs::remove_dir(parent);
let _ = fs.remove_dir(parent);
dir = parent.parent();
} else {
break;

10
crates/notfiles/src/main.rs
View File

@@ -5,7 +5,7 @@ use std::fs;
use notcore::Config;
use notfiles::cli::{Cli, Command};
use notfiles::linker::{LinkOptions, State};
use notfiles::package::resolve_packages;
use notfiles::package::resolve_packages_with_store;
use notfiles::{adapters, linker, status};
fn main() -> Result<()> {
@@ -26,7 +26,7 @@ fn main() -> Result<()> {
let fs = &adapters::FileStoreImpl;
let config = Config::load(&dotfiles_dir)?;
let mut state = State::load(&dotfiles_dir, fs)?;
let pkgs = resolve_packages(&dotfiles_dir, &packages)?;
let pkgs = resolve_packages_with_store(&dotfiles_dir, &packages, fs)?;
let opts = LinkOptions {
force,
no_backup,
@@ -74,7 +74,7 @@ fn main() -> Result<()> {
.collect::<Vec<_>>()
} else {
// Validate requested packages exist in state
let _ = resolve_packages(&dotfiles_dir, &packages).or_else(|_| {
let _ = resolve_packages_with_store(&dotfiles_dir, &packages, fs).or_else(|_| {
// Package dir might be gone but state entries exist — that's fine for unlink
Ok::<Vec<String>, anyhow::Error>(packages.clone())
});
@@ -111,10 +111,10 @@ fn main() -> Result<()> {
let fs = &adapters::FileStoreImpl;
let config = Config::load(&dotfiles_dir)?;
let state = State::load(&dotfiles_dir, fs)?;
let pkgs = resolve_packages(&dotfiles_dir, &packages)?;
let pkgs = resolve_packages_with_store(&dotfiles_dir, &packages, fs)?;
for pkg in &pkgs {
let entries = status::package_status(&dotfiles_dir, &config, &state, pkg);
let entries = status::package_status(&dotfiles_dir, &config, &state, pkg, fs);
status::print_status(pkg, &entries);
}
}

55
crates/notfiles/src/package.rs
View File

@@ -1,17 +1,29 @@
use std::fs;
use std::path::{Path, PathBuf};
use crate::adapters::FileStoreImpl;
use crate::ignore::IgnoreMatcher;
use crate::ports::FileStore;
use notcore::{Config, NotfilesError};
/// Discover available packages (subdirectories of the dotfiles dir).
pub fn discover_packages(dotfiles_dir: &Path) -> Result<Vec<String>, NotfilesError> {
discover_packages_with_store(dotfiles_dir, &FileStoreImpl)
}
pub fn discover_packages_with_store(
dotfiles_dir: &Path,
fs: &dyn FileStore,
) -> Result<Vec<String>, NotfilesError> {
let mut packages = Vec::new();
for entry in fs::read_dir(dotfiles_dir)? {
let entry = entry?;
let path = entry.path();
if path.is_dir() {
let name = entry.file_name().to_string_lossy().to_string();
// TODO(#19): Replace implicit top-level directory discovery with an explicit
// package contract so mixed-purpose repos do not accidentally link
// operational directories like scripts/ or docs/.
for path in fs.read_dir(dotfiles_dir)? {
if fs.is_dir(&path) {
let name = path
.file_name()
.map(|name| name.to_string_lossy().to_string())
.unwrap_or_default();
// Skip hidden dirs and common non-package dirs
if !name.starts_with('.') {
packages.push(name);
@@ -28,7 +40,15 @@ pub fn resolve_packages(
dotfiles_dir: &Path,
requested: &[String],
) -> Result<Vec<String>, NotfilesError> {
let available = discover_packages(dotfiles_dir)?;
resolve_packages_with_store(dotfiles_dir, requested, &FileStoreImpl)
}
pub fn resolve_packages_with_store(
dotfiles_dir: &Path,
requested: &[String],
fs: &dyn FileStore,
) -> Result<Vec<String>, NotfilesError> {
let available = discover_packages_with_store(dotfiles_dir, fs)?;
if requested.is_empty() {
return Ok(available);
}
@@ -45,11 +65,20 @@ pub fn collect_files(
package_dir: &Path,
config: &Config,
package_name: &str,
) -> Result<Vec<PathBuf>, NotfilesError> {
collect_files_with_store(package_dir, config, package_name, &FileStoreImpl)
}
pub fn collect_files_with_store(
package_dir: &Path,
config: &Config,
package_name: &str,
fs: &dyn FileStore,
) -> Result<Vec<PathBuf>, NotfilesError> {
let patterns = config.ignore_patterns_for(package_name);
let matcher = IgnoreMatcher::new(&patterns)?;
let mut files = Vec::new();
walk_dir(package_dir, package_dir, &matcher, &mut files)?;
walk_dir(package_dir, package_dir, &matcher, &mut files, fs)?;
files.sort();
Ok(files)
}
@@ -59,18 +88,17 @@ fn walk_dir(
current: &Path,
matcher: &IgnoreMatcher,
files: &mut Vec<PathBuf>,
fs: &dyn FileStore,
) -> Result<(), NotfilesError> {
for entry in fs::read_dir(current)? {
let entry = entry?;
let path = entry.path();
for path in fs.read_dir(current)? {
let relative = path.strip_prefix(base).unwrap().to_path_buf();
if matcher.is_ignored(&relative) {
continue;
}
if path.is_dir() {
walk_dir(base, &path, matcher, files)?;
if fs.is_dir(&path) {
walk_dir(base, &path, matcher, files, fs)?;
} else {
files.push(relative);
}
@@ -81,6 +109,7 @@ fn walk_dir(
#[cfg(test)]
mod tests {
use super::*;
use std::fs;
use tempfile::TempDir;
#[test]

9
crates/notfiles/src/ports.rs
View File

@@ -5,6 +5,9 @@ use std::path::{Path, PathBuf};
/// This trait defines the interface for all file system interactions in the notfiles
/// linker, enabling testing with mock implementations and potential future backends.
pub trait FileStore {
/// Read entire file to bytes.
fn read(&self, path: &Path) -> Result<Vec<u8>, std::io::Error>;
/// Read entire file to string.
fn read_to_string(&self, path: &Path) -> Result<String, std::io::Error>;
@@ -20,6 +23,9 @@ pub trait FileStore {
/// Recursively remove a directory and all its contents.
fn remove_dir_all(&self, path: &Path) -> Result<(), std::io::Error>;
/// Remove an empty directory.
fn remove_dir(&self, path: &Path) -> Result<(), std::io::Error>;
/// Read the target of a symbolic link.
fn read_link(&self, path: &Path) -> Result<PathBuf, std::io::Error>;
@@ -32,6 +38,9 @@ pub trait FileStore {
/// Create a directory and all missing parent directories.
fn create_dir_all(&self, path: &Path) -> Result<(), std::io::Error>;
/// List direct children of a directory.
fn read_dir(&self, path: &Path) -> Result<Vec<PathBuf>, std::io::Error>;
/// Create a symbolic link at `link` pointing to `target`.
///
/// On Unix systems, this creates a symlink. On Windows with symlink support enabled,

27
crates/notfiles/src/status.rs
View File

@@ -1,8 +1,8 @@
use std::fs;
use std::path::{Path, PathBuf};
use crate::linker::State;
use crate::package::collect_files;
use crate::package::collect_files_with_store;
use crate::ports::FileStore;
use notcore::{Config, Method, expand_tilde};
#[derive(Debug, PartialEq)]
@@ -37,6 +37,7 @@ pub fn package_status(
config: &Config,
state: &State,
package: &str,
fs: &dyn FileStore,
) -> Vec<StatusEntry> {
let mut results = Vec::new();
let package_dir = dotfiles_dir.join(package);
@@ -47,7 +48,7 @@ pub fn package_status(
};
// Check files that should exist
if let Ok(files) = collect_files(&package_dir, config, package) {
if let Ok(files) = collect_files_with_store(&package_dir, config, package, fs) {
for relative in &files {
let source = package_dir.join(relative);
let target = target_base.join(relative);
@@ -55,25 +56,31 @@ pub fn package_status(
let status = match method {
Method::Symlink => {
if let Ok(link_target) = fs::read_link(&target) {
if let Ok(link_target) = fs.read_link(&target) {
if link_target == source {
FileStatus::Linked
} else {
FileStatus::Conflict
}
} else if target.exists() {
} else if fs.exists(&target) {
FileStatus::Conflict
} else {
FileStatus::Missing
}
}
Method::Copy => {
let has_state = state.entries.iter().any(|e| {
e.package == package && e.target == target.to_string_lossy().as_ref()
let tracked = state.entries.iter().any(|e| {
e.package == package
&& e.target == target.to_string_lossy().as_ref()
&& e.source == source.to_string_lossy().as_ref()
});
if has_state && target.exists() {
let content_matches = match (fs.read(&source), fs.read(&target)) {
(Ok(source_bytes), Ok(target_bytes)) => source_bytes == target_bytes,
_ => false,
};
if tracked && content_matches {
FileStatus::Copied
} else if target.exists() {
} else if fs.exists(&target) {
FileStatus::Conflict
} else {
FileStatus::Missing
@@ -92,7 +99,7 @@ pub fn package_status(
// Check for orphans: entries in state that no longer have a source file
for entry in state.entries_for_package(package) {
let source = PathBuf::from(&entry.source);
if !source.exists() {
if !fs.exists(&source) {
let target = PathBuf::from(&entry.target);
// Only add if we didn't already report this target
if !results.iter().any(|r| r.target == target) {

83
crates/notfiles/tests/integration.rs
View File

@@ -303,6 +303,89 @@ method = "copy"
assert!(!ssh_config.exists());
}
#[test]
fn test_status_flags_diverged_copy_as_conflict() {
let tmp = TempDir::new().unwrap();
setup_dotfiles(&tmp);
let dotfiles = tmp.path().join("dotfiles");
let target = tmp.path().join("home");
let ssh = dotfiles.join("ssh");
fs::create_dir_all(ssh.join(".ssh")).unwrap();
fs::write(ssh.join(".ssh/config"), "Host *\n AddKeysToAgent yes").unwrap();
let config = format!(
r#"[defaults]
target = "{}"
ignore = [".git", ".DS_Store", "README.md", "LICENSE", "notfiles.toml", ".notfiles-state.toml"]
[packages.ssh]
method = "copy"
"#,
target.display()
);
fs::write(dotfiles.join("notfiles.toml"), config).unwrap();
let (_, _, ok) = run(&dotfiles, &["link", "ssh"]);
assert!(ok);
let ssh_config = target.join(".ssh/config");
fs::write(&ssh_config, "Host github.com\n User joe\n").unwrap();
let (stdout, _, ok) = run(&dotfiles, &["status", "ssh"]);
assert!(ok);
assert!(
stdout.contains("conflict"),
"diverged copy should be reported as conflict, stdout={stdout}"
);
}
#[test]
fn test_unlink_preserves_diverged_copy_and_state() {
let tmp = TempDir::new().unwrap();
setup_dotfiles(&tmp);
let dotfiles = tmp.path().join("dotfiles");
let target = tmp.path().join("home");
let ssh = dotfiles.join("ssh");
fs::create_dir_all(ssh.join(".ssh")).unwrap();
fs::write(ssh.join(".ssh/config"), "Host *\n AddKeysToAgent yes").unwrap();
let config = format!(
r#"[defaults]
target = "{}"
ignore = [".git", ".DS_Store", "README.md", "LICENSE", "notfiles.toml", ".notfiles-state.toml"]
[packages.ssh]
method = "copy"
"#,
target.display()
);
fs::write(dotfiles.join("notfiles.toml"), config).unwrap();
let (_, _, ok) = run(&dotfiles, &["link", "ssh"]);
assert!(ok);
let ssh_config = target.join(".ssh/config");
fs::write(&ssh_config, "Host github.com\n User joe\n").unwrap();
let (stdout, stderr, ok) = run(&dotfiles, &["unlink", "ssh", "--verbose"]);
assert!(
ok,
"unlink should not fail: stdout={stdout} stderr={stderr}"
);
assert!(
ssh_config.exists(),
"diverged copy should not be removed during unlink"
);
let state = fs::read_to_string(dotfiles.join(".notfiles-state.toml")).unwrap();
assert!(
state.contains(".ssh/config"),
"state entry must be retained when unlink skips a diverged copy: {state}"
);
}
#[test]
fn test_dry_run() {
let tmp = TempDir::new().unwrap();

6
crates/notgraph/tests/integration.rs
View File

@@ -60,7 +60,7 @@ fn cyclic_fixture_module_graph_builds() {
/// write_all produces report.html, report.md, and report.json in the output dir.
#[test]
fn write_all_creates_expected_files() {
use types::{CrateGraph, GraphStats, ModuleGraph};
use types::{CrateGraph, ModuleGraph};
let out = TempDir::new().unwrap();
let crate_graph = CrateGraph {
@@ -162,7 +162,7 @@ fn write_all_heatmap_does_not_panic_for_single_crate() {
/// ModuleGraph entry in the HTML output.
#[test]
fn write_all_renders_per_crate_module_graphs() {
use types::{CrateGraph, ModuleGraph};
use types::CrateGraph;
let out = TempDir::new().unwrap();
let mg = module_graph::build("clean".to_string(), &fixture_src("clean")).unwrap();
@@ -170,7 +170,7 @@ fn write_all_renders_per_crate_module_graphs() {
nodes: vec!["clean".to_string()],
edges: vec![],
};
let stats = analysis::analyse(&crate_graph, &[mg.clone()], 3);
let stats = analysis::analyse(&crate_graph, std::slice::from_ref(&mg), 3);
let symbol_tables: Vec<types::SymbolTable> = vec![];
emit::write_all(out.path(), &crate_graph, &[mg], &stats, &symbol_tables).unwrap();

6
crates/nothooks/src/lib.rs
View File

@@ -14,6 +14,10 @@ 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 {
pub fn run_phase(
hooks: &[HookSpec],
phase: &HookPhase,
runner: &HookRunner,
) -> anyhow::Result<Report> {
runner.run_phase(hooks, phase)
}

2
crates/nothooks/src/main.rs
View File

@@ -62,7 +62,7 @@ fn main() -> Result<()> {
HookRunner::new(state_dir)
};
let report = run_phase(&file.hooks, &phase, &runner);
let report = run_phase(&file.hooks, &phase, &runner)?;
report.print();
if report.has_failures() {

37
crates/nothooks/src/runner.rs
View File

@@ -1,7 +1,8 @@
use crate::HookResult;
use crate::state::HookState;
use anyhow::{Context, Result};
use notcore::{HookPhase, HookSpec, Report, StepStatus};
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process::Command;
fn resolve_interpreter(spec: &HookSpec) -> Result<String, String> {
@@ -51,8 +52,9 @@ impl HookRunner {
/// 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();
pub fn run_phase(&self, hooks: &[HookSpec], phase: &HookPhase) -> Result<Report> {
let mut state = HookState::load(&self.state_dir)
.with_context(|| format!("loading hook state from {}", self.state_dir.display()))?;
let mut state_dirty = false;
let mut report = Report::default();
@@ -67,10 +69,12 @@ impl HookRunner {
}
if state_dirty {
let _ = state.save(&self.state_dir);
state
.save(&self.state_dir)
.with_context(|| format!("saving hook state into {}", self.state_dir.display()))?;
}
report
Ok(report)
}
/// Execute a single hook, using the caller-owned `state` and `dirty` flag
@@ -89,11 +93,12 @@ impl HookRunner {
Ok(i) => i,
Err(msg) => return HookResult::Failed(msg),
};
let script = self.resolve_script_path(&spec.script);
// SAFETY: `interp` is derived from the file extension or an explicit `interpreter` field
// in HookSpec (both come from config, not untrusted shell input). `spec.script` is a
// filesystem path from config. Both are passed as discrete args with no shell involved,
// so argument injection is not possible.
let result = Command::new(&interp).arg(&spec.script).status();
let result = Command::new(&interp).arg(&script).status();
match result {
Ok(status) if status.success() => {
@@ -112,13 +117,25 @@ impl HookRunner {
///
/// 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();
pub fn run_hook(&self, spec: &HookSpec) -> Result<HookResult> {
let mut state = HookState::load(&self.state_dir)
.with_context(|| format!("loading hook state from {}", self.state_dir.display()))?;
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);
state
.save(&self.state_dir)
.with_context(|| format!("saving hook state into {}", self.state_dir.display()))?;
}
Ok(result)
}
fn resolve_script_path(&self, script: &str) -> PathBuf {
let script = Path::new(script);
if script.is_absolute() {
script.to_path_buf()
} else {
self.state_dir.join(script)
}
result
}
}

4
crates/nothooks/src/state.rs
View File

@@ -22,7 +22,9 @@ impl HookState {
pub fn save(&self, dir: &Path) -> Result<()> {
let path = dir.join(STATE_FILE);
std::fs::write(path, toml::to_string(self)?)?;
let tmp = dir.join(format!("{STATE_FILE}.tmp"));
std::fs::write(&tmp, toml::to_string(self)?)?;
std::fs::rename(&tmp, &path)?;
Ok(())
}

32
crates/nothooks/tests/integration.rs
View File

@@ -19,7 +19,7 @@ fn test_hook_success() {
let dir = TempDir::new().unwrap();
let spec = make_hook_script(&dir, "ok-hook", "echo hello");
let runner = HookRunner::new(dir.path().to_path_buf());
let result = runner.run_hook(&spec);
let result = runner.run_hook(&spec).unwrap();
assert!(matches!(result, HookResult::Ok));
}
@@ -28,7 +28,7 @@ fn test_hook_failure() {
let dir = TempDir::new().unwrap();
let spec = make_hook_script(&dir, "fail-hook", "exit 1");
let runner = HookRunner::new(dir.path().to_path_buf());
let result = runner.run_hook(&spec);
let result = runner.run_hook(&spec).unwrap();
assert!(matches!(result, HookResult::Failed(_)));
}
@@ -39,11 +39,11 @@ fn test_setup_hook_skipped_on_rerun() {
spec.phase = notcore::HookPhase::Setup;
let runner = HookRunner::new(dir.path().to_path_buf());
let r1 = runner.run_hook(&spec);
let r1 = runner.run_hook(&spec).unwrap();
assert!(matches!(r1, HookResult::Ok));
// Second run — should be skipped
let r2 = runner.run_hook(&spec);
let r2 = runner.run_hook(&spec).unwrap();
assert!(matches!(r2, HookResult::Skipped));
}
@@ -54,9 +54,29 @@ fn test_setup_hook_force_reruns() {
spec.phase = notcore::HookPhase::Setup;
let runner = HookRunner::new(dir.path().to_path_buf());
runner.run_hook(&spec);
runner.run_hook(&spec).unwrap();
let runner2 = HookRunner::with_force(dir.path().to_path_buf());
let r2 = runner2.run_hook(&spec);
let r2 = runner2.run_hook(&spec).unwrap();
assert!(matches!(r2, HookResult::Ok));
}
#[test]
fn test_relative_script_path_resolves_from_state_dir() {
let dir = TempDir::new().unwrap();
let scripts_dir = dir.path().join("scripts");
fs::create_dir_all(&scripts_dir).unwrap();
let script = scripts_dir.join("relative.sh");
fs::write(&script, "echo relative-ok").unwrap();
let spec = HookSpec {
name: "relative".to_string(),
script: "scripts/relative.sh".to_string(),
phase: HookPhase::Dot,
interpreter: Some("sh".to_string()),
};
let runner = HookRunner::new(dir.path().to_path_buf());
let result = runner.run_hook(&spec).unwrap();
assert!(matches!(result, HookResult::Ok));
}

12
crates/notstrap/src/lib.rs
View File

@@ -157,6 +157,7 @@ pub fn run(opts: BootstrapOptions) -> Result<Report> {
}
Err(e) => {
report.add("link dotfiles", StepStatus::Failed(e.to_string()));
return Ok(report);
}
}
@@ -171,7 +172,13 @@ pub fn run(opts: BootstrapOptions) -> Result<Report> {
(HookPhase::Dot, "dot hooks"),
(HookPhase::Setup, "setup hooks"),
] {
let phase_report = run_phase(&cfg.hooks, &phase, &runner);
let phase_report = match run_phase(&cfg.hooks, &phase, &runner) {
Ok(report) => report,
Err(e) => {
report.add(label, StepStatus::Failed(e.to_string()));
return Ok(report);
}
};
let failed = phase_report
.steps
.iter()
@@ -183,6 +190,9 @@ pub fn run(opts: BootstrapOptions) -> Result<Report> {
StepStatus::Ok
};
report.add(label, summary);
if failed > 0 {
return Ok(report);
}
}
Ok(report)