diff --git a/CHANGELOG.md b/CHANGELOG.md index 44e898f60db..411c1ffe703 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ ## Unreleased +#### 🚀 Updates + +- Added caching around `bun bun.lockb` commands, instead of running them for every task. +- Updated and loosened identifier naming restrictions. +- Updated environment variable substitution to support different outputs when a variable is missing, + based on a trailing flag syntax. + - `$FOO` or `${FOO}` - If variable is missing, keeps the original syntax (current default). + - `$FOO?` or `${FOO?}` - If variable is missing, replaces with an empty string. + - `$FOO!` or `${FOO!}` - Ignores variable substitution and preserves the syntax (without !). + +#### 🐞 Fixes + +- Fixed an issue where an affected task with files that contain non-standard characters would fail + to run because Bash expansion fails. We now quote file paths that contain `*`, `$`, `+`, and `[]`. + #### ⚙️ Internal - Updated Rust to v1.81. diff --git a/Cargo.lock b/Cargo.lock index 8b6360e372a..b438774020d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3185,6 +3185,7 @@ dependencies = [ "moon_utils", "proto_core", "rustc-hash 2.0.0", + "scc", "starbase_utils", "tracing", ] @@ -3588,6 +3589,7 @@ dependencies = [ "moon_utils", "proto_core", "rustc-hash 2.0.0", + "scc", "starbase_styles", "starbase_utils", "tracing", diff --git a/crates/cli/tests/snapshots/run_test__output_styles__stream.snap b/crates/cli/tests/snapshots/run_test__output_styles__stream.snap index 10c02458d26..70381d15cad 100644 --- a/crates/cli/tests/snapshots/run_test__output_styles__stream.snap +++ b/crates/cli/tests/snapshots/run_test__output_styles__stream.snap @@ -4,13 +4,11 @@ expression: assert.output() --- ▪▪▪▪ npm install ▪▪▪▪ outputStyles:stream - outputStyles:stream | stdout +outputStyles:stream | stdout ▪▪▪▪ outputStyles:stream (100ms) ▪▪▪▪ outputStyles:streamPrimary (no op) Tasks: 2 completed Time: 100ms - outputStyles:stream | stderr - - +outputStyles:stream | stderr diff --git a/crates/common/src/id.rs b/crates/common/src/id.rs index a810f32db37..922b9dc0485 100644 --- a/crates/common/src/id.rs +++ b/crates/common/src/id.rs @@ -13,7 +13,7 @@ pub static ID_CLEAN: OnceLock = OnceLock::new(); #[derive(Error, Debug, Diagnostic)] #[diagnostic(code(id::invalid_format))] -#[error("Invalid format for {}, may only contain alpha-numeric characters, dashes (-), slashes (/), underscores (_), and dots (.), and must start with an alpha character.", .0.style(Style::Id))] +#[error("Invalid format for {}, may only contain alpha-numeric characters, dashes (-), slashes (/), underscores (_), and periods (.).", .0.style(Style::Id))] pub struct IdError(String); #[derive(Clone, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] @@ -24,9 +24,8 @@ impl Id { let id = id.as_ref(); // The @ is to support npm package scopes! - let pattern = ID_PATTERN.get_or_init(|| { - Regex::new(format!("^([A-Za-z@_]{{1}}{})$", ID_CHARS).as_str()).unwrap() - }); + let pattern = + ID_PATTERN.get_or_init(|| Regex::new(format!("^(@?{})$", ID_CHARS).as_str()).unwrap()); if !pattern.is_match(id) { return Err(IdError(id.to_owned())); @@ -40,7 +39,7 @@ impl Id { // with a leading -, causing pattern failures let id = id.as_ref().replace('@', ""); - // This is to clean and ID and remove unwanted characters + // This is to clean an ID and remove unwanted characters let pattern = ID_CLEAN.get_or_init(|| Regex::new(r"[^0-9A-Za-z/\._-]+").unwrap()); Id::new(pattern.replace_all(&id, "-")) diff --git a/crates/config/src/patterns.rs b/crates/config/src/patterns.rs index c25409412a9..41e977defe7 100644 --- a/crates/config/src/patterns.rs +++ b/crates/config/src/patterns.rs @@ -12,8 +12,14 @@ macro_rules! pattern { pattern!(ENV_VAR, "\\$([A-Z0-9_]+)"); // $ENV_VAR pattern!(ENV_VAR_DISTINCT, "^\\$([A-Z0-9_]+)$"); // $ENV_VAR pattern!(ENV_VAR_GLOB_DISTINCT, "^\\$([A-Z0-9_*]+)$"); // $ENV_* -pattern!(ENV_VAR_SUBSTITUTE, "\\$(?:\\{([A-Z0-9_]+)\\}|([A-Z0-9_]+))"); // $ENV_VAR, ${ENV_VAR} -pattern!(ENV_VAR_SUBSTITUTE_STRICT, "\\$\\{([A-Z0-9_]+)\\}"); // ${ENV_VAR} +pattern!( + ENV_VAR_SUBSTITUTE, + "(?:\\$\\{(?P[A-Z0-9_]+)(?P[!?]{1})?\\})|(?:\\$(?P[A-Z0-9_]+)(?P[!?]{1})?)" +); // $ENV_VAR, ${ENV_VAR} +pattern!( + ENV_VAR_SUBSTITUTE_STRICT, + "\\$\\{(?P[A-Z0-9_]+)(?P[!?]{1})?\\}" +); // ${ENV_VAR} // Task tokens diff --git a/crates/project-expander/src/expander_context.rs b/crates/project-expander/src/expander_context.rs index 38f4162ed8c..0c857bc9d64 100644 --- a/crates/project-expander/src/expander_context.rs +++ b/crates/project-expander/src/expander_context.rs @@ -45,30 +45,87 @@ pub fn substitute_env_var( patterns::ENV_VAR_SUBSTITUTE.replace_all( value, |caps: &patterns::Captures| { - // First with wrapping {} then without: ${FOO} -> $FOO - let name = caps.get(1).or_else(|| caps.get(2)).unwrap().as_str(); + let Some(name) = caps.name("name1") + .or_else(|| caps.name("name2")) + .map(|cap| cap.as_str()) + else { + return String::new(); + }; + + let flag = caps.name("flag1").or_else(|| caps.name("flag2")).map(|cap| cap.as_str()); // If the variable is referencing itself, don't pull // from the local map, and instead only pull from the // system environment. Otherwise we hit recursion! - let maybe_var = if !base_name.is_empty() && base_name == name { - env::var(name).ok() - } else { - env_map.get(name).cloned().or_else(|| env::var(name).ok()) + let get_replacement_value = || { + if !base_name.is_empty() && base_name == name { + env::var(name).ok() + } else { + env_map.get(name).cloned().or_else(|| env::var(name).ok()) + } }; - match maybe_var { - Some(var) => var, - None => { + match flag { + // Don't substitute + Some("!") => { + format!("${name}") + }, + // Substitute with empty string when missing + Some("?") =>{ + debug!( + "Task value `{}` contains the environment variable ${}, but this variable is not set. Replacing with an empty value.", + value, + name + ); + + get_replacement_value().unwrap_or_default() + }, + // Substitute with self when missing + _ => { debug!( - "Task value `{}` contains the environment variable ${}, but this variable is not set. Not substituting and keeping as-is.", + "Task value `{}` contains the environment variable ${}, but this variable is not set. Not substituting and keeping as-is. Append with ? or ! to change outcome.", value, name ); - caps.get(0).unwrap().as_str().to_owned() + get_replacement_value() + .unwrap_or_else(|| caps.get(0).unwrap().as_str().to_owned()) } } }) .to_string() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn handles_flags_when_missing() { + let envs = FxHashMap::default(); + + assert_eq!(substitute_env_var("", "$KEY", &envs), "$KEY"); + assert_eq!(substitute_env_var("", "${KEY}", &envs), "${KEY}"); + + assert_eq!(substitute_env_var("", "$KEY!", &envs), "$KEY"); + assert_eq!(substitute_env_var("", "${KEY!}", &envs), "$KEY"); + + assert_eq!(substitute_env_var("", "$KEY?", &envs), ""); + assert_eq!(substitute_env_var("", "${KEY?}", &envs), ""); + } + + #[test] + fn handles_flags_when_not_missing() { + let mut envs = FxHashMap::default(); + envs.insert("KEY".to_owned(), "value".to_owned()); + + assert_eq!(substitute_env_var("", "$KEY", &envs), "value"); + assert_eq!(substitute_env_var("", "${KEY}", &envs), "value"); + + assert_eq!(substitute_env_var("", "$KEY!", &envs), "$KEY"); + assert_eq!(substitute_env_var("", "${KEY!}", &envs), "$KEY"); + + assert_eq!(substitute_env_var("", "$KEY?", &envs), "value"); + assert_eq!(substitute_env_var("", "${KEY?}", &envs), "value"); + } +} diff --git a/crates/project-graph/src/projects_locator.rs b/crates/project-graph/src/projects_locator.rs index d6eaed20359..7e0b79d61a0 100644 --- a/crates/project-graph/src/projects_locator.rs +++ b/crates/project-graph/src/projects_locator.rs @@ -4,11 +4,11 @@ use moon_config::{ProjectSourceEntry, ProjectsSourcesList}; use moon_vcs::BoxedVcs; use starbase_utils::{fs, glob}; use std::path::Path; -use tracing::{instrument, warn}; +use tracing::{debug, instrument, warn}; /// Infer a project name from a source path, by using the name of /// the project folder. -pub fn infer_project_id_and_source( +fn infer_project_id_and_source( path: &str, workspace_root: &Path, ) -> miette::Result { @@ -113,10 +113,17 @@ where } } - sources.push(infer_project_id_and_source( - &project_source, - workspace_root, - )?) + let (id, source) = infer_project_id_and_source(&project_source, workspace_root)?; + + if id.starts_with(".") { + debug!( + id = id.as_str(), + source = source.as_str(), + "Received a project for a hidden folder. These are not supported through globs, but can be mapped explicitly with project sources!" + ); + } else { + sources.push((id, source)); + } } } diff --git a/crates/project-graph/tests/project_graph_test.rs b/crates/project-graph/tests/project_graph_test.rs index fa2c1efdd1f..e70ff882212 100644 --- a/crates/project-graph/tests/project_graph_test.rs +++ b/crates/project-graph/tests/project_graph_test.rs @@ -193,8 +193,7 @@ mod project_graph { } #[tokio::test] - #[should_panic(expected = "Invalid format for .foo")] - async fn errors_for_dot_folders() { + async fn filters_dot_folders() { let sandbox = create_sandbox("dependencies"); sandbox.create_file(".foo/moon.yml", ""); diff --git a/crates/task-runner/src/command_builder.rs b/crates/task-runner/src/command_builder.rs index 047fe5feffe..0d83bcda8af 100644 --- a/crates/task-runner/src/command_builder.rs +++ b/crates/task-runner/src/command_builder.rs @@ -272,8 +272,16 @@ impl<'task> CommandBuilder<'task> { self.command.arg_if_missing("."); } else { // Mimic relative from ("./") - self.command - .args(files.iter().map(|file| format!("./{file}"))); + self.command.args(files.iter().map(|file| { + let arg = format!("./{file}"); + + // Escape files with special characters + if arg.contains(['*', '$', '+', '[', ']']) { + format!("\"{arg}\"") + } else { + arg + } + })); } } diff --git a/crates/task-runner/src/command_executor.rs b/crates/task-runner/src/command_executor.rs index d196b35acd5..5070e034bb5 100644 --- a/crates/task-runner/src/command_executor.rs +++ b/crates/task-runner/src/command_executor.rs @@ -250,12 +250,16 @@ impl<'task> CommandExecutor<'task> { }; // Transitive targets may run concurrently, so differentiate them with a prefix. - if self.stream && (is_ci_env() || !is_primary || context.primary_targets.len() > 1) { - let prefix_max_width = context - .primary_targets - .iter() - .map(|target| target.id.len()) - .max(); + if !is_primary || is_ci_env() || context.primary_targets.len() > 1 { + let prefix_max_width = if context.primary_targets.len() > 1 { + context + .primary_targets + .iter() + .map(|target| target.id.len()) + .max() + } else { + None + }; self.command .set_prefix(&self.task.target.id, prefix_max_width); diff --git a/crates/task-runner/tests/__fixtures__/builder/project/moon.yml b/crates/task-runner/tests/__fixtures__/builder/project/moon.yml index 34558271915..580c8de1af8 100644 --- a/crates/task-runner/tests/__fixtures__/builder/project/moon.yml +++ b/crates/task-runner/tests/__fixtures__/builder/project/moon.yml @@ -7,3 +7,4 @@ tasks: inputs: - 'input.txt' - 'file.*' + - 'routes/**/*' diff --git a/crates/task-runner/tests/command_builder_test.rs b/crates/task-runner/tests/command_builder_test.rs index 023c061513d..48116e631aa 100644 --- a/crates/task-runner/tests/command_builder_test.rs +++ b/crates/task-runner/tests/command_builder_test.rs @@ -367,5 +367,43 @@ mod command_builder { assert_eq!(get_args(&command), vec!["arg", "--opt", "./input.txt"]); } + + #[tokio::test] + async fn quotes_files_with_special_chars() { + let container = TaskRunnerContainer::new("builder").await; + + let mut context = ActionContext::default(); + context.affected_only = true; + context.touched_files.insert("project/file.txt".into()); + context.touched_files.insert("project/routes/*.ts".into()); + context + .touched_files + .insert("project/routes/[id].ts".into()); + context + .touched_files + .insert("project/routes/$slug.tsx".into()); + context + .touched_files + .insert("project/routes/+page.svelte".into()); + + let command = container + .create_command_with_config(context, |task, _| { + task.options.affected_files = Some(TaskOptionAffectedFiles::Args); + }) + .await; + + assert_eq!( + get_args(&command), + vec![ + "arg", + "--opt", + "./file.txt", + "\"./routes/$slug.tsx\"", + "\"./routes/*.ts\"", + "\"./routes/+page.svelte\"", + "\"./routes/[id].ts\"" + ] + ); + } } } diff --git a/legacy/bun/lang/src/bun_lockb.rs b/legacy/bun/lang/src/bun_lockb.rs index b3bdf6c2e1b..b647902a375 100644 --- a/legacy/bun/lang/src/bun_lockb.rs +++ b/legacy/bun/lang/src/bun_lockb.rs @@ -2,11 +2,12 @@ use cached::proc_macro::cached; use miette::IntoDiagnostic; use moon_lang::LockfileDependencyVersions; use rustc_hash::FxHashMap; +use std::sync::Arc; use yarn_lock_parser::{parse_str, Entry}; #[cached(result)] pub fn load_lockfile_dependencies( - lockfile_text: String, + lockfile_text: Arc, ) -> miette::Result { let mut deps: LockfileDependencyVersions = FxHashMap::default(); diff --git a/legacy/bun/tool/Cargo.toml b/legacy/bun/tool/Cargo.toml index e027f00af8e..77b894a37fa 100644 --- a/legacy/bun/tool/Cargo.toml +++ b/legacy/bun/tool/Cargo.toml @@ -16,6 +16,7 @@ moon_utils = { path = "../../core/utils" } miette = { workspace = true } proto_core = { workspace = true } rustc-hash = { workspace = true } +scc = { workspace = true } starbase_utils = { workspace = true } tracing = { workspace = true } diff --git a/legacy/bun/tool/src/bun_tool.rs b/legacy/bun/tool/src/bun_tool.rs index 28b9ba1f1d7..63341a9bb8b 100644 --- a/legacy/bun/tool/src/bun_tool.rs +++ b/legacy/bun/tool/src/bun_tool.rs @@ -12,6 +12,7 @@ use moon_utils::get_workspace_root; use proto_core::flow::install::InstallOptions; use proto_core::{Id, ProtoEnvironment, Tool as ProtoTool, UnresolvedVersionSpec}; use rustc_hash::FxHashMap; +use scc::hash_cache::Entry; use starbase_utils::fs; use std::env; use std::path::{Path, PathBuf}; @@ -34,6 +35,8 @@ pub struct BunTool { console: Arc, + lockfile_cache: scc::HashCache>, + proto_env: Arc, } @@ -50,6 +53,7 @@ impl BunTool { global: false, tool: load_tool_plugin(&Id::raw("bun"), &proto, config.plugin.as_ref().unwrap()) .await?, + lockfile_cache: scc::HashCache::default(), proto_env: proto, }; @@ -62,6 +66,37 @@ impl BunTool { Ok(bun) } + + // Bun lockfiles are binary, so we need to convert them to text first + // using Bun itself! + async fn load_lockfile(&self, cwd: &Path) -> miette::Result> { + let key = cwd.to_path_buf(); + + let cache = match self.lockfile_cache.entry_async(key).await { + Entry::Occupied(o) => o.get().clone(), + Entry::Vacant(v) => { + let yarn_lock = cwd.join("yarn.lock"); + + let content = if yarn_lock.exists() { + Arc::new(fs::read_file(yarn_lock)?) + } else { + let mut cmd = self.create_command(&())?; + cmd.arg("bun.lockb"); + cmd.cwd(cwd); + + let output = cmd.create_async().exec_capture_output().await?; + + Arc::new(output_to_string(&output.stdout)) + }; + + v.put_entry(content.clone()); + + content + } + }; + + Ok(cache) + } } #[async_trait] @@ -189,17 +224,9 @@ impl DependencyManager<()> for BunTool { return Ok(FxHashMap::default()); }; - // Bun lockfiles are binary, so we need to convert them to text first - // using Bun itself! - let mut cmd = self.create_command(&())?; - cmd.arg("bun.lockb"); - cmd.cwd(lockfile_path.parent().unwrap()); - - let output = cmd.create_async().exec_capture_output().await?; - - Ok(load_lockfile_dependencies(output_to_string( - &output.stdout, - ))?) + Ok(load_lockfile_dependencies( + self.load_lockfile(lockfile_path.parent().unwrap()).await?, + )?) } #[instrument(skip_all)] diff --git a/legacy/node/tool/Cargo.toml b/legacy/node/tool/Cargo.toml index 3e0faa28a41..869e5161562 100644 --- a/legacy/node/tool/Cargo.toml +++ b/legacy/node/tool/Cargo.toml @@ -16,6 +16,7 @@ moon_utils = { path = "../../core/utils" } miette = { workspace = true } proto_core = { workspace = true } rustc-hash = { workspace = true } +scc = { workspace = true } starbase_styles = { workspace = true } starbase_utils = { workspace = true } tracing = { workspace = true } diff --git a/legacy/node/tool/src/bun_tool.rs b/legacy/node/tool/src/bun_tool.rs index 1d2702d88b3..6a2dfbad264 100644 --- a/legacy/node/tool/src/bun_tool.rs +++ b/legacy/node/tool/src/bun_tool.rs @@ -13,9 +13,10 @@ use moon_utils::get_workspace_root; use proto_core::flow::install::InstallOptions; use proto_core::{Id, ProtoEnvironment, Tool as ProtoTool, UnresolvedVersionSpec}; use rustc_hash::FxHashMap; +use scc::hash_cache::Entry; use starbase_utils::fs; use std::env; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::Arc; use tracing::instrument; @@ -28,6 +29,8 @@ pub struct BunTool { console: Arc, + lockfile_cache: scc::HashCache>, + proto_env: Arc, } @@ -45,6 +48,7 @@ impl BunTool { .await?, config, proto_env, + lockfile_cache: scc::HashCache::default(), console, }) } @@ -67,6 +71,37 @@ impl BunTool { Ok(cmd) } + + // Bun lockfiles are binary, so we need to convert them to text first + // using Bun itself! + async fn load_lockfile(&self, cwd: &Path) -> miette::Result> { + let key = cwd.to_path_buf(); + + let cache = match self.lockfile_cache.entry_async(key).await { + Entry::Occupied(o) => o.get().clone(), + Entry::Vacant(v) => { + let yarn_lock = cwd.join("yarn.lock"); + + let content = if yarn_lock.exists() { + Arc::new(fs::read_file(yarn_lock)?) + } else { + let mut cmd = self.internal_create_command()?; + cmd.arg("bun.lockb"); + cmd.cwd(cwd); + + let output = cmd.create_async().exec_capture_output().await?; + + Arc::new(output_to_string(&output.stdout)) + }; + + v.put_entry(content.clone()); + + content + } + }; + + Ok(cache) + } } #[async_trait] @@ -196,17 +231,9 @@ impl DependencyManager for BunTool { return Ok(FxHashMap::default()); }; - // Bun lockfiles are binary, so we need to convert them to text first - // using Bun itself! - let mut cmd = self.internal_create_command()?; - cmd.arg("bun.lockb"); - cmd.cwd(lockfile_path.parent().unwrap()); - - let output = cmd.create_async().exec_capture_output().await?; - - Ok(bun::load_lockfile_dependencies(output_to_string( - &output.stdout, - ))?) + Ok(bun::load_lockfile_dependencies( + self.load_lockfile(lockfile_path.parent().unwrap()).await?, + )?) } #[instrument(skip_all)]