diff --git a/src/config.rs b/src/config.rs index ef32f3c..4d89487 100644 --- a/src/config.rs +++ b/src/config.rs @@ -72,11 +72,31 @@ impl Default for Config { /// Folder hooks configurations #[derive(Debug, serde::Deserialize)] +#[serde(from = "RawFolderConfig")] pub(crate) struct FolderConfig { /// Hooks array pub hooks: Vec, } +/// Folder hooks configurations as deserialized, before hook indexes are assigned +#[derive(serde::Deserialize)] +struct RawFolderConfig { + /// Hooks array + hooks: Vec, +} + +impl From for FolderConfig { + fn from(raw: RawFolderConfig) -> Self { + let mut hooks = raw.hooks; + // Assign hook indexes during deserialization so that every parsing path gets + // unique ones, they identify each hook at runtime + for (index, hook) in hooks.iter_mut().enumerate() { + hook.index = index; + } + Self { hooks } + } +} + /// Path string with ~ replaced, and canonicalized #[derive(Clone, Debug, Hash, PartialEq, Eq)] pub(crate) struct NormalizedPath(PathBuf); @@ -126,6 +146,9 @@ pub(crate) struct FolderHook { pub command: Vec, /// Allow concurrent runs for the same hook pub allow_concurrent: Option, + /// Unique hook index, assigned when parsing, used to identify the hook at runtime + #[serde(skip)] + pub index: usize, } /// Deserialize filter into a glob matcher to validate glob expression diff --git a/src/hook.rs b/src/hook.rs index adc32fa..e25b295 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -4,7 +4,6 @@ use std::{ collections::HashSet, path::{Path, PathBuf}, process::{Child, Command, Stdio}, - ptr, sync::{Arc, Mutex, mpsc}, time::Duration, }; @@ -12,14 +11,14 @@ use std::{ use crate::config; /// Unique identifier for a folder hook +#[cfg_attr(test, derive(Debug))] #[derive(Clone, Eq, Hash, PartialEq)] pub(crate) struct FolderHookId(usize); impl FolderHookId { - /// Create unique identifier for hook + /// Create unique identifier for hook, from its parse time index pub(crate) fn from_hook(hook: &config::FolderHook) -> Self { - let val = ptr::from_ref(hook) as usize; - Self(val) + Self(hook.index) } } @@ -111,6 +110,7 @@ mod tests { filter: None, command: command.iter().map(|a| (*a).to_owned()).collect(), allow_concurrent, + index: 0, } } @@ -221,4 +221,24 @@ mod tests { thread::sleep(Duration::from_millis(10)); } } + + /// Hook identity must be identical for clones of the same hook (the hook map + /// stores clones), and unique across different hooks + #[test] + fn hook_id_is_stable_and_unique() { + let hook0 = hook(&["true"], None); + let hook1 = config::FolderHook { + index: 1, + ..hook0.clone() + }; + + assert_eq!( + FolderHookId::from_hook(&hook0), + FolderHookId::from_hook(&hook0.clone()) + ); + assert_ne!( + FolderHookId::from_hook(&hook0), + FolderHookId::from_hook(&hook1) + ); + } }