From 8d8495c89cca93512bd14895d21f53d601d5d191 Mon Sep 17 00:00:00 2001 From: Ray Andrew Date: Wed, 10 Jun 2026 22:49:00 -0700 Subject: [PATCH] feat(apply): warn when explicit dotfile targets collide with glob directories --- crates/doot-cli/src/commands/apply.rs | 93 +++++++++++++++++++++++ crates/doot-lang/src/planner/scheduler.rs | 17 ++++- 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/crates/doot-cli/src/commands/apply.rs b/crates/doot-cli/src/commands/apply.rs index e7e4e44..e84405f 100644 --- a/crates/doot-cli/src/commands/apply.rs +++ b/crates/doot-cli/src/commands/apply.rs @@ -32,6 +32,12 @@ pub fn run(config_path: Option, dry_run: bool, prune: bool) -> anyhow:: // Get environment variables to expose to hook scripts let hook_env = evaluator.get_hook_env(); + // Warn about likely-mistaken directory targets before expanding globs, while + // result.dotfiles still holds only the explicit (non-glob) blocks. + for warning in directory_target_collisions(&result.dotfiles, &result.dotfile_patterns) { + eprintln!("warning: {warning}"); + } + // Expand glob patterns from dotfiles: blocks let glob_count = expand_dotfile_patterns(&mut result.dotfiles, &result.dotfile_patterns, &source_dir); @@ -943,6 +949,40 @@ pub(crate) fn template_outdated( Ok(false) } +/// Returns warnings for explicit dotfile blocks whose target is exactly a glob +/// pattern's directory target. For a directory target, doot appends the source +/// filename at deploy time, so the explicit entry lands on the *same path* the +/// glob already produces — they collide instead of the explicit block overriding +/// the glob. Targeting the specific file (`… / ""`) is what makes the +/// override fire. (`validate_dotfile_targets` can't catch this: the raw target +/// fields differ — `…/bin` vs `…/bin/wb` — only the deployed paths coincide.) +fn directory_target_collisions( + dotfiles: &[DotfileConfig], + patterns: &[DotfilesPattern], +) -> Vec { + let mut warnings = Vec::new(); + for df in dotfiles { + for pat in patterns { + if df.target == pat.target_base { + let file = df + .source + .file_name() + .map(|f| f.to_string_lossy().into_owned()) + .unwrap_or_else(|| "".to_string()); + warnings.push(format!( + "dotfile '{}' targets the directory '{}', which is also a glob target. \ + It will collide with the glob instead of overriding it. \ + Did you mean: target = ... / \"{}\"", + df.source.display(), + df.target.display(), + file, + )); + } + } + } + warnings +} + /// Expands dotfile patterns (from `dotfiles:` blocks) into individual DotfileConfig entries. /// Returns the number of entries added. fn expand_dotfile_patterns( @@ -1059,3 +1099,56 @@ fn merge_specializations(dotfiles: &mut Vec, glob_count: usize) { dotfiles.remove(idx); } } + +#[cfg(test)] +mod tests { + use super::*; + use doot_lang::evaluator::{DeployMode, DotfilesSource}; + + fn explicit(source: &str, target: &str) -> DotfileConfig { + DotfileConfig { + source: PathBuf::from(source), + target: PathBuf::from(target), + template: false, + permissions: vec![], + owner: None, + deploy: DeployMode::default(), + link_patterns: vec![], + copy_patterns: vec![], + exclude_paths: vec![], + exclude_sources: vec![], + } + } + + fn glob_pattern(pattern: &str, target_base: &str) -> DotfilesPattern { + DotfilesPattern { + source: DotfilesSource::Pattern(pattern.to_string()), + target_base: PathBuf::from(target_base), + template: false, + permissions: vec![], + owner: None, + deploy: DeployMode::default(), + link_patterns: vec![], + copy_patterns: vec![], + } + } + + #[test] + fn warns_when_explicit_target_is_glob_directory() { + // explicit `bin/wb` -> ~/.local/bin (the directory) collides with `bin/*`. + let dotfiles = vec![explicit("bin/wb", "/home/u/.local/bin")]; + let patterns = vec![glob_pattern("bin/*", "/home/u/.local/bin")]; + let warnings = directory_target_collisions(&dotfiles, &patterns); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("wb")); + assert!(warnings[0].contains("collide")); + } + + #[test] + fn no_warning_for_specific_file_target() { + // explicit `bin/wb` -> ~/.local/bin/wb (a file) overrides correctly. + let dotfiles = vec![explicit("bin/wb", "/home/u/.local/bin/wb")]; + let patterns = vec![glob_pattern("bin/*", "/home/u/.local/bin")]; + assert!(directory_target_collisions(&dotfiles, &patterns).is_empty()); + } +} diff --git a/crates/doot-lang/src/planner/scheduler.rs b/crates/doot-lang/src/planner/scheduler.rs index 3e1cab5..b9af741 100644 --- a/crates/doot-lang/src/planner/scheduler.rs +++ b/crates/doot-lang/src/planner/scheduler.rs @@ -181,8 +181,20 @@ pub fn validate_dotfile_targets( index_b: j, }); } else { - // Different source + same target = override, j depends on i + // Different source + same target = override, j depends on i. + // Layering is supported, but two sources fighting over one file + // is usually a mistake, so surface it as a warning (last wins). graph.add_edge(&format!("dotfile_{}", i), &format!("dotfile_{}", j)); + warnings.push(DotfileWarning { + message: format!( + "'{}' and '{}' both deploy to '{}'; the later entry wins", + a.source.display(), + b.source.display(), + target_a.display() + ), + index_a: i, + index_b: j, + }); } continue; } @@ -334,6 +346,9 @@ mod tests { assert!(result.errors.is_empty()); // Second entry should come after first assert_eq!(result.ordered_indices, vec![0, 1]); + // Two different sources hitting one target is allowed (last wins) but warns. + assert_eq!(result.warnings.len(), 1); + assert!(result.warnings[0].message.contains("both deploy to")); } #[test]