feat(apply): warn when explicit dotfile targets collide with glob directories
This commit is contained in:
parent
0eb4d38392
commit
8d8495c89c
2 changed files with 109 additions and 1 deletions
|
|
@ -32,6 +32,12 @@ pub fn run(config_path: Option<PathBuf>, dry_run: bool, prune: bool) -> anyhow::
|
||||||
// Get environment variables to expose to hook scripts
|
// Get environment variables to expose to hook scripts
|
||||||
let hook_env = evaluator.get_hook_env();
|
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
|
// Expand glob patterns from dotfiles: blocks
|
||||||
let glob_count =
|
let glob_count =
|
||||||
expand_dotfile_patterns(&mut result.dotfiles, &result.dotfile_patterns, &source_dir);
|
expand_dotfile_patterns(&mut result.dotfiles, &result.dotfile_patterns, &source_dir);
|
||||||
|
|
@ -943,6 +949,40 @@ pub(crate) fn template_outdated(
|
||||||
Ok(false)
|
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 (`… / "<name>"`) 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<String> {
|
||||||
|
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(|| "<name>".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.
|
/// Expands dotfile patterns (from `dotfiles:` blocks) into individual DotfileConfig entries.
|
||||||
/// Returns the number of entries added.
|
/// Returns the number of entries added.
|
||||||
fn expand_dotfile_patterns(
|
fn expand_dotfile_patterns(
|
||||||
|
|
@ -1059,3 +1099,56 @@ fn merge_specializations(dotfiles: &mut Vec<DotfileConfig>, glob_count: usize) {
|
||||||
dotfiles.remove(idx);
|
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());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -181,8 +181,20 @@ pub fn validate_dotfile_targets(
|
||||||
index_b: j,
|
index_b: j,
|
||||||
});
|
});
|
||||||
} else {
|
} 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));
|
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;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
@ -334,6 +346,9 @@ mod tests {
|
||||||
assert!(result.errors.is_empty());
|
assert!(result.errors.is_empty());
|
||||||
// Second entry should come after first
|
// Second entry should come after first
|
||||||
assert_eq!(result.ordered_indices, vec![0, 1]);
|
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]
|
#[test]
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue