From 14e50c351b78d903c9c1ca540f5ca0cad8633b39 Mon Sep 17 00:00:00 2001 From: jdx <216188+jdx@users.noreply.github.com> Date: Tue, 5 Nov 2024 16:09:59 -0600 Subject: [PATCH] fix: use project_root for task execution Fixe #1660 --- src/cli/run.rs | 50 +++++++++------ src/cli/tasks/edit.rs | 11 +--- src/config/config_file/mise_toml.rs | 2 + src/config/mod.rs | 26 +++++--- src/task/mod.rs | 95 +++++++++++++---------------- src/task/task_script_parser.rs | 24 ++++---- src/tera.rs | 42 +++++++------ src/ui/ctrlc.rs | 2 +- 8 files changed, 134 insertions(+), 118 deletions(-) diff --git a/src/cli/run.rs b/src/cli/run.rs index 72ff1aeb6..bc0f47125 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -224,7 +224,7 @@ impl Run { trace!("running task: {task}"); if let Err(err) = self.run_task(&env, &task) { let prefix = task.estyled_prefix(); - eprintln!("{prefix} {} {err}", style::ered("ERROR"),); + eprintln!("{prefix} {} {err:?}", style::ered("ERROR"),); let _ = tx_err.send((task.clone(), Error::get_exit_status(&err))); } } @@ -269,7 +269,7 @@ impl Run { fn run_task(&self, env: &BTreeMap, task: &Task) -> Result<()> { let prefix = task.estyled_prefix(); - if !self.force && self.sources_are_fresh(task) { + if !self.force && self.sources_are_fresh(task)? { eprintln!("{prefix} sources up-to-date, skipping"); return Ok(()); } @@ -322,7 +322,11 @@ impl Run { prefix: &str, ) -> Result<()> { let script = script.trim_start(); - let cmd = trunc(&style::ebold(format!("$ {script}")).bright().to_string()); + let cmd = trunc( + &style::ebold(format!("$ {script} {args}", args = args.join(" "))) + .bright() + .to_string(), + ); eprintln!("{prefix} {cmd}"); if script.starts_with("#!") { @@ -423,9 +427,15 @@ impl Run { if self.raw(task) { cmd.with_raw(); } - if let Some(cd) = &self.cd.as_ref().or(task.dir.as_ref()) { - cmd = cmd.current_dir(cd); + let dir = self.cwd(task)?; + if !dir.exists() { + eprintln!( + "{prefix} {} task directory does not exist: {}", + style::eyellow("WARN"), + display_path(&dir) + ); } + cmd = cmd.current_dir(dir); if self.dry_run { return Ok(()); } @@ -496,23 +506,23 @@ impl Run { Ok(()) } - fn sources_are_fresh(&self, task: &Task) -> bool { + fn sources_are_fresh(&self, task: &Task) -> Result { if task.sources.is_empty() && task.outputs.is_empty() { - return false; + return Ok(false); } let run = || -> Result { - let sources = self.get_last_modified(&self.cwd(task), &task.sources)?; - let outputs = self.get_last_modified(&self.cwd(task), &task.outputs)?; + let sources = self.get_last_modified(&self.cwd(task)?, &task.sources)?; + let outputs = self.get_last_modified(&self.cwd(task)?, &task.outputs)?; trace!("sources: {sources:?}, outputs: {outputs:?}"); match (sources, outputs) { (Some(sources), Some(outputs)) => Ok(sources < outputs), _ => Ok(false), } }; - run().unwrap_or_else(|err| { + Ok(run().unwrap_or_else(|err| { warn!("sources_are_fresh: {err}"); false - }) + })) } fn add_failed_task(&self, task: Task, status: Option) { @@ -549,13 +559,17 @@ impl Run { Ok(last_mod) } - fn cwd(&self, task: &Task) -> PathBuf { - self.cd - .as_ref() - .or(task.dir.as_ref()) - .cloned() - .or_else(|| CONFIG.project_root.clone()) - .unwrap_or_else(|| env::current_dir().unwrap().clone()) + fn cwd(&self, task: &Task) -> Result { + if let Some(d) = &self.cd { + Ok(d.clone()) + } else if let Some(d) = task.dir()? { + Ok(d) + } else { + Ok(CONFIG + .project_root + .clone() + .unwrap_or_else(|| env::current_dir().unwrap())) + } } fn save_checksum(&self, task: &Task) -> Result<()> { diff --git a/src/cli/tasks/edit.rs b/src/cli/tasks/edit.rs index e9f12b5ab..c4ced614c 100644 --- a/src/cli/tasks/edit.rs +++ b/src/cli/tasks/edit.rs @@ -29,14 +29,9 @@ impl TasksEdit { .cloned() .map_or_else( || { - let path = config - .project_root - .as_ref() - .unwrap_or(&env::current_dir()?) - .join(".mise") - .join("tasks") - .join(&self.task); - Task::from_path(&path) + let project_root = config.project_root.clone().unwrap_or(env::current_dir()?); + let path = project_root.join(".mise").join("tasks").join(&self.task); + Task::from_path(&path, path.parent().unwrap(), &project_root) }, Ok, )?; diff --git a/src/config/config_file/mise_toml.rs b/src/config/config_file/mise_toml.rs index 3d3b60861..6cf2ff39e 100644 --- a/src/config/config_file/mise_toml.rs +++ b/src/config/config_file/mise_toml.rs @@ -96,8 +96,10 @@ impl MiseToml { rf.context .insert("config_root", path.parent().unwrap().to_str().unwrap()); rf.path = path.to_path_buf(); + let project_root = rf.project_root().map(|p| p.to_path_buf()); for task in rf.tasks.0.values_mut() { task.config_source.clone_from(&rf.path); + task.config_root = project_root.clone(); } // trace!("{}", rf.dump()?); Ok(rf) diff --git a/src/config/mod.rs b/src/config/mod.rs index 559ef573a..ff4d2f441 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -301,7 +301,7 @@ impl Config { let file_tasks = includes .into_par_iter() .flat_map(|p| { - self.load_tasks_includes(&p).unwrap_or_else(|err| { + self.load_tasks_includes(&p, dir).unwrap_or_else(|err| { warn!("loading tasks in {}: {err}", display_path(&p)); vec![] }) @@ -348,19 +348,24 @@ impl Config { fn load_global_tasks(&self) -> Result> { let cf = self.config_files.get(&*env::MISE_GLOBAL_CONFIG_FILE); + let config_root = cf.and_then(|cf| cf.project_root()).unwrap_or(&*env::HOME); Ok(self .load_config_tasks(&cf) .into_iter() - .chain(self.load_file_tasks(&cf)) + .chain(self.load_file_tasks(&cf, config_root)) .collect()) } fn load_system_tasks(&self) -> Result> { let cf = self.config_files.get(&dirs::SYSTEM.join("config.toml")); + let config_root = cf + .and_then(|cf| cf.project_root()) + .map(|p| p.to_path_buf()) + .unwrap_or_default(); Ok(self .load_config_tasks(&cf) .into_iter() - .chain(self.load_file_tasks(&cf)) + .chain(self.load_file_tasks(&cf, &config_root)) .collect()) } @@ -374,7 +379,7 @@ impl Config { } #[allow(clippy::borrowed_box)] - fn load_file_tasks(&self, cf: &Option<&Box>) -> Vec { + fn load_file_tasks(&self, cf: &Option<&Box>, config_root: &Path) -> Vec { let includes = match cf { Some(cf) => cf .task_config() @@ -389,15 +394,16 @@ impl Config { includes .into_iter() .flat_map(|p| { - self.load_tasks_includes(&p).unwrap_or_else(|err| { - warn!("loading tasks in {}: {err}", display_path(&p)); - vec![] - }) + self.load_tasks_includes(&p, config_root) + .unwrap_or_else(|err| { + warn!("loading tasks in {}: {err}", display_path(&p)); + vec![] + }) }) .collect() } - fn load_tasks_includes(&self, root: &Path) -> Result> { + fn load_tasks_includes(&self, root: &Path, config_root: &Path) -> Result> { if !root.is_dir() { return Ok(vec![]); } @@ -411,7 +417,7 @@ impl Config { .try_collect::<_, Vec, _>()? .into_par_iter() .filter(|p| file::is_executable(p)) - .map(|path| Task::from_path(&path)) + .map(|path| Task::from_path(&path, root, config_root)) .collect() } diff --git a/src/task/mod.rs b/src/task/mod.rs index f183fcbd5..c22327a1d 100644 --- a/src/task/mod.rs +++ b/src/task/mod.rs @@ -40,6 +40,8 @@ pub struct Task { pub aliases: Vec, #[serde(skip)] pub config_source: PathBuf, + #[serde(skip)] + pub config_root: Option, #[serde(default)] pub depends: Vec, #[serde(default)] @@ -93,7 +95,7 @@ impl Debug for EitherStringOrBool { } impl Task { - pub fn from_path(path: &Path) -> Result { + pub fn from_path(path: &Path, prefix: &Path, config_root: &Path) -> Result { let info = file::read_to_string(path)? .lines() .filter_map(|line| { @@ -113,16 +115,15 @@ impl Task { map }); let info = toml::Value::Table(info); - let config_root = - config_root(&path).ok_or_else(|| eyre!("config root not found: {}", path.display()))?; let mut tera_ctx = BASE_CONTEXT.clone(); tera_ctx.insert("config_root", &config_root); let p = TomlParser::new(&info, get_tera(Some(config_root)), tera_ctx); // trace!("task info: {:#?}", info); let task = Task { - name: name_from_path(config_root, path)?, + name: name_from_path(prefix, path)?, config_source: path.to_path_buf(), + config_root: Some(config_root.to_path_buf()), hide: !file::is_executable(path) || p.parse_bool("hide").unwrap_or_default(), aliases: p .parse_array("alias")? @@ -183,7 +184,8 @@ impl Task { spec.cmd.name = self.name.clone(); (spec, vec![]) } else { - let (scripts, spec) = TaskScriptParser::new(cwd).parse_run_scripts(&self.run)?; + let (scripts, spec) = + TaskScriptParser::new(cwd).parse_run_scripts(&self.config_root, &self.run)?; (spec, scripts) }; spec.name = self.name.clone(); @@ -251,12 +253,36 @@ impl Task { let idx = self.name.chars().map(|c| c as usize).sum::() % COLORS.len(); style::ereset() + &style::estyle(self.prefix()).fg(COLORS[idx]).to_string() } + + pub fn dir(&self) -> Result> { + if let Some(dir) = &self.dir { + // TODO: memoize + // let dir = self.dir_rendered.get_or_try_init(|| -> Result { + let dir = dir.to_string_lossy().to_string(); + let mut tera = get_tera(self.config_root.as_deref()); + let mut ctx = BASE_CONTEXT.clone(); + if let Some(config_root) = &self.config_root { + ctx.insert("config_root", config_root); + } + let dir = tera.render_str(&dir, &ctx)?; + let dir = file::replace_path(&dir); + if dir.is_absolute() { + Ok(Some(dir.to_path_buf())) + } else if let Some(root) = &self.config_root { + Ok(Some(root.join(dir))) + } else { + Ok(Some(dir.clone())) + } + } else { + Ok(self.config_root.clone()) + } + } } -fn name_from_path(root: impl AsRef, path: impl AsRef) -> Result { +fn name_from_path(prefix: impl AsRef, path: impl AsRef) -> Result { Ok(path .as_ref() - .strip_prefix(root) + .strip_prefix(prefix) .map(|p| match p { p if p.starts_with("mise-tasks") => p.strip_prefix("mise-tasks"), p if p.starts_with(".mise-tasks") => p.strip_prefix(".mise-tasks"), @@ -288,6 +314,7 @@ impl Default for Task { description: "".to_string(), aliases: vec![], config_source: PathBuf::new(), + config_root: None, depends: vec![], env: BTreeMap::new(), dir: None, @@ -354,32 +381,6 @@ impl TreeItem for (&Graph, NodeIndex) { } } -fn config_root(config_source: &impl AsRef) -> Option<&Path> { - for ancestor in config_source.as_ref().ancestors() { - if ancestor.ends_with("mise-tasks") { - return ancestor.parent(); - } - - if ancestor.ends_with(".mise-tasks") { - return ancestor.parent(); - } - - if ancestor.ends_with(".mise/tasks") { - return ancestor.parent()?.parent(); - } - - if ancestor.ends_with(".config/mise/tasks") { - return ancestor.parent()?.parent()?.parent(); - } - - if ancestor.ends_with("mise/tasks") { - return ancestor.parent()?.parent(); - } - } - - config_source.as_ref().parent() -} - pub trait GetMatchingExt { fn get_matching(&self, pat: &str) -> Result>; } @@ -409,12 +410,12 @@ where mod tests { use std::path::Path; - use pretty_assertions::assert_eq; - + use crate::dirs; use crate::task::Task; use crate::test::reset; + use pretty_assertions::assert_eq; - use super::{config_root, name_from_path}; + use super::name_from_path; #[test] fn test_from_path() { @@ -422,7 +423,12 @@ mod tests { let test_cases = [(".mise/tasks/filetask", "filetask", vec!["ft"])]; for (path, name, aliases) in test_cases { - let t = Task::from_path(Path::new(path)).unwrap(); + let t = Task::from_path( + Path::new(path), + Path::new(".mise/tasks"), + Path::new(dirs::CWD.as_ref().unwrap()), + ) + .unwrap(); assert_eq!(t.name, name); assert_eq!(t.aliases, aliases); } @@ -453,19 +459,4 @@ mod tests { assert!(name_from_path(root, path).is_err()) } } - - #[test] - fn test_config_root() { - reset(); - let test_cases = [ - ("/base", Some(Path::new("/"))), - ("/base/.mise/tasks", Some(Path::new("/base"))), - ("/base/.config/mise/tasks", Some(Path::new("/base"))), - ("/base/mise/tasks", Some(Path::new("/base"))), - ]; - - for (src, expected) in test_cases { - assert_eq!(config_root(&src), expected) - } - } } diff --git a/src/task/task_script_parser.rs b/src/task/task_script_parser.rs index b879175bd..8e5043ee2 100644 --- a/src/task/task_script_parser.rs +++ b/src/task/task_script_parser.rs @@ -8,22 +8,22 @@ use xx::regex; pub struct TaskScriptParser { dir: Option, - ctx: tera::Context, } impl TaskScriptParser { pub fn new(dir: Option) -> Self { - TaskScriptParser { - dir, - ctx: BASE_CONTEXT.clone(), - } + TaskScriptParser { dir } } fn get_tera(&self) -> tera::Tera { get_tera(self.dir.as_deref()) } - pub fn parse_run_scripts(&self, scripts: &[String]) -> Result<(Vec, usage::Spec)> { + pub fn parse_run_scripts( + &self, + config_root: &Option, + scripts: &[String], + ) -> Result<(Vec, usage::Spec)> { let mut tera = self.get_tera(); let input_args = Arc::new(Mutex::new(vec![])); let template_key = |name| format!("MISE_TASK_ARG:{name}:MISE_TASK_ARG"); @@ -257,9 +257,11 @@ impl TaskScriptParser { } } }); + let mut ctx = BASE_CONTEXT.clone(); + ctx.insert("config_root", config_root); let scripts = scripts .iter() - .map(|s| tera.render_str(s, &self.ctx).unwrap()) + .map(|s| tera.render_str(s, &ctx).unwrap()) .collect(); let mut cmd = usage::SpecCommand::default(); // TODO: ensure no gaps in args, e.g.: 1,2,3,4,5 @@ -327,7 +329,7 @@ mod tests { reset(); let parser = TaskScriptParser::new(None); let scripts = vec!["echo {{ arg(i=0, name='foo') }}".to_string()]; - let (scripts, spec) = parser.parse_run_scripts(&scripts).unwrap(); + let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap(); assert_eq!(scripts, vec!["echo MISE_TASK_ARG:foo:MISE_TASK_ARG"]); let arg0 = spec.cmd.args.first().unwrap(); assert_eq!(arg0.name, "foo"); @@ -342,7 +344,7 @@ mod tests { reset(); let parser = TaskScriptParser::new(None); let scripts = vec!["echo {{ arg(var=true) }}".to_string()]; - let (scripts, spec) = parser.parse_run_scripts(&scripts).unwrap(); + let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap(); assert_eq!(scripts, vec!["echo MISE_TASK_ARG:0:MISE_TASK_ARG"]); let arg0 = spec.cmd.args.first().unwrap(); assert_eq!(arg0.name, "0"); @@ -361,7 +363,7 @@ mod tests { reset(); let parser = TaskScriptParser::new(None); let scripts = vec!["echo {{ flag(name='foo') }}".to_string()]; - let (scripts, spec) = parser.parse_run_scripts(&scripts).unwrap(); + let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap(); assert_eq!(scripts, vec!["echo MISE_TASK_ARG:foo:MISE_TASK_ARG"]); let flag = spec.cmd.flags.iter().find(|f| &f.name == "foo").unwrap(); assert_eq!(&flag.name, "foo"); @@ -377,7 +379,7 @@ mod tests { reset(); let parser = TaskScriptParser::new(None); let scripts = vec!["echo {{ option(name='foo') }}".to_string()]; - let (scripts, spec) = parser.parse_run_scripts(&scripts).unwrap(); + let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap(); assert_eq!(scripts, vec!["echo MISE_TASK_ARG:foo:MISE_TASK_ARG"]); let option = spec.cmd.flags.iter().find(|f| &f.name == "foo").unwrap(); assert_eq!(&option.name, "foo"); diff --git a/src/tera.rs b/src/tera.rs index a19da8808..541ea8d80 100644 --- a/src/tera.rs +++ b/src/tera.rs @@ -29,25 +29,8 @@ pub static BASE_CONTEXT: Lazy = Lazy::new(|| { context }); -pub fn get_tera(dir: Option<&Path>) -> Tera { +static TERA: Lazy = Lazy::new(|| { let mut tera = Tera::default(); - let dir = dir.map(PathBuf::from); - tera.register_function( - "exec", - move |args: &HashMap| -> tera::Result { - match args.get("command") { - Some(Value::String(command)) => { - let mut cmd = cmd("bash", ["-c", command]).full_env(&*env::PRISTINE_ENV); - if let Some(dir) = &dir { - cmd = cmd.dir(dir); - } - let result = cmd.read()?; - Ok(Value::String(result)) - } - _ => Err("exec command must be a string".into()), - } - }, - ); tera.register_function( "arch", move |_args: &HashMap| -> tera::Result { @@ -308,6 +291,29 @@ pub fn get_tera(dir: Option<&Path>) -> Tera { }, ); + tera +}); + +pub fn get_tera(dir: Option<&Path>) -> Tera { + let mut tera = TERA.clone(); + let dir = dir.map(PathBuf::from); + tera.register_function( + "exec", + move |args: &HashMap| -> tera::Result { + match args.get("command") { + Some(Value::String(command)) => { + let mut cmd = cmd("bash", ["-c", command]).full_env(&*env::PRISTINE_ENV); + if let Some(dir) = &dir { + cmd = cmd.dir(dir); + } + let result = cmd.read()?; + Ok(Value::String(result)) + } + _ => Err("exec command must be a string".into()), + } + }, + ); + tera } diff --git a/src/ui/ctrlc.rs b/src/ui/ctrlc.rs index 661840caf..7ccb8f710 100644 --- a/src/ui/ctrlc.rs +++ b/src/ui/ctrlc.rs @@ -15,7 +15,7 @@ pub fn init() { thread::spawn(move || { let mut signals = Signals::new([SIGINT]).unwrap(); let _handle = signals.handle(); - if let Some(_signal) = signals.into_iter().next() { + while let Some(_signal) = signals.forever().next() { if SHOW_CURSOR.load(Ordering::Relaxed) { let _ = Term::stderr().show_cursor(); }