Rust performance and maintainability wins

src/config/mod.rs

@@ -3,11 +3,26 @@ pub mod os;
 pub mod types;
 pub mod validate;
 
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use crate::error::{Error, Result};
 use types::AppConfig;
 
+/// Return the directory that relative paths inside a config file should be
+/// resolved against. For local configs this is the config file's parent
+/// directory (after canonicalizing); for URLs and unresolvable paths we fall
+/// back to `fallback`.
+pub fn resolve_config_dir(path_or_url: &str, fallback: &Path) -> PathBuf {
+    if is_url(path_or_url) {
+        return fallback.to_path_buf();
+    }
+    Path::new(path_or_url)
+        .canonicalize()
+        .ok()
+        .and_then(|p| p.parent().map(Path::to_path_buf))
+        .unwrap_or_else(|| fallback.to_path_buf())
+}
+
 /// Load a config from a local path or URL, auto-detecting format from extension.
 /// If the path has no extension, tries `.yaml`, `.yml`, then `.json`.
 ///

src/config/types.rs

@@ -150,30 +150,60 @@ impl<'de> Deserialize<'de> for CommandEntry {
 impl std::fmt::Display for CommandEntry {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            CommandEntry::Copy(args) => write!(f, "copy: {} -> {}", args.src, args.target),
-            CommandEntry::Symlink(args) => write!(f, "symlink: {} -> {}", args.src, args.target),
-            CommandEntry::Clone(args) => write!(f, "clone: {} -> {}", args.url, args.target),
-            CommandEntry::Run(args) => {
-                let cmds = args.all_command_strings();
-                if cmds.is_empty() {
-                    write!(f, "run: (no commands)")
-                } else if cmds.len() == 1 {
-                    write!(f, "run: {}", cmds[0])
-                } else {
-                    write!(f, "run: {} commands", cmds.len())
-                }
-            }
-            CommandEntry::MachineSetup(args) => {
-                write!(f, "machine_setup: {}", args.config)?;
-                if let Some(task) = &args.task {
-                    write!(f, " (task: {task})")?;
-                }
-                Ok(())
-            }
+            CommandEntry::Copy(args) => args.fmt(f),
+            CommandEntry::Symlink(args) => args.fmt(f),
+            CommandEntry::Clone(args) => args.fmt(f),
+            CommandEntry::Run(args) => args.fmt(f),
+            CommandEntry::MachineSetup(args) => args.fmt(f),
+        }
+    }
+}
+
+impl std::fmt::Display for CopyArgs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let prefix = if self.sudo { "copy (sudo)" } else { "copy" };
+        write!(f, "{prefix}: {} -> {}", self.src, self.target)
+    }
+}
+
+impl std::fmt::Display for SymlinkArgs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let prefix = if self.sudo {
+            "symlink (sudo)"
+        } else {
+            "symlink"
+        };
+        write!(f, "{prefix}: {} -> {}", self.src, self.target)
+    }
+}
+
+impl std::fmt::Display for CloneArgs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "clone: {} -> {}", self.url, self.target)
+    }
+}
+
+impl std::fmt::Display for RunArgs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut iter = self.all_command_strings();
+        match (iter.next(), iter.next()) {
+            (None, _) => write!(f, "run: (no commands)"),
+            (Some(c), None) => write!(f, "run: {c}"),
+            (Some(_), Some(_)) => write!(f, "run: {} commands", 2 + iter.count()),
         }
     }
 }
 
+impl std::fmt::Display for MachineSetupArgs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "machine_setup: {}", self.config)?;
+        if let Some(task) = &self.task {
+            write!(f, " (task: {task})")?;
+        }
+        Ok(())
+    }
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct CopyArgs {
     pub src: String,
@@ -230,14 +260,17 @@ pub struct RunArgs {
 }
 
 impl RunArgs {
-    /// Get all command strings regardless of mode (for display purposes).
-    pub fn all_command_strings(&self) -> Vec<&str> {
-        let mut cmds = Vec::new();
-        cmds.extend(self.commands.as_slice().iter().map(|s| s.as_str()));
-        cmds.extend(self.install.as_slice().iter().map(|s| s.as_str()));
-        cmds.extend(self.update.as_slice().iter().map(|s| s.as_str()));
-        cmds.extend(self.uninstall.as_slice().iter().map(|s| s.as_str()));
-        cmds
+    /// Iterate all command strings regardless of mode (for display purposes).
+    /// Returns an iterator so callers that only need count/first/is_empty
+    /// don't force an intermediate Vec allocation.
+    pub fn all_command_strings(&self) -> impl Iterator<Item = &str> {
+        self.commands
+            .as_slice()
+            .iter()
+            .chain(self.install.as_slice().iter())
+            .chain(self.update.as_slice().iter())
+            .chain(self.uninstall.as_slice().iter())
+            .map(|s| s.as_str())
     }
 
     /// Get commands for a specific mode.
@@ -310,15 +343,16 @@ impl<'de> Deserialize<'de> for StringOrVec {
 impl AppConfig {
     /// Check if any commands in the selected tasks require sudo.
     pub fn requires_sudo(&self, task_names: &[String]) -> bool {
+        let selected: std::collections::HashSet<&str> =
+            task_names.iter().map(String::as_str).collect();
         self.tasks
             .iter()
-            .filter(|(name, _)| task_names.iter().any(|t| t == *name))
+            .filter(|(name, _)| selected.contains(name.as_str()))
             .any(|(_, task)| {
                 task.commands.iter().any(|cmd| match cmd {
-                    CommandEntry::Run(args) => args
-                        .all_command_strings()
-                        .iter()
-                        .any(|s| s.contains("sudo")),
+                    CommandEntry::Run(args) => {
+                        args.all_command_strings().any(|s| s.contains("sudo"))
+                    }
                     CommandEntry::Copy(args) => args.sudo,
                     CommandEntry::Symlink(args) => args.sudo,
                     _ => false,

src/config/validate.rs

@@ -146,7 +146,7 @@ pub fn validate_config(config: &AppConfig, config_dir: &Path) -> Vec<ValidationI
         for cmd in &task.commands {
             match cmd {
                 CommandEntry::Run(args) => {
-                    if args.all_command_strings().is_empty() {
+                    if args.all_command_strings().next().is_none() {
                         issues.push(ValidationIssue {
                             task_name: name.clone(),
                             message: format!("Run command has no commands defined: {cmd}"),

src/engine/commands/clone.rs

@@ -1,11 +1,11 @@
 use async_trait::async_trait;
-use tokio::io::{AsyncBufReadExt, BufReader};
 use tokio::process::Command;
 
 use crate::config::types::CloneArgs;
 use crate::engine::context::CommandContext;
 use crate::error::{Error, Result};
 use crate::utils::path::expand_path;
+use crate::utils::process;
 
 use super::CommandExecutor;
 
@@ -75,7 +75,7 @@ impl CommandExecutor for CloneCommand {
     }
 
     fn description(&self) -> String {
-        format!("clone: {} -> {}", self.args.url, self.args.target)
+        self.args.to_string()
     }
 }
 
@@ -93,44 +93,15 @@ async fn run_git_command(
         cmd.current_dir(dir);
     }
 
-    let mut child = cmd
+    let child = cmd
         .spawn()
         .map_err(|e| Error::GitFailed(format!("Failed to spawn git: {e}")))?;
 
-    let stdout_handle = child.stdout.take().map(|stdout| {
-        let ctx_clone = ctx.clone();
-        tokio::spawn(async move {
-            let reader = BufReader::new(stdout);
-            let mut lines = reader.lines();
-            while let Ok(Some(line)) = lines.next_line().await {
-                ctx_clone.log(line);
-            }
-        })
-    });
-
-    let stderr_handle = child.stderr.take().map(|stderr| {
-        let ctx_clone = ctx.clone();
-        tokio::spawn(async move {
-            let reader = BufReader::new(stderr);
-            let mut lines = reader.lines();
-            while let Ok(Some(line)) = lines.next_line().await {
-                ctx_clone.log(line);
-            }
-        })
-    });
-
-    let status = child
-        .wait()
+    // git emits progress on stderr — don't tag those lines as errors.
+    let status = process::stream_and_wait(child, ctx, process::StderrLabel::Plain)
         .await
         .map_err(|e| Error::GitFailed(format!("Failed to wait for git: {e}")))?;
 
-    if let Some(h) = stdout_handle {
-        let _ = h.await;
-    }
-    if let Some(h) = stderr_handle {
-        let _ = h.await;
-    }
-
     if !status.success() {
         return Err(Error::GitFailed(format!(
             "git {} exited with code {}",

src/engine/commands/copy.rs

@@ -1,11 +1,10 @@
 use async_trait::async_trait;
 use std::path::Path;
-use walkdir::WalkDir;
 
 use crate::config::types::CopyArgs;
 use crate::engine::context::CommandContext;
 use crate::error::{Error, Result};
-use crate::utils::path::{expand_path, should_ignore};
+use crate::utils::path::{expand_path, walk_relative};
 use crate::utils::sudo;
 
 use super::CommandExecutor;
@@ -74,31 +73,20 @@ impl CommandExecutor for CopyCommand {
                 remove_file(&dest, use_sudo)?;
             }
         } else {
-            for entry in WalkDir::new(&src).into_iter().filter_map(|e| e.ok()) {
-                if entry.file_type().is_file() {
-                    let relative = entry.path().strip_prefix(&src).unwrap();
-                    if should_ignore(relative, &self.args.ignore) {
-                        continue;
-                    }
-                    let dest = target.join(relative);
-                    if dest.exists() {
-                        ctx.log(format!("Removing: {}", dest.display()));
-                        remove_file(&dest, use_sudo)?;
-                    }
+            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)?;
                 }
-            }
+                Ok(())
+            })?;
         }
 
         Ok(())
     }
 
     fn description(&self) -> String {
-        let prefix = if self.args.sudo {
-            "copy (sudo)"
-        } else {
-            "copy"
-        };
-        format!("{prefix}: {} -> {}", self.args.src, self.args.target)
+        self.args.to_string()
     }
 }
 
@@ -156,20 +144,11 @@ fn copy_directory(
     use_sudo: bool,
     ctx: &CommandContext,
 ) -> Result<()> {
-    for entry in WalkDir::new(src).into_iter().filter_map(|e| e.ok()) {
-        let relative = entry.path().strip_prefix(src).unwrap();
-
-        if should_ignore(relative, ignore) {
-            continue;
-        }
-
-        let dest = target.join(relative);
-
+    walk_relative(src, target, ignore, |entry, dest| {
         if entry.file_type().is_dir() {
-            mkdir(&dest, use_sudo)?;
+            mkdir(dest, use_sudo)
         } else {
-            copy_file(entry.path(), &dest, use_sudo, ctx)?;
+            copy_file(entry.path(), dest, use_sudo, ctx)
         }
-    }
-    Ok(())
+    })
 }

src/engine/commands/mod.rs

@@ -22,13 +22,15 @@ pub trait CommandExecutor: Send + Sync {
     fn description(&self) -> String;
 }
 
-/// Create a command executor from a config entry.
-pub fn create_executor(entry: &CommandEntry) -> Box<dyn CommandExecutor> {
+/// Create a command executor from a config entry. Takes ownership so the
+/// args struct moves directly into the executor without an intermediate
+/// clone inside each match arm.
+pub fn create_executor(entry: CommandEntry) -> Box<dyn CommandExecutor> {
     match entry {
-        CommandEntry::Copy(args) => Box::new(copy::CopyCommand::new(args.clone())),
-        CommandEntry::Symlink(args) => Box::new(symlink::SymlinkCommand::new(args.clone())),
-        CommandEntry::Clone(args) => Box::new(clone::CloneCommand::new(args.clone())),
-        CommandEntry::Run(args) => Box::new(run::RunCommand::new(args.clone())),
-        CommandEntry::MachineSetup(args) => Box::new(setup::SetupCommand::new(args.clone())),
+        CommandEntry::Copy(args) => Box::new(copy::CopyCommand::new(args)),
+        CommandEntry::Symlink(args) => Box::new(symlink::SymlinkCommand::new(args)),
+        CommandEntry::Clone(args) => Box::new(clone::CloneCommand::new(args)),
+        CommandEntry::Run(args) => Box::new(run::RunCommand::new(args)),
+        CommandEntry::MachineSetup(args) => Box::new(setup::SetupCommand::new(args)),
     }
 }