From 611219247d0142b6d0ecd5357b21b1a595ddf7e9 Mon Sep 17 00:00:00 2001 From: Edward Wibowo Date: Fri, 9 Apr 2021 22:17:03 +0800 Subject: [PATCH 1/4] refactor: move link logic to a separate module --- src/bin/ambit/cmd.rs | 500 ----------------------------- src/bin/ambit/main.rs | 98 +++--- src/cmd.rs | 90 ++++++ src/config/mod.rs | 13 +- src/{bin/ambit => }/directories.rs | 30 +- src/lib.rs | 3 + src/linker.rs | 443 +++++++++++++++++++++++++ tests/integration_tests.rs | 82 +---- 8 files changed, 630 insertions(+), 629 deletions(-) delete mode 100644 src/bin/ambit/cmd.rs create mode 100644 src/cmd.rs rename src/{bin/ambit => }/directories.rs (86%) create mode 100644 src/linker.rs diff --git a/src/bin/ambit/cmd.rs b/src/bin/ambit/cmd.rs deleted file mode 100644 index a3e46f8..0000000 --- a/src/bin/ambit/cmd.rs +++ /dev/null @@ -1,500 +0,0 @@ -// Symlink function is dependent on OS -#[cfg(unix)] -use std::os::unix::fs::symlink; -#[cfg(windows)] -use std::os::windows::fs::symlink_file as symlink; -use std::{ - fs, - io::{self, Write}, - path::{Path, PathBuf}, - process::Command, -}; - -use patmatch::{MatchOptions, Pattern}; -use walkdir::WalkDir; - -use ambit::{ - config::{self, ast::Spec, Entry}, - error::{AmbitError, AmbitResult}, -}; - -use crate::directories::{AmbitPath, AmbitPathKind, AMBIT_PATHS, CONFIG_NAME}; - -// Initialize config and repository directory -fn ensure_paths_exist(force: bool) -> AmbitResult<()> { - if !AMBIT_PATHS.config.exists() { - AMBIT_PATHS.config.ensure_parent_dirs_exist()?; - AMBIT_PATHS.config.create()?; - } - if AMBIT_PATHS.repo.exists() && !force { - // Dotfile repository should not be overwritten if force is false - return Err(AmbitError::Other( - "Dotfile repository already exists.\nUse '-f' flag to overwrite.".to_owned(), - )); - } else if AMBIT_PATHS.repo.exists() { - // Repository directory exists but force is enabled - AMBIT_PATHS.repo.remove()?; - } - Ok(()) -} - -// Fetch entries from config file and return as vector -fn get_config_entries(config_path: &AmbitPath) -> AmbitResult> { - let content = config_path.as_string()?; - config::get_entries(content.chars().peekable()) - .collect::, _>>() - .map_err(AmbitError::Parse) -} - -// Return if link_name is symlinked to target (link_name -> target). -fn is_symlinked(link_name: &Path, target: &Path) -> bool { - fs::read_link(link_name) - .map(|link_path| link_path == *target) - .unwrap_or(false) -} - -// Return a vector of PathBufs that match a pattern relative to the given start_path. -fn get_paths_from_spec(spec: &Spec, start_path: PathBuf) -> AmbitResult> { - let mut paths: Vec = Vec::new(); - for entry in spec.into_iter() { - if !entry.contains('*') && !entry.contains('?') { - // The entry does not contain any pattern matching characters. - // This is a definitive path so we can simply push it. - paths.push(PathBuf::from(&entry)); - } else { - // The only valid path at the start is the starting path. - // This will be replaced at every iteration/depth. - let mut valid_paths: Vec = vec![start_path.clone()]; - let components: Vec<_> = Path::new(&entry) - .components() - .map(|comp| comp.as_os_str().to_string_lossy()) - .collect(); - // To find matching files and directories, an entry as part of the spec is split into components. - // For each component, a pattern is compiled and a vector of paths that match this pattern is found. - // With the vector produced from the previous component, the process is repeated with the ancestor paths equal to the said vector. - for (i, component) in components.iter().enumerate() { - let mut new_valid_paths: Vec = Vec::new(); - let expected_path_kind = if i < components.len() - 1 { - // There are still more components to go, expect a directory. - AmbitPathKind::Directory - } else { - // No more components, expect a file. - AmbitPathKind::File - }; - let pattern = Pattern::compile( - &component, - MatchOptions::WILDCARDS | MatchOptions::UNKNOWN_CHARS, - ); - for ancestor_path in &valid_paths { - for path in fs::read_dir(ancestor_path)? { - let path = path?.path(); - // Validify the current path. - if let Some(file_name) = path.file_name() { - if match expected_path_kind { - AmbitPathKind::File => path.is_file(), - AmbitPathKind::Directory => path.is_dir(), - } && pattern.matches(&file_name.to_string_lossy()) - { - new_valid_paths.push(path); - } - } - } - } - valid_paths = new_valid_paths; - } - // Strip prefix from all paths. - for path in valid_paths { - paths.push(path.strip_prefix(&start_path)?.to_path_buf()); - } - } - } - Ok(paths) -} - -// Return vector over path pairs in the form of `(repo_file, host_file)` from given entry. -fn get_ambit_paths_from_entry(entry: &Entry) -> AmbitResult> { - let left_entry_start = if entry.right.is_some() { - PathBuf::from(AMBIT_PATHS.repo.to_str()?) - } else { - PathBuf::from(AMBIT_PATHS.home.to_str()?) - }; - let left_paths = get_paths_from_spec(&entry.left, left_entry_start)?; - let right_paths = if let Some(entry_right) = &entry.right { - Some(get_paths_from_spec( - &entry_right, - PathBuf::from(AMBIT_PATHS.home.to_str()?), - )?) - } else { - // The right entry does not exist. Treat the left entry as both the repo and host paths. - None - }; - // The number of left and right paths may be different due to pattern matching. - // An error is thrown if they have different sizes. - if let Some(right_paths) = &right_paths { - if left_paths.len() != right_paths.len() { - // Format the vector of PathBuf as a string delimited by a newline. - let format_paths = |paths: &Vec| { - paths - .iter() - .map(|path| path.as_path().display().to_string()) - .collect::>() - .join("\n") - }; - return Err(AmbitError::Other(format!( - "Entry has imbalanced left and right side due to pattern matching\nAttempted to sync:\n{}\nwith:\n{}", - format_paths(&left_paths), format_paths(right_paths), - ))); - } - } - let mut paths = Vec::new(); - for (i, repo_path) in left_paths.iter().enumerate() { - let host_path = if let Some(ref right_paths) = right_paths { - &right_paths[i] - } else { - repo_path - }; - paths.push(( - AmbitPath::new(AMBIT_PATHS.repo.path.join(repo_path), AmbitPathKind::File), - AmbitPath::new(AMBIT_PATHS.home.path.join(host_path), AmbitPathKind::File), - )) - } - Ok(paths) -} - -// Recursively search dotfile repository for config path. -fn get_repo_config_paths(stop_at_first_found: bool) -> Vec { - let mut repo_config_paths = Vec::new(); - for dir_entry in WalkDir::new(&AMBIT_PATHS.repo.path) { - if let Ok(dir_entry) = dir_entry { - let path = dir_entry.path(); - if let Some(file_name) = path.file_name() { - if file_name == CONFIG_NAME { - repo_config_paths.push(path.to_path_buf()); - if stop_at_first_found { - break; - } - } - } - } - } - repo_config_paths -} - -// Prompt user for confirmation with message. -fn prompt_confirm(message: &str) -> AmbitResult { - print!("{} [Y/n] ", message); - io::stdout().flush()?; - let mut answer = String::new(); - io::stdin().read_line(&mut answer)?; - Ok(answer.trim().to_lowercase() == "y") -} - -// Initialize an empty dotfile repository -pub fn init(force: bool) -> AmbitResult<()> { - ensure_paths_exist(force)?; - AMBIT_PATHS.repo.create()?; - // Initialize an empty git repository - git(vec!["init"])?; - Ok(()) -} - -// Clone an existing dotfile repository with given origin -pub fn clone(force: bool, arguments: Vec<&str>) -> AmbitResult<()> { - ensure_paths_exist(force)?; - // Clone will handle creating the repository directory - let repo_path = AMBIT_PATHS.repo.to_str()?; - let status = Command::new("git") - .arg("clone") - .args(arguments) - .args(vec!["--", repo_path]) - .status()?; - match status.success() { - true => { - println!("Successfully cloned repository to {}", repo_path); - Ok(()) - } - false => Err(AmbitError::Other("Failed to clone repository".to_owned())), - } -} - -// Check ambit configuration for errors -pub fn check() -> AmbitResult<()> { - get_config_entries(&AMBIT_PATHS.config)?; - Ok(()) -} - -// Sync files in dotfile repository to system through symbolic links -pub fn sync( - dry_run: bool, - quiet: bool, - move_files: bool, - use_repo_config: bool, - use_repo_config_if_required: bool, - use_any_repo_config: bool, -) -> AmbitResult<()> { - // Only symlink if repo and git directories exist - if !(AMBIT_PATHS.repo.exists() && AMBIT_PATHS.git.exists()) { - return Err(AmbitError::Other( - "Dotfile repository does not exist. Run `init` or `clone` before syncing.".to_owned(), - )); - } - let mut successful_syncs: usize = 0; // Number of syncs that actually occurred - let mut total_syncs: usize = 0; - let mut link = |repo_file: AmbitPath, host_file: AmbitPath| -> AmbitResult<()> { - // already_symlinked holds whether host_file already links to repo_file - let already_symlinked = is_symlinked(&host_file.path, &repo_file.path); - // cache for later - let host_file_exists = host_file.exists(); - let repo_file_exists = repo_file.exists(); - - if host_file_exists && !already_symlinked && !move_files { - // Host file already exists but is not symlinked correctly - return Err(AmbitError::Sync { - host_file_path: host_file.path, - repo_file_path: repo_file.path, - error: Box::new(AmbitError::Other( - "Host file already exists and is not correctly symlinked".to_owned(), - )), - }); - } - if !repo_file_exists && !move_files { - return Err(AmbitError::Sync { - host_file_path: host_file.path, - repo_file_path: repo_file.path, - error: Box::new(AmbitError::Other( - "Repository file does not exist".to_owned(), - )), - }); - } - if !already_symlinked { - let mut moved = false; - if !dry_run { - if host_file_exists && !repo_file_exists && move_files { - // Automatically move the file into the repo - repo_file.ensure_parent_dirs_exist()?; - fs::rename(&host_file.path, &repo_file.path)?; - moved = true; - } else { - host_file.ensure_parent_dirs_exist()?; - } - // Attempt to perform symlink - if let Err(e) = symlink(&repo_file.path, &host_file.path) { - // Symlink went wrong - return Err(AmbitError::Sync { - host_file_path: host_file.path, - repo_file_path: repo_file.path, - error: Box::new(AmbitError::Io(e)), - }); - } - successful_syncs += 1; - } - if !quiet { - let action = match moved { - true => "Moved", - false => match !dry_run { - true => "Synced", - false => "Ignored", - }, - }; - println!( - "{} {} -> {}", - action, - host_file.path.display(), - repo_file.path.display() - ); - } - } - total_syncs += 1; - Ok(()) - }; - let entries = if use_repo_config || !AMBIT_PATHS.config.exists() { - if !use_repo_config { - // Ask user if they want to search for repo config. - println!( - "No configuration file found in {}", - AMBIT_PATHS.config.path.display() - ); - // No need to prompt if `use_repo_config_if_required` is true. - if !use_repo_config_if_required - && !prompt_confirm("Search for configuration in repository?")? - { - println!("Ignoring sync..."); - return Ok(()); - } - } - println!( - "Searching for {} in {}...", - CONFIG_NAME, - AMBIT_PATHS.repo.path.display() - ); - let repo_config_paths = get_repo_config_paths(use_any_repo_config); - let mut repo_config = None; - // Iterate through repo configuration files that were found. - for path in repo_config_paths { - if use_any_repo_config - || prompt_confirm(format!("Repo config found: {}. Use?", path.display()).as_str())? - { - // config.ambit file has been found in repo and user has accepted it. - repo_config = Some(AmbitPath::new(path, AmbitPathKind::File)); - break; - } - } - match repo_config { - Some(repo_config) => get_config_entries(&repo_config)?, - None => { - return Err(AmbitError::Other( - "Could not find configuration file in dotfile repository.".to_owned(), - )); - } - } - } else { - get_config_entries(&AMBIT_PATHS.config)? - }; - for entry in entries { - let paths = get_ambit_paths_from_entry(&entry)?; - for (repo_file, host_file) in paths { - link(repo_file, host_file)?; - } - } - // Report the number of files symlinked - println!( - "sync result ({} total): {} synced; {} ignored", - total_syncs, - successful_syncs, - total_syncs - successful_syncs, - ); - Ok(()) -} - -// Remove all symlinks and delete host files. -pub fn clean() -> AmbitResult<()> { - let entries = get_config_entries(&AMBIT_PATHS.config)?; - let mut total_syncs: usize = 0; - let mut deletions: usize = 0; - for entry in entries { - let paths = get_ambit_paths_from_entry(&entry)?; - for (repo_file, host_file) in paths { - if is_symlinked(&host_file.path, &repo_file.path) { - host_file.remove()?; - deletions += 1; - } - total_syncs += 1; - } - } - println!( - "clean result ({} total): {} deleted: {} ignored", - total_syncs, - deletions, - total_syncs - deletions - ); - Ok(()) -} - -// Run git commands from the dotfile repository -pub fn git(arguments: Vec<&str>) -> AmbitResult<()> { - // The path to repository (git-dir) and the working tree (work-tree) is - // passed to ensure that git commands are run from the dotfile repository - let output = Command::new("git") - .args(&[ - ["--git-dir=", AMBIT_PATHS.git.to_str()?].concat(), - ["--work-tree=", AMBIT_PATHS.repo.to_str()?].concat(), - ]) - .args(arguments) - .output()?; - io::stdout().write_all(&output.stdout)?; - io::stdout().write_all(&output.stderr)?; - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::get_paths_from_spec; - use ambit::config::ast::Spec; - use std::{ - collections::HashSet, - fs::{self, File}, - path::PathBuf, - }; - - fn test_spec(spec_str: &str, existing_paths: &[&str], expected_paths: &[PathBuf]) { - let spec = Spec::from(spec_str); - let dir_path = tempfile::tempdir().unwrap().into_path(); - // Create paths. - for path in existing_paths { - let path = dir_path.join(path); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent).unwrap(); - } - File::create(path).unwrap(); - } - let paths = get_paths_from_spec(&spec, dir_path).unwrap(); - // Assert that there are no duplicates as they would be removed when collected into a HashSet. - assert_eq!(paths.len(), expected_paths.len()); - let paths: HashSet<&PathBuf> = paths.iter().collect(); - // Use a HashSet as order of paths should not matter. - assert_eq!(paths, expected_paths.iter().collect::>()); - } - - #[test] - fn get_paths_from_spec_without_pattern() { - test_spec( - "a/b/c", - &["c/b/a", "a/b/c"], - &[PathBuf::from("a").join("b").join("c")], - ); - } - - #[test] - fn get_paths_from_spec_ignore_parent() { - // This will resolve to a/b/c because if the user explicitly specifies a file (without pattern matching characters) - // its existence has to be verified at the symlinking stage which would error if it doesn't exist. - // This is to inform the user that the file does not exist. - // This differs from a pattern matching spec that will not resolve if the file does not exist. - test_spec("a/b/c", &["a/b"], &[PathBuf::from("a").join("b").join("c")]); - } - - #[test] - fn get_paths_from_spec_adjacent_wildcard() { - test_spec( - ".config/*/*", - &[ - ".config/foo", - ".config/bar", - ".config/hello", - ".config/nvim/init.vim", - ".config/ambit/config.ambit", - ".config/ambit/repo/.vimrc", - ], - &[ - PathBuf::from(".config").join("nvim").join("init.vim"), - PathBuf::from(".config").join("ambit").join("config.ambit"), - ], - ); - } - - #[test] - fn get_paths_from_spec_with_unknown_char() { - test_spec( - "Pictures/*.???", - &[ - "Pictures/foo.jpg", - "Pictures/bar.png", - "Pictures/hello.svg", - // The following 2 should be ignored. - "Pictures/world.webp", - "Pictures/image.jpeg", - ], - &[ - PathBuf::from("Pictures").join("foo.jpg"), - PathBuf::from("Pictures").join("bar.png"), - PathBuf::from("Pictures").join("hello.svg"), - ], - ); - } - - #[cfg(not(target_os = "windows"))] - #[test] - fn get_paths_from_spec_with_escaped_char() { - test_spec("x\\*y", &["x*y", "xay", "xaay"], &[PathBuf::from("x*y")]); - } -} diff --git a/src/bin/ambit/main.rs b/src/bin/ambit/main.rs index a3587f2..802a016 100644 --- a/src/bin/ambit/main.rs +++ b/src/bin/ambit/main.rs @@ -1,11 +1,13 @@ -mod cmd; -mod directories; - use clap::{App, AppSettings, Arg, SubCommand}; use std::process; -use ambit::error::{self, AmbitResult}; +use ambit::{ + cmd, + directories::AMBIT_PATHS, + error::{self, AmbitResult}, + linker::{self, Linker}, +}; // Return instance of ambit application fn get_app() -> App<'static, 'static> { @@ -13,6 +15,16 @@ fn get_app() -> App<'static, 'static> { .short("f") .long("force") .help("Overwrite currently initialized dotfile repository"); + let linker_args = &[ + force_arg.clone(), + Arg::with_name("dry-run") + .long("dry-run") + .help("If set, do not actually symlink the files"), + Arg::with_name("quiet") + .long("quiet") + .short("q") + .help("Don't report individual symlinks"), + ]; App::new("ambit") .about("Dotfile manager") @@ -38,43 +50,17 @@ fn get_app() -> App<'static, 'static> { .subcommand( SubCommand::with_name("sync") .about("Sync files in dotfile repository to system through symbolic links") - .arg( - Arg::with_name("dry-run") - .long("dry-run") - .help("If set, do not actually symlink the files"), - ) - .arg( - Arg::with_name("quiet") - .long("quiet") - .short("q") - .help("Don't report individual symlinks"), - ) - .arg( - Arg::with_name("move") - .long("move") - .short("m") - .help("Move host files into dotfile repository if needed") - .long_help("Will automatically move host files into repository if they don't already exist in the repository and then symlink them"), - ) - .arg( - Arg::with_name("use-repo-config") - .long("use-repo-config") - .help("Recursively search dotfile repository for configuration file and use it to sync") - ) - .arg( - Arg::with_name("use-repo-config-if-required") - .long("use-repo-config-if-required") - .help("Search for configuration file in dotfile repository if configuration in default location does not exist") - ) - .arg( - Arg::with_name("use-any-repo-config-found") - .long("use-any-repo-config-found") - .help("Use first repository configuration found after recursive search") - ) + .args(linker_args), ) .subcommand( SubCommand::with_name("clean") - .about("Remove all symlinks and delete host files") + .about("Remove all symlinks and delete host files") + .args(linker_args), + ) + .subcommand( + SubCommand::with_name("move") + .about("Move host files into dotfile repository if needed") + .args(linker_args), ) .subcommand(SubCommand::with_name("check").about("Check ambit configuration for errors")) } @@ -95,23 +81,25 @@ fn run() -> AmbitResult<()> { cmd::git(git_arguments)?; } else if matches.is_present("check") { cmd::check()?; - } else if let Some(matches) = matches.subcommand_matches("sync") { - let dry_run = matches.is_present("dry-run"); - let quiet = matches.is_present("quiet"); - let move_files = matches.is_present("move"); - let use_repo_config = matches.is_present("use-repo-config"); - let use_repo_config_if_required = matches.is_present("use-repo-config-if-required"); - let use_any_repo_config = matches.is_present("use-any-repo-config-found"); - cmd::sync( - dry_run, - quiet, - move_files, - use_repo_config, - use_repo_config_if_required, - use_any_repo_config, - )?; - } else if matches.is_present("clean") { - cmd::clean()?; + } else { + type LinkerAction = fn(&Linker) -> AmbitResult<()>; + let linker_commands: &[(&str, LinkerAction)] = &[ + ("sync", Linker::sync_paths), + ("move", Linker::move_paths), + ("clean", Linker::clean_paths), + ]; + for (subcommand, func) in linker_commands { + if let Some(matches) = matches.subcommand_matches(subcommand) { + let options = linker::Options { + force: matches.is_present("force"), + dry_run: matches.is_present("dry-run"), + quiet: matches.is_present("quiet"), + }; + let linker = Linker::new(&AMBIT_PATHS, options)?; + func(&linker)?; + break; + } + } } Ok(()) } diff --git a/src/cmd.rs b/src/cmd.rs new file mode 100644 index 0000000..9744ba3 --- /dev/null +++ b/src/cmd.rs @@ -0,0 +1,90 @@ +// Symlink function is dependent on OS +use crate::{ + config, + directories::AMBIT_PATHS, + error::{AmbitError, AmbitResult}, +}; +use std::{ + io::{self, Write}, + process::Command, +}; + +// Initialize config and repository directory +fn ensure_repo_exists(force: bool) -> AmbitResult<()> { + if AMBIT_PATHS.repo.exists() + // No need to prompt if force is true. + && !force + // Ask user if they want to overwrite. + && !prompt_confirm("A repository already exists. Overwrite?")? + { + return Err(AmbitError::Other( + "Dotfile repository already exists.\nUse '-f' flag to overwrite.".to_owned(), + )); + } else if AMBIT_PATHS.repo.exists() { + // Remove if either force is enabled or if the user confirmed to overwrite. + AMBIT_PATHS.repo.remove()?; + } + Ok(()) +} + +// Prompt user for confirmation with message. +pub fn prompt_confirm(message: &str) -> AmbitResult { + print!("{} [Y/n] ", message); + io::stdout().flush()?; + let mut answer = String::new(); + io::stdin().read_line(&mut answer)?; + Ok(answer.trim().to_lowercase() == "y") +} + +// Initialize an empty dotfile repository +pub fn init(force: bool) -> AmbitResult<()> { + if !AMBIT_PATHS.config.exists() { + AMBIT_PATHS.config.create()?; + } + ensure_repo_exists(force)?; + AMBIT_PATHS.repo.create()?; + // Initialize an empty git repository + git(vec!["init"])?; + Ok(()) +} + +// Clone an existing dotfile repository with given origin +pub fn clone(force: bool, arguments: Vec<&str>) -> AmbitResult<()> { + ensure_repo_exists(force)?; + // Clone will handle creating the repository directory + let repo_path = AMBIT_PATHS.repo.to_str()?; + let status = Command::new("git") + .arg("clone") + .args(arguments) + .args(vec!["--", repo_path]) + .status()?; + match status.success() { + true => { + println!("Successfully cloned repository to {}", repo_path); + Ok(()) + } + false => Err(AmbitError::Other("Failed to clone repository".to_owned())), + } +} + +// Check ambit configuration for errors +pub fn check() -> AmbitResult<()> { + config::get_entries(&AMBIT_PATHS.config)?; + Ok(()) +} + +// Run git commands from the dotfile repository +pub fn git(arguments: Vec<&str>) -> AmbitResult<()> { + // The path to repository (git-dir) and the working tree (work-tree) is + // passed to ensure that git commands are run from the dotfile repository + let output = Command::new("git") + .args(&[ + ["--git-dir=", AMBIT_PATHS.git.to_str()?].concat(), + ["--work-tree=", AMBIT_PATHS.repo.to_str()?].concat(), + ]) + .args(arguments) + .output()?; + io::stdout().write_all(&output.stdout)?; + io::stdout().write_all(&output.stderr)?; + Ok(()) +} diff --git a/src/config/mod.rs b/src/config/mod.rs index 6030b79..0cb9185 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -9,7 +9,11 @@ pub use parser::Parser; use std::error::Error; use std::fmt::{self, Display, Formatter}; -use std::iter::Peekable; + +use crate::{ + directories::AmbitPath, + error::{AmbitError, AmbitResult}, +}; #[derive(PartialEq, Eq, Debug, Clone, Copy)] pub enum ParseErrorType { @@ -42,7 +46,8 @@ impl From for ParseError { pub type ParseResult = std::result::Result; -pub fn get_entries>(char_iter: Peekable) -> Parser> { - let lex = Lexer::new(char_iter); - Parser::new(lex.peekable()) +pub fn get_entries(config_path: &AmbitPath) -> AmbitResult> { + Parser::new(Lexer::new(config_path.as_string()?.chars().peekable()).peekable()) + .collect::, _>>() + .map_err(AmbitError::Parse) } diff --git a/src/bin/ambit/directories.rs b/src/directories.rs similarity index 86% rename from src/bin/ambit/directories.rs rename to src/directories.rs index 4ec0744..a7ffdd8 100644 --- a/src/bin/ambit/directories.rs +++ b/src/directories.rs @@ -3,25 +3,47 @@ use std::{ env, fs::{self, File}, io::Read, + ops::Deref, path::PathBuf, }; -use ambit::error::{AmbitError, AmbitResult}; +use crate::error::{AmbitError, AmbitResult}; pub const CONFIG_NAME: &str = "config.ambit"; -#[derive(PartialEq, Eq, Debug)] +#[derive(PartialEq, Eq, Debug, Clone)] pub enum AmbitPathKind { File, Directory, } -#[derive(PartialEq, Eq, Debug)] +#[derive(PartialEq, Eq, Debug, Clone)] pub struct AmbitPath { pub path: PathBuf, kind: AmbitPathKind, } +impl From<&PathBuf> for AmbitPath { + fn from(path: &PathBuf) -> Self { + let kind = if path.is_file() { + AmbitPathKind::File + } else { + AmbitPathKind::Directory + }; + Self { + path: path.to_path_buf(), + kind, + } + } +} + +impl Deref for AmbitPath { + type Target = PathBuf; + fn deref(&self) -> &Self::Target { + &self.path + } +} + impl AmbitPath { pub fn new(path: PathBuf, kind: AmbitPathKind) -> Self { Self { path, kind } @@ -78,6 +100,7 @@ impl AmbitPath { pub fn create(&self) -> AmbitResult<()> { match self.kind { AmbitPathKind::File => { + self.ensure_parent_dirs_exist()?; File::create(&self.path)?; } AmbitPathKind::Directory => { @@ -96,6 +119,7 @@ impl AmbitPath { } } +#[derive(Debug, Clone)] pub struct AmbitPaths { pub home: AmbitPath, pub config: AmbitPath, diff --git a/src/lib.rs b/src/lib.rs index 7404805..0f2e1a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,2 +1,5 @@ +pub mod cmd; pub mod config; +pub mod directories; pub mod error; +pub mod linker; diff --git a/src/linker.rs b/src/linker.rs new file mode 100644 index 0000000..43715d7 --- /dev/null +++ b/src/linker.rs @@ -0,0 +1,443 @@ +// Linker handles sync and move operations. +use crate::{ + cmd, + config::{self, ast::Spec, Entry}, + directories::{self, AmbitPath, AmbitPathKind, AmbitPaths}, + error::{AmbitError, AmbitResult}, +}; +use patmatch::{MatchOptions, Pattern}; +#[cfg(unix)] +use std::os::unix::fs::symlink; +#[cfg(windows)] +use std::os::windows::fs::symlink_file as symlink; +use std::{ + fs, + path::{Path, PathBuf}, +}; +use walkdir::WalkDir; + +#[derive(Debug)] +pub struct Options { + pub force: bool, + pub dry_run: bool, + pub quiet: bool, +} + +// Return if link_name is symlinked to target (link_name -> target). +fn is_symlinked(link_name: &Path, target: &Path) -> bool { + fs::read_link(link_name) + .map(|link_path| link_path == *target) + .unwrap_or(false) +} + +// Return a vector of PathBufs that match a pattern relative to the given start_path. +fn get_paths_from_spec(spec: &Spec, start_path: PathBuf) -> AmbitResult> { + let mut paths: Vec = Vec::new(); + for entry in spec.into_iter() { + if !entry.contains('*') && !entry.contains('?') { + // The entry does not contain any pattern matching characters. + // This is a definitive path so we can simply push it. + paths.push(PathBuf::from(&entry)); + } else { + // The only valid path at the start is the starting path. + // This will be replaced at every iteration/depth. + let mut valid_paths: Vec = vec![start_path.clone()]; + let components: Vec<_> = Path::new(&entry) + .components() + .map(|comp| comp.as_os_str().to_string_lossy()) + .collect(); + // To find matching files and directories, an entry as part of the spec is split into components. + // For each component, a pattern is compiled and a vector of paths that match this pattern is found. + // With the vector produced from the previous component, the process is repeated with the ancestor paths equal to the said vector. + for (i, component) in components.iter().enumerate() { + let mut new_valid_paths: Vec = Vec::new(); + let expected_path_kind = if i < components.len() - 1 { + // There are still more components to go, expect a directory. + AmbitPathKind::Directory + } else { + // No more components, expect a file. + AmbitPathKind::File + }; + let pattern = Pattern::compile( + &component, + MatchOptions::WILDCARDS | MatchOptions::UNKNOWN_CHARS, + ); + for ancestor_path in &valid_paths { + for path in fs::read_dir(ancestor_path)? { + let path = path?.path(); + // Validify the current path. + if let Some(file_name) = path.file_name() { + if match expected_path_kind { + AmbitPathKind::File => path.is_file(), + AmbitPathKind::Directory => path.is_dir(), + } && pattern.matches(&file_name.to_string_lossy()) + { + new_valid_paths.push(path); + } + } + } + } + valid_paths = new_valid_paths; + } + // Strip prefix from all paths. + for path in valid_paths { + paths.push(path.strip_prefix(&start_path)?.to_path_buf()); + } + } + } + Ok(paths) +} + +#[derive(Debug)] +pub struct Linker { + paths: AmbitPaths, + options: Options, +} + +impl Linker { + pub fn new(paths: &AmbitPaths, options: Options) -> AmbitResult { + // Only symlink if repo and git directories exist + if !paths.repo.exists() || !paths.git.exists() { + Err(AmbitError::Other( + "Dotfile repository does not exist. Run `init` or `clone`.".to_owned(), + )) + } else { + Ok(Self { + paths: paths.clone(), + options, + }) + } + } + + pub fn clean_paths(&self) -> AmbitResult<()> { + let config_path = self.find_config_path(self.options.force)?; + let entries = config::get_entries(&config_path)?; + let mut total_syncs: usize = 0; + let mut deletions: usize = 0; + for entry in entries { + let paths = self.get_ambit_paths_from_entry(&entry)?; + for (repo_file, host_file) in paths { + if is_symlinked(&host_file.path, &repo_file.path) { + if !self.options.dry_run { + host_file.remove()?; + } + deletions += 1; + if !self.options.quiet { + let action = if self.options.dry_run { + "Ignored" + } else { + "Removed" + }; + println!("{} {}", action, host_file.path.display()); + } + } + total_syncs += 1; + } + } + // Final clean metrics. + println!( + "clean result ({} total): {} deleted: {} ignored", + total_syncs, + deletions, + total_syncs - deletions + ); + Ok(()) + } + + pub fn sync_paths(&self) -> AmbitResult<()> { + let mut total: usize = 0; + let config_path = self.find_config_path(self.options.force)?; + let mut symlink_pairs = Vec::new(); + for entry in config::get_entries(&config_path)? { + for (repo_file, host_file) in self.get_ambit_paths_from_entry(&entry)? { + // Only push into symlink_pairs if it hasn't been symlinkd already. + if !is_symlinked(&host_file.path, &repo_file.path) { + symlink_pairs.push((repo_file, host_file)); + } + total += 1; + } + } + for (repo_file, host_file) in &symlink_pairs { + if !repo_file.exists() { + return Err(AmbitError::Other(format!( + "Repository file {} must exist to be synced. Consider using `move`.", + repo_file.path.display() + ))); + } + // symlink_pairs only contains pairs of paths that aren't already symlinked. + // This means that if host_file exists at this point, it isn't correctly symlinked. + if host_file.exists() { + return Err(AmbitError::Other(format!( + "Host file {} already exists and is not correctly symlinked.", + host_file.path.display() + ))); + } + } + for (repo_file, host_file) in &symlink_pairs { + if !self.options.dry_run { + host_file.ensure_parent_dirs_exist()?; + // Attempt to symlink. + if let Err(e) = symlink(&repo_file.path, &host_file.path) { + // Symlink went wrong + return Err(AmbitError::Sync { + host_file_path: PathBuf::from(&host_file.path), + repo_file_path: PathBuf::from(&repo_file.path), + error: Box::new(AmbitError::Io(e)), + }); + } + } + if !self.options.quiet { + let action = if self.options.dry_run { + "Ignored" + } else { + "Synced" + }; + println!( + "{} {} -> {}", + action, + host_file.path.display(), + repo_file.path.display() + ); + } + } + let total_synced: usize = if self.options.dry_run { + 0 + } else { + symlink_pairs.len() + }; + // Final sync metrics. + println!( + "sync result ({} total): {} synced: {} ignored", + total, + total_synced, + total - total_synced, + ); + Ok(()) + } + + pub fn move_paths(&self) -> AmbitResult<()> { + let mut total: usize = 0; + let mut total_moved: usize = 0; + let config_path = self.find_config_path(self.options.force)?; + for entry in config::get_entries(&config_path)? { + for (repo_file, host_file) in self.get_ambit_paths_from_entry(&entry)? { + total += 1; + if !repo_file.exists() && host_file.exists() { + if !self.options.dry_run { + total_moved += 1; + fs::rename(host_file.as_path(), repo_file.as_path())?; + } + if !self.options.quiet { + let action = if self.options.dry_run { + "Ignored moving" + } else { + "Moved" + }; + println!( + "{} {} to {}", + action, + host_file.display(), + repo_file.display() + ); + } + } + } + } + // Final moved metrics. + println!( + "move result ({} total): {} moved: {} ignored", + total, + total_moved, + total - total_moved, + ); + Ok(()) + } + + fn find_config_path(&self, force: bool) -> AmbitResult { + let mut new_config_path = None; + if !self.paths.config.exists() { + if force || cmd::prompt_confirm("Search for configuration in repository?")? { + println!( + "Searching for {} in {}...", + directories::CONFIG_NAME, + self.paths.repo.display() + ); + // Pass force because only the first repo config path is needed. + for path in self.get_repo_config_paths(force) { + if force || cmd::prompt_confirm(format!("Use {}?", path.display()).as_str())? { + new_config_path = Some(AmbitPath::new(path, AmbitPathKind::File)); + break; + } + } + } + } else { + new_config_path = Some(AmbitPath::from(&self.paths.config.path)); + } + new_config_path + .ok_or_else(|| AmbitError::Other("Could not locate configuration file.".to_owned())) + } + + // Recursively search dotfile repository for config path. + fn get_repo_config_paths(&self, stop_at_first_found: bool) -> Vec { + let mut repo_config_paths = Vec::new(); + for dir_entry in WalkDir::new(self.paths.repo.as_path()) { + if let Ok(dir_entry) = dir_entry { + let path = dir_entry.path(); + if let Some(file_name) = path.file_name() { + if file_name == directories::CONFIG_NAME { + repo_config_paths.push(path.to_path_buf()); + if stop_at_first_found { + break; + } + } + } + } + } + repo_config_paths + } + + // Return vector over path pairs in the form of `(repo_file, host_file)` from given entry. + fn get_ambit_paths_from_entry( + &self, + entry: &Entry, + ) -> AmbitResult> { + // Only search left paths from repo. + let left_paths = + get_paths_from_spec(&entry.left, PathBuf::from(self.paths.repo.to_str()?))?; + let right_paths = if let Some(entry_right) = &entry.right { + Some(get_paths_from_spec( + &entry_right, + PathBuf::from(self.paths.home.to_str()?), + )?) + } else { + // The right entry does not exist. Treat the left entry as both the repo and host paths. + None + }; + // The number of left and right paths may be different due to pattern matching. + // An error is thrown if they have different sizes. + if let Some(right_paths) = &right_paths { + if left_paths.len() != right_paths.len() { + // Format the vector of PathBuf as a string delimited by a newline. + let format_paths = |paths: &Vec| { + paths + .iter() + .map(|path| path.as_path().display().to_string()) + .collect::>() + .join("\n") + }; + return Err(AmbitError::Other(format!( + "Entry has imbalanced left and right side due to pattern matching\nAttempted to sync:\n{}\nwith:\n{}", + format_paths(&left_paths), format_paths(right_paths), + ))); + } + } + let mut paths = Vec::new(); + for (i, repo_path) in left_paths.iter().enumerate() { + let host_path = if let Some(ref right_paths) = right_paths { + &right_paths[i] + } else { + repo_path + }; + paths.push(( + AmbitPath::new(self.paths.repo.join(repo_path), AmbitPathKind::File), + AmbitPath::new(self.paths.home.path.join(host_path), AmbitPathKind::File), + )) + } + Ok(paths) + } +} + +#[cfg(test)] +mod tests { + use super::get_paths_from_spec; + use crate::config::ast::Spec; + use std::{ + collections::HashSet, + fs::{self, File}, + path::PathBuf, + }; + + fn test_spec(spec_str: &str, existing_paths: &[&str], expected_paths: &[PathBuf]) { + let spec = Spec::from(spec_str); + let dir_path = tempfile::tempdir().unwrap().into_path(); + // Create paths. + for path in existing_paths { + let path = dir_path.join(path); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).unwrap(); + } + File::create(path).unwrap(); + } + let paths = get_paths_from_spec(&spec, dir_path).unwrap(); + // Assert that there are no duplicates as they would be removed when collected into a HashSet. + assert_eq!(paths.len(), expected_paths.len()); + let paths: HashSet<&PathBuf> = paths.iter().collect(); + // Use a HashSet as order of paths should not matter. + assert_eq!(paths, expected_paths.iter().collect::>()); + } + + #[test] + fn get_paths_from_spec_without_pattern() { + test_spec( + "a/b/c", + &["c/b/a", "a/b/c"], + &[PathBuf::from("a").join("b").join("c")], + ); + } + + #[test] + fn get_paths_from_spec_ignore_parent() { + // This will resolve to a/b/c because if the user explicitly specifies a file (without pattern matching characters) + // its existence has to be verified at the symlinking stage which would error if it doesn't exist. + // This is to inform the user that the file does not exist. + // This differs from a pattern matching spec that will not resolve if the file does not exist. + test_spec("a/b/c", &["a/b"], &[PathBuf::from("a").join("b").join("c")]); + } + + #[test] + fn get_paths_from_spec_adjacent_wildcard() { + test_spec( + ".config/*/*", + &[ + ".config/foo", + ".config/bar", + ".config/hello", + ".config/nvim/init.vim", + ".config/ambit/config.ambit", + ".config/ambit/repo/.vimrc", + ], + &[ + PathBuf::from(".config").join("nvim").join("init.vim"), + PathBuf::from(".config").join("ambit").join("config.ambit"), + ], + ); + } + + #[test] + fn get_paths_from_spec_with_unknown_char() { + test_spec( + "Pictures/*.???", + &[ + "Pictures/foo.jpg", + "Pictures/bar.png", + "Pictures/hello.svg", + // The following 2 should be ignored. + "Pictures/world.webp", + "Pictures/image.jpeg", + ], + &[ + PathBuf::from("Pictures").join("foo.jpg"), + PathBuf::from("Pictures").join("bar.png"), + PathBuf::from("Pictures").join("hello.svg"), + ], + ); + } + + #[cfg(not(target_os = "windows"))] + #[test] + fn get_paths_from_spec_with_escaped_char() { + test_spec("x\\*y", &["x*y", "xay", "xaay"], &[PathBuf::from("x*y")]); + } + + // TODO: Add more tests +} diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 80e868a..0263957 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -156,9 +156,10 @@ fn clone_repo_already_exists() { fn sync_without_repo() { // Error should occur if attempting to sync without initializing. // `with_repo_path` is omitted here. - AmbitTester::default().arg("sync").assert().stderr( - "ERROR: Dotfile repository does not exist. Run `init` or `clone` before syncing.\n", - ); + AmbitTester::default() + .arg("sync") + .assert() + .stderr("ERROR: Dotfile repository does not exist. Run `init` or `clone`.\n"); } #[test] @@ -202,22 +203,17 @@ fn sync_normal() { } #[test] -fn sync_move_normal() { +fn move_normal() { let temp_dir = TempDir::new().unwrap(); AmbitTester::from_temp_dir(&temp_dir) .with_repo_path() .with_config("repo.txt => host.txt;") .with_host_file("host.txt") - .args(vec!["sync", "-m"]) + .arg("move") .assert() .success(); - // The new `repo.txt` should now exist + // The new `repo.txt` should now exist. assert!(temp_dir.path().join("repo").join("repo.txt").exists()); - // The symlink should still succeed - assert!(is_symlinked( - temp_dir.path().join("host.txt"), - temp_dir.path().join("repo").join("repo.txt") - )); } #[test] @@ -252,7 +248,7 @@ fn sync_creates_host_parent_directories() { } #[test] -fn sync_use_repo_config_option() { +fn sync_use_nested_repo_config() { // If --use-repo-config flag is passed, recursively search for configuration in repository and use that. let temp_dir = TempDir::new().unwrap(); // Set repo_config_path to /a/b/config.ambit to ensure recursive search works. @@ -262,32 +258,11 @@ fn sync_use_repo_config_option() { .join("a") .join("b") .join("config.ambit"); - AmbitTester::from_temp_dir(&temp_dir) - .with_repo_file("repo.txt") - .with_file_with_content(&repo_config_path, "repo.txt => host.txt;") - .args(vec!["sync", "--use-repo-config"]) - .write_stdin("Y") // 'Y' should be synonymous to 'y' - .assert() - .success(); - assert!(is_symlinked( - temp_dir.path().join("host.txt"), - temp_dir.path().join("repo").join("repo.txt"), - )); -} - -#[test] -fn sync_with_missing_config_answer_yes() { - // Sync without existing configuration file and answer yes to use repo configuration instead. - let temp_dir = TempDir::new().unwrap(); - let repo_config_path = temp_dir.path().join("repo").join("config.ambit"); AmbitTester::from_temp_dir(&temp_dir) .with_repo_file("repo.txt") .with_file_with_content(&repo_config_path, "repo.txt => host.txt;") .arg("sync") - // Answer 'y' twice: - // 1. Accept to search for configuration. - // 2. Accept to use repo config that was found. - .write_stdin("y\ny") + .write_stdin("Y\nY") // 'Y' should be synonymous to 'y' .assert() .success(); assert!(is_symlinked( @@ -307,7 +282,7 @@ fn sync_with_missing_config_answer_no() { .arg("sync") .write_stdin("n") .assert() - .success(); + .failure(); // Should not be symlinked. assert!(!is_symlinked( temp_dir.path().join("host.txt"), @@ -316,35 +291,13 @@ fn sync_with_missing_config_answer_no() { } #[test] -fn sync_use_repo_config_if_required() { - // Sync without existing configuration file and answer yes to use repo configuration instead. - let temp_dir = TempDir::new().unwrap(); - let repo_config_path = temp_dir.path().join("repo").join("config.ambit"); - AmbitTester::from_temp_dir(&temp_dir) - .with_repo_file("repo.txt") - .with_file_with_content(&repo_config_path, "repo.txt => host.txt;") - .args(vec!["sync", "--use-repo-config-if-required"]) - // Only need to input 'y' once as --use-repo-config-if-required means that it will always - // search for repo config if config in default location does not exist. - .write_stdin("y") - .assert() - .success(); - assert!(is_symlinked( - temp_dir.path().join("host.txt"), - temp_dir.path().join("repo").join("repo.txt"), - )); -} - -#[test] -fn sync_use_any_repo_config_found() { +fn sync_force() { let temp_dir = TempDir::new().unwrap(); let repo_path = temp_dir.path().join("repo"); AmbitTester::from_temp_dir(&temp_dir) .with_repo_file("repo.txt") .with_file_with_content(&repo_path.join("config.ambit"), "repo.txt => host.txt;") - .args(vec!["sync", "--use-any-repo-config-found"]) - // With --use-any-repo-config-found flag, only one 'y' needs to be passed - .write_stdin("y") + .args(vec!["sync", "-f"]) .assert() .success(); assert!(is_symlinked( @@ -354,19 +307,14 @@ fn sync_use_any_repo_config_found() { } #[test] -fn sync_use_any_repo_config_found_if_required() { +fn sync_without_force() { let temp_dir = TempDir::new().unwrap(); let repo_path = temp_dir.path().join("repo"); AmbitTester::from_temp_dir(&temp_dir) .with_repo_file("repo.txt") .with_file_with_content(&repo_path.join("config.ambit"), "repo.txt => host.txt;") - // Combine flags --use-any-repo-config-found and --use-repo-config-if-required. - // This should mean that nothing has to be written to stdin. - .args(vec![ - "sync", - "--use-any-repo-config-found", - "--use-repo-config-if-required", - ]) + .arg("sync") + .write_stdin("y\ny") .assert() .success(); assert!(is_symlinked( From e1dd52afd4c4616897becb32c38244abf3de31a1 Mon Sep 17 00:00:00 2001 From: Edward Wibowo Date: Fri, 9 Apr 2021 23:22:15 +0800 Subject: [PATCH 2/4] fix: disable pattern matching for host paths --- src/linker.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/linker.rs b/src/linker.rs index 43715d7..4264c22 100644 --- a/src/linker.rs +++ b/src/linker.rs @@ -31,7 +31,11 @@ fn is_symlinked(link_name: &Path, target: &Path) -> bool { } // Return a vector of PathBufs that match a pattern relative to the given start_path. -fn get_paths_from_spec(spec: &Spec, start_path: PathBuf) -> AmbitResult> { +fn get_paths_from_spec( + spec: &Spec, + start_path: PathBuf, + allow_pattern: bool, +) -> AmbitResult> { let mut paths: Vec = Vec::new(); for entry in spec.into_iter() { if !entry.contains('*') && !entry.contains('?') { @@ -39,6 +43,11 @@ fn get_paths_from_spec(spec: &Spec, start_path: PathBuf) -> AmbitResult = vec![start_path.clone()]; @@ -303,11 +312,12 @@ impl Linker { ) -> AmbitResult> { // Only search left paths from repo. let left_paths = - get_paths_from_spec(&entry.left, PathBuf::from(self.paths.repo.to_str()?))?; + get_paths_from_spec(&entry.left, PathBuf::from(self.paths.repo.to_str()?), true)?; let right_paths = if let Some(entry_right) = &entry.right { Some(get_paths_from_spec( &entry_right, PathBuf::from(self.paths.home.to_str()?), + false, )?) } else { // The right entry does not exist. Treat the left entry as both the repo and host paths. @@ -368,7 +378,7 @@ mod tests { } File::create(path).unwrap(); } - let paths = get_paths_from_spec(&spec, dir_path).unwrap(); + let paths = get_paths_from_spec(&spec, dir_path, true).unwrap(); // Assert that there are no duplicates as they would be removed when collected into a HashSet. assert_eq!(paths.len(), expected_paths.len()); let paths: HashSet<&PathBuf> = paths.iter().collect(); From 60b17312d7fd285f656a96dfd7dfb84bdb9ac654 Mon Sep 17 00:00:00 2001 From: Edward Wibowo Date: Sat, 10 Apr 2021 11:29:47 +0800 Subject: [PATCH 3/4] fix: catch non-existent repo files earlier --- src/linker.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/linker.rs b/src/linker.rs index 4264c22..cf4e504 100644 --- a/src/linker.rs +++ b/src/linker.rs @@ -159,29 +159,25 @@ impl Linker { let mut symlink_pairs = Vec::new(); for entry in config::get_entries(&config_path)? { for (repo_file, host_file) in self.get_ambit_paths_from_entry(&entry)? { + if !repo_file.exists() { + return Err(AmbitError::Other(format!( + "Repository file {} must exist to be synced. Consider using `move`.", + repo_file.path.display() + ))); + } // Only push into symlink_pairs if it hasn't been symlinkd already. if !is_symlinked(&host_file.path, &repo_file.path) { + if host_file.exists() { + return Err(AmbitError::Other(format!( + "Host file {} already exists and is not correctly symlinked.", + host_file.path.display() + ))); + } symlink_pairs.push((repo_file, host_file)); } total += 1; } } - for (repo_file, host_file) in &symlink_pairs { - if !repo_file.exists() { - return Err(AmbitError::Other(format!( - "Repository file {} must exist to be synced. Consider using `move`.", - repo_file.path.display() - ))); - } - // symlink_pairs only contains pairs of paths that aren't already symlinked. - // This means that if host_file exists at this point, it isn't correctly symlinked. - if host_file.exists() { - return Err(AmbitError::Other(format!( - "Host file {} already exists and is not correctly symlinked.", - host_file.path.display() - ))); - } - } for (repo_file, host_file) in &symlink_pairs { if !self.options.dry_run { host_file.ensure_parent_dirs_exist()?; From 9e5463c5920df37638242e615de6b3cd905ab805 Mon Sep 17 00:00:00 2001 From: Edward Wibowo Date: Fri, 4 Jun 2021 21:47:57 +0800 Subject: [PATCH 4/4] refactor: reflect review changes --- src/bin/ambit/main.rs | 8 +-- src/cmd.rs | 28 ++++---- src/config/ast.rs | 2 +- src/config/strgen.rs | 5 +- src/directories.rs | 5 +- src/linker.rs | 160 ++++++++++++++++++++++-------------------- 6 files changed, 105 insertions(+), 103 deletions(-) diff --git a/src/bin/ambit/main.rs b/src/bin/ambit/main.rs index 802a016..e17acbb 100644 --- a/src/bin/ambit/main.rs +++ b/src/bin/ambit/main.rs @@ -88,6 +88,7 @@ fn run() -> AmbitResult<()> { ("move", Linker::move_paths), ("clean", Linker::clean_paths), ]; + // Iterate through sync, move, and clean commands and execute corresponding function. for (subcommand, func) in linker_commands { if let Some(matches) = matches.subcommand_matches(subcommand) { let options = linker::Options { @@ -145,10 +146,9 @@ mod tests { #[test] fn git_arguments_with_hyphen() { let matches = arguments_list!("git", "status", "-v", "--short"); - let git_arguments: Option> = match matches.subcommand_matches("git") { - Some(matches) => Some(matches.values_of("GIT_ARGUMENTS").unwrap().collect()), - None => None, - }; + let git_arguments: Option> = matches + .subcommand_matches("git") + .map(|matches| matches.values_of("GIT_ARGUMENTS").unwrap().collect()); assert_eq!(git_arguments, Some(vec!["status", "-v", "--short"])); } diff --git a/src/cmd.rs b/src/cmd.rs index 74f107f..0639a1e 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -10,8 +10,9 @@ use std::{ }; // Initialize config and repository directory -fn ensure_repo_exists(force: bool) -> AmbitResult<()> { - if AMBIT_PATHS.repo.exists() +fn ensure_no_repo_conflicts(force: bool) -> AmbitResult<()> { + let repo_exists = AMBIT_PATHS.repo.exists(); + if repo_exists // No need to prompt if force is true. && !force // Ask user if they want to overwrite. @@ -20,7 +21,7 @@ fn ensure_repo_exists(force: bool) -> AmbitResult<()> { return Err(AmbitError::Other( "Dotfile repository already exists.\nUse '-f' flag to overwrite.".to_owned(), )); - } else if AMBIT_PATHS.repo.exists() { + } else if repo_exists { // Remove if either force is enabled or if the user confirmed to overwrite. AMBIT_PATHS.repo.remove()?; } @@ -38,10 +39,7 @@ pub fn prompt_confirm(message: &str) -> AmbitResult { // Initialize an empty dotfile repository pub fn init(force: bool) -> AmbitResult<()> { - if !AMBIT_PATHS.config.exists() { - AMBIT_PATHS.config.create()?; - } - ensure_repo_exists(force)?; + ensure_no_repo_conflicts(force)?; AMBIT_PATHS.repo.create()?; // Initialize an empty git repository git(vec!["init"])?; @@ -49,13 +47,13 @@ pub fn init(force: bool) -> AmbitResult<()> { } // Clone an existing dotfile repository with given origin -pub fn clone(force: bool, arguments: Vec<&str>) -> AmbitResult<()> { - ensure_repo_exists(force)?; +pub fn clone(force: bool, clone_arguments: Vec<&str>) -> AmbitResult<()> { + ensure_no_repo_conflicts(force)?; // Clone will handle creating the repository directory let repo_path = AMBIT_PATHS.repo.to_str()?; let status = Command::new("git") .arg("clone") - .args(arguments) + .args(clone_arguments) .args(vec!["--", repo_path]) .status()?; match status.success() { @@ -74,7 +72,7 @@ pub fn check() -> AmbitResult<()> { } // Run git commands from the dotfile repository -pub fn git(arguments: Vec<&str>) -> AmbitResult<()> { +pub fn git(git_arguments: Vec<&str>) -> AmbitResult<()> { // The path to repository (git-dir) and the working tree (work-tree) is // passed to ensure that git commands are run from the dotfile repository let mut command = Command::new("git"); @@ -82,10 +80,10 @@ pub fn git(arguments: Vec<&str>) -> AmbitResult<()> { ["--git-dir=", AMBIT_PATHS.git.to_str()?].concat(), ["--work-tree=", AMBIT_PATHS.repo.to_str()?].concat(), ]); - command.args(arguments); + command.args(git_arguments); // Conditional compilation so that this still compiles on Windows. #[cfg(unix)] - fn exec_git_cmd(mut command: Command) -> AmbitResult<()> { + fn exec_git_command(mut command: Command) -> AmbitResult<()> { use std::os::unix::process::CommandExt; // Try to replace this process with the `git` process. // This is to allow stuff like terminal colors. @@ -94,12 +92,12 @@ pub fn git(arguments: Vec<&str>) -> AmbitResult<()> { Err(AmbitError::Io(command.exec())) } #[cfg(not(unix))] - fn exec_git_cmd(mut command: Command) -> AmbitResult<()> { + fn exec_git_command(mut command: Command) -> AmbitResult<()> { // Not easy to do this on other systems, just use defaults let output = command.output()?; io::stdout().write_all(&output.stdout)?; io::stdout().write_all(&output.stderr)?; Ok(()) } - exec_git_cmd(command) + exec_git_command(command) } diff --git a/src/config/ast.rs b/src/config/ast.rs index 08396f1..4fbb071 100644 --- a/src/config/ast.rs +++ b/src/config/ast.rs @@ -93,7 +93,7 @@ impl MatchExpr { } } -// A comma seperated list of `T`s, with optional trailing comma. +// A comma separated list of `T`s, with optional trailing comma. // (The delimiters are passed to the parse() function.) #[derive(PartialEq, Eq, Debug, Clone)] pub struct CommaList { diff --git a/src/config/strgen.rs b/src/config/strgen.rs index 45291ab..5d2f4d7 100644 --- a/src/config/strgen.rs +++ b/src/config/strgen.rs @@ -267,10 +267,7 @@ impl MatchExpr { fn raw_iter(&self) -> MatchIter { MatchIter { expr: &self, - spec_iter: match self.resolve() { - Some(spec) => Some(spec.raw_iter()), - None => None, - }, + spec_iter: self.resolve().map(|spec| spec.raw_iter()), } } } diff --git a/src/directories.rs b/src/directories.rs index a7ffdd8..51f4008 100644 --- a/src/directories.rs +++ b/src/directories.rs @@ -153,10 +153,7 @@ impl AmbitPaths { // Attempt to fetch path from env if set fn get_path_from_env(key: &str) -> Option { - match env::var_os(key) { - Some(path) => Some(PathBuf::from(path)), - None => None, - } + env::var_os(key).map(PathBuf::from) } } diff --git a/src/linker.rs b/src/linker.rs index cf4e504..bc8effc 100644 --- a/src/linker.rs +++ b/src/linker.rs @@ -6,6 +6,7 @@ use crate::{ error::{AmbitError, AmbitResult}, }; use patmatch::{MatchOptions, Pattern}; +// Allow symlink to be executed with same function name. #[cfg(unix)] use std::os::unix::fs::symlink; #[cfg(windows)] @@ -51,10 +52,18 @@ fn get_paths_from_spec( // The only valid path at the start is the starting path. // This will be replaced at every iteration/depth. let mut valid_paths: Vec = vec![start_path.clone()]; - let components: Vec<_> = Path::new(&entry) + let components: Result, _> = Path::new(&entry) .components() - .map(|comp| comp.as_os_str().to_string_lossy()) + .map(|comp| { + comp.as_os_str().to_str().ok_or_else(|| { + AmbitError::Other(format!( + "The path \"{}\" contains non-UTF8 characters.", + entry + )) + }) + }) .collect(); + let components = components?; // To find matching files and directories, an entry as part of the spec is split into components. // For each component, a pattern is compiled and a vector of paths that match this pattern is found. // With the vector produced from the previous component, the process is repeated with the ancestor paths equal to the said vector. @@ -118,30 +127,26 @@ impl Linker { } } + // Remove all symlinked host files. pub fn clean_paths(&self) -> AmbitResult<()> { - let config_path = self.find_config_path(self.options.force)?; - let entries = config::get_entries(&config_path)?; let mut total_syncs: usize = 0; let mut deletions: usize = 0; - for entry in entries { - let paths = self.get_ambit_paths_from_entry(&entry)?; - for (repo_file, host_file) in paths { - if is_symlinked(&host_file.path, &repo_file.path) { - if !self.options.dry_run { - host_file.remove()?; - } + for (repo_file, host_file) in self.get_paths_from_config()? { + if is_symlinked(&host_file.path, &repo_file.path) { + if !self.options.dry_run { + host_file.remove()?; deletions += 1; - if !self.options.quiet { - let action = if self.options.dry_run { - "Ignored" - } else { - "Removed" - }; - println!("{} {}", action, host_file.path.display()); - } } - total_syncs += 1; + if !self.options.quiet { + let action = if self.options.dry_run { + "Ignored" + } else { + "Removed" + }; + println!("{} {}", action, host_file.path.display()); + } } + total_syncs += 1; } // Final clean metrics. println!( @@ -153,37 +158,33 @@ impl Linker { Ok(()) } + // Symlink host files to corresponding repo files. pub fn sync_paths(&self) -> AmbitResult<()> { let mut total: usize = 0; - let config_path = self.find_config_path(self.options.force)?; let mut symlink_pairs = Vec::new(); - for entry in config::get_entries(&config_path)? { - for (repo_file, host_file) in self.get_ambit_paths_from_entry(&entry)? { - if !repo_file.exists() { + for (repo_file, host_file) in self.get_paths_from_config()? { + if !repo_file.exists() { + return Err(AmbitError::Other(format!( + "Repository file {} must exist to be synced. Consider using `move`.", + repo_file.path.display() + ))); + } + // Only push into symlink_pairs if the file hasn't been symlinked already. + if !is_symlinked(&host_file.path, &repo_file.path) { + if host_file.exists() { return Err(AmbitError::Other(format!( - "Repository file {} must exist to be synced. Consider using `move`.", - repo_file.path.display() + "Host file {} already exists and is not correctly symlinked.", + host_file.path.display() ))); } - // Only push into symlink_pairs if it hasn't been symlinkd already. - if !is_symlinked(&host_file.path, &repo_file.path) { - if host_file.exists() { - return Err(AmbitError::Other(format!( - "Host file {} already exists and is not correctly symlinked.", - host_file.path.display() - ))); - } - symlink_pairs.push((repo_file, host_file)); - } - total += 1; + symlink_pairs.push((repo_file, host_file)); } + total += 1; } for (repo_file, host_file) in &symlink_pairs { if !self.options.dry_run { host_file.ensure_parent_dirs_exist()?; - // Attempt to symlink. if let Err(e) = symlink(&repo_file.path, &host_file.path) { - // Symlink went wrong return Err(AmbitError::Sync { host_file_path: PathBuf::from(&host_file.path), repo_file_path: PathBuf::from(&repo_file.path), @@ -220,32 +221,31 @@ impl Linker { Ok(()) } + // Move existing host files to corresponding non-existing repo files. pub fn move_paths(&self) -> AmbitResult<()> { let mut total: usize = 0; let mut total_moved: usize = 0; - let config_path = self.find_config_path(self.options.force)?; - for entry in config::get_entries(&config_path)? { - for (repo_file, host_file) in self.get_ambit_paths_from_entry(&entry)? { - total += 1; - if !repo_file.exists() && host_file.exists() { - if !self.options.dry_run { - total_moved += 1; - fs::rename(host_file.as_path(), repo_file.as_path())?; - } - if !self.options.quiet { - let action = if self.options.dry_run { - "Ignored moving" - } else { - "Moved" - }; - println!( - "{} {} to {}", - action, - host_file.display(), - repo_file.display() - ); - } - } + for (repo_file, host_file) in self.get_paths_from_config()? { + let moveable = !repo_file.exists() && host_file.exists(); + total += 1; + if moveable && !self.options.dry_run { + total_moved += 1; + fs::rename(host_file.as_path(), repo_file.as_path())?; + } + if !self.options.quiet { + let action = if self.options.dry_run { + "Ignored moving" + } else if !moveable { + "Ignored moving (already synced)" + } else { + "Moved" + }; + println!( + "{} {} to {}", + action, + host_file.display(), + repo_file.display() + ); } } // Final moved metrics. @@ -285,15 +285,16 @@ impl Linker { // Recursively search dotfile repository for config path. fn get_repo_config_paths(&self, stop_at_first_found: bool) -> Vec { let mut repo_config_paths = Vec::new(); - for dir_entry in WalkDir::new(self.paths.repo.as_path()) { - if let Ok(dir_entry) = dir_entry { - let path = dir_entry.path(); - if let Some(file_name) = path.file_name() { - if file_name == directories::CONFIG_NAME { - repo_config_paths.push(path.to_path_buf()); - if stop_at_first_found { - break; - } + for dir_entry in WalkDir::new(self.paths.repo.as_path()) + .into_iter() + .flatten() + { + let path = dir_entry.path(); + if let Some(file_name) = path.file_name() { + if file_name == directories::CONFIG_NAME { + repo_config_paths.push(path.to_path_buf()); + if stop_at_first_found { + break; } } } @@ -301,11 +302,20 @@ impl Linker { repo_config_paths } + fn get_paths_from_config(&self) -> AmbitResult> { + Ok( + config::get_entries(&self.find_config_path(self.options.force)?)? + .into_iter() + // Flatten results. + .flat_map(|entry| self.get_paths_from_entry(&entry)) + // Flatten vectors. + .flatten() + .collect(), + ) + } + // Return vector over path pairs in the form of `(repo_file, host_file)` from given entry. - fn get_ambit_paths_from_entry( - &self, - entry: &Entry, - ) -> AmbitResult> { + fn get_paths_from_entry(&self, entry: &Entry) -> AmbitResult> { // Only search left paths from repo. let left_paths = get_paths_from_spec(&entry.left, PathBuf::from(self.paths.repo.to_str()?), true)?;