diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..5cace2f --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,118 @@ +# machine_setup Context + +The domain and architecture vocabulary for `machine_setup` — a declarative +machine-configuration tool that runs tasks defined in YAML/JSON. This file is +the canonical naming reference; prefer these terms (and avoid their listed +aliases) in code, comments, and reviews. + +## Language + +### Configuration + +**Task**: +A named unit of setup work, made of an ordered list of commands, with optional +OS filter, conditions, dependencies, and retry. +_Avoid_: job, step, action. + +**Command entry**: +One declarative operation inside a task — `copy`, `symlink`, `clone`, `run`, or +`machine_setup`. The `CommandEntry` enum in the config. +_Avoid_: step, instruction. (Do **not** shorten to "command" — see Flagged +ambiguities.) + +**Sub-config**: +A nested configuration pulled in by a `machine_setup` command entry and executed +by its own runner one nesting level deeper. +_Avoid_: child config, included config. + +### Execution + +**Mode**: +The execution intent applied to a run — `Install`, `Update`, or `Uninstall`. +Derived once from the CLI verb; the only verbs the engine acts on. +_Avoid_: action, command, operation. + +**Runner**: +The component that orders tasks, applies skip rules, and drives each task's +command entries to completion, emitting events as it goes (`TaskRunner`). +_Avoid_: engine (too broad), executor (means something narrower here). + +**Command executor**: +The thing that runs one command entry for the current mode — one per command +entry type, behind the `CommandExecutor` interface (single `execute` method). +_Avoid_: handler, command (see Flagged ambiguities). + +**Task event**: +A message emitted by the runner describing execution progress, consumed by the +TUI or the plain logger (`TaskEvent`). +_Avoid_: message, log, signal. + +**History**: +The persisted record of which tasks are currently installed, used to skip +already-installed tasks unless forced. +_Avoid_: state, cache, ledger. + +### Architecture seams + +These name the deepened modules introduced to concentrate behavior; reviews +should refer to them by these names. + +**Task graph**: +The single home for everything derived from `depends_on` edges — topological +order, parallel layers, cycle detection, and missing-edge detection +(`TaskGraph`). Both the runner and the validator read from it. +_Avoid_: dependency resolver, DAG, scheduler. + +**File ops**: +The privilege seam for filesystem primitives (`mkdir`, copy, symlink, removal), +with two adapters — **DirectFs** (`std::fs`) and **SudoFs** (`sudo`) — chosen +once per command from its `sudo` flag (`FileOps`). +_Avoid_: fs helper, file utils. + +**Tree materialization**: +The shared traversal behind `copy` and `symlink`: destination resolution (the +file-vs-directory target rule) plus the install/uninstall walk, parameterized by +a per-file operation. +_Avoid_: file walker, copier. + +## Relationships + +- A **Task** contains one or more **Command entries** and may declare + dependencies on other tasks. +- The **Task graph** orders **Tasks**; the **Runner** executes them in that + order under one **Mode**. +- The **Runner** turns each **Command entry** into a **Command executor** and + calls `execute`, which acts according to the current **Mode**. +- The `copy` and `symlink` **Command executors** drive **Tree materialization**, + which performs each file operation through a **File ops** adapter. +- A `machine_setup` **Command entry** loads a **Sub-config** and runs it with a + nested **Runner**. +- The **Runner** emits **Task events** and consults/updates **History**. + +## Example dialogue + +> **Dev:** "When the runner hits a `copy` command entry in uninstall mode, who +> decides whether to use sudo?" +> **Maintainer:** "The copy executor picks a **File ops** adapter once from the +> entry's `sudo` flag, then hands it to **tree materialization**. The executor +> and the traversal never branch on sudo again — that decision lives behind the +> seam." +> +> **Dev:** "And the file-vs-directory target rule?" +> **Maintainer:** "That's `resolve_single_file_dest` inside tree +> materialization — one pure function, shared by copy and symlink, install and +> uninstall." + +## Flagged ambiguities + +- **"Command" was overloaded three ways** — resolved into three distinct terms: + - **Command entry** (`CommandEntry`): a declarative op in the config + (`copy`/`symlink`/…). + - **Mode** (`Mode`): the execution intent (install/update/uninstall) — split + out from the CLI `Command` so engine code stops carrying dead arms for + non-execution verbs (`list`/`validate`/`completions`). + - **CLI command** (`cli::Command`): the clap subcommand the user types, + including non-execution verbs. Maps to a **Mode** for execution verbs only. + Use the qualified term; never a bare "command". +- **"Engine" vs "Runner"** — the crate has an `engine` module, but the executing + component is the **Runner**. Say "runner" for the thing that runs tasks. diff --git a/src/config/graph.rs b/src/config/graph.rs new file mode 100644 index 0000000..9659382 --- /dev/null +++ b/src/config/graph.rs @@ -0,0 +1,357 @@ +//! The task dependency graph. +//! +//! `depends_on` edges are consumed in three ways: the runner needs a +//! topological order to execute in, the runner needs dependency *layers* to +//! parallelize within, and validation needs to report missing edges and +//! cycles. Previously each was a separate hand-rolled traversal (two in the +//! runner, one in the validator), so cycle detection in particular existed +//! twice with different code. [`TaskGraph`] is the single home for all of it: +//! build it once from the task map, then ask it for an order, for layers, or +//! for diagnostics. + +use std::borrow::Cow; +use std::collections::{HashMap, HashSet, VecDeque}; + +use indexmap::IndexMap; + +use super::types::TaskConfig; +use crate::error::{Error, Result}; + +/// A view over tasks and their `depends_on` edges. Borrows the task map; cheap +/// to construct. +pub struct TaskGraph<'a> { + tasks: &'a IndexMap, +} + +impl<'a> TaskGraph<'a> { + pub fn new(tasks: &'a IndexMap) -> Self { + Self { tasks } + } + + /// Every `(task, missing_dependency)` pair where a task depends on a name + /// that isn't defined. Used by validation to report each broken edge. + pub fn missing_dependencies(&self) -> Vec<(String, String)> { + let mut missing = Vec::new(); + for (name, task) in self.tasks { + for dep in &task.depends_on { + if !self.tasks.contains_key(dep) { + missing.push((name.clone(), dep.clone())); + } + } + } + missing + } + + /// Find one dependency cycle, if any, returned as the path of task names + /// forming it (e.g. `["a", "b", "a"]`). Returns `None` when the graph is + /// acyclic. Only the first cycle encountered is reported. + pub fn find_cycle(&self) -> Option> { + #[derive(PartialEq, Clone, Copy)] + enum Color { + White, + Gray, + Black, + } + + let mut colors: HashMap<&str, Color> = self + .tasks + .keys() + .map(|k| (k.as_str(), Color::White)) + .collect(); + + fn dfs<'b>( + node: &'b str, + tasks: &'b IndexMap, + colors: &mut HashMap<&'b str, Color>, + path: &mut Vec<&'b str>, + ) -> Option> { + colors.insert(node, Color::Gray); + path.push(node); + + if let Some(task) = tasks.get(node) { + for dep in &task.depends_on { + match colors.get(dep.as_str()).copied() { + Some(Color::Gray) => { + // Back edge — reconstruct the cycle path. + let start = path.iter().position(|&n| n == dep.as_str()).unwrap(); + let mut cycle: Vec = + path[start..].iter().map(|s| s.to_string()).collect(); + cycle.push(dep.clone()); + return Some(cycle); + } + Some(Color::White) | None => { + if let Some(found) = dfs(dep, tasks, colors, path) { + return Some(found); + } + } + Some(Color::Black) => {} + } + } + } + + path.pop(); + colors.insert(node, Color::Black); + None + } + + let keys: Vec<&str> = self.tasks.keys().map(|k| k.as_str()).collect(); + for &node in &keys { + if colors.get(node).copied() == Some(Color::White) { + let mut path = Vec::new(); + if let Some(cycle) = dfs(node, self.tasks, &mut colors, &mut path) { + return Some(cycle); + } + } + } + None + } + + /// Topologically order the `requested` tasks together with all of their + /// transitive dependencies, so every task appears after the tasks it + /// depends on. + /// + /// When none of the requested tasks declare a dependency, the input order + /// is preserved and borrowed (no allocation). Errors with + /// [`Error::MissingDependency`] if an edge points at an unknown task, or + /// [`Error::CyclicDependency`] if the requested subgraph contains a cycle. + pub fn topo_order<'r>(&self, requested: &'r [String]) -> Result> { + let has_deps = requested + .iter() + .filter_map(|n| self.tasks.get(n)) + .any(|t| !t.depends_on.is_empty()); + if !has_deps { + return Ok(Cow::Borrowed(requested)); + } + + // Collect requested tasks plus their transitive dependencies. + let mut needed: HashSet = HashSet::new(); + let mut queue: VecDeque = requested.iter().cloned().collect(); + while let Some(name) = queue.pop_front() { + if needed.contains(&name) { + continue; + } + if let Some(task) = self.tasks.get(&name) { + needed.insert(name.clone()); + for dep in &task.depends_on { + if !self.tasks.contains_key(dep) { + return Err(Error::MissingDependency(name.clone(), dep.clone())); + } + if !needed.contains(dep) { + queue.push_back(dep.clone()); + } + } + } + } + + // In-degree map and dependents adjacency over the needed subgraph. + let mut in_degree: HashMap<&str, usize> = HashMap::new(); + let mut dependents: HashMap<&str, Vec<&str>> = HashMap::new(); + for name in &needed { + in_degree.entry(name.as_str()).or_insert(0); + if let Some(task) = self.tasks.get(name) { + for dep in &task.depends_on { + if needed.contains(dep) { + *in_degree.entry(name.as_str()).or_insert(0) += 1; + dependents + .entry(dep.as_str()) + .or_default() + .push(name.as_str()); + } + } + } + } + + // Kahn's algorithm. + let mut queue: VecDeque<&str> = in_degree + .iter() + .filter(|(_, °)| deg == 0) + .map(|(&name, _)| name) + .collect(); + let mut sorted = Vec::new(); + while let Some(node) = queue.pop_front() { + sorted.push(node.to_string()); + if let Some(deps) = dependents.get(node) { + for &dep in deps { + if let Some(deg) = in_degree.get_mut(dep) { + *deg -= 1; + if *deg == 0 { + queue.push_back(dep); + } + } + } + } + } + + if sorted.len() != needed.len() { + let remaining: Vec = needed + .iter() + .filter(|n| !sorted.contains(n)) + .cloned() + .collect(); + return Err(Error::CyclicDependency(remaining.join(", "))); + } + + Ok(Cow::Owned(sorted)) + } + + /// Group an already-ordered task list into dependency layers: tasks in the + /// same layer have no dependencies on one another and can run in parallel. + /// A task lands one layer deeper than its deepest dependency. + pub fn layers(&self, ordered: &[String]) -> Vec> { + let mut layers: Vec> = Vec::new(); + let mut task_layer: HashMap<&str, usize> = HashMap::new(); + + for name in ordered { + let layer = if let Some(task) = self.tasks.get(name) { + task.depends_on + .iter() + .filter_map(|dep| task_layer.get(dep.as_str())) + .max() + .map(|&l| l + 1) + .unwrap_or(0) + } else { + 0 + }; + + task_layer.insert(name.as_str(), layer); + while layers.len() <= layer { + layers.push(Vec::new()); + } + layers[layer].push(name.clone()); + } + + layers + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::types::*; + + fn task(deps: &[&str]) -> TaskConfig { + TaskConfig { + commands: vec![], + os: Default::default(), + parallel: false, + only_if: Default::default(), + skip_if: Default::default(), + depends_on: deps.iter().map(|s| s.to_string()).collect(), + retry: 0, + } + } + + fn graph_of(pairs: &[(&str, &[&str])]) -> IndexMap { + let mut map = IndexMap::new(); + for (name, deps) in pairs { + map.insert(name.to_string(), task(deps)); + } + map + } + + fn pos(order: &[String], name: &str) -> usize { + order.iter().position(|n| n == name).unwrap() + } + + #[test] + fn test_topo_no_deps_preserves_and_borrows() { + let tasks = graph_of(&[("a", &[]), ("b", &[]), ("c", &[])]); + let g = TaskGraph::new(&tasks); + let requested = vec!["a".to_string(), "b".to_string(), "c".to_string()]; + let ordered = g.topo_order(&requested).unwrap(); + assert!(matches!(ordered, Cow::Borrowed(_))); + assert_eq!(&*ordered, &requested[..]); + } + + #[test] + fn test_topo_orders_dependencies_first() { + // c -> b -> a + let tasks = graph_of(&[("a", &[]), ("b", &["a"]), ("c", &["b"])]); + let g = TaskGraph::new(&tasks); + let requested = ["c".to_string()]; + let ordered = g.topo_order(&requested).unwrap(); + // Requesting c pulls in b and a transitively, ordered before c. + assert!(pos(&ordered, "a") < pos(&ordered, "b")); + assert!(pos(&ordered, "b") < pos(&ordered, "c")); + } + + #[test] + fn test_topo_diamond() { + // d depends on b and c, both depend on a. + let tasks = graph_of(&[("a", &[]), ("b", &["a"]), ("c", &["a"]), ("d", &["b", "c"])]); + let g = TaskGraph::new(&tasks); + let requested = ["d".to_string()]; + let ordered = g.topo_order(&requested).unwrap(); + assert_eq!(ordered.len(), 4); + assert!(pos(&ordered, "a") < pos(&ordered, "b")); + assert!(pos(&ordered, "a") < pos(&ordered, "c")); + assert!(pos(&ordered, "b") < pos(&ordered, "d")); + assert!(pos(&ordered, "c") < pos(&ordered, "d")); + } + + #[test] + fn test_topo_missing_dependency_errors() { + let tasks = graph_of(&[("a", &["ghost"])]); + let g = TaskGraph::new(&tasks); + let requested = ["a".to_string()]; + let err = g.topo_order(&requested).unwrap_err(); + assert!(matches!(err, Error::MissingDependency(t, d) if t == "a" && d == "ghost")); + } + + #[test] + fn test_topo_cycle_errors() { + let tasks = graph_of(&[("a", &["b"]), ("b", &["a"])]); + let g = TaskGraph::new(&tasks); + let requested = ["a".to_string()]; + let err = g.topo_order(&requested).unwrap_err(); + assert!(matches!(err, Error::CyclicDependency(_))); + } + + #[test] + fn test_layers_groups_independent_tasks() { + let tasks = graph_of(&[("a", &[]), ("b", &["a"]), ("c", &["a"]), ("d", &["b", "c"])]); + let g = TaskGraph::new(&tasks); + let requested = ["d".to_string()]; + let ordered = g.topo_order(&requested).unwrap(); + let layers = g.layers(&ordered); + // a alone, then {b,c}, then d. + assert_eq!(layers[0], vec!["a".to_string()]); + assert_eq!(layers.len(), 3); + let mut middle = layers[1].clone(); + middle.sort(); + assert_eq!(middle, vec!["b".to_string(), "c".to_string()]); + assert_eq!(layers[2], vec!["d".to_string()]); + } + + #[test] + fn test_find_cycle_detects_and_reports_path() { + let tasks = graph_of(&[("a", &["b"]), ("b", &["c"]), ("c", &["a"])]); + let g = TaskGraph::new(&tasks); + let cycle = g.find_cycle().expect("cycle exists"); + // First and last element close the loop. + assert_eq!(cycle.first(), cycle.last()); + assert!(cycle.len() >= 4); + } + + #[test] + fn test_find_cycle_none_when_acyclic() { + let tasks = graph_of(&[("a", &[]), ("b", &["a"])]); + let g = TaskGraph::new(&tasks); + assert!(g.find_cycle().is_none()); + } + + #[test] + fn test_missing_dependencies_lists_all() { + let tasks = graph_of(&[("a", &["x"]), ("b", &["y", "a"])]); + let g = TaskGraph::new(&tasks); + let mut missing = g.missing_dependencies(); + missing.sort(); + assert_eq!( + missing, + vec![ + ("a".to_string(), "x".to_string()), + ("b".to_string(), "y".to_string()) + ] + ); + } +} diff --git a/src/config/mod.rs b/src/config/mod.rs index d849326..f612021 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,3 +1,4 @@ +pub mod graph; pub mod history; pub mod os; pub mod types; diff --git a/src/config/types.rs b/src/config/types.rs index 18222dd..d32bdba 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -273,34 +273,32 @@ impl RunArgs { .map(|s| s.as_str()) } - /// Get commands for a specific mode. - pub fn commands_for_mode(&self, mode: &crate::cli::Command) -> &[String] { + /// Get commands for a specific execution mode. + pub fn commands_for_mode(&self, mode: crate::engine::mode::Mode) -> &[String] { + use crate::engine::mode::Mode; match mode { - crate::cli::Command::Install => { + Mode::Install => { if !self.install.as_slice().is_empty() { self.install.as_slice() } else { self.commands.as_slice() } } - crate::cli::Command::Update => { + Mode::Update => { if !self.update.as_slice().is_empty() { self.update.as_slice() } else { - // In v1, update only runs if explicitly defined + // Update only runs if explicitly defined. &[] } } - crate::cli::Command::Uninstall => { + Mode::Uninstall => { if !self.uninstall.as_slice().is_empty() { self.uninstall.as_slice() } else { &[] } } - crate::cli::Command::List - | crate::cli::Command::Validate - | crate::cli::Command::Completions { .. } => &[], } } } @@ -442,18 +440,24 @@ update: "npm update" uninstall: "npm uninstall" "#; let args: RunArgs = serde_yaml::from_str(yaml).unwrap(); - assert_eq!( - args.commands_for_mode(&crate::cli::Command::Install), - &["npm install"] - ); - assert_eq!( - args.commands_for_mode(&crate::cli::Command::Update), - &["npm update"] - ); - assert_eq!( - args.commands_for_mode(&crate::cli::Command::Uninstall), - &["npm uninstall"] - ); + use crate::engine::mode::Mode; + assert_eq!(args.commands_for_mode(Mode::Install), &["npm install"]); + assert_eq!(args.commands_for_mode(Mode::Update), &["npm update"]); + assert_eq!(args.commands_for_mode(Mode::Uninstall), &["npm uninstall"]); + } + + #[test] + fn test_run_args_install_falls_back_to_commands() { + // When no mode-specific `install` is set, install mode uses `commands`. + let yaml = r#" +commands: "echo shared" +"#; + let args: RunArgs = serde_yaml::from_str(yaml).unwrap(); + use crate::engine::mode::Mode; + assert_eq!(args.commands_for_mode(Mode::Install), &["echo shared"]); + // Update/uninstall don't fall back to `commands`. + assert!(args.commands_for_mode(Mode::Update).is_empty()); + assert!(args.commands_for_mode(Mode::Uninstall).is_empty()); } #[test] diff --git a/src/config/validate.rs b/src/config/validate.rs index 167cd31..1d61ee4 100644 --- a/src/config/validate.rs +++ b/src/config/validate.rs @@ -1,5 +1,6 @@ use std::path::Path; +use super::graph::TaskGraph; use super::types::{AppConfig, CommandEntry}; use crate::utils::shell::validate_env_key; @@ -25,86 +26,27 @@ pub struct ValidationIssue { pub severity: Severity, } -/// Validate depends_on references exist and detect cycles. +/// Validate depends_on references exist and detect cycles, using the shared +/// [`TaskGraph`] so ordering and validation agree on the same logic. fn validate_dependencies(config: &AppConfig, issues: &mut Vec) { - let task_names: std::collections::HashSet<&str> = - config.tasks.keys().map(|s| s.as_str()).collect(); + let graph = TaskGraph::new(&config.tasks); - // Check all depends_on references exist - for (name, task) in &config.tasks { - for dep in &task.depends_on { - if !task_names.contains(dep.as_str()) { - issues.push(ValidationIssue { - task_name: name.clone(), - message: format!("depends_on references unknown task: '{dep}'"), - severity: Severity::Error, - }); - } - } + // Report each broken edge. + for (name, dep) in graph.missing_dependencies() { + issues.push(ValidationIssue { + task_name: name, + message: format!("depends_on references unknown task: '{dep}'"), + severity: Severity::Error, + }); } - // Detect cycles using DFS with coloring - use std::collections::HashMap; - #[derive(PartialEq)] - enum Color { - White, - Gray, - Black, - } - let mut colors: HashMap<&str, Color> = config - .tasks - .keys() - .map(|k| (k.as_str(), Color::White)) - .collect(); - - fn dfs<'a>( - node: &'a str, - config: &'a AppConfig, - colors: &mut HashMap<&'a str, Color>, - path: &mut Vec<&'a str>, - ) -> Option> { - colors.insert(node, Color::Gray); - path.push(node); - - if let Some(task) = config.tasks.get(node) { - for dep in &task.depends_on { - match colors.get(dep.as_str()) { - Some(Color::Gray) => { - // Found a cycle — build the cycle path - let cycle_start = path.iter().position(|&n| n == dep.as_str()).unwrap(); - let mut cycle: Vec = - path[cycle_start..].iter().map(|s| s.to_string()).collect(); - cycle.push(dep.clone()); - return Some(cycle); - } - Some(Color::White) | None => { - if let Some(cycle) = dfs(dep, config, colors, path) { - return Some(cycle); - } - } - Some(Color::Black) => {} - } - } - } - - path.pop(); - colors.insert(node, Color::Black); - None - } - - let task_keys: Vec<&str> = config.tasks.keys().map(|k| k.as_str()).collect(); - for &node in &task_keys { - if colors.get(node) == Some(&Color::White) { - let mut path = Vec::new(); - if let Some(cycle) = dfs(node, config, &mut colors, &mut path) { - issues.push(ValidationIssue { - task_name: cycle[0].clone(), - message: format!("Cyclic dependency detected: {}", cycle.join(" -> ")), - severity: Severity::Error, - }); - break; // Report one cycle at a time - } - } + // Report one cycle, if any. + if let Some(cycle) = graph.find_cycle() { + issues.push(ValidationIssue { + task_name: cycle[0].clone(), + message: format!("Cyclic dependency detected: {}", cycle.join(" -> ")), + severity: Severity::Error, + }); } } diff --git a/src/engine/commands/clone.rs b/src/engine/commands/clone.rs index f15aeeb..68c1026 100644 --- a/src/engine/commands/clone.rs +++ b/src/engine/commands/clone.rs @@ -3,6 +3,7 @@ use tokio::process::Command; use crate::config::types::CloneArgs; use crate::engine::context::CommandContext; +use crate::engine::mode::Mode; use crate::error::{Error, Result}; use crate::utils::path::expand_path; use crate::utils::process; @@ -21,18 +22,49 @@ impl CloneCommand { #[async_trait] impl CommandExecutor for CloneCommand { - async fn install(&self, ctx: &CommandContext) -> Result<()> { + async fn execute(&self, ctx: &CommandContext) -> Result<()> { + match ctx.mode { + Mode::Install => self.clone_repo(ctx).await, + Mode::Update => self.pull_repo(ctx).await, + Mode::Uninstall => self.remove_repo(ctx).await, + } + } + + fn description(&self) -> String { + self.args.to_string() + } +} + +impl CloneCommand { + async fn clone_repo(&self, ctx: &CommandContext) -> Result<()> { let target = expand_path(&self.args.target, Some(&ctx.config_dir)); - // Check if already cloned + // Already cloned — fall through to a pull instead. if target.join(".git").exists() { ctx.log(format!( "Repository already exists at {}, running update instead", target.display() )); - return self.update(ctx).await; + return self.git_pull(&target, ctx).await; } + self.git_clone(&target, ctx).await + } + + async fn pull_repo(&self, ctx: &CommandContext) -> Result<()> { + let target = expand_path(&self.args.target, Some(&ctx.config_dir)); + + // Not cloned yet — fall through to a clone instead. + if !target.join(".git").exists() { + ctx.log("Repository not found, running install instead"); + return self.git_clone(&target, ctx).await; + } + + self.git_pull(&target, ctx).await + } + + /// Clone the repository into `target`. Non-recursive low-level action. + async fn git_clone(&self, target: &std::path::Path, ctx: &CommandContext) -> Result<()> { ctx.log(format!( "Cloning {} into {}", self.args.url, @@ -51,19 +83,13 @@ impl CommandExecutor for CloneCommand { .await } - async fn update(&self, ctx: &CommandContext) -> Result<()> { - let target = expand_path(&self.args.target, Some(&ctx.config_dir)); - - if !target.join(".git").exists() { - ctx.log("Repository not found, running install instead"); - return self.install(ctx).await; - } - + /// Pull the latest changes in `target`. Non-recursive low-level action. + async fn git_pull(&self, target: &std::path::Path, ctx: &CommandContext) -> Result<()> { ctx.log(format!("Pulling latest in {}", target.display())); - run_git_command(&["pull"], Some(&target), ctx).await + run_git_command(&["pull"], Some(target), ctx).await } - async fn uninstall(&self, ctx: &CommandContext) -> Result<()> { + async fn remove_repo(&self, ctx: &CommandContext) -> Result<()> { let target = expand_path(&self.args.target, Some(&ctx.config_dir)); if target.exists() { @@ -73,10 +99,6 @@ impl CommandExecutor for CloneCommand { Ok(()) } - - fn description(&self) -> String { - self.args.to_string() - } } async fn run_git_command( diff --git a/src/engine/commands/copy.rs b/src/engine/commands/copy.rs index 1178487..cc022f3 100644 --- a/src/engine/commands/copy.rs +++ b/src/engine/commands/copy.rs @@ -3,10 +3,12 @@ use std::path::Path; use crate::config::types::CopyArgs; use crate::engine::context::CommandContext; +use crate::engine::mode::Mode; use crate::error::{Error, Result}; -use crate::utils::path::{expand_path, walk_relative}; -use crate::utils::sudo; +use crate::utils::path::expand_path; +use super::fs_ops::{self, FileOps}; +use super::tree; use super::CommandExecutor; pub struct CopyCommand { @@ -21,10 +23,22 @@ impl CopyCommand { #[async_trait] impl CommandExecutor for CopyCommand { - async fn install(&self, ctx: &CommandContext) -> Result<()> { + async fn execute(&self, ctx: &CommandContext) -> Result<()> { + match ctx.mode { + Mode::Install | Mode::Update => self.apply(ctx), + Mode::Uninstall => self.remove(ctx), + } + } + + fn description(&self) -> String { + self.args.to_string() + } +} + +impl CopyCommand { + fn apply(&self, ctx: &CommandContext) -> Result<()> { let src = expand_path(&self.args.src, Some(&ctx.config_dir)); let target = expand_path(&self.args.target, Some(&ctx.config_dir)); - let use_sudo = self.args.sudo; if !src.exists() { return Err(Error::PathError(format!( @@ -33,86 +47,35 @@ impl CommandExecutor for CopyCommand { ))); } - if src.is_file() { - let dest = if target.extension().is_some() || !target.is_dir() { - if let Some(parent) = target.parent() { - mkdir(parent, use_sudo)?; - } - target.clone() - } else { - mkdir(&target, use_sudo)?; - target.join(src.file_name().unwrap()) - }; - copy_file(&src, &dest, use_sudo, ctx)?; - } else { - mkdir(&target, use_sudo)?; - copy_directory(&src, &target, &self.args.ignore, use_sudo, ctx)?; - } - - Ok(()) - } - - async fn update(&self, ctx: &CommandContext) -> Result<()> { - ctx.log("Copy update: re-running install"); - self.install(ctx).await + let ops = fs_ops::select(self.args.sudo); + tree::install_tree( + ops.as_ref(), + &src, + &target, + &self.args.ignore, + |file, dest| copy_one(ops.as_ref(), file, dest, ctx), + ) } - async fn uninstall(&self, ctx: &CommandContext) -> Result<()> { + fn remove(&self, ctx: &CommandContext) -> Result<()> { let src = expand_path(&self.args.src, Some(&ctx.config_dir)); let target = expand_path(&self.args.target, Some(&ctx.config_dir)); - let use_sudo = self.args.sudo; - if src.is_file() { - let dest = if target.extension().is_some() || !target.is_dir() { - target.clone() - } else { - target.join(src.file_name().unwrap()) - }; + let ops = fs_ops::select(self.args.sudo); + tree::uninstall_tree(&src, &target, &self.args.ignore, |dest| { if dest.exists() { ctx.log(format!("Removing: {}", dest.display())); - remove_file(&dest, use_sudo)?; - } - } else { - walk_relative(&src, &target, &self.args.ignore, |entry, dest| { - if entry.file_type().is_file() && dest.exists() { - ctx.log(format!("Removing: {}", dest.display())); - remove_file(dest, use_sudo)?; - } + ops.remove_file(dest) + } else { Ok(()) - })?; - } - - Ok(()) - } - - fn description(&self) -> String { - self.args.to_string() - } -} - -fn mkdir(path: &Path, use_sudo: bool) -> Result<()> { - if path.is_dir() { - return Ok(()); - } - if use_sudo { - sudo::sudo_mkdir(path) - } else { - std::fs::create_dir_all(path)?; - Ok(()) - } -} - -fn remove_file(path: &Path, use_sudo: bool) -> Result<()> { - if use_sudo { - sudo::sudo_remove(path) - } else { - std::fs::remove_file(path)?; - Ok(()) + } + }) } } -fn copy_file(src: &Path, dest: &Path, use_sudo: bool, ctx: &CommandContext) -> Result<()> { - // Skip if target is newer +/// Copy a single file, skipping when the destination is already at least as +/// new as the source. +fn copy_one(ops: &dyn FileOps, src: &Path, dest: &Path, ctx: &CommandContext) -> Result<()> { if dest.exists() { if let (Ok(src_meta), Ok(dest_meta)) = (std::fs::metadata(src), std::fs::metadata(dest)) { if let (Ok(src_mod), Ok(dest_mod)) = (src_meta.modified(), dest_meta.modified()) { @@ -125,30 +88,64 @@ fn copy_file(src: &Path, dest: &Path, use_sudo: bool, ctx: &CommandContext) -> R } ctx.log(format!("Copying: {} -> {}", src.display(), dest.display())); + ops.copy_file(src, dest) +} - if use_sudo { - sudo::sudo_copy(src, dest) - } else { - if let Some(parent) = dest.parent() { - std::fs::create_dir_all(parent)?; - } - std::fs::copy(src, dest)?; - Ok(()) +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::commands::fs_ops::RecordingFs; + use tempfile::tempdir; + + #[test] + fn test_copy_one_skips_when_dest_newer() { + let dir = tempdir().unwrap(); + let src = dir.path().join("s.txt"); + let dest = dir.path().join("d.txt"); + std::fs::write(&src, b"old").unwrap(); + // Create dest after src so its mtime is >= src. + std::fs::write(&dest, b"new").unwrap(); + + let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); + let ctx = CommandContext { + event_tx: tx, + mode: Mode::Install, + config_dir: dir.path().to_path_buf(), + temp_dir: dir.path().to_path_buf(), + default_shell: crate::config::types::Shell::Bash, + task_name: "t".to_string(), + depth: 0, + }; + + let ops = RecordingFs::default(); + copy_one(&ops, &src, &dest, &ctx).unwrap(); + // Skipped: no copy_file recorded. + assert!(ops.calls().is_empty()); } -} -fn copy_directory( - src: &Path, - target: &Path, - ignore: &[String], - use_sudo: bool, - ctx: &CommandContext, -) -> Result<()> { - walk_relative(src, target, ignore, |entry, dest| { - if entry.file_type().is_dir() { - mkdir(dest, use_sudo) - } else { - copy_file(entry.path(), dest, use_sudo, ctx) - } - }) + #[test] + fn test_copy_one_copies_when_dest_missing() { + let dir = tempdir().unwrap(); + let src = dir.path().join("s.txt"); + let dest = dir.path().join("d.txt"); + std::fs::write(&src, b"data").unwrap(); + + let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); + let ctx = CommandContext { + event_tx: tx, + mode: Mode::Install, + config_dir: dir.path().to_path_buf(), + temp_dir: dir.path().to_path_buf(), + default_shell: crate::config::types::Shell::Bash, + task_name: "t".to_string(), + depth: 0, + }; + + let ops = RecordingFs::default(); + copy_one(&ops, &src, &dest, &ctx).unwrap(); + assert_eq!( + ops.calls(), + vec![format!("copy_file {} {}", src.display(), dest.display())] + ); + } } diff --git a/src/engine/commands/fs_ops.rs b/src/engine/commands/fs_ops.rs new file mode 100644 index 0000000..89fcd5e --- /dev/null +++ b/src/engine/commands/fs_ops.rs @@ -0,0 +1,270 @@ +//! Filesystem operations behind a privilege seam. +//! +//! The `copy` and `symlink` executors need the same handful of filesystem +//! primitives, each of which has two implementations: a direct one (`std::fs`) +//! and a privileged one (shelling out to `sudo`). Rather than fork on +//! `use_sudo` at every call site — and duplicate the platform-specific symlink +//! and removal logic across both executors — the fork is decided once, when an +//! executor picks an adapter, and every primitive call goes through the +//! [`FileOps`] interface. +//! +//! Two adapters means a real seam: [`DirectFs`] and [`SudoFs`] both exist and +//! are selected at runtime by [`select`]. + +use std::path::Path; + +use crate::error::Result; +use crate::utils::sudo; + +/// The filesystem primitives `copy` and `symlink` need. Each method hides the +/// privilege decision and any platform-specific handling from callers. +pub trait FileOps: Send + Sync { + /// Create `path` and any missing parent directories (like `mkdir -p`). + fn mkdir_p(&self, path: &Path) -> Result<()>; + + /// Copy a single regular file from `src` to `dest`. + fn copy_file(&self, src: &Path, dest: &Path) -> Result<()>; + + /// Create a symlink at `dest` pointing to `src`. + fn create_symlink(&self, src: &Path, dest: &Path) -> Result<()>; + + /// Remove a regular file at `path`. + fn remove_file(&self, path: &Path) -> Result<()>; + + /// Force-remove whatever exists at `path` — a file, or a directory and all + /// its contents. + fn remove_path(&self, path: &Path) -> Result<()>; + + /// Remove a symlink at `path`, handling directory symlinks on platforms + /// (Windows) that distinguish them from file symlinks. + fn remove_symlink(&self, path: &Path) -> Result<()>; +} + +/// Pick the adapter for a command based on whether it requested `sudo`. +pub fn select(use_sudo: bool) -> Box { + if use_sudo { + Box::new(SudoFs) + } else { + Box::new(DirectFs) + } +} + +/// Direct filesystem access via `std::fs`. +pub struct DirectFs; + +impl FileOps for DirectFs { + fn mkdir_p(&self, path: &Path) -> Result<()> { + std::fs::create_dir_all(path)?; + Ok(()) + } + + fn copy_file(&self, src: &Path, dest: &Path) -> Result<()> { + if let Some(parent) = dest.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::copy(src, dest)?; + Ok(()) + } + + fn create_symlink(&self, src: &Path, dest: &Path) -> Result<()> { + #[cfg(unix)] + { + std::os::unix::fs::symlink(src, dest)?; + } + + #[cfg(windows)] + { + if src.is_dir() { + std::os::windows::fs::symlink_dir(src, dest)?; + } else { + std::os::windows::fs::symlink_file(src, dest)?; + } + } + + Ok(()) + } + + fn remove_file(&self, path: &Path) -> Result<()> { + std::fs::remove_file(path)?; + Ok(()) + } + + fn remove_path(&self, path: &Path) -> Result<()> { + if path.is_dir() { + std::fs::remove_dir_all(path)?; + } else { + std::fs::remove_file(path)?; + } + Ok(()) + } + + fn remove_symlink(&self, path: &Path) -> Result<()> { + #[cfg(windows)] + { + if path.is_dir() { + std::fs::remove_dir(path)?; + return Ok(()); + } + } + std::fs::remove_file(path)?; + Ok(()) + } +} + +/// Privileged filesystem access via `sudo`. +pub struct SudoFs; + +impl FileOps for SudoFs { + fn mkdir_p(&self, path: &Path) -> Result<()> { + sudo::sudo_mkdir(path) + } + + fn copy_file(&self, src: &Path, dest: &Path) -> Result<()> { + sudo::sudo_copy(src, dest) + } + + fn create_symlink(&self, src: &Path, dest: &Path) -> Result<()> { + sudo::sudo_symlink(src, dest) + } + + fn remove_file(&self, path: &Path) -> Result<()> { + sudo::sudo_remove(path) + } + + fn remove_path(&self, path: &Path) -> Result<()> { + // `rm -rf` removes files and directories alike. + sudo::sudo_remove_dir(path) + } + + fn remove_symlink(&self, path: &Path) -> Result<()> { + // `rm -f` on a symlink removes the link itself, file or dir target. + sudo::sudo_remove(path) + } +} + +/// An adapter that records the operations requested, without touching the real +/// filesystem — lets tests across the command executors assert *what* sequence +/// of operations an executor asked for. Test-only. +#[cfg(test)] +#[derive(Default)] +pub struct RecordingFs { + ops: std::sync::Mutex>, +} + +#[cfg(test)] +impl RecordingFs { + /// The operations recorded so far, in order. + pub fn calls(&self) -> Vec { + self.ops.lock().unwrap().clone() + } + fn record(&self, op: String) { + self.ops.lock().unwrap().push(op); + } +} + +#[cfg(test)] +impl FileOps for RecordingFs { + fn mkdir_p(&self, path: &Path) -> Result<()> { + self.record(format!("mkdir_p {}", path.display())); + Ok(()) + } + fn copy_file(&self, src: &Path, dest: &Path) -> Result<()> { + self.record(format!("copy_file {} {}", src.display(), dest.display())); + Ok(()) + } + fn create_symlink(&self, src: &Path, dest: &Path) -> Result<()> { + self.record(format!( + "create_symlink {} {}", + src.display(), + dest.display() + )); + Ok(()) + } + fn remove_file(&self, path: &Path) -> Result<()> { + self.record(format!("remove_file {}", path.display())); + Ok(()) + } + fn remove_path(&self, path: &Path) -> Result<()> { + self.record(format!("remove_path {}", path.display())); + Ok(()) + } + fn remove_symlink(&self, path: &Path) -> Result<()> { + self.record(format!("remove_symlink {}", path.display())); + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + #[test] + fn test_select_returns_distinct_adapters() { + // Smoke test: both branches construct without panicking. + let _direct = select(false); + let _sudo = select(true); + } + + #[test] + fn test_direct_mkdir_and_copy_roundtrip() { + let dir = tempdir().unwrap(); + let ops = DirectFs; + + let src = dir.path().join("src.txt"); + std::fs::write(&src, b"hello").unwrap(); + + let nested = dir.path().join("a/b/c"); + ops.mkdir_p(&nested).unwrap(); + assert!(nested.is_dir()); + + // copy_file creates intermediate parents on its own. + let dest = dir.path().join("out/copied.txt"); + ops.copy_file(&src, &dest).unwrap(); + assert_eq!(std::fs::read(&dest).unwrap(), b"hello"); + } + + #[test] + fn test_direct_remove_file_and_path() { + let dir = tempdir().unwrap(); + let ops = DirectFs; + + let file = dir.path().join("f.txt"); + std::fs::write(&file, b"x").unwrap(); + ops.remove_file(&file).unwrap(); + assert!(!file.exists()); + + let subdir = dir.path().join("tree/inner"); + std::fs::create_dir_all(&subdir).unwrap(); + std::fs::write(subdir.join("leaf"), b"y").unwrap(); + ops.remove_path(&dir.path().join("tree")).unwrap(); + assert!(!dir.path().join("tree").exists()); + } + + #[cfg(unix)] + #[test] + fn test_direct_create_and_remove_symlink() { + let dir = tempdir().unwrap(); + let ops = DirectFs; + + let src = dir.path().join("target.txt"); + std::fs::write(&src, b"z").unwrap(); + let link = dir.path().join("link.txt"); + + ops.create_symlink(&src, &link).unwrap(); + assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); + + ops.remove_symlink(&link).unwrap(); + assert!(link.symlink_metadata().is_err()); + // The link target is untouched. + assert!(src.exists()); + } + + #[test] + fn test_recording_fs_captures_calls() { + let ops = RecordingFs::default(); + ops.mkdir_p(Path::new("/a")).unwrap(); + ops.copy_file(Path::new("/a/s"), Path::new("/a/d")).unwrap(); + assert_eq!(ops.calls(), vec!["mkdir_p /a", "copy_file /a/s /a/d"]); + } +} diff --git a/src/engine/commands/mod.rs b/src/engine/commands/mod.rs index f9c95eb..7ed17ee 100644 --- a/src/engine/commands/mod.rs +++ b/src/engine/commands/mod.rs @@ -1,8 +1,10 @@ pub mod clone; pub mod copy; +pub mod fs_ops; pub mod run; pub mod setup; pub mod symlink; +pub mod tree; use async_trait::async_trait; @@ -12,11 +14,13 @@ use crate::error::Result; use super::context::CommandContext; /// Trait for executable commands. +/// +/// A single `execute` entry point reads the execution mode from `ctx.mode`, +/// so mode dispatch happens exactly once (inside the executor) rather than +/// once here and again at the call site. #[async_trait] pub trait CommandExecutor: Send + Sync { - async fn install(&self, ctx: &CommandContext) -> Result<()>; - async fn update(&self, ctx: &CommandContext) -> Result<()>; - async fn uninstall(&self, ctx: &CommandContext) -> Result<()>; + async fn execute(&self, ctx: &CommandContext) -> Result<()>; /// Short description for display. fn description(&self) -> String; diff --git a/src/engine/commands/run.rs b/src/engine/commands/run.rs index 5d6c034..717e87a 100644 --- a/src/engine/commands/run.rs +++ b/src/engine/commands/run.rs @@ -3,6 +3,7 @@ use tokio::process::Command; use crate::config::types::RunArgs; use crate::engine::context::CommandContext; +use crate::engine::mode::Mode; use crate::error::{Error, Result}; use crate::utils::{process, shell}; @@ -20,16 +21,8 @@ impl RunCommand { #[async_trait] impl CommandExecutor for RunCommand { - async fn install(&self, ctx: &CommandContext) -> Result<()> { - run_for_mode(&self.args, &crate::cli::Command::Install, ctx).await - } - - async fn update(&self, ctx: &CommandContext) -> Result<()> { - run_for_mode(&self.args, &crate::cli::Command::Update, ctx).await - } - - async fn uninstall(&self, ctx: &CommandContext) -> Result<()> { - run_for_mode(&self.args, &crate::cli::Command::Uninstall, ctx).await + async fn execute(&self, ctx: &CommandContext) -> Result<()> { + run_for_mode(&self.args, ctx.mode, ctx).await } fn description(&self) -> String { @@ -37,11 +30,7 @@ impl CommandExecutor for RunCommand { } } -async fn run_for_mode( - args: &RunArgs, - mode: &crate::cli::Command, - ctx: &CommandContext, -) -> Result<()> { +async fn run_for_mode(args: &RunArgs, mode: Mode, ctx: &CommandContext) -> Result<()> { let commands = args.commands_for_mode(mode); if commands.is_empty() { ctx.log(format!("No commands defined for mode: {mode}")); diff --git a/src/engine/commands/setup.rs b/src/engine/commands/setup.rs index 9b7c2dc..5d90c6f 100644 --- a/src/engine/commands/setup.rs +++ b/src/engine/commands/setup.rs @@ -19,15 +19,7 @@ impl SetupCommand { #[async_trait] impl CommandExecutor for SetupCommand { - async fn install(&self, ctx: &CommandContext) -> Result<()> { - run_sub_config(&self.args, ctx).await - } - - async fn update(&self, ctx: &CommandContext) -> Result<()> { - run_sub_config(&self.args, ctx).await - } - - async fn uninstall(&self, ctx: &CommandContext) -> Result<()> { + async fn execute(&self, ctx: &CommandContext) -> Result<()> { run_sub_config(&self.args, ctx).await } @@ -56,10 +48,9 @@ async fn run_sub_config(args: &MachineSetupArgs, ctx: &CommandContext) -> Result // and unresolvable paths fall back to the parent's config_dir. let sub_config_dir = crate::config::resolve_config_dir(&config_str, &ctx.config_dir); - let runner = - crate::engine::runner::TaskRunner::new(config, ctx.mode.clone(), ctx.event_tx.clone()) - .with_config_dir(sub_config_dir) - .with_depth(ctx.depth + 1); + let runner = crate::engine::runner::TaskRunner::new(config, ctx.mode, ctx.event_tx.clone()) + .with_config_dir(sub_config_dir) + .with_depth(ctx.depth + 1); if let Some(task_name) = &args.task { runner.run_single_task(task_name, false).await diff --git a/src/engine/commands/symlink.rs b/src/engine/commands/symlink.rs index 04202a1..679038a 100644 --- a/src/engine/commands/symlink.rs +++ b/src/engine/commands/symlink.rs @@ -1,11 +1,14 @@ use async_trait::async_trait; +use std::path::Path; use crate::config::types::SymlinkArgs; use crate::engine::context::CommandContext; +use crate::engine::mode::Mode; use crate::error::{Error, Result}; -use crate::utils::path::{expand_path, walk_relative}; -use crate::utils::sudo; +use crate::utils::path::expand_path; +use super::fs_ops::{self, FileOps}; +use super::tree; use super::CommandExecutor; pub struct SymlinkCommand { @@ -20,10 +23,22 @@ impl SymlinkCommand { #[async_trait] impl CommandExecutor for SymlinkCommand { - async fn install(&self, ctx: &CommandContext) -> Result<()> { + async fn execute(&self, ctx: &CommandContext) -> Result<()> { + match ctx.mode { + Mode::Install | Mode::Update => self.apply(ctx), + Mode::Uninstall => self.remove(ctx), + } + } + + fn description(&self) -> String { + self.args.to_string() + } +} + +impl SymlinkCommand { + fn apply(&self, ctx: &CommandContext) -> Result<()> { let src = expand_path(&self.args.src, Some(&ctx.config_dir)); let target = expand_path(&self.args.target, Some(&ctx.config_dir)); - let use_sudo = self.args.sudo; if !src.exists() { return Err(Error::PathError(format!( @@ -32,141 +47,148 @@ impl CommandExecutor for SymlinkCommand { ))); } - if src.is_file() { - let dest = if target.extension().is_some() || !target.is_dir() { - if let Some(parent) = target.parent() { - mkdir(parent, use_sudo)?; - } - target.clone() - } else { - mkdir(&target, use_sudo)?; - target.join(src.file_name().unwrap()) - }; - create_symlink(&src, &dest, self.args.force, use_sudo, ctx)?; - } else { - mkdir(&target, use_sudo)?; - walk_relative(&src, &target, &self.args.ignore, |entry, dest| { - if entry.file_type().is_dir() { - mkdir(dest, use_sudo) - } else { - create_symlink(entry.path(), dest, self.args.force, use_sudo, ctx) - } - })?; - } - - Ok(()) - } - - async fn update(&self, ctx: &CommandContext) -> Result<()> { - self.install(ctx).await + let ops = fs_ops::select(self.args.sudo); + let force = self.args.force; + tree::install_tree( + ops.as_ref(), + &src, + &target, + &self.args.ignore, + |file, dest| symlink_one(ops.as_ref(), file, dest, force, ctx), + ) } - async fn uninstall(&self, ctx: &CommandContext) -> Result<()> { + fn remove(&self, ctx: &CommandContext) -> Result<()> { let src = expand_path(&self.args.src, Some(&ctx.config_dir)); let target = expand_path(&self.args.target, Some(&ctx.config_dir)); - let use_sudo = self.args.sudo; - - if src.is_file() { - let dest = if target.extension().is_some() || !target.is_dir() { - target - } else { - target.join(src.file_name().unwrap()) - }; - remove_symlink(&dest, use_sudo, ctx)?; - } else { - walk_relative(&src, &target, &self.args.ignore, |entry, dest| { - if entry.file_type().is_file() { - remove_symlink(dest, use_sudo, ctx)?; - } - Ok(()) - })?; - } - - Ok(()) - } - - fn description(&self) -> String { - self.args.to_string() - } -} -fn mkdir(path: &std::path::Path, use_sudo: bool) -> Result<()> { - if path.is_dir() { - return Ok(()); - } - if use_sudo { - sudo::sudo_mkdir(path) - } else { - std::fs::create_dir_all(path)?; - Ok(()) + let ops = fs_ops::select(self.args.sudo); + tree::uninstall_tree(&src, &target, &self.args.ignore, |dest| { + remove_link(ops.as_ref(), dest, ctx) + }) } } -fn create_symlink( - src: &std::path::Path, - dest: &std::path::Path, +/// Create one symlink at `dest` pointing to `src`. When something already +/// exists at `dest`, either replace it (`force`) or skip it. +fn symlink_one( + ops: &dyn FileOps, + src: &Path, + dest: &Path, force: bool, - use_sudo: bool, ctx: &CommandContext, ) -> Result<()> { if dest.exists() || dest.symlink_metadata().is_ok() { if force { ctx.log(format!("Removing existing: {}", dest.display())); - if use_sudo { - if dest.is_dir() { - sudo::sudo_remove_dir(dest)?; - } else { - sudo::sudo_remove(dest)?; - } - } else if dest.is_dir() { - std::fs::remove_dir_all(dest)?; - } else { - std::fs::remove_file(dest)?; - } + ops.remove_path(dest)?; } else { ctx.log(format!("Skipping (already exists): {}", dest.display())); return Ok(()); } } - if let Some(parent) = dest.parent() { - mkdir(parent, use_sudo)?; - } - ctx.log(format!("Symlink: {} -> {}", src.display(), dest.display())); - - if use_sudo { - sudo::sudo_symlink(src, dest) - } else { - #[cfg(unix)] - std::os::unix::fs::symlink(src, dest)?; - - #[cfg(windows)] - { - if src.is_dir() { - std::os::windows::fs::symlink_dir(src, dest)?; - } else { - std::os::windows::fs::symlink_file(src, dest)?; - } - } - - Ok(()) - } + ops.create_symlink(src, dest) } -fn remove_symlink(dest: &std::path::Path, use_sudo: bool, ctx: &CommandContext) -> Result<()> { +/// Remove the symlink an install would have created at `dest`, if present. +fn remove_link(ops: &dyn FileOps, dest: &Path, ctx: &CommandContext) -> Result<()> { if dest.symlink_metadata().is_ok() { ctx.log(format!("Removing symlink: {}", dest.display())); - if use_sudo { - sudo::sudo_remove(dest)?; - } else { - #[cfg(windows)] - if dest.is_dir() { - std::fs::remove_dir(dest)?; - return Ok(()); - } - std::fs::remove_file(dest)?; - } + ops.remove_symlink(dest)?; } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::commands::fs_ops::RecordingFs; + use tempfile::tempdir; + + fn ctx_for( + dir: &Path, + ) -> ( + CommandContext, + tokio::sync::mpsc::UnboundedReceiver, + ) { + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let ctx = CommandContext { + event_tx: tx, + mode: Mode::Install, + config_dir: dir.to_path_buf(), + temp_dir: dir.to_path_buf(), + default_shell: crate::config::types::Shell::Bash, + task_name: "t".to_string(), + depth: 0, + }; + (ctx, rx) + } + + #[test] + fn test_symlink_one_creates_when_absent() { + let dir = tempdir().unwrap(); + let src = dir.path().join("s"); + std::fs::write(&src, b"x").unwrap(); + let dest = dir.path().join("link"); + let (ctx, _rx) = ctx_for(dir.path()); + + let ops = RecordingFs::default(); + symlink_one(&ops, &src, &dest, false, &ctx).unwrap(); + assert_eq!( + ops.calls(), + vec![format!( + "create_symlink {} {}", + src.display(), + dest.display() + )] + ); + } + + #[test] + fn test_symlink_one_skips_existing_without_force() { + let dir = tempdir().unwrap(); + let src = dir.path().join("s"); + std::fs::write(&src, b"x").unwrap(); + let dest = dir.path().join("existing"); + std::fs::write(&dest, b"old").unwrap(); + let (ctx, _rx) = ctx_for(dir.path()); + + let ops = RecordingFs::default(); + symlink_one(&ops, &src, &dest, false, &ctx).unwrap(); + // Skipped: nothing touched. + assert!(ops.calls().is_empty()); + } + + #[test] + fn test_symlink_one_force_removes_then_creates() { + let dir = tempdir().unwrap(); + let src = dir.path().join("s"); + std::fs::write(&src, b"x").unwrap(); + let dest = dir.path().join("existing"); + std::fs::write(&dest, b"old").unwrap(); + let (ctx, _rx) = ctx_for(dir.path()); + + let ops = RecordingFs::default(); + symlink_one(&ops, &src, &dest, true, &ctx).unwrap(); + assert_eq!( + ops.calls(), + vec![ + format!("remove_path {}", dest.display()), + format!("create_symlink {} {}", src.display(), dest.display()), + ] + ); + } + + #[test] + fn test_remove_link_noop_when_absent() { + let dir = tempdir().unwrap(); + let dest = dir.path().join("nope"); + let (ctx, _rx) = ctx_for(dir.path()); + + let ops = RecordingFs::default(); + remove_link(&ops, &dest, &ctx).unwrap(); + assert!(ops.calls().is_empty()); + } +} diff --git a/src/engine/commands/tree.rs b/src/engine/commands/tree.rs new file mode 100644 index 0000000..afa8c22 --- /dev/null +++ b/src/engine/commands/tree.rs @@ -0,0 +1,263 @@ +//! Tree materialization shared by the `copy` and `symlink` executors. +//! +//! Both executors face the same shape: map a source — a single file, or a +//! whole directory tree — onto a target, honoring an ignore list, and either +//! apply an operation per file (install) or undo it per file (uninstall). The +//! only genuine difference between them is what happens to one file (copy its +//! bytes vs. create a symlink), so that is the single parameter callers supply. +//! +//! Destination resolution — the "is the target a file path or a directory to +//! drop the file into" rule — lives here as one pure, table-tested function +//! instead of being copy-pasted across four install/uninstall bodies. + +use std::path::{Path, PathBuf}; + +use crate::error::Result; +use crate::utils::path::walk_relative; + +use super::fs_ops::FileOps; + +/// Where a single source file lands, and which directory (if any) must exist +/// before it is placed there. +#[derive(Debug, PartialEq, Eq)] +pub struct ResolvedDest { + pub dest: PathBuf, + /// Directory to create before writing `dest`, or `None` if not needed. + pub dir_to_create: Option, +} + +/// Decide the destination for mapping a single source *file* onto `target`. +/// +/// If `target` looks like a file path — it has an extension, or it isn't an +/// existing directory — the file lands at `target` itself and `target`'s +/// parent must exist. Otherwise `target` is an existing directory and the file +/// lands inside it under its own file name. +pub fn resolve_single_file_dest(src: &Path, target: &Path) -> ResolvedDest { + if target.extension().is_some() || !target.is_dir() { + ResolvedDest { + dest: target.to_path_buf(), + dir_to_create: target.parent().map(Path::to_path_buf), + } + } else { + let name = src + .file_name() + .expect("source file path always has a final component"); + ResolvedDest { + dest: target.join(name), + dir_to_create: Some(target.to_path_buf()), + } + } +} + +/// Apply an operation across a source onto `target`. +/// +/// For a single file, resolves the destination, ensures the needed directory +/// exists via `ops`, then calls `on_file(src_file, dest)`. For a directory, +/// creates `target` and walks the source (honoring `ignore`), creating +/// directories via `ops` and calling `on_file` for each file. Because WalkDir +/// yields a directory before its contents, every file's parent already exists +/// by the time `on_file` runs. +pub fn install_tree( + ops: &dyn FileOps, + src: &Path, + target: &Path, + ignore: &[String], + mut on_file: F, +) -> Result<()> +where + F: FnMut(&Path, &Path) -> Result<()>, +{ + if src.is_file() { + let resolved = resolve_single_file_dest(src, target); + if let Some(dir) = &resolved.dir_to_create { + ops.mkdir_p(dir)?; + } + on_file(src, &resolved.dest)?; + } else { + ops.mkdir_p(target)?; + walk_relative(src, target, ignore, |entry, dest| { + if entry.file_type().is_dir() { + ops.mkdir_p(dest) + } else { + on_file(entry.path(), dest) + } + })?; + } + Ok(()) +} + +/// Undo an operation across a source onto `target`. +/// +/// For a single file, resolves the destination and calls `on_dest(dest)`. For +/// a directory, walks the source and calls `on_dest` for each file's mapped +/// destination. Directory destinations are left in place — only the files an +/// install would have created are targeted. +pub fn uninstall_tree(src: &Path, target: &Path, ignore: &[String], mut on_dest: F) -> Result<()> +where + F: FnMut(&Path) -> Result<()>, +{ + if src.is_file() { + let dest = resolve_single_file_dest(src, target).dest; + on_dest(&dest)?; + } else { + walk_relative(src, target, ignore, |entry, dest| { + if entry.file_type().is_file() { + on_dest(dest) + } else { + Ok(()) + } + })?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::commands::fs_ops::RecordingFs; + use std::cell::RefCell; + use tempfile::tempdir; + + #[test] + fn test_resolve_dest_target_with_extension_is_file_path() { + // Target has an extension → treated as the file path itself. + let r = resolve_single_file_dest(Path::new("/src/a.txt"), Path::new("/dst/b.txt")); + assert_eq!(r.dest, PathBuf::from("/dst/b.txt")); + assert_eq!(r.dir_to_create, Some(PathBuf::from("/dst"))); + } + + #[test] + fn test_resolve_dest_nonexistent_extensionless_target_is_file_path() { + // No extension and not an existing dir → still treated as a file path. + let r = resolve_single_file_dest(Path::new("/src/a.txt"), Path::new("/dst/newname")); + assert_eq!(r.dest, PathBuf::from("/dst/newname")); + assert_eq!(r.dir_to_create, Some(PathBuf::from("/dst"))); + } + + #[test] + fn test_resolve_dest_existing_dir_target_appends_filename() { + let dir = tempdir().unwrap(); + // An existing, extensionless directory → file drops inside it. + let r = resolve_single_file_dest(Path::new("/src/a.txt"), dir.path()); + assert_eq!(r.dest, dir.path().join("a.txt")); + assert_eq!(r.dir_to_create, Some(dir.path().to_path_buf())); + } + + #[test] + fn test_install_tree_single_file_creates_parent_then_applies() { + let dir = tempdir().unwrap(); + let src = dir.path().join("file.txt"); + std::fs::write(&src, b"data").unwrap(); + let target = dir.path().join("out/file.txt"); + + let ops = RecordingFs::default(); + let applied = RefCell::new(Vec::new()); + install_tree(&ops, &src, &target, &[], |s, d| { + applied + .borrow_mut() + .push(format!("{} -> {}", s.display(), d.display())); + Ok(()) + }) + .unwrap(); + + // Parent dir created via ops; file op invoked once with resolved dest. + assert_eq!( + ops.calls(), + vec![format!("mkdir_p {}", dir.path().join("out").display())] + ); + assert_eq!( + applied.into_inner(), + vec![format!("{} -> {}", src.display(), target.display())] + ); + } + + #[test] + fn test_install_tree_directory_mkdirs_and_applies_per_file() { + let dir = tempdir().unwrap(); + let src = dir.path().join("src"); + std::fs::create_dir_all(src.join("sub")).unwrap(); + std::fs::write(src.join("a.txt"), b"a").unwrap(); + std::fs::write(src.join("sub/b.txt"), b"b").unwrap(); + let target = dir.path().join("dst"); + + let ops = RecordingFs::default(); + let files = RefCell::new(Vec::new()); + install_tree(&ops, &src, &target, &[], |s, _d| { + files + .borrow_mut() + .push(s.file_name().unwrap().to_string_lossy().into_owned()); + Ok(()) + }) + .unwrap(); + + let mut applied = files.into_inner(); + applied.sort(); + assert_eq!(applied, vec!["a.txt", "b.txt"]); + // Directories were created via ops (target + sub at least). + let calls = ops.calls(); + assert!(calls.iter().any(|c| c.contains("dst"))); + assert!(calls.iter().any(|c| c.contains("sub"))); + } + + #[test] + fn test_install_tree_honors_ignore() { + let dir = tempdir().unwrap(); + let src = dir.path().join("src"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::write(src.join("keep.txt"), b"k").unwrap(); + std::fs::write(src.join("README.md"), b"r").unwrap(); + let target = dir.path().join("dst"); + + let ops = RecordingFs::default(); + let files = RefCell::new(Vec::new()); + install_tree(&ops, &src, &target, &["README.md".to_string()], |s, _d| { + files + .borrow_mut() + .push(s.file_name().unwrap().to_string_lossy().into_owned()); + Ok(()) + }) + .unwrap(); + + assert_eq!(files.into_inner(), vec!["keep.txt"]); + } + + #[test] + fn test_uninstall_tree_single_file_targets_resolved_dest() { + let dir = tempdir().unwrap(); + let src = dir.path().join("file.txt"); + std::fs::write(&src, b"data").unwrap(); + let target = dir.path().join("out/file.txt"); + + let removed = RefCell::new(Vec::new()); + uninstall_tree(&src, &target, &[], |d| { + removed.borrow_mut().push(d.display().to_string()); + Ok(()) + }) + .unwrap(); + + assert_eq!(removed.into_inner(), vec![target.display().to_string()]); + } + + #[test] + fn test_uninstall_tree_directory_targets_files_only() { + let dir = tempdir().unwrap(); + let src = dir.path().join("src"); + std::fs::create_dir_all(src.join("sub")).unwrap(); + std::fs::write(src.join("a.txt"), b"a").unwrap(); + std::fs::write(src.join("sub/b.txt"), b"b").unwrap(); + let target = dir.path().join("dst"); + + let removed = RefCell::new(Vec::new()); + uninstall_tree(&src, &target, &[], |d| { + removed + .borrow_mut() + .push(d.file_name().unwrap().to_string_lossy().into_owned()); + Ok(()) + }) + .unwrap(); + + let mut got = removed.into_inner(); + got.sort(); + assert_eq!(got, vec!["a.txt", "b.txt"]); + } +} diff --git a/src/engine/context.rs b/src/engine/context.rs index 9fd71f2..7ac465c 100644 --- a/src/engine/context.rs +++ b/src/engine/context.rs @@ -1,10 +1,10 @@ use std::path::PathBuf; use tokio::sync::mpsc; -use crate::cli::Command; use crate::config::types::Shell; use super::event::TaskEvent; +use super::mode::Mode; /// Context passed to each command during execution. #[derive(Clone)] @@ -13,7 +13,7 @@ pub struct CommandContext { pub event_tx: mpsc::UnboundedSender, /// Current execution mode (install/update/uninstall). - pub mode: Command, + pub mode: Mode, /// Directory where the config file is located (for resolving relative paths). pub config_dir: PathBuf, diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 91fee66..f945476 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -1,4 +1,5 @@ pub mod commands; pub mod context; pub mod event; +pub mod mode; pub mod runner; diff --git a/src/engine/mode.rs b/src/engine/mode.rs new file mode 100644 index 0000000..d2200ed --- /dev/null +++ b/src/engine/mode.rs @@ -0,0 +1,72 @@ +use crate::cli::Command; + +/// The execution modes the engine actually acts on. +/// +/// Distinct from the CLI [`Command`], which also carries non-execution verbs +/// (`list`, `validate`, `completions`) that never reach the engine. Keeping +/// `Mode` separate means every match over an execution mode is exhaustive in +/// three real arms instead of carrying dead arms for verbs that can't occur. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Mode { + Install, + Update, + Uninstall, +} + +impl Mode { + /// Map a CLI command to an execution mode. Returns `None` for verbs that + /// don't drive task execution (`list`, `validate`, `completions`); the + /// caller handles those before the engine is ever constructed. + pub fn from_command(command: &Command) -> Option { + match command { + Command::Install => Some(Mode::Install), + Command::Update => Some(Mode::Update), + Command::Uninstall => Some(Mode::Uninstall), + Command::List | Command::Validate | Command::Completions { .. } => None, + } + } +} + +impl std::fmt::Display for Mode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Mode::Install => write!(f, "install"), + Mode::Update => write!(f, "update"), + Mode::Uninstall => write!(f, "uninstall"), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_from_command_execution_verbs() { + assert_eq!(Mode::from_command(&Command::Install), Some(Mode::Install)); + assert_eq!(Mode::from_command(&Command::Update), Some(Mode::Update)); + assert_eq!( + Mode::from_command(&Command::Uninstall), + Some(Mode::Uninstall) + ); + } + + #[test] + fn test_from_command_non_execution_verbs() { + assert_eq!(Mode::from_command(&Command::List), None); + assert_eq!(Mode::from_command(&Command::Validate), None); + assert_eq!( + Mode::from_command(&Command::Completions { + shell: clap_complete::Shell::Bash + }), + None + ); + } + + #[test] + fn test_display() { + assert_eq!(Mode::Install.to_string(), "install"); + assert_eq!(Mode::Update.to_string(), "update"); + assert_eq!(Mode::Uninstall.to_string(), "uninstall"); + } +} diff --git a/src/engine/runner.rs b/src/engine/runner.rs index b30b4cd..8decccb 100644 --- a/src/engine/runner.rs +++ b/src/engine/runner.rs @@ -1,9 +1,7 @@ -use std::borrow::Cow; -use std::collections::{HashMap, HashSet, VecDeque}; use std::path::{Path, PathBuf}; use tokio::sync::mpsc; -use crate::cli::Command; +use crate::config::graph::TaskGraph; use crate::config::history::History; use crate::config::types::{AppConfig, TaskConfig}; use crate::error::{Error, Result}; @@ -12,21 +10,26 @@ use crate::utils::path::expand_path; use super::commands::{create_executor, CommandExecutor}; use super::context::CommandContext; use super::event::TaskEvent; +use super::mode::Mode; pub struct TaskRunner { config: AppConfig, - mode: Command, + mode: Mode, event_tx: mpsc::UnboundedSender, config_dir: PathBuf, depth: usize, } +/// Running counts of task outcomes across all layers of a run. +#[derive(Default)] +struct Tally { + succeeded: usize, + failed: usize, + skipped: usize, +} + impl TaskRunner { - pub fn new( - config: AppConfig, - mode: Command, - event_tx: mpsc::UnboundedSender, - ) -> Self { + pub fn new(config: AppConfig, mode: Mode, event_tx: mpsc::UnboundedSender) -> Self { Self { config, mode, @@ -62,99 +65,28 @@ impl TaskRunner { /// Run specific tasks by name. pub async fn run_tasks(&self, task_names: &[String], force: bool) -> Result<()> { - // Resolve dependency order via topological sort. When no task has + // Resolve dependency order via the task graph. When no task has // dependencies, this borrows `task_names` instead of cloning it. - let ordered = self.topological_sort(task_names)?; + let graph = TaskGraph::new(&self.config.tasks); + let ordered = graph.topo_order(task_names)?; let ordered: &[String] = &ordered; let temp_dir = expand_path(&self.config.temp_dir, None); let mut history = History::load(&temp_dir).unwrap_or_default(); - let mut succeeded = 0usize; - let mut failed = 0usize; - let mut skipped = 0usize; - - if self.config.parallel { - // Parallel execution with dependency layers - let layers = self.dependency_layers(ordered); - - for layer in layers { - let mut handles = Vec::new(); - - for name in &layer { - let task_config = &self.config.tasks[name]; - - if let Some(reason) = self.should_skip(task_config, name, force, &history) { - self.send(TaskEvent::TaskSkipped { - task_name: name.clone(), - reason, - }); - skipped += 1; - continue; - } - - let ctx = self.create_context(name, &temp_dir); - let task = task_config.clone(); - let mode = self.mode.clone(); - let depth = self.depth; - let name = name.clone(); - handles.push(tokio::spawn(async move { - let result = run_task_with_retry(&name, &task, &ctx, mode, depth).await; - (name, result) - })); - } - - for handle in handles { - match handle.await { - Ok((name, Ok(()))) => { - self.update_history(&mut history, &name); - succeeded += 1; - } - Ok((name, Err(e))) => { - self.send(TaskEvent::TaskFailed { - task_name: name, - error: e.to_string(), - }); - failed += 1; - } - Err(_) => { - failed += 1; - } - } - } - } + // Both execution modes are the same loop over layers; sequential is the + // degenerate case where each task is its own layer (so the join below + // runs them one at a time, in dependency order). + let layers: Vec> = if self.config.parallel { + graph.layers(ordered) } else { - // Sequential execution - for name in ordered { - let task_config = &self.config.tasks[name]; - - if let Some(reason) = self.should_skip(task_config, name, force, &history) { - self.send(TaskEvent::TaskSkipped { - task_name: name.clone(), - reason, - }); - skipped += 1; - continue; - } + ordered.iter().map(|name| vec![name.clone()]).collect() + }; - let ctx = self.create_context(name, &temp_dir); - - match run_task_with_retry(name, task_config, &ctx, self.mode.clone(), self.depth) - .await - { - Ok(()) => { - self.update_history(&mut history, name); - succeeded += 1; - } - Err(e) => { - self.send(TaskEvent::TaskFailed { - task_name: name.clone(), - error: e.to_string(), - }); - failed += 1; - } - } - } + let mut tally = Tally::default(); + for layer in &layers { + self.run_layer(layer, force, &temp_dir, &mut history, &mut tally) + .await; } // Save history @@ -163,18 +95,73 @@ impl TaskRunner { } self.send(TaskEvent::AllDone { - succeeded, - failed, - skipped, + succeeded: tally.succeeded, + failed: tally.failed, + skipped: tally.skipped, }); - if failed > 0 { - Err(Error::Other(format!("{failed} task(s) failed"))) + if tally.failed > 0 { + Err(Error::Other(format!("{} task(s) failed", tally.failed))) } else { Ok(()) } } + /// Run one dependency layer: skip what should be skipped, spawn the rest, + /// then join — recording each task's outcome into `tally` and history. A + /// layer of one task (sequential mode) runs that task to completion before + /// the caller advances to the next layer. + async fn run_layer( + &self, + layer: &[String], + force: bool, + temp_dir: &Path, + history: &mut History, + tally: &mut Tally, + ) { + let mut handles = Vec::new(); + + for name in layer { + let task_config = &self.config.tasks[name]; + + if let Some(reason) = self.should_skip(task_config, name, force, history) { + self.send(TaskEvent::TaskSkipped { + task_name: name.clone(), + reason, + }); + tally.skipped += 1; + continue; + } + + let ctx = self.create_context(name, temp_dir); + let task = task_config.clone(); + let name = name.clone(); + handles.push(tokio::spawn(async move { + let result = run_task_with_retry(&name, &task, &ctx).await; + (name, result) + })); + } + + for handle in handles { + match handle.await { + Ok((name, Ok(()))) => { + self.update_history(history, &name); + tally.succeeded += 1; + } + Ok((name, Err(e))) => { + self.send(TaskEvent::TaskFailed { + task_name: name, + error: e.to_string(), + }); + tally.failed += 1; + } + Err(_) => { + tally.failed += 1; + } + } + } + } + /// Check if a task should be skipped (OS filter, conditions, history). fn should_skip( &self, @@ -205,129 +192,13 @@ impl TaskRunner { } // Check history - if self.mode == Command::Install && !force && history.is_installed(name) { + if self.mode == Mode::Install && !force && history.is_installed(name) { return Some("Already installed (use --force to reinstall)".to_string()); } None } - /// Topological sort of tasks respecting depends_on. - /// Returns tasks in dependency order. Includes transitive dependencies - /// of the requested tasks. Borrows the input when no sorting is needed. - fn topological_sort<'a>(&self, requested: &'a [String]) -> Result> { - let all_tasks = &self.config.tasks; - - // If no task has dependencies, preserve original order - let has_deps = requested - .iter() - .filter_map(|n| all_tasks.get(n)) - .any(|t| !t.depends_on.is_empty()); - if !has_deps { - return Ok(Cow::Borrowed(requested)); - } - - // Collect all needed tasks (requested + transitive deps) - let mut needed: HashSet = HashSet::new(); - let mut queue: VecDeque = requested.iter().cloned().collect(); - while let Some(name) = queue.pop_front() { - if needed.contains(&name) { - continue; - } - if let Some(task) = all_tasks.get(&name) { - needed.insert(name.clone()); - for dep in &task.depends_on { - if !all_tasks.contains_key(dep) { - return Err(Error::MissingDependency(name.clone(), dep.clone())); - } - if !needed.contains(dep) { - queue.push_back(dep.clone()); - } - } - } - } - - // Build in-degree map for needed tasks - let mut in_degree: HashMap<&str, usize> = HashMap::new(); - let mut dependents: HashMap<&str, Vec<&str>> = HashMap::new(); - for name in &needed { - in_degree.entry(name.as_str()).or_insert(0); - if let Some(task) = all_tasks.get(name) { - for dep in &task.depends_on { - if needed.contains(dep) { - *in_degree.entry(name.as_str()).or_insert(0) += 1; - dependents - .entry(dep.as_str()) - .or_default() - .push(name.as_str()); - } - } - } - } - - // Kahn's algorithm - let mut queue: VecDeque<&str> = in_degree - .iter() - .filter(|(_, °)| deg == 0) - .map(|(&name, _)| name) - .collect(); - let mut sorted = Vec::new(); - - while let Some(node) = queue.pop_front() { - sorted.push(node.to_string()); - if let Some(deps) = dependents.get(node) { - for &dep in deps { - if let Some(deg) = in_degree.get_mut(dep) { - *deg -= 1; - if *deg == 0 { - queue.push_back(dep); - } - } - } - } - } - - if sorted.len() != needed.len() { - let remaining: Vec = needed - .iter() - .filter(|n| !sorted.contains(n)) - .cloned() - .collect(); - return Err(Error::CyclicDependency(remaining.join(", "))); - } - - Ok(Cow::Owned(sorted)) - } - - /// Group tasks into dependency layers for parallel execution. - /// Tasks in the same layer have no dependencies on each other. - fn dependency_layers(&self, ordered: &[String]) -> Vec> { - let all_tasks = &self.config.tasks; - let mut layers: Vec> = Vec::new(); - let mut task_layer: HashMap<&str, usize> = HashMap::new(); - - for name in ordered { - let layer = if let Some(task) = all_tasks.get(name) { - task.depends_on - .iter() - .filter_map(|dep| task_layer.get(dep.as_str())) - .max() - .map(|&l| l + 1) - .unwrap_or(0) - } else { - 0 - }; - - task_layer.insert(name.as_str(), layer); - while layers.len() <= layer { - layers.push(Vec::new()); - } - layers[layer].push(name.clone()); - } - - layers - } - /// Get ordered task names (for list command / TUI display). #[allow(dead_code)] pub fn task_names(&self) -> Vec { @@ -343,7 +214,7 @@ impl TaskRunner { fn create_context(&self, task_name: &str, temp_dir: &Path) -> CommandContext { CommandContext { event_tx: self.event_tx.clone(), - mode: self.mode.clone(), + mode: self.mode, config_dir: self.config_dir.clone(), temp_dir: temp_dir.to_path_buf(), default_shell: self.config.default_shell.clone(), @@ -354,10 +225,9 @@ impl TaskRunner { fn update_history(&self, history: &mut History, task_name: &str) { match self.mode { - Command::Install => history.mark_installed(task_name), - Command::Update => history.mark_updated(task_name), - Command::Uninstall => history.mark_uninstalled(task_name), - Command::List | Command::Validate | Command::Completions { .. } => {} + Mode::Install => history.mark_installed(task_name), + Mode::Update => history.mark_updated(task_name), + Mode::Uninstall => history.mark_uninstalled(task_name), } } @@ -367,17 +237,11 @@ impl TaskRunner { } /// Run a task with retry support. -async fn run_task_with_retry( - name: &str, - task: &TaskConfig, - ctx: &CommandContext, - mode: Command, - depth: usize, -) -> Result<()> { +async fn run_task_with_retry(name: &str, task: &TaskConfig, ctx: &CommandContext) -> Result<()> { let max_attempts = task.retry + 1; for attempt in 1..=max_attempts { - match run_task(name, task, ctx, mode.clone(), depth).await { + match run_task(name, task, ctx).await { Ok(()) => return Ok(()), Err(e) if attempt < max_attempts => { let _ = ctx.event_tx.send(TaskEvent::TaskRetry { @@ -395,17 +259,11 @@ async fn run_task_with_retry( unreachable!() } -async fn run_task( - name: &str, - task: &TaskConfig, - ctx: &CommandContext, - mode: Command, - depth: usize, -) -> Result<()> { +async fn run_task(name: &str, task: &TaskConfig, ctx: &CommandContext) -> Result<()> { let _ = ctx.event_tx.send(TaskEvent::TaskStarted { task_name: name.to_string(), command_count: task.commands.len(), - depth, + depth: ctx.depth, }); let executors: Vec> = @@ -417,10 +275,7 @@ async fn run_task( for executor in executors { let ctx = ctx.clone(); - let mode = mode.clone(); - handles.push(tokio::spawn(async move { - run_command(executor.as_ref(), &ctx, mode).await - })); + handles.push(tokio::spawn(async move { executor.execute(&ctx).await })); } for handle in handles { @@ -435,7 +290,7 @@ async fn run_task( command_desc: desc.clone(), }); - match run_command(executor.as_ref(), ctx, mode.clone()).await { + match executor.execute(ctx).await { Ok(()) => { let _ = ctx.event_tx.send(TaskEvent::CommandCompleted { task_name: name.to_string(), @@ -460,16 +315,3 @@ async fn run_task( Ok(()) } - -async fn run_command( - executor: &dyn CommandExecutor, - ctx: &CommandContext, - mode: Command, -) -> Result<()> { - match mode { - Command::Install => executor.install(ctx).await, - Command::Update => executor.update(ctx).await, - Command::Uninstall => executor.uninstall(ctx).await, - Command::List | Command::Validate | Command::Completions { .. } => Ok(()), - } -} diff --git a/src/main.rs b/src/main.rs index 832bf45..e2970e0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,6 +7,7 @@ use tokio_util::sync::CancellationToken; use cli::{Cli, Command}; use engine::event::TaskEvent; +use engine::mode::Mode; use engine::runner::TaskRunner; #[tokio::main] @@ -84,9 +85,13 @@ async fn main() -> anyhow::Result<()> { pre_authenticate_sudo(); } + // All non-execution verbs (list/validate/completions) returned above, so + // the remaining command always maps to an execution mode. + let mode = Mode::from_command(&cli.command) + .expect("list/validate/completions are handled before this point"); + // Set up runner (moves app_config) - let runner = - TaskRunner::new(app_config, cli.command.clone(), event_tx).with_config_dir(config_dir); + let runner = TaskRunner::new(app_config, mode, event_tx).with_config_dir(config_dir); let force = cli.force; let task_names_clone = task_names.clone(); @@ -103,7 +108,7 @@ async fn main() -> anyhow::Result<()> { }); // Run TUI in foreground (blocks until user quits) - tui::run(event_rx, task_names, cli.command, cancel).await?; + tui::run(event_rx, task_names, mode, cancel).await?; // Abort engine if still running engine_handle.abort(); diff --git a/src/tui/app.rs b/src/tui/app.rs index 8e41f9a..0bd7328 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -1,5 +1,5 @@ -use crate::cli::Command; use crate::engine::event::TaskEvent; +use crate::engine::mode::Mode; /// Status of a task in the TUI. #[derive(Debug, Clone, PartialEq, Eq)] @@ -61,7 +61,7 @@ impl TaskState { pub struct App { pub tasks: Vec, pub selected: usize, - pub mode: Command, + pub mode: Mode, pub log_scroll: usize, pub done: bool, pub succeeded: usize, @@ -78,7 +78,7 @@ pub struct App { } impl App { - pub fn new(task_names: Vec, mode: Command) -> Self { + pub fn new(task_names: Vec, mode: Mode) -> Self { let filtered_indices: Vec = (0..task_names.len()).collect(); let tasks = task_names.into_iter().map(TaskState::new).collect(); Self { diff --git a/src/tui/mod.rs b/src/tui/mod.rs index d2c8e66..0280435 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -28,7 +28,7 @@ fn restore_terminal() { pub async fn run( mut event_rx: mpsc::UnboundedReceiver, task_names: Vec, - mode: crate::cli::Command, + mode: crate::engine::mode::Mode, cancel: CancellationToken, ) -> anyhow::Result<()> { // Install panic hook that restores the terminal diff --git a/tests/integration.rs b/tests/integration.rs index fcbbb9c..efd61c1 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -2,13 +2,13 @@ use std::fs; use tempfile::tempdir; use tokio::sync::mpsc; -use machine_setup::cli::Command; use machine_setup::config; use machine_setup::engine::event::TaskEvent; +use machine_setup::engine::mode::Mode; use machine_setup::engine::runner::TaskRunner; /// Helper: run a config string and collect all events. -async fn run_config(yaml: &str, mode: Command) -> Vec { +async fn run_config(yaml: &str, mode: Mode) -> Vec { let dir = tempdir().unwrap(); let config_path = dir.path().join("config.yaml"); fs::write(&config_path, yaml).unwrap(); @@ -79,7 +79,7 @@ tasks: - run: commands: "echo hello_world" "#, - Command::Install, + Mode::Install, ) .await; @@ -99,7 +99,7 @@ tasks: - "echo line_one" - "echo line_two" "#, - Command::Install, + Mode::Install, ) .await; @@ -120,7 +120,7 @@ tasks: update: "echo updating" uninstall: "echo removing" "#, - Command::Install, + Mode::Install, ) .await; @@ -140,7 +140,7 @@ tasks: install: "echo installing" update: "echo updating" "#, - Command::Update, + Mode::Update, ) .await; @@ -160,7 +160,7 @@ tasks: MY_VAR: "test_value_123" commands: "echo $MY_VAR" "#, - Command::Install, + Mode::Install, ) .await; @@ -187,7 +187,7 @@ tasks: - run: commands: "exit 1" "#, - Command::Install, + Mode::Install, ) .await; @@ -215,7 +215,7 @@ tasks: commands: "echo should_not_run" "# ), - Command::Install, + Mode::Install, ) .await; @@ -244,7 +244,7 @@ tasks: commands: "echo correct_os" "# ), - Command::Install, + Mode::Install, ) .await; @@ -283,7 +283,7 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, _rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; // Verify file was copied @@ -326,7 +326,7 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, _rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; assert!(target_dir.join("keep.txt").exists()); @@ -364,7 +364,7 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, _rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; let link = target_dir.join("dotfile"); @@ -395,14 +395,14 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx1, _rx1) = mpsc::unbounded_channel(); let runner1 = - TaskRunner::new(config, Command::Install, tx1).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx1).with_config_dir(dir.path().to_path_buf()); let _ = runner1.run_all(false).await; // Second run (should skip) let config2 = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx2, mut rx2) = mpsc::unbounded_channel(); let runner2 = - TaskRunner::new(config2, Command::Install, tx2).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config2, Mode::Install, tx2).with_config_dir(dir.path().to_path_buf()); let _ = runner2.run_all(false).await; tokio::time::sleep(std::time::Duration::from_millis(50)).await; @@ -437,14 +437,14 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx1, _rx1) = mpsc::unbounded_channel(); let runner1 = - TaskRunner::new(config, Command::Install, tx1).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx1).with_config_dir(dir.path().to_path_buf()); let _ = runner1.run_all(false).await; // Second run with force let config2 = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx2, mut rx2) = mpsc::unbounded_channel(); let runner2 = - TaskRunner::new(config2, Command::Install, tx2).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config2, Mode::Install, tx2).with_config_dir(dir.path().to_path_buf()); let _ = runner2.run_all(true).await; tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -475,7 +475,7 @@ tasks: - run: commands: "echo task_b" "#, - Command::Install, + Mode::Install, ) .await; @@ -496,7 +496,7 @@ tasks: - run: commands: "echo cmd_2" "#, - Command::Install, + Mode::Install, ) .await; @@ -541,7 +541,7 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, mut rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; let mut events = Vec::new(); @@ -567,7 +567,7 @@ async fn test_json_config() { let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, mut rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; let mut events = Vec::new(); @@ -602,7 +602,7 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, mut rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; let mut events = Vec::new(); @@ -697,7 +697,7 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, mut rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -721,7 +721,7 @@ tasks: - run: commands: "echo should_not_run" "#, - Command::Install, + Mode::Install, ) .await; @@ -755,7 +755,7 @@ tasks: let config = config::load_config(config_path.to_str().unwrap()).unwrap(); let (tx, mut rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -778,7 +778,7 @@ tasks: - run: commands: "echo skip_if_ran" "#, - Command::Install, + Mode::Install, ) .await; @@ -803,7 +803,7 @@ tasks: - run: commands: "echo first_task" "#, - Command::Install, + Mode::Install, ) .await; @@ -842,7 +842,7 @@ tasks: - run: commands: "echo task_a" "#, - Command::Install, + Mode::Install, ) .await; @@ -959,7 +959,7 @@ tasks: let (tx, mut rx) = mpsc::unbounded_channel(); let runner = - TaskRunner::new(config, Command::Install, tx).with_config_dir(dir.path().to_path_buf()); + TaskRunner::new(config, Mode::Install, tx).with_config_dir(dir.path().to_path_buf()); let _ = runner.run_all(true).await; tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -987,7 +987,7 @@ tasks: - run: commands: "exit 1" "#, - Command::Install, + Mode::Install, ) .await; @@ -1008,7 +1008,7 @@ tasks: - run: commands: "exit 1" "#, - Command::Install, + Mode::Install, ) .await; @@ -1018,3 +1018,239 @@ tasks: .any(|e| matches!(e, TaskEvent::TaskRetry { .. }))); assert!(task_failed(&events, "no_retry")); } + +// ─── Copy/symlink lifecycle tests (install → uninstall) ─── + +/// Run a config at `config_path` once in the given mode, against `base_dir`. +async fn run_at(config_path: &std::path::Path, base_dir: &std::path::Path, mode: Mode) { + let config = config::load_config(config_path.to_str().unwrap()).unwrap(); + let (tx, _rx) = mpsc::unbounded_channel(); + let runner = TaskRunner::new(config, mode, tx).with_config_dir(base_dir.to_path_buf()); + let _ = runner.run_all(true).await; +} + +#[tokio::test] +async fn test_copy_uninstall_removes_copied_files() { + let dir = tempdir().unwrap(); + let src_dir = dir.path().join("source"); + let target_dir = dir.path().join("target"); + fs::create_dir_all(src_dir.join("nested")).unwrap(); + fs::write(src_dir.join("a.txt"), "a").unwrap(); + fs::write(src_dir.join("nested/b.txt"), "b").unwrap(); + + let config_path = dir.path().join("config.yaml"); + fs::write( + &config_path, + format!( + r#" +tasks: + copy_task: + commands: + - copy: + src: "{}" + target: "{}" +"#, + src_dir.to_string_lossy().replace('\\', "/"), + target_dir.to_string_lossy().replace('\\', "/"), + ), + ) + .unwrap(); + + run_at(&config_path, dir.path(), Mode::Install).await; + assert!(target_dir.join("a.txt").exists()); + assert!(target_dir.join("nested/b.txt").exists()); + + run_at(&config_path, dir.path(), Mode::Uninstall).await; + // Copied files are removed; the source is untouched. + assert!(!target_dir.join("a.txt").exists()); + assert!(!target_dir.join("nested/b.txt").exists()); + assert!(src_dir.join("a.txt").exists()); +} + +#[tokio::test] +async fn test_symlink_uninstall_removes_link_keeps_source() { + let dir = tempdir().unwrap(); + let src_dir = dir.path().join("source"); + let target_dir = dir.path().join("target"); + fs::create_dir_all(&src_dir).unwrap(); + fs::write(src_dir.join("dotfile"), "content").unwrap(); + + let config_path = dir.path().join("config.yaml"); + fs::write( + &config_path, + format!( + r#" +tasks: + link_task: + commands: + - symlink: + src: "{}" + target: "{}" +"#, + src_dir.to_string_lossy().replace('\\', "/"), + target_dir.to_string_lossy().replace('\\', "/"), + ), + ) + .unwrap(); + + run_at(&config_path, dir.path(), Mode::Install).await; + let link = target_dir.join("dotfile"); + assert!(link.symlink_metadata().is_ok()); + + run_at(&config_path, dir.path(), Mode::Uninstall).await; + assert!(link.symlink_metadata().is_err()); + // Source file survives. + assert!(src_dir.join("dotfile").exists()); +} + +#[tokio::test] +async fn test_symlink_force_overwrites_existing_file() { + let dir = tempdir().unwrap(); + let src_dir = dir.path().join("source"); + let target_dir = dir.path().join("target"); + fs::create_dir_all(&src_dir).unwrap(); + fs::create_dir_all(&target_dir).unwrap(); + fs::write(src_dir.join("dotfile"), "from-source").unwrap(); + // A real file already sits where the symlink should go. + fs::write(target_dir.join("dotfile"), "pre-existing").unwrap(); + + let config_path = dir.path().join("config.yaml"); + fs::write( + &config_path, + format!( + r#" +tasks: + link_task: + commands: + - symlink: + src: "{}" + target: "{}" + force: true +"#, + src_dir.to_string_lossy().replace('\\', "/"), + target_dir.to_string_lossy().replace('\\', "/"), + ), + ) + .unwrap(); + + run_at(&config_path, dir.path(), Mode::Install).await; + + let link = target_dir.join("dotfile"); + let meta = link.symlink_metadata().unwrap(); + assert!( + meta.file_type().is_symlink(), + "force should replace the file with a symlink" + ); + // Reading through the link yields the source content. + assert_eq!(fs::read_to_string(&link).unwrap(), "from-source"); +} + +#[tokio::test] +async fn test_symlink_without_force_skips_existing_file() { + let dir = tempdir().unwrap(); + let src_dir = dir.path().join("source"); + let target_dir = dir.path().join("target"); + fs::create_dir_all(&src_dir).unwrap(); + fs::create_dir_all(&target_dir).unwrap(); + fs::write(src_dir.join("dotfile"), "from-source").unwrap(); + fs::write(target_dir.join("dotfile"), "pre-existing").unwrap(); + + let config_path = dir.path().join("config.yaml"); + fs::write( + &config_path, + format!( + r#" +tasks: + link_task: + commands: + - symlink: + src: "{}" + target: "{}" +"#, + src_dir.to_string_lossy().replace('\\', "/"), + target_dir.to_string_lossy().replace('\\', "/"), + ), + ) + .unwrap(); + + run_at(&config_path, dir.path(), Mode::Install).await; + + // Without force, the pre-existing real file is left in place. + let link = target_dir.join("dotfile"); + assert!(!link.symlink_metadata().unwrap().file_type().is_symlink()); + assert_eq!(fs::read_to_string(&link).unwrap(), "pre-existing"); +} + +#[tokio::test] +async fn test_parallel_respects_dependency_layers() { + // parallel: true must still honor depends_on: the dependency completes + // before the dependent starts, exercising the graph's layering. + let events = run_config( + r#" +parallel: true +tasks: + dependent: + depends_on: ["base"] + commands: + - run: + commands: "echo dependent_task" + base: + commands: + - run: + commands: "echo base_task" +"#, + Mode::Install, + ) + .await; + + assert!(task_completed(&events, "base")); + assert!(task_completed(&events, "dependent")); + + let base_done = events + .iter() + .position(|e| matches!(e, TaskEvent::TaskCompleted { task_name } if task_name == "base")) + .unwrap(); + let dependent_start = events + .iter() + .position( + |e| matches!(e, TaskEvent::TaskStarted { task_name, .. } if task_name == "dependent"), + ) + .unwrap(); + assert!(base_done < dependent_start); +} + +#[tokio::test] +async fn test_copy_single_file_into_directory_target() { + // A single source file with an existing directory target lands inside the + // directory under its own name (the dest-resolution rule). + let dir = tempdir().unwrap(); + let src_file = dir.path().join("config.toml"); + fs::write(&src_file, "x = 1").unwrap(); + let target_dir = dir.path().join("dest_dir"); + fs::create_dir_all(&target_dir).unwrap(); + + let config_path = dir.path().join("config.yaml"); + fs::write( + &config_path, + format!( + r#" +tasks: + copy_one: + commands: + - copy: + src: "{}" + target: "{}" +"#, + src_file.to_string_lossy().replace('\\', "/"), + target_dir.to_string_lossy().replace('\\', "/"), + ), + ) + .unwrap(); + + run_at(&config_path, dir.path(), Mode::Install).await; + assert!(target_dir.join("config.toml").exists()); + assert_eq!( + fs::read_to_string(target_dir.join("config.toml")).unwrap(), + "x = 1" + ); +}