From 2600bfbc9fab14c5e666f07ae966f1b1b7344c32 Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Sun, 18 Jul 2021 18:36:50 +0800 Subject: [PATCH 01/12] Add CopyTarget struct --- src/config.rs | 28 ++++++++++++++++++++++++++-- src/deploy.rs | 6 ++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index 83a2d30..c2004c5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -23,6 +23,15 @@ pub struct SymbolicTarget { pub condition: Option, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[serde(deny_unknown_fields)] +pub struct CopyTarget { + pub target: PathBuf, + pub owner: Option, + #[serde(rename = "if")] + pub condition: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] #[serde(deny_unknown_fields)] pub struct TemplateTarget { @@ -39,6 +48,7 @@ pub struct TemplateTarget { pub enum FileTarget { Automatic(PathBuf), Symbolic(SymbolicTarget), + Copy(CopyTarget), ComplexTemplate(TemplateTarget), } @@ -347,6 +357,7 @@ impl<'de> Deserialize<'de> for FileTarget { enum This { Automatic(PathBuf), Symbolic(SymbolicTarget), + Copy(CopyTarget), #[serde(rename = "template")] ComplexTemplate(TemplateTarget), } @@ -371,6 +382,7 @@ impl FileTarget { match self { FileTarget::Automatic(path) => &path, FileTarget::Symbolic(SymbolicTarget { target, .. }) + | FileTarget::Copy(CopyTarget { target, .. }) | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) => &target, } } @@ -379,6 +391,7 @@ impl FileTarget { match self { FileTarget::Automatic(ref mut path) => *path = new_path.into(), FileTarget::Symbolic(SymbolicTarget { target, .. }) + | FileTarget::Copy(CopyTarget { target, .. }) | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) => { *target = new_path.into() } @@ -388,8 +401,9 @@ impl FileTarget { pub fn condition(&self) -> Option<&String> { match self { FileTarget::Automatic(_) => None, - FileTarget::Symbolic(SymbolicTarget { condition, .. }) => condition.as_ref(), - FileTarget::ComplexTemplate(TemplateTarget { condition, .. }) => condition.as_ref(), + FileTarget::Symbolic(SymbolicTarget { condition, .. }) + | FileTarget::Copy(CopyTarget { condition, .. }) + | FileTarget::ComplexTemplate(TemplateTarget { condition, .. }) => condition.as_ref(), } } } @@ -410,6 +424,16 @@ impl> From for SymbolicTarget { } } +impl> From for CopyTarget { + fn from(input: T) -> Self { + CopyTarget { + target: input.into(), + owner: None, + condition: None, + } + } +} + impl> From for TemplateTarget { fn from(input: T) -> Self { TemplateTarget { diff --git a/src/deploy.rs b/src/deploy.rs index 86c48cd..7595876 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -96,6 +96,9 @@ Proceeding by copying instead of symlinking." FileTarget::Symbolic(target) => { desired_symlinks.insert(source, target); } + FileTarget::Copy(target) => { + warn!("Copy target not yet implemented"); + } FileTarget::ComplexTemplate(target) => { desired_templates.insert(source, target); } @@ -108,6 +111,9 @@ Proceeding by copying instead of symlinking." FileTarget::Symbolic(target) => { desired_templates.insert(source, target.into_template()); } + FileTarget::Copy(target) => { + warn!("Copy target not yet implemented"); + } FileTarget::ComplexTemplate(target) => { desired_templates.insert(source, target); } From 0ac042290b41b8f0ba6741308783a85731f1af4f Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Sun, 18 Jul 2021 18:57:41 +0800 Subject: [PATCH 02/12] Add caching & (un)deploy logic for copy target --- src/actions.rs | 47 ++++++++++++++++++++++++++++++- src/config.rs | 1 + src/deploy.rs | 75 ++++++++++++++++++++++++++++++++++++++++++++++++-- src/init.rs | 1 + 4 files changed, 120 insertions(+), 4 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index 2bf93c3..a5c4db7 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -4,15 +4,17 @@ use anyhow::{Context, Result}; use crossterm::style::Colorize; use handlebars::Handlebars; -use crate::config::{SymbolicTarget, TemplateTarget, Variables}; +use crate::config::{CopyTarget, SymbolicTarget, TemplateTarget, Variables}; use crate::difference; use crate::filesystem::{Filesystem, SymlinkComparison, TemplateComparison}; #[cfg_attr(test, mockall::automock)] pub trait ActionRunner { fn delete_symlink(&mut self, source: &Path, target: &Path) -> Result; + fn delete_copy(&mut self, source: &Path, target: &Path) -> Result; fn delete_template(&mut self, source: &Path, cache: &Path, target: &Path) -> Result; fn create_symlink(&mut self, source: &Path, target: &SymbolicTarget) -> Result; + fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result; fn create_template( &mut self, source: &Path, @@ -20,6 +22,7 @@ pub trait ActionRunner { target: &TemplateTarget, ) -> Result; fn update_symlink(&mut self, source: &Path, target: &SymbolicTarget) -> Result; + fn update_copy(&mut self, source: &Path, target: &CopyTarget) -> Result; fn update_template( &mut self, source: &Path, @@ -58,12 +61,18 @@ impl<'a> ActionRunner for RealActionRunner<'a> { fn delete_symlink(&mut self, source: &Path, target: &Path) -> Result { delete_symlink(source, target, self.fs, self.force) } + fn delete_copy(&mut self, source: &Path, target: &Path) -> Result { + delete_copy(source, target, self.fs, self.force) + } fn delete_template(&mut self, source: &Path, cache: &Path, target: &Path) -> Result { delete_template(source, cache, target, self.fs, self.force) } fn create_symlink(&mut self, source: &Path, target: &SymbolicTarget) -> Result { create_symlink(source, target, self.fs, self.force) } + fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result { + create_copy(source, target, self.fs, self.force) + } fn create_template( &mut self, source: &Path, @@ -83,6 +92,9 @@ impl<'a> ActionRunner for RealActionRunner<'a> { fn update_symlink(&mut self, source: &Path, target: &SymbolicTarget) -> Result { update_symlink(source, target, self.fs, self.force) } + fn update_copy(&mut self, source: &Path, target: &CopyTarget) -> Result { + update_copy(source, target, self.fs, self.force) + } fn update_template( &mut self, source: &Path, @@ -158,6 +170,17 @@ fn perform_symlink_target_deletion(fs: &mut dyn Filesystem, target: &Path) -> Re Ok(()) } +/// Returns true if copy should be deleted from cache +pub fn delete_copy( + source: &Path, + target: &Path, + fs: &mut dyn Filesystem, + force: bool, +) -> Result { + warn!("Delete copy not yet implemented"); + Ok(false) +} + /// Returns true if template should be deleted from cache pub fn delete_template( source: &Path, @@ -299,6 +322,17 @@ pub fn create_symlink( } } +/// Returns true if copy should be added to cache +pub fn create_copy( + source: &Path, + target: &CopyTarget, + fs: &mut dyn Filesystem, + force: bool, +) -> Result { + warn!("Create copy not yet implemented"); + Ok(false) +} + /// Returns true if the template should be added to cache pub fn create_template( source: &Path, @@ -456,6 +490,17 @@ pub fn update_symlink( } } +/// Returns true if the symlink wasn't skipped +pub fn update_copy( + source: &Path, + target: &CopyTarget, + fs: &mut dyn Filesystem, + force: bool, +) -> Result { + warn!("Update copy not yet implemented"); + Ok(false) +} + /// Returns true if the template was not skipped #[allow(clippy::too_many_arguments)] pub fn update_template( diff --git a/src/config.rs b/src/config.rs index c2004c5..8bc75a7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -159,6 +159,7 @@ pub fn load_configuration( #[serde(deny_unknown_fields)] pub struct Cache { pub symlinks: BTreeMap, + pub copies: BTreeMap, pub templates: BTreeMap, } diff --git a/src/deploy.rs b/src/deploy.rs index 7595876..7a05678 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use crate::actions::{self, ActionRunner, RealActionRunner}; use crate::args::Options; -use crate::config::{self, Cache, FileTarget, SymbolicTarget, TemplateTarget}; +use crate::config::{self, Cache, CopyTarget, FileTarget, SymbolicTarget, TemplateTarget}; use crate::display_error; use crate::filesystem::{self, load_file, Filesystem}; use crate::handlebars_helpers::create_new_handlebars; @@ -78,6 +78,7 @@ Proceeding by copying instead of symlinking." }; let mut desired_symlinks = BTreeMap::::new(); + let mut desired_copies = BTreeMap::::new(); let mut desired_templates = BTreeMap::::new(); for (source, target) in config.files { @@ -97,7 +98,7 @@ Proceeding by copying instead of symlinking." desired_symlinks.insert(source, target); } FileTarget::Copy(target) => { - warn!("Copy target not yet implemented"); + desired_copies.insert(source, target); } FileTarget::ComplexTemplate(target) => { desired_templates.insert(source, target); @@ -112,7 +113,7 @@ Proceeding by copying instead of symlinking." desired_templates.insert(source, target.into_template()); } FileTarget::Copy(target) => { - warn!("Copy target not yet implemented"); + desired_copies.insert(source, target); } FileTarget::ComplexTemplate(target) => { desired_templates.insert(source, target); @@ -134,6 +135,7 @@ Proceeding by copying instead of symlinking." let (suggest_force, mut error_occurred) = run_deploy( &mut runner, &desired_symlinks, + &desired_copies, &desired_templates, &mut cache, &opt, @@ -211,6 +213,16 @@ pub fn undeploy(opt: Options) -> Result { ); } + for (deleted_copy, target) in cache.copies.clone() { + execute_action( + actions::delete_copy(&deleted_copy, &target, fs, opt.force), + || cache.copies.remove(&deleted_copy), + || format!("delete copy {:?} -> {:?}", deleted_copy, target), + &mut suggest_force, + &mut error_occurred, + ); + } + for (deleted_template, target) in cache.templates.clone() { execute_action( actions::delete_template( @@ -257,6 +269,7 @@ pub fn undeploy(opt: Options) -> Result { fn run_deploy( runner: &mut A, desired_symlinks: &BTreeMap, + desired_copies: &BTreeMap, desired_templates: &BTreeMap, cache: &mut Cache, opt: &Options, @@ -270,6 +283,11 @@ fn run_deploy( .iter() .map(|(k, v)| (k.clone(), v.clone())) .collect(); + let existing_copies: BTreeSet<(PathBuf, PathBuf)> = cache + .copies + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); let existing_templates: BTreeSet<(PathBuf, PathBuf)> = cache .templates .iter() @@ -280,6 +298,10 @@ fn run_deploy( .iter() .map(|(k, v)| ((k.clone(), v.target.clone()), v)) .collect(); + let desired_copies: BTreeMap<(PathBuf, PathBuf), _> = desired_copies + .iter() + .map(|(k, v)| ((k.clone(), v.target.clone()), v)) + .collect(); let desired_templates: BTreeMap<(PathBuf, PathBuf), _> = desired_templates .iter() .map(|(k, v)| ((k.clone(), v.target.clone()), v)) @@ -300,6 +322,16 @@ fn run_deploy( ); } + for (source, target) in existing_copies.difference(&desired_copies.keys().cloned().collect()) { + execute_action( + runner.delete_copy(source, &target), + || resulting_cache.copies.remove(source), + || format!("delete copy {:?} -> {:?}", source, target), + &mut suggest_force, + &mut error_occurred, + ); + } + for (source, target) in existing_templates.difference(&desired_templates.keys().cloned().collect()) { @@ -334,6 +366,28 @@ fn run_deploy( ); } + for (source, target_path) in desired_copies + .keys() + .cloned() + .collect::>() + .difference(&existing_copies) + { + let target = desired_copies + .get(&(source.into(), target_path.into())) + .unwrap(); + execute_action( + runner.create_copy(&source, &target), + || { + resulting_cache + .copies + .insert(source.clone(), target_path.clone()) + }, + || format!("create copy {:?} -> {:?}", source, target_path), + &mut suggest_force, + &mut error_occurred, + ); + } + for (source, target_path) in desired_templates .keys() .cloned() @@ -371,6 +425,21 @@ fn run_deploy( ); } + for (source, target_path) in + existing_copies.intersection(&desired_copies.keys().cloned().collect()) + { + let target = desired_copies + .get(&(source.into(), target_path.into())) + .unwrap(); + execute_action( + runner.update_copy(&source, &target), + || (), + || format!("update copy {:?} -> {:?}", source, target_path), + &mut suggest_force, + &mut error_occurred, + ); + } + for (source, target_path) in existing_templates.intersection(&desired_templates.keys().cloned().collect()) { diff --git a/src/init.rs b/src/init.rs index bacb0bf..0354d64 100644 --- a/src/init.rs +++ b/src/init.rs @@ -42,6 +42,7 @@ pub fn init(opt: Options) -> Result<()> { &opt.cache_file, config::Cache { symlinks: BTreeMap::default(), + copies: BTreeMap::default(), templates: BTreeMap::default(), }, ) From 7e44dd363f99af14629f8e02fd96829272449837 Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Sun, 18 Jul 2021 20:05:18 +0800 Subject: [PATCH 03/12] Add copy target actions and comparison --- src/actions.rs | 160 ++++++++++++++++++++++++++++++++++++++++++++-- src/filesystem.rs | 87 +++++++++++++++++++++++++ 2 files changed, 240 insertions(+), 7 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index a5c4db7..66c3d9a 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -6,7 +6,7 @@ use handlebars::Handlebars; use crate::config::{CopyTarget, SymbolicTarget, TemplateTarget, Variables}; use crate::difference; -use crate::filesystem::{Filesystem, SymlinkComparison, TemplateComparison}; +use crate::filesystem::{CopyComparison, Filesystem, SymlinkComparison, TemplateComparison}; #[cfg_attr(test, mockall::automock)] pub trait ActionRunner { @@ -177,8 +177,49 @@ pub fn delete_copy( fs: &mut dyn Filesystem, force: bool, ) -> Result { - warn!("Delete copy not yet implemented"); - Ok(false) + info!("{} copy {:?} -> {:?}", "[-]".red(), source, target); + + let comparison = fs + .compare_copy(source, target) + .context("detect copy's current state")?; + debug!("Current state: {}", comparison); + + match comparison { + CopyComparison::Identical | CopyComparison::OnlyTargetExists => { + debug!("Performing deletion"); + perform_copy_target_deletion(fs, target).context("perform copy target deletion")?; + Ok(true) + } + CopyComparison::OnlySourceExists | CopyComparison::BothMissing => { + warn!( + "Deleting copy {:?} -> {:?} but target doesn't exist. Removing from cache anyways.", + source, target + ); + Ok(true) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory if force => { + warn!( + "Deleting copy {:?} -> {:?} but {}. Forcing.", + source, target, comparison + ); + perform_copy_target_deletion(fs, target).context("perform copy target deletion")?; + Ok(true) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory => { + error!( + "Deleting {:?} -> {:?} but {}. Skipping.", + source, target, comparison + ); + Ok(false) + } + } +} + +fn perform_copy_target_deletion(fs: &mut dyn Filesystem, target: &Path) -> Result<()> { + fs.remove_file(target).context("remove copy")?; + fs.delete_parents(target, false) + .context("delete parents of copy")?; + Ok(()) } /// Returns true if template should be deleted from cache @@ -329,8 +370,58 @@ pub fn create_copy( fs: &mut dyn Filesystem, force: bool, ) -> Result { - warn!("Create copy not yet implemented"); - Ok(false) + info!("{} copy {:?} -> {:?}", "[+]".green(), source, target.target); + + let comparison = fs + .compare_copy(source, &target.target) + .context("detect copy's current state")?; + debug!("Current state: {}", comparison); + + match comparison { + CopyComparison::OnlySourceExists => { + debug!("Performing creation"); + fs.create_dir_all( + target + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &target.target, &target.owner) + .context("create target copy")?; + Ok(true) + } + CopyComparison::Identical => { + warn!("Creating copy {:?} -> {:?} but target already exists and points at source. Adding to cache anyways", source, target.target); + Ok(true) + } + CopyComparison::OnlyTargetExists | CopyComparison::BothMissing => { + error!( + "Creating copy {:?} -> {:?} but {}. Skipping.", + source, target.target, comparison + ); + Ok(false) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory if force => { + warn!( + "Creating copy {:?} -> {:?} but {}. Forcing.", + source, target.target, comparison + ); + fs.remove_file(&target.target) + .context("remove copy target while forcing")?; + fs.copy_file(source, &target.target, &target.owner) + .context("create target copy")?; + Ok(true) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory => { + error!( + "Creating copy {:?} -> {:?} but {}. Skipping.", + source, target.target, comparison + ); + Ok(false) + } + } } /// Returns true if the template should be added to cache @@ -497,8 +588,63 @@ pub fn update_copy( fs: &mut dyn Filesystem, force: bool, ) -> Result { - warn!("Update copy not yet implemented"); - Ok(false) + debug!("Updating copy {:?} -> {:?}...", source, target.target); + + let comparison = fs + .compare_copy(&source, &target.target) + .context("detect copy's current state")?; + debug!("Current state: {}", comparison); + + match comparison { + CopyComparison::Identical => { + debug!("Performing update"); + fs.set_owner(&target.target, &target.owner) + .context("set target copy owner")?; + Ok(true) + } + CopyComparison::OnlyTargetExists | CopyComparison::BothMissing => { + error!( + "Updating copy {:?} -> {:?} but source is missing. Skipping.", + source, target.target + ); + Ok(false) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory if force => { + warn!( + "Updating copy {:?} -> {:?} but {}. Forcing.", + source, target.target, comparison + ); + fs.remove_file(&target.target) + .context("remove copy target while forcing")?; + fs.copy_file(source, &target.target, &target.owner) + .context("create target copy")?; + Ok(true) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory => { + error!( + "Updating copy {:?} -> {:?} but {}. Skipping.", + source, target.target, comparison + ); + Ok(false) + } + CopyComparison::OnlySourceExists => { + warn!( + "Updating copy {:?} -> {:?} but {}. Creating it anyways.", + source, target.target, comparison + ); + fs.create_dir_all( + target + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &target.target, &target.owner) + .context("create target copy")?; + Ok(true) + } + } } /// Returns true if the template was not skipped diff --git a/src/filesystem.rs b/src/filesystem.rs index a44a871..289e41a 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -46,6 +46,9 @@ pub trait Filesystem { /// Check state of expected symlink on disk fn compare_symlink(&mut self, source: &Path, link: &Path) -> Result; + /// Check state of expected copy on disk + fn compare_copy(&mut self, source: &Path, link: &Path) -> Result; + /// Check state of expected symbolic link on disk fn compare_template(&mut self, target: &Path, cache: &Path) -> Result; @@ -114,6 +117,15 @@ impl Filesystem for RealFilesystem { Ok(compare_symlink(&source, source_state, link_state)) } + fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { + let source_state = get_file_state(source).context("get source state")?; + trace!("Source state: {:#?}", source_state); + let target_state = get_file_state(target).context("get target state")?; + trace!("Target state: {:#?}", target_state); + + Ok(compare_copy(source_state, target_state)) + } + fn compare_template(&mut self, target: &Path, cache: &Path) -> Result { let target_state = get_file_state(target).context("get state of target")?; trace!("Target state: {:#?}", target_state); @@ -281,6 +293,15 @@ impl Filesystem for RealFilesystem { Ok(compare_symlink(&source, source_state, link_state)) } + fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { + let source_state = get_file_state(source).context("get source state")?; + trace!("Source state: {:#?}", source_state); + let target_state = get_file_state(target).context("get target state")?; + trace!("Target state: {:#?}", target_state); + + Ok(compare_copy(source_state, target_state)) + } + fn compare_template(&mut self, target: &Path, cache: &Path) -> Result { let target_state = get_file_state(target).context("get state of target")?; let cache_state = get_file_state(cache).context("get state of cache")?; @@ -593,6 +614,27 @@ impl Filesystem for DryRunFilesystem { Ok(compare_symlink(&source, source_state, link_state)) } + fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { + let source_state = if let Some(state) = self.file_states.get(source) { + debug!("Cached (probably not actual) source state: {:?}", state); + state.clone() + } else { + let state = get_file_state(source).context("get source state")?; + debug!("Source state: {:?}", state); + state + }; + let target_state = if let Some(state) = self.file_states.get(target) { + debug!("Cached (probably not actual) target state: {:?}", state); + state.clone() + } else { + let state = get_file_state(target).context("get target state")?; + debug!("Target state: {:?}", state); + state + }; + + Ok(compare_copy(source_state, target_state)) + } + fn compare_template(&mut self, target: &Path, cache: &Path) -> Result { let target_state = if let Some(state) = self.file_states.get(target) { debug!("Cached (probably not actual) target state: {:?}", state); @@ -776,6 +818,51 @@ fn compare_symlink( } } +#[derive(Debug, PartialEq)] +pub enum CopyComparison { + Identical, + OnlySourceExists, + OnlyTargetExists, + Changed, + TargetNotRegularFileOrDirectory, + BothMissing, +} + +impl std::fmt::Display for CopyComparison { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + use self::CopyComparison::*; + match self { + Identical => "target and source's contents are equal", + OnlySourceExists => "target doesn't exist", + OnlyTargetExists => "source doesn't exist", + Changed => "target contents were changed", + TargetNotRegularFileOrDirectory => "target is not a regular file or directory", + BothMissing => "source and target are missing", + } + .fmt(f) + } +} + +fn compare_copy(source_state: FileState, target_state: FileState) -> CopyComparison { + info!( + "source state: {:?} target_state: {:?}", + source_state, target_state + ); + match (source_state, target_state) { + (FileState::File(t), FileState::File(c)) => { + if t == c { + CopyComparison::Identical + } else { + CopyComparison::Changed + } + } + (FileState::File(_), FileState::Missing) => CopyComparison::OnlySourceExists, + (FileState::Missing, FileState::File(_)) => CopyComparison::OnlyTargetExists, + (FileState::Missing, FileState::Missing) => CopyComparison::BothMissing, + _ => CopyComparison::TargetNotRegularFileOrDirectory, + } +} + #[derive(Debug, PartialEq)] pub enum TemplateComparison { Identical, From 427c96473f5c721d968d463960c74eebe8dca7e6 Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Sun, 18 Jul 2021 20:22:42 +0800 Subject: [PATCH 04/12] Support copying non-text files as a different user --- src/filesystem.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index 289e41a..8c85817 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -455,8 +455,8 @@ impl Filesystem for RealFilesystem { use std::io::Write; if let Some(owner) = owner { - let contents = std::fs::read_to_string(source) - .context("read source file contents as current user")?; + let contents = + std::fs::read(source).context("read source file contents as current user")?; let mut child = self .sudo(format!( "Copying {:?} -> {:?} as user {:?}", @@ -477,7 +477,7 @@ impl Filesystem for RealFilesystem { .stdin .as_ref() .expect("has stdin") - .write_all(contents.as_bytes()) + .write_all(&contents) .context("give input to tee")?; let success = child.wait().context("wait for sudo tee")?.success(); From ee988f2c96470949c0de484cec2f48e8246b0179 Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Sun, 18 Jul 2021 20:31:42 +0800 Subject: [PATCH 05/12] Add copy target to tests --- src/deploy.rs | 53 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/src/deploy.rs b/src/deploy.rs index 7a05678..22736f4 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -501,14 +501,18 @@ mod test { fn high_level_simple() { // State let a_out: SymbolicTarget = "a_out".into(); - let b_out: TemplateTarget = "b_out".into(); + let b_out: CopyTarget = "b_out".into(); + let c_out: TemplateTarget = "c_out".into(); let desired_symlinks = maplit::btreemap! { PathBuf::from("a_in") => a_out.clone() }; - let desired_templates = maplit::btreemap! { + let desired_copies = maplit::btreemap! { PathBuf::from("b_in") => b_out.clone() }; + let desired_templates = maplit::btreemap! { + PathBuf::from("c_in") => c_out.clone() + }; // Test high level let mut runner = actions::MockActionRunner::new(); @@ -521,13 +525,19 @@ mod test { .with(function(path_eq("a_in")), eq(a_out)) .in_sequence(&mut seq) .returning(|_, _| Ok(true)); + runner + .expect_create_copy() + .times(1) + .with(function(path_eq("b_in")), eq(b_out)) + .in_sequence(&mut seq) + .returning(|_, _| Ok(true)); runner .expect_create_template() .times(1) .with( - function(path_eq("b_in")), - function(path_eq("cache/b_in")), - eq(b_out), + function(path_eq("c_in")), + function(path_eq("cache/c_in")), + eq(c_out), ) .in_sequence(&mut seq) .returning(|_, _, _| Ok(true)); @@ -535,6 +545,7 @@ mod test { let (suggest_force, error_occurred) = run_deploy( &mut runner, &desired_symlinks, + &desired_copies, &desired_templates, &mut cache, &Options { @@ -548,23 +559,29 @@ mod test { assert_eq!(error_occurred, false); assert!(cache.symlinks.contains_key(&PathBuf::from("a_in"))); - assert!(cache.templates.contains_key(&PathBuf::from("b_in"))); + assert!(cache.copies.contains_key(&PathBuf::from("b_in"))); + assert!(cache.templates.contains_key(&PathBuf::from("c_in"))); assert_eq!(cache.symlinks.len(), 1); assert_eq!(cache.templates.len(), 1); + assert_eq!(cache.copies.len(), 1); } #[test] fn high_level_skip() { // Setup let a_out: SymbolicTarget = "a_out".into(); - let b_out: TemplateTarget = "b_out".into(); + let b_out: CopyTarget = "b_out".into(); + let c_out: TemplateTarget = "c_out".into(); let desired_symlinks = maplit::btreemap! { PathBuf::from("a_in") => a_out.clone() }; - let desired_templates = maplit::btreemap! { + let desired_copies = maplit::btreemap! { PathBuf::from("b_in") => b_out.clone() }; + let desired_templates = maplit::btreemap! { + PathBuf::from("c_in") => c_out.clone() + }; let mut runner = actions::MockActionRunner::new(); let mut seq = mockall::Sequence::new(); @@ -577,13 +594,19 @@ mod test { .with(function(path_eq("a_in")), eq(a_out)) .in_sequence(&mut seq) .returning(|_, _| Err(anyhow::anyhow!("oh no"))); + runner + .expect_create_copy() + .times(1) + .with(function(path_eq("b_in")), eq(b_out)) + .in_sequence(&mut seq) + .returning(|_, _| Ok(false)); runner .expect_create_template() .times(1) .with( - function(path_eq("b_in")), - function(path_eq("cache/b_in")), - eq(b_out), + function(path_eq("c_in")), + function(path_eq("cache/c_in")), + eq(c_out), ) .in_sequence(&mut seq) .returning(|_, _, _| Ok(false)); @@ -592,6 +615,7 @@ mod test { let (suggest_force, error_occurred) = run_deploy( &mut runner, &desired_symlinks, + &desired_copies, &desired_templates, &mut cache, &Options { @@ -606,6 +630,7 @@ mod test { assert_eq!(cache.symlinks.len(), 0); assert_eq!(cache.templates.len(), 0); + assert_eq!(cache.copies.len(), 0); } #[test] @@ -623,6 +648,7 @@ mod test { symlinks: maplit::btreemap! { PathBuf::from("a_in") => "a_out_old".into() }, + copies: BTreeMap::new(), templates: BTreeMap::new(), }; @@ -645,6 +671,7 @@ mod test { &mut runner, &desired_symlinks, &BTreeMap::new(), + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), @@ -673,6 +700,7 @@ mod test { let mut seq = mockall::Sequence::new(); let mut cache = Cache { symlinks: BTreeMap::new(), + copies: BTreeMap::new(), templates: maplit::btreemap! { PathBuf::from("a_in") => "a_out_old".into() }, @@ -701,6 +729,7 @@ mod test { &mut runner, &desired_symlinks, &BTreeMap::new(), + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), @@ -728,6 +757,7 @@ mod test { let mut seq = mockall::Sequence::new(); let mut cache = Cache { symlinks: BTreeMap::new(), + copies: BTreeMap::new(), templates: maplit::btreemap! { PathBuf::from("a_in") => "a_out_old".into() }, @@ -750,6 +780,7 @@ mod test { &mut runner, &desired_symlinks, &BTreeMap::new(), + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), From 1a95c610e0bba8d970925a934f7ef57deca328ad Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Sun, 18 Jul 2021 21:10:31 +0800 Subject: [PATCH 06/12] Warn if file set as template is not templated --- src/actions.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index 66c3d9a..841b8fa 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -742,13 +742,16 @@ pub(crate) fn perform_template_deploy( handlebars: &Handlebars<'_>, variables: &Variables, ) -> Result<()> { - let file_contents = fs + let original_file_contents = fs .read_to_string(&source) .context("read template source file")?; - let file_contents = target.apply_actions(file_contents); + let file_contents = target.apply_actions(original_file_contents.clone()); let rendered = handlebars .render_template(&file_contents, variables) .context("render template")?; + if original_file_contents == rendered { + warn!("File {:?} is specified as 'template' but is not a templated file. Consider using 'copy' instead.", source); + } // Cache fs.create_dir_all(&cache.parent().context("get parent of cache file")?, &None) From 2c6587cfab749e13ea958eb3edadf2835f12dab8 Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Tue, 20 Jul 2021 12:20:24 +0800 Subject: [PATCH 07/12] Silence clippy --- src/config.rs | 15 ++++++--------- src/deploy.rs | 40 +++++++++++++++++++-------------------- src/handlebars_helpers.rs | 38 ++++++++++++------------------------- 3 files changed, 38 insertions(+), 55 deletions(-) diff --git a/src/config.rs b/src/config.rs index 8bc75a7..269d8ab 100644 --- a/src/config.rs +++ b/src/config.rs @@ -236,7 +236,7 @@ fn merge_configuration_files( // Patch each package with included.toml's for included_path in &local.includes { || -> Result<()> { - let mut included: IncludedConfig = filesystem::load_file(&included_path) + let mut included: IncludedConfig = filesystem::load_file(included_path) .and_then(|c| c.ok_or_else(|| anyhow::anyhow!("file not found"))) .context("load file")?; @@ -381,7 +381,7 @@ impl<'de> Deserialize<'de> for FileTarget { impl FileTarget { pub fn path(&self) -> &Path { match self { - FileTarget::Automatic(path) => &path, + FileTarget::Automatic(path) => path, FileTarget::Symbolic(SymbolicTarget { target, .. }) | FileTarget::Copy(CopyTarget { target, .. }) | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) => &target, @@ -584,17 +584,14 @@ mod tests { .file, FileTarget::ComplexTemplate(PathBuf::from("~/.QuarticCat").into()), ); - assert_eq!( - parse( - r#" + assert!(parse( + r#" [file] target = '~/.QuarticCat' type = 'symbolic' append = 'whatever' "#, - ) - .is_err(), - true - ); + ) + .is_err()); } } diff --git a/src/deploy.rs b/src/deploy.rs index 22736f4..7db8c31 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -138,7 +138,7 @@ Proceeding by copying instead of symlinking." &desired_copies, &desired_templates, &mut cache, - &opt, + opt, ); // === Post-deploy === @@ -314,7 +314,7 @@ fn run_deploy( existing_symlinks.difference(&desired_symlinks.keys().cloned().collect()) { execute_action( - runner.delete_symlink(&source, &target), + runner.delete_symlink(source, target), || resulting_cache.symlinks.remove(source), || format!("delete symlink {:?} -> {:?}", source, target), &mut suggest_force, @@ -324,7 +324,7 @@ fn run_deploy( for (source, target) in existing_copies.difference(&desired_copies.keys().cloned().collect()) { execute_action( - runner.delete_copy(source, &target), + runner.delete_copy(source, target), || resulting_cache.copies.remove(source), || format!("delete copy {:?} -> {:?}", source, target), &mut suggest_force, @@ -336,7 +336,7 @@ fn run_deploy( existing_templates.difference(&desired_templates.keys().cloned().collect()) { execute_action( - runner.delete_template(&source, &opt.cache_directory.join(&source), &target), + runner.delete_template(source, &opt.cache_directory.join(&source), target), || resulting_cache.templates.remove(source), || format!("delete template {:?} -> {:?}", source, target), &mut suggest_force, @@ -354,7 +354,7 @@ fn run_deploy( .get(&(source.into(), target_path.into())) .unwrap(); execute_action( - runner.create_symlink(&source, &target), + runner.create_symlink(source, target), || { resulting_cache .symlinks @@ -376,7 +376,7 @@ fn run_deploy( .get(&(source.into(), target_path.into())) .unwrap(); execute_action( - runner.create_copy(&source, &target), + runner.create_copy(source, target), || { resulting_cache .copies @@ -398,7 +398,7 @@ fn run_deploy( .get(&(source.into(), target_path.into())) .unwrap(); execute_action( - runner.create_template(&source, &opt.cache_directory.join(&source), &target), + runner.create_template(source, &opt.cache_directory.join(&source), target), || { resulting_cache .templates @@ -417,7 +417,7 @@ fn run_deploy( .get(&(source.into(), target_path.into())) .unwrap(); execute_action( - runner.update_symlink(&source, &target), + runner.update_symlink(source, target), || (), || format!("update symlink {:?} -> {:?}", source, target_path), &mut suggest_force, @@ -432,7 +432,7 @@ fn run_deploy( .get(&(source.into(), target_path.into())) .unwrap(); execute_action( - runner.update_copy(&source, &target), + runner.update_copy(source, target), || (), || format!("update copy {:?} -> {:?}", source, target_path), &mut suggest_force, @@ -447,7 +447,7 @@ fn run_deploy( .get(&(source.into(), target_path.into())) .unwrap(); execute_action( - runner.update_template(&source, &opt.cache_directory.join(&source), &target), + runner.update_template(source, &opt.cache_directory.join(&source), target), || (), || format!("update template {:?} -> {:?}", source, target_path), &mut suggest_force, @@ -555,8 +555,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert!(cache.symlinks.contains_key(&PathBuf::from("a_in"))); assert!(cache.copies.contains_key(&PathBuf::from("b_in"))); @@ -625,8 +625,8 @@ mod test { }, ); - assert_eq!(suggest_force, true); - assert_eq!(error_occurred, true); + assert!(suggest_force); + assert!(error_occurred); assert_eq!(cache.symlinks.len(), 0); assert_eq!(cache.templates.len(), 0); @@ -680,8 +680,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert_eq!(cache.symlinks.len(), 1); assert_eq!(cache.templates.len(), 0); @@ -738,8 +738,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert_eq!(cache.symlinks.len(), 1); assert_eq!(cache.templates.len(), 0); @@ -789,8 +789,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert_eq!(cache.symlinks.len(), 1); assert_eq!(cache.templates.len(), 0); diff --git a/src/handlebars_helpers.rs b/src/handlebars_helpers.rs index 673a405..c193095 100644 --- a/src/handlebars_helpers.rs +++ b/src/handlebars_helpers.rs @@ -326,21 +326,11 @@ mod test { }; let handlebars = create_new_handlebars(&mut config).unwrap(); - assert_eq!( - eval_condition(&handlebars, &config.variables, "foo").unwrap(), - true - ); - assert_eq!( - eval_condition(&handlebars, &config.variables, "bar").unwrap(), - false - ); - assert_eq!( - eval_condition(&handlebars, &config.variables, "dotter.packages.default").unwrap(), - true - ); - assert_eq!( - eval_condition(&handlebars, &config.variables, "dotter.packages.nonexist").unwrap(), - false + assert!(eval_condition(&handlebars, &config.variables, "foo").unwrap(),); + assert!(!eval_condition(&handlebars, &config.variables, "bar").unwrap(),); + assert!(eval_condition(&handlebars, &config.variables, "dotter.packages.default").unwrap(),); + assert!( + !eval_condition(&handlebars, &config.variables, "dotter.packages.nonexist").unwrap(), ); } @@ -354,18 +344,14 @@ mod test { }; let handlebars = create_new_handlebars(&mut config).unwrap(); - assert_eq!( - eval_condition( - &handlebars, - &config.variables, - "(is_executable \"no_such_executable_please\")" - ) - .unwrap(), - false - ); - assert_eq!( + assert!(!eval_condition( + &handlebars, + &config.variables, + "(is_executable \"no_such_executable_please\")" + ) + .unwrap(),); + assert!( eval_condition(&handlebars, &config.variables, "(eq (math \"5+5\") \"10\")").unwrap(), - true ); } } From 32c3508ceaa99000b9e3f755b0188cff6ecb0c74 Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Tue, 20 Jul 2021 12:26:16 +0800 Subject: [PATCH 08/12] Rename TargetNotRegularFileOrDirectory --- src/actions.rs | 12 ++++++------ src/filesystem.rs | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index 841b8fa..0e59ffd 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -197,7 +197,7 @@ pub fn delete_copy( ); Ok(true) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory if force => { + CopyComparison::Changed | CopyComparison::TargetNotRegularFile if force => { warn!( "Deleting copy {:?} -> {:?} but {}. Forcing.", source, target, comparison @@ -205,7 +205,7 @@ pub fn delete_copy( perform_copy_target_deletion(fs, target).context("perform copy target deletion")?; Ok(true) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory => { + CopyComparison::Changed | CopyComparison::TargetNotRegularFile => { error!( "Deleting {:?} -> {:?} but {}. Skipping.", source, target, comparison @@ -403,7 +403,7 @@ pub fn create_copy( ); Ok(false) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory if force => { + CopyComparison::Changed | CopyComparison::TargetNotRegularFile if force => { warn!( "Creating copy {:?} -> {:?} but {}. Forcing.", source, target.target, comparison @@ -414,7 +414,7 @@ pub fn create_copy( .context("create target copy")?; Ok(true) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory => { + CopyComparison::Changed | CopyComparison::TargetNotRegularFile => { error!( "Creating copy {:?} -> {:?} but {}. Skipping.", source, target.target, comparison @@ -609,7 +609,7 @@ pub fn update_copy( ); Ok(false) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory if force => { + CopyComparison::Changed | CopyComparison::TargetNotRegularFile if force => { warn!( "Updating copy {:?} -> {:?} but {}. Forcing.", source, target.target, comparison @@ -620,7 +620,7 @@ pub fn update_copy( .context("create target copy")?; Ok(true) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFileOrDirectory => { + CopyComparison::Changed | CopyComparison::TargetNotRegularFile => { error!( "Updating copy {:?} -> {:?} but {}. Skipping.", source, target.target, comparison diff --git a/src/filesystem.rs b/src/filesystem.rs index 8c85817..f255540 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -824,7 +824,7 @@ pub enum CopyComparison { OnlySourceExists, OnlyTargetExists, Changed, - TargetNotRegularFileOrDirectory, + TargetNotRegularFile, BothMissing, } @@ -836,7 +836,7 @@ impl std::fmt::Display for CopyComparison { OnlySourceExists => "target doesn't exist", OnlyTargetExists => "source doesn't exist", Changed => "target contents were changed", - TargetNotRegularFileOrDirectory => "target is not a regular file or directory", + TargetNotRegularFile => "target is not a regular file", BothMissing => "source and target are missing", } .fmt(f) @@ -859,7 +859,7 @@ fn compare_copy(source_state: FileState, target_state: FileState) -> CopyCompari (FileState::File(_), FileState::Missing) => CopyComparison::OnlySourceExists, (FileState::Missing, FileState::File(_)) => CopyComparison::OnlyTargetExists, (FileState::Missing, FileState::Missing) => CopyComparison::BothMissing, - _ => CopyComparison::TargetNotRegularFileOrDirectory, + _ => CopyComparison::TargetNotRegularFile, } } From 85e3b697b9bf98811d13450cd5bcbd6d25a8a42a Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Tue, 20 Jul 2021 12:40:24 +0800 Subject: [PATCH 09/12] Fallback to copy target when symlinks are not enabled --- src/config.rs | 14 ++++++-------- src/deploy.rs | 53 ++++++++++++++++++++------------------------------- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/config.rs b/src/config.rs index 269d8ab..413964b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -447,14 +447,12 @@ impl> From for TemplateTarget { } } -impl SymbolicTarget { - pub fn into_template(self) -> TemplateTarget { - TemplateTarget { - target: self.target, - owner: self.owner, - condition: self.condition, - prepend: None, - append: None, +impl From for CopyTarget { + fn from(st: SymbolicTarget) -> Self { + CopyTarget { + target: st.target, + owner: st.owner, + condition: st.condition, } } } diff --git a/src/deploy.rs b/src/deploy.rs index 7db8c31..46df6f7 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -82,42 +82,31 @@ Proceeding by copying instead of symlinking." let mut desired_templates = BTreeMap::::new(); for (source, target) in config.files { - if symlinks_enabled { - match target { - FileTarget::Automatic(target) => { - if fs - .is_template(&source) - .context(format!("check whether {:?} is a template", source))? - { - desired_templates.insert(source, target.into()); - } else { - desired_symlinks.insert(source, target.into()); - } + match target { + FileTarget::Automatic(target) => { + if fs + .is_template(&source) + .context(format!("check whether {:?} is a template", source))? + { + desired_templates.insert(source, target.into()); + } else if symlinks_enabled { + desired_symlinks.insert(source, target.into()); + } else { + desired_copies.insert(source, target.into()); } - FileTarget::Symbolic(target) => { + } + FileTarget::Symbolic(target) => { + if symlinks_enabled { desired_symlinks.insert(source, target); - } - FileTarget::Copy(target) => { - desired_copies.insert(source, target); - } - FileTarget::ComplexTemplate(target) => { - desired_templates.insert(source, target); + } else { + desired_copies.insert(source, target.into()); } } - } else { - match target { - FileTarget::Automatic(target) => { - desired_templates.insert(source, target.into()); - } - FileTarget::Symbolic(target) => { - desired_templates.insert(source, target.into_template()); - } - FileTarget::Copy(target) => { - desired_copies.insert(source, target); - } - FileTarget::ComplexTemplate(target) => { - desired_templates.insert(source, target); - } + FileTarget::Copy(target) => { + desired_copies.insert(source, target); + } + FileTarget::ComplexTemplate(target) => { + desired_templates.insert(source, target); } } } From c854e91774dfa0a4eee55d2f3253e413e19d8ccb Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Tue, 20 Jul 2021 15:59:42 +0800 Subject: [PATCH 10/12] Store file contents as Vec --- src/filesystem.rs | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index f255540..075c0e9 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -149,7 +149,15 @@ impl Filesystem for RealFilesystem { } fn read_to_string(&mut self, path: &Path) -> Result { - fs::read_to_string(path).context("read from file") + match fs::read_to_string(path) { + Ok(s) => Ok(s), + Err(e) if e.kind() == ErrorKind::InvalidData => { + anyhow::bail!( + "invalid UTF-8 in template source - consider using copy target instead" + ) + } + Err(e) => Err(anyhow::anyhow!(e)), + } } fn write(&mut self, path: &Path, content: String) -> Result<()> { @@ -342,7 +350,15 @@ impl Filesystem for RealFilesystem { } fn read_to_string(&mut self, path: &Path) -> Result { - fs::read_to_string(path).context("read from file") + match fs::read_to_string(path) { + Ok(s) => Ok(s), + Err(e) if e.kind() == ErrorKind::InvalidData => { + anyhow::bail!( + "invalid UTF-8 in template source - consider using copy target instead" + ) + } + Err(e) => Err(anyhow::anyhow!(e)), + } } fn write(&mut self, path: &Path, content: String) -> Result<()> { @@ -569,8 +585,7 @@ pub struct DryRunFilesystem { #[derive(Debug, Clone, PartialEq)] enum FileState { - /// None if file is invalid UTF-8 - File(Option), + File(Vec), SymbolicLink(PathBuf), Directory, Missing, @@ -670,7 +685,12 @@ impl Filesystem for DryRunFilesystem { fn read_to_string(&mut self, path: &Path) -> Result { debug!("Reading contents of file {:?}", path); match self.get_state(path).context("get file state")? { - FileState::File(s) => Ok(s.context("invalid utf-8 in template source")?), + FileState::File(bytes) => match String::from_utf8(bytes) { + Ok(s) => Ok(s), + Err(_) => anyhow::bail!( + "invalid UTF-8 in template source - consider using copy target instead" + ), + }, _ => anyhow::bail!("writing to non-file"), } } @@ -678,7 +698,7 @@ impl Filesystem for DryRunFilesystem { fn write(&mut self, path: &Path, content: String) -> Result<()> { debug!("Writing contents {:?} to file {:?}", content, path); self.file_states - .insert(path.into(), FileState::File(Some(content))); + .insert(path.into(), FileState::File(content.into_bytes())); Ok(()) } @@ -765,9 +785,8 @@ fn get_file_state(path: &Path) -> Result { return Ok(FileState::Directory); } - match fs::read_to_string(path) { - Ok(f) => Ok(FileState::File(Some(f))), - Err(e) if e.kind() == ErrorKind::InvalidData => Ok(FileState::File(None)), + match fs::read(path) { + Ok(f) => Ok(FileState::File(f)), Err(e) if e.kind() == ErrorKind::NotFound => Ok(FileState::Missing), Err(e) => Err(e).context("read contents of file that isn't symbolic or directory")?, } @@ -1073,7 +1092,7 @@ mod test { // Verify all actions assert_eq!( fs.file_states.get(&PathBuf::from("source")), - Some(&FileState::File(Some("{{name}}".into()))) + Some(&FileState::File("{{name}}".into())) ); assert_eq!( fs.file_states.get(&PathBuf::from("cache_dir")), @@ -1081,7 +1100,7 @@ mod test { ); assert_eq!( fs.file_states.get(&PathBuf::from("cache_dir/cache")), - Some(&FileState::File(Some("John".into()))) + Some(&FileState::File("John".into())) ); assert_eq!( fs.file_states.get(&PathBuf::from("target_dir")), @@ -1089,7 +1108,7 @@ mod test { ); assert_eq!( fs.file_states.get(&PathBuf::from("target_dir/target")), - Some(&FileState::File(Some("John".into()))) + Some(&FileState::File("John".into())) ); } From 0d0d7e49adde3d112415a59b75469a6101af82ad Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Tue, 20 Jul 2021 16:19:22 +0800 Subject: [PATCH 11/12] Check if hook needs to be rendered --- src/hooks.rs | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/hooks.rs b/src/hooks.rs index 8342e7f..7089497 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -4,6 +4,8 @@ use handlebars::Handlebars; use std::path::Path; use std::process::Command; +use crate::filesystem::{Filesystem, RealFilesystem}; + pub(crate) fn run_hook( location: &Path, cache_dir: &Path, @@ -15,23 +17,35 @@ pub(crate) fn run_hook( return Ok(()); } - let mut script_file = cache_dir.join(location); - if cfg!(windows) { - script_file.set_extension("bat"); + let fs: &mut dyn Filesystem = &mut RealFilesystem::new(false); + + // Default to current location + let mut script_file = location.into(); + + // If it is templated, then render it into cache and run from there + if fs + .is_template(location) + .context(format!("check whether {:?} is a template", location))? + { + script_file = cache_dir.join(location); + if cfg!(windows) { + script_file.set_extension("bat"); + } + + debug!("Rendering script {:?} -> {:?}", location, script_file); + + crate::actions::perform_template_deploy( + location, + &script_file, + &std::env::temp_dir().join("dotter_temp").into(), + fs, + handlebars, + variables, + ) + .context("deploy script")?; } - debug!("Rendering script {:?} -> {:?}", location, script_file); - - crate::actions::perform_template_deploy( - location, - &script_file, - &std::env::temp_dir().join("dotter_temp").into(), - &mut crate::filesystem::RealFilesystem::new(false), - handlebars, - variables, - ) - .context("deploy script")?; - - debug!("Running script file "); + + debug!("Running script file"); let mut child = if cfg!(windows) { Command::new(script_file) .spawn() From 694c83ecfc042de666227e6e351d51daca543846 Mon Sep 17 00:00:00 2001 From: PlayerNameHere Date: Tue, 20 Jul 2021 18:31:00 +0800 Subject: [PATCH 12/12] Remove unnecessary dyn type annotation --- src/hooks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks.rs b/src/hooks.rs index 7089497..98a27bc 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -17,7 +17,7 @@ pub(crate) fn run_hook( return Ok(()); } - let fs: &mut dyn Filesystem = &mut RealFilesystem::new(false); + let fs = &mut RealFilesystem::new(false); // Default to current location let mut script_file = location.into();