diff --git a/cli/src/main.rs b/cli/src/main.rs index 5ee0eb0..39b7858 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -12,6 +12,7 @@ fn main() { search_root, ignore, dry_run, + batch_confirm, } = Args::parse(); let path = if path.as_os_str() == "." { @@ -49,7 +50,7 @@ fn main() { } let mut io = TerminalIO; - match run(&cases, &mut io, dry_run) { + match run(&cases, &mut io, dry_run, batch_confirm) { Ok(summary) => { if summary.total() > 0 { println!("{summary}"); @@ -82,4 +83,8 @@ struct Args { /// Preview changes without modifying the filesystem #[arg(short, long)] dry_run: bool, + + /// Collect all decisions, show summary, then confirm before applying + #[arg(short, long)] + batch_confirm: bool, } diff --git a/core/src/fuzzy.rs b/core/src/fuzzy.rs index ff60858..5578a14 100644 --- a/core/src/fuzzy.rs +++ b/core/src/fuzzy.rs @@ -46,7 +46,7 @@ fn dir_components(path: &Path) -> Vec { .collect() } -fn score_candidate(broken: &BrokenSymlink, candidate: &Path, search_root: &Path) -> f64 { +fn score_candidate(broken: &BrokenSymlink, candidate: &Path, search_root: &Path) -> (f64, usize) { let target_name = broken .target .file_name() @@ -77,7 +77,8 @@ fn score_candidate(broken: &BrokenSymlink, candidate: &Path, search_root: &Path) .unwrap_or(0); let depth_penalty = depth as f64 * 0.1; - filename_score * 10.0 + path_score * 3.0 + depth_penalty + let score = filename_score * 10.0 + path_score * 3.0 + depth_penalty; + (score, shared) } pub fn find_candidates( @@ -123,11 +124,25 @@ pub fn find_candidates( }) .map(|e| { let path = e.into_path(); - let score = score_candidate(broken, &path, search_root); - ScoredCandidate { path, score } + let (score, shared_dirs) = score_candidate(broken, &path, search_root); + ScoredCandidate { + path, + score, + shared_dirs, + basename_count: 0, + } }) .collect(); + let target_name = broken.target.file_name(); + let basename_count = candidates + .iter() + .filter(|c| c.path.file_name() == target_name) + .count(); + for c in &mut candidates { + c.basename_count = basename_count; + } + candidates.sort_by(|a, b| { a.score .partial_cmp(&b.score) @@ -140,6 +155,8 @@ pub fn find_candidates( pub struct ScoredCandidate { pub path: PathBuf, pub score: f64, + pub shared_dirs: usize, + pub basename_count: usize, } pub const DEFAULT_IGNORE: &[&str] = &[ diff --git a/core/src/resolver/action.rs b/core/src/resolver/action.rs index 05596bd..5874b9f 100644 --- a/core/src/resolver/action.rs +++ b/core/src/resolver/action.rs @@ -40,10 +40,14 @@ mod tests { ScoredCandidate { path: "/first/candidate.txt".into(), score: 1.0, + shared_dirs: 0, + basename_count: 2, }, ScoredCandidate { path: "/second/candidate.txt".into(), score: 2.0, + shared_dirs: 0, + basename_count: 2, }, ], ) diff --git a/core/src/resolver/display.rs b/core/src/resolver/display.rs index 77e7f90..7a5503e 100644 --- a/core/src/resolver/display.rs +++ b/core/src/resolver/display.rs @@ -1,6 +1,9 @@ use std::fmt; -use super::model::RepairCase; +use super::{ + model::RepairCase, + safety::{format_warnings, relink_warnings}, +}; pub fn present(w: &mut impl fmt::Write, case: &RepairCase) -> fmt::Result { format_header(w, case)?; @@ -18,18 +21,40 @@ pub fn format_header(w: &mut impl fmt::Write, case: &RepairCase) -> fmt::Result } pub fn format_candidates(w: &mut impl fmt::Write, case: &RepairCase) -> fmt::Result { - let RepairCase { ref candidates, .. } = *case; + let RepairCase { + ref candidates, + ref original_target, + .. + } = *case; if candidates.is_empty() { writeln!(w, " no candidates found") } else { + let target_basename = original_target + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or(""); for (i, candidate) in candidates.iter().enumerate() { + let basename_note = if candidate.basename_count <= 1 { + "only match".to_string() + } else { + format!( + "{} files named {target_basename} found", + candidate.basename_count + ) + }; writeln!( w, - " [{}] {} (score: {:.2})", + " [{}] {} (score: {:.2}, {} shared dirs, {})", i + 1, candidate.path.display(), - candidate.score + candidate.score, + candidate.shared_dirs, + basename_note )?; + let warnings = relink_warnings(&case.link, original_target, &candidate.path); + if !warnings.is_empty() { + write!(w, "{}", format_warnings(&warnings))?; + } } Ok(()) } @@ -62,10 +87,14 @@ mod tests { ScoredCandidate { path: "/home/user/target.txt".into(), score: 3.20, + shared_dirs: 0, + basename_count: 2, }, ScoredCandidate { path: "/archive/target.txt".into(), score: 4.50, + shared_dirs: 0, + basename_count: 2, }, ], ) @@ -82,6 +111,8 @@ mod tests { vec![ScoredCandidate { path: "/home/user/target.txt".into(), score: 3.20, + shared_dirs: 0, + basename_count: 1, }], ) } @@ -97,8 +128,10 @@ mod tests { fn candidates_listed_with_scores() { let mut out = String::new(); format_candidates(&mut out, &case_with_candidates()).unwrap(); - assert!(out.contains("[1] /home/user/target.txt (score: 3.20)")); - assert!(out.contains("[2] /archive/target.txt (score: 4.50)")); + assert!(out.contains("[1] /home/user/target.txt (score: 3.20")); + assert!(out.contains("[2] /archive/target.txt (score: 4.50")); + assert!(out.contains("shared dirs")); + assert!(out.contains("files named target.txt found")); } #[test] diff --git a/core/src/resolver/fs_ops.rs b/core/src/resolver/fs_ops.rs index 84eb4d1..dcbc3af 100644 --- a/core/src/resolver/fs_ops.rs +++ b/core/src/resolver/fs_ops.rs @@ -3,7 +3,7 @@ use std::{ path::{Path, PathBuf}, }; -use super::model::Action; +use super::{model::Action, safety::would_create_loop}; pub fn execute(link: &Path, action: &Action, dry_run: bool) -> Result<(), FsError> { match action { @@ -18,6 +18,12 @@ pub fn execute(link: &Path, action: &Action, dry_run: bool) -> Result<(), FsErro }) } Action::Relink(target) => { + if would_create_loop(link, target) { + return Err(FsError::WouldCreateLoop { + link: link.to_path_buf(), + target: target.clone(), + }); + } if dry_run { return Ok(()); } @@ -52,6 +58,14 @@ impl fmt::Display for FsError { target.display() ) } + Self::WouldCreateLoop { link, target } => { + write!( + f, + "refused: relinking {} -> {} would create a symlink loop", + link.display(), + target.display() + ) + } } } } @@ -67,6 +81,10 @@ pub enum FsError { target: PathBuf, source: std::io::Error, }, + WouldCreateLoop { + link: PathBuf, + target: PathBuf, + }, } #[cfg(test)] @@ -157,6 +175,20 @@ mod tests { assert!(msg.contains("not found")); } + #[test] + fn would_create_loop_refused() { + let temp = TempDir::new().unwrap(); + let a = temp.path().join("a"); + let b = temp.path().join("b"); + symlink(&b, &a).unwrap(); + symlink(&a, &b).unwrap(); + + let result = execute(&a, &Action::Relink(b), false); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.to_string().contains("loop")); + } + #[test] fn error_display_symlink_includes_both_paths() { let err = FsError::SymlinkFailed { diff --git a/core/src/resolver/mod.rs b/core/src/resolver/mod.rs index d33057a..6b0530d 100644 --- a/core/src/resolver/mod.rs +++ b/core/src/resolver/mod.rs @@ -5,6 +5,7 @@ pub(crate) mod fs_ops; pub(crate) mod input; pub mod io; pub mod model; +pub(crate) mod safety; pub(crate) mod session; pub use display::present; diff --git a/core/src/resolver/model.rs b/core/src/resolver/model.rs index b4a6bf8..d4a4be4 100644 --- a/core/src/resolver/model.rs +++ b/core/src/resolver/model.rs @@ -53,7 +53,7 @@ pub enum Action { Skip, } -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct Summary { pub relinked: usize, pub removed: usize, @@ -115,6 +115,8 @@ mod tests { vec![ScoredCandidate { path: "/candidate".into(), score: 1.0, + shared_dirs: 0, + basename_count: 1, }], ); assert!(case.has_candidates()); diff --git a/core/src/resolver/safety.rs b/core/src/resolver/safety.rs new file mode 100644 index 0000000..6e95854 --- /dev/null +++ b/core/src/resolver/safety.rs @@ -0,0 +1,141 @@ +use std::{fs, path::Path}; + +#[cfg(unix)] +use std::os::unix::fs::MetadataExt; + +pub fn would_create_loop(link: &Path, new_target: &Path) -> bool { + let link = link.to_path_buf(); + let mut current = new_target.to_path_buf(); + let mut visited = std::collections::HashSet::new(); + for _ in 0..64 { + if current == link { + return true; + } + if visited.contains(¤t) { + return true; + } + visited.insert(current.clone()); + let meta = match fs::symlink_metadata(¤t) { + Ok(m) => m, + Err(_) => return false, + }; + if !meta.file_type().is_symlink() { + return false; + } + current = match fs::read_link(¤t) { + Ok(t) => { + if t.is_absolute() { + t + } else { + current.parent().unwrap_or(Path::new(".")).join(t) + } + } + Err(_) => return false, + }; + } + false +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RelinkWarning { + CrossFilesystem { link_dev: u64, target_dev: u64 }, + RelativeToAbsolute, + AbsoluteToRelative, +} + +pub fn relink_warnings( + link: &Path, + original_target: &Path, + new_target: &Path, +) -> Vec { + let mut warnings = Vec::new(); + + if original_target.is_relative() && new_target.is_absolute() { + warnings.push(RelinkWarning::RelativeToAbsolute); + } + if original_target.is_absolute() && new_target.is_relative() { + warnings.push(RelinkWarning::AbsoluteToRelative); + } + + #[cfg(unix)] + { + let link_dev = fs::metadata(link).ok().map(|m| m.dev()).unwrap_or(0); + let target_canon = new_target + .canonicalize() + .unwrap_or_else(|_| new_target.to_path_buf()); + let target_dev = fs::metadata(&target_canon) + .ok() + .map(|m| m.dev()) + .unwrap_or(0); + if link_dev != 0 && target_dev != 0 && link_dev != target_dev { + warnings.push(RelinkWarning::CrossFilesystem { + link_dev, + target_dev, + }); + } + } + + warnings +} + +pub fn format_warnings(warnings: &[RelinkWarning]) -> String { + let mut out = String::new(); + for w in warnings { + match w { + RelinkWarning::CrossFilesystem { .. } => { + out.push_str(" ⚠ cross-filesystem (link and target on different mounts)\n"); + } + RelinkWarning::RelativeToAbsolute => { + out.push_str(" ⚠ was relative, would become absolute\n"); + } + RelinkWarning::AbsoluteToRelative => { + out.push_str(" ⚠ was absolute, would become relative\n"); + } + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::fs::symlink; + use tempfile::TempDir; + + #[test] + fn loop_a_to_b_to_a() { + let temp = TempDir::new().unwrap(); + let a = temp.path().join("a"); + let b = temp.path().join("b"); + symlink(&b, &a).unwrap(); + symlink(&a, &b).unwrap(); + + assert!(would_create_loop(&a, &b)); + assert!(would_create_loop(&b, &a)); + } + + #[test] + fn no_loop_simple_relink() { + let temp = TempDir::new().unwrap(); + let link = temp.path().join("link"); + let target = temp.path().join("target"); + std::fs::write(&target, b"x").unwrap(); + symlink("/nonexistent", &link).unwrap(); + + assert!(!would_create_loop(&link, &target)); + } + + #[test] + fn relative_to_absolute_warning() { + let temp = TempDir::new().unwrap(); + let link = temp.path().join("link"); + let target = temp.path().join("target"); + std::fs::write(&target, b"x").unwrap(); + + let w = relink_warnings(&link, Path::new("../foo"), &target); + assert!( + w.iter() + .any(|x| matches!(x, RelinkWarning::RelativeToAbsolute)) + ); + } +} diff --git a/core/src/resolver/session.rs b/core/src/resolver/session.rs index 9af723b..b2b7661 100644 --- a/core/src/resolver/session.rs +++ b/core/src/resolver/session.rs @@ -10,9 +10,27 @@ use super::{ model::{Action, RepairCase, Summary}, }; -pub fn run(cases: &[RepairCase], io: &mut impl ResolverIO, dry_run: bool) -> io::Result { +pub fn run( + cases: &[RepairCase], + io: &mut impl ResolverIO, + dry_run: bool, + batch_confirm: bool, +) -> io::Result { let mut summary = Summary::default(); + if batch_confirm { + run_batch(cases, io, dry_run) + } else { + run_immediate(cases, io, dry_run, &mut summary) + } +} + +fn run_immediate( + cases: &[RepairCase], + io: &mut impl ResolverIO, + dry_run: bool, + summary: &mut Summary, +) -> io::Result { for case in cases { let mut buf = String::new(); present(&mut buf, case).unwrap(); @@ -34,6 +52,60 @@ pub fn run(cases: &[RepairCase], io: &mut impl ResolverIO, dry_run: bool) -> io: io.write_str("\n")?; } + Ok(std::mem::take(summary)) +} + +fn run_batch(cases: &[RepairCase], io: &mut impl ResolverIO, dry_run: bool) -> io::Result { + let mut planned: Vec<(&RepairCase, Action)> = Vec::new(); + + for case in cases { + let mut buf = String::new(); + present(&mut buf, case).unwrap(); + io.write_str(&buf)?; + + let action = prompt_until_resolved(case, io)?; + planned.push((case, action)); + io.write_str("\n")?; + } + + let mut would_relink = 0; + let mut would_skip = 0; + let mut would_remove = 0; + for (_, a) in &planned { + match a { + Action::Relink(_) => would_relink += 1, + Action::Skip => would_skip += 1, + Action::Remove => would_remove += 1, + } + } + + io.write_str(&format!( + "Will {} {}, skip {}, remove {}. Proceed? [y/N] ", + if dry_run { "simulate" } else { "relink" }, + would_relink, + would_skip, + would_remove + ))?; + let confirm = io.read_line()?; + if !matches!(confirm.trim().to_ascii_lowercase().as_str(), "y" | "yes") { + io.write_str("Aborted. No changes made.\n")?; + return Ok(Summary::default()); + } + + let mut summary = Summary::default(); + for (case, action) in planned { + match execute(&case.link, &action, dry_run) { + Ok(()) => { + format_outcome(io, &action, dry_run)?; + summary.record(&action); + } + Err(e) => { + io.write_str(&format!(" error: {e}\n"))?; + summary.record(&Action::Skip); + } + } + } + Ok(summary) } @@ -111,6 +183,8 @@ mod tests { vec![ScoredCandidate { path: target, score: 3.20, + shared_dirs: 0, + basename_count: 1, }], ) } @@ -122,7 +196,7 @@ mod tests { let expected_target = case.candidates[0].path.clone(); let mut io = MockIO::new(vec!["1"]); - let summary = run(&[case], &mut io, false).unwrap(); + let summary = run(&[case], &mut io, false, false).unwrap(); assert_eq!(summary.relinked, 1); let resolved = fs::read_link(temp.path().join("my_link")).unwrap(); @@ -136,7 +210,7 @@ mod tests { let case = setup_case(&temp); let mut io = MockIO::new(vec!["s"]); - let summary = run(&[case], &mut io, false).unwrap(); + let summary = run(&[case], &mut io, false, false).unwrap(); assert_eq!(summary.skipped, 1); assert!(temp.path().join("my_link").symlink_metadata().is_ok()); @@ -148,7 +222,7 @@ mod tests { let case = setup_case(&temp); let mut io = MockIO::new(vec!["r", "y"]); - let summary = run(&[case], &mut io, false).unwrap(); + let summary = run(&[case], &mut io, false, false).unwrap(); assert_eq!(summary.removed, 1); assert!(temp.path().join("my_link").symlink_metadata().is_err()); @@ -161,7 +235,7 @@ mod tests { let case = setup_case(&temp); let mut io = MockIO::new(vec!["r", "n", "s"]); - let summary = run(&[case], &mut io, false).unwrap(); + let summary = run(&[case], &mut io, false, false).unwrap(); assert_eq!(summary.skipped, 1); assert!(temp.path().join("my_link").symlink_metadata().is_ok()); @@ -173,7 +247,7 @@ mod tests { let case = setup_case(&temp); let mut io = MockIO::new(vec!["xyz", "s"]); - let summary = run(&[case], &mut io, false).unwrap(); + let summary = run(&[case], &mut io, false, false).unwrap(); assert_eq!(summary.skipped, 1); assert!(io.output().contains("unrecognized input")); @@ -187,7 +261,7 @@ mod tests { let case = setup_case(&temp); let mut io = MockIO::new(vec!["c", custom_target.to_str().unwrap()]); - let summary = run(&[case], &mut io, false).unwrap(); + let summary = run(&[case], &mut io, false, false).unwrap(); assert_eq!(summary.relinked, 1); let resolved = fs::read_link(temp.path().join("my_link")).unwrap(); @@ -200,7 +274,7 @@ mod tests { let case = setup_case(&temp); let mut io = MockIO::new(vec!["c", "", "s"]); - let summary = run(&[case], &mut io, false).unwrap(); + let summary = run(&[case], &mut io, false, false).unwrap(); assert_eq!(summary.skipped, 1); } @@ -211,7 +285,7 @@ mod tests { let case = setup_case(&temp); let mut io = MockIO::new(vec!["1"]); - let summary = run(&[case], &mut io, true).unwrap(); + let summary = run(&[case], &mut io, true, false).unwrap(); assert_eq!(summary.relinked, 1); let still_broken = fs::read_link(temp.path().join("my_link")).unwrap(); @@ -233,6 +307,8 @@ mod tests { vec![ScoredCandidate { path: t1, score: 1.0, + shared_dirs: 0, + basename_count: 1, }], ); @@ -242,10 +318,39 @@ mod tests { let mut io = MockIO::new(vec!["1", "s"]); - let summary = run(&[case1, case2], &mut io, false).unwrap(); + let summary = run(&[case1, case2], &mut io, false, false).unwrap(); assert_eq!(summary.relinked, 1); assert_eq!(summary.skipped, 1); assert_eq!(summary.total(), 2); } + + #[test] + fn batch_confirm_decline_no_changes() { + let temp = TempDir::new().unwrap(); + let case = setup_case(&temp); + let mut io = MockIO::new(vec!["1", "n"]); + + let summary = run(&[case], &mut io, false, true).unwrap(); + + assert_eq!(summary.relinked, 0); + assert_eq!(summary.total(), 0); + assert!(io.output().contains("Aborted")); + let still_broken = fs::read_link(temp.path().join("my_link")).unwrap(); + assert_eq!(still_broken, PathBuf::from("/nonexistent")); + } + + #[test] + fn batch_confirm_accept_applies_all() { + let temp = TempDir::new().unwrap(); + let case = setup_case(&temp); + let expected_target = case.candidates[0].path.clone(); + let mut io = MockIO::new(vec!["1", "y"]); + + let summary = run(&[case], &mut io, false, true).unwrap(); + + assert_eq!(summary.relinked, 1); + let resolved = fs::read_link(temp.path().join("my_link")).unwrap(); + assert_eq!(resolved, expected_target); + } }