From 8863f424d700fda37922066bc19072df3eb481a0 Mon Sep 17 00:00:00 2001 From: Miles Johnson Date: Fri, 3 Jan 2025 17:37:50 -0800 Subject: [PATCH] new: Support glob based targets. (#1776) * Update locator. * Update graph. * Update ci. * Add locator tests. * Add tests. --- CHANGELOG.md | 2 + .../action-graph/src/action_graph_builder.rs | 77 +++++++- .../action-graph/tests/action_graph_test.rs | 83 +++++++++ ...om_requirements__runs_by_project_glob.snap | 22 +++ ...n_from_requirements__runs_by_tag_glob.snap | 28 +++ ..._from_requirements__runs_by_task_glob.snap | 33 ++++ crates/app/src/commands/ci.rs | 30 ++-- crates/target/src/target_locator.rs | 57 +++++- crates/target/tests/target_locator_test.rs | 167 ++++++++++++++++++ 9 files changed, 467 insertions(+), 32 deletions(-) create mode 100644 crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_project_glob.snap create mode 100644 crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_tag_glob.snap create mode 100644 crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_task_glob.snap create mode 100644 crates/target/tests/target_locator_test.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 741903349aa..856fc8ba5a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ - Deprecated the top-level `platform` setting from `moon.yml`, use `toolchain.default` instead. - Additionally, the toolchain can now be inferred from the top-level `language` setting and any config files in the project/workspace root. This pattern is preferred when possible. +- Added the ability to run targets in `moon run` and `moon ci` using a glob-like syntax. + - For example: `:build-*`, `app-*:build`, `#tag-{foo,bar}:build`, etc. - Updated task option `runInCI` to support the values "always" (always run) and "affected" (only run if affected, same as `true`). - Updated the `extends` setting in `.moon/workspace.yml`, `toolchain.yml`, and `tasks.yml`, to diff --git a/crates/action-graph/src/action_graph_builder.rs b/crates/action-graph/src/action_graph_builder.rs index 4129184edbf..06b4684d014 100644 --- a/crates/action-graph/src/action_graph_builder.rs +++ b/crates/action-graph/src/action_graph_builder.rs @@ -540,13 +540,76 @@ impl<'app> ActionGraphBuilder<'app> { // Track the qualified as an initial target for locator in reqs.target_locators.clone() { - initial_targets.push(match locator { - TargetLocator::Qualified(target) => target, - TargetLocator::TaskFromWorkingDir(task_id) => Target::new( - &self.workspace_graph.get_project_from_path(None)?.id, - task_id, - )?, - }); + match locator { + TargetLocator::GlobMatch { + project_glob, + task_glob, + scope, + .. + } => { + let mut is_all = false; + let mut do_query = false; + let mut projects = vec![]; + + // Query for all applicable projects first since we can't + // query projects + tasks at the same time + if let Some(glob) = project_glob { + let query = if let Some(tag_glob) = glob.strip_prefix('#') { + format!("tag~{tag_glob}") + } else { + format!("project~{glob}") + }; + + projects = self.workspace_graph.query_projects(build_query(&query)?)?; + do_query = !projects.is_empty(); + } else { + match scope { + Some(TargetScope::All) => { + is_all = true; + do_query = true; + } + _ => { + // Don't query for the other scopes, + // since they're not valid from the run context + } + }; + } + + // Then query for all tasks within the queried projects + if do_query { + let mut query = format!("task~{task_glob}"); + + if !is_all { + query = format!( + "project=[{}] && {query}", + projects + .into_iter() + .map(|project| project.id.to_string()) + .collect::>() + .join(",") + ); + } + + let tasks = self.workspace_graph.query_tasks(build_query(&query)?)?; + + initial_targets.extend( + tasks + .into_iter() + .map(|task| task.target.clone()) + .collect::>(), + ); + } + } + TargetLocator::Qualified(target) => { + initial_targets.push(target); + } + TargetLocator::TaskFromWorkingDir(task_id) => { + initial_targets.push(Target::new( + &self.workspace_graph.get_project_from_path(None)?.id, + task_id, + )?); + } + }; } // Determine affected tasks before building diff --git a/crates/action-graph/tests/action_graph_test.rs b/crates/action-graph/tests/action_graph_test.rs index 785328f1174..cd02a2833cb 100644 --- a/crates/action-graph/tests/action_graph_test.rs +++ b/crates/action-graph/tests/action_graph_test.rs @@ -1742,6 +1742,89 @@ mod action_graph { assert_snapshot!(graph.to_dot()); } + #[tokio::test] + async fn runs_by_task_glob() { + let sandbox = create_sandbox("tasks"); + let container = ActionGraphContainer::new(sandbox.path()).await; + let mut builder = container.create_builder(); + + builder + .run_from_requirements(RunRequirements { + target_locators: FxHashSet::from_iter([ + TargetLocator::parse(":*-dependency").unwrap(), + TargetLocator::parse(":{a,c}").unwrap(), + ]), + ..Default::default() + }) + .unwrap(); + + let graph = builder.build(); + + assert_snapshot!(graph.to_dot()); + } + + #[tokio::test] + async fn runs_by_tag_glob() { + let sandbox = create_sandbox("tasks"); + let container = ActionGraphContainer::new(sandbox.path()).await; + let mut builder = container.create_builder(); + + builder + .run_from_requirements(RunRequirements { + target_locators: FxHashSet::from_iter([ + TargetLocator::parse("#front*:build").unwrap() + ]), + ..Default::default() + }) + .unwrap(); + + let graph = builder.build(); + + assert_snapshot!(graph.to_dot()); + } + + #[tokio::test] + async fn runs_by_project_glob() { + let sandbox = create_sandbox("tasks"); + let container = ActionGraphContainer::new(sandbox.path()).await; + let mut builder = container.create_builder(); + + builder + .run_from_requirements(RunRequirements { + target_locators: FxHashSet::from_iter([TargetLocator::parse( + "c{lient,ommon}:test", + ) + .unwrap()]), + ..Default::default() + }) + .unwrap(); + + let graph = builder.build(); + + assert_snapshot!(graph.to_dot()); + } + + #[tokio::test] + async fn returns_empty_result_for_no_glob_match() { + let sandbox = create_sandbox("tasks"); + let container = ActionGraphContainer::new(sandbox.path()).await; + let mut builder = container.create_builder(); + + builder + .run_from_requirements(RunRequirements { + target_locators: FxHashSet::from_iter([TargetLocator::parse( + "{foo,bar}:task-*", + ) + .unwrap()]), + ..Default::default() + }) + .unwrap(); + + let graph = builder.build(); + + assert!(graph.is_empty()); + } + #[tokio::test] async fn computes_context() { let sandbox = create_sandbox("tasks"); diff --git a/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_project_glob.snap b/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_project_glob.snap new file mode 100644 index 00000000000..02eec2be4e9 --- /dev/null +++ b/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_project_glob.snap @@ -0,0 +1,22 @@ +--- +source: crates/action-graph/tests/action_graph_test.rs +expression: graph.to_dot() +--- +digraph { + 0 [ label="SyncWorkspace" ] + 1 [ label="SetupToolchain(system)" ] + 2 [ label="SyncProject(system, client)" ] + 3 [ label="SyncProject(system, server)" ] + 4 [ label="SyncProject(system, common)" ] + 5 [ label="SyncProject(system, base)" ] + 6 [ label="RunTask(client:test)" ] + 1 -> 0 [ ] + 3 -> 1 [ ] + 5 -> 1 [ ] + 4 -> 1 [ ] + 4 -> 5 [ ] + 2 -> 1 [ ] + 2 -> 3 [ ] + 2 -> 4 [ ] + 6 -> 2 [ ] +} diff --git a/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_tag_glob.snap b/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_tag_glob.snap new file mode 100644 index 00000000000..6ecdd1bb594 --- /dev/null +++ b/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_tag_glob.snap @@ -0,0 +1,28 @@ +--- +source: crates/action-graph/tests/action_graph_test.rs +expression: graph.to_dot() +--- +digraph { + 0 [ label="SyncWorkspace" ] + 1 [ label="SetupToolchain(system)" ] + 2 [ label="SyncProject(system, client)" ] + 3 [ label="SyncProject(system, server)" ] + 4 [ label="SyncProject(system, common)" ] + 5 [ label="SyncProject(system, base)" ] + 6 [ label="RunTask(client:build)" ] + 7 [ label="RunTask(server:build)" ] + 8 [ label="RunTask(common:build)" ] + 1 -> 0 [ ] + 3 -> 1 [ ] + 5 -> 1 [ ] + 4 -> 1 [ ] + 4 -> 5 [ ] + 2 -> 1 [ ] + 2 -> 3 [ ] + 2 -> 4 [ ] + 7 -> 3 [ ] + 8 -> 4 [ ] + 6 -> 2 [ ] + 6 -> 7 [ ] + 6 -> 8 [ ] +} diff --git a/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_task_glob.snap b/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_task_glob.snap new file mode 100644 index 00000000000..58438a8740e --- /dev/null +++ b/crates/action-graph/tests/snapshots/action_graph_test__action_graph__run_from_requirements__runs_by_task_glob.snap @@ -0,0 +1,33 @@ +--- +source: crates/action-graph/tests/action_graph_test.rs +expression: graph.to_dot() +--- +digraph { + 0 [ label="SyncWorkspace" ] + 1 [ label="SetupToolchain(system)" ] + 2 [ label="SyncProject(system, ci)" ] + 3 [ label="RunTask(ci:ci2-dependency)" ] + 4 [ label="RunTask(ci:ci3-dependency)" ] + 5 [ label="RunTask(ci:ci4-dependency)" ] + 6 [ label="SyncProject(system, deps-affected)" ] + 7 [ label="RunTask(deps-affected:a)" ] + 8 [ label="RunTask(deps-affected:b)" ] + 9 [ label="RunTask(deps-affected:c)" ] + 10 [ label="SyncProject(system, deps)" ] + 11 [ label="RunTask(deps:a)" ] + 12 [ label="RunTask(deps:c)" ] + 1 -> 0 [ ] + 2 -> 1 [ ] + 3 -> 2 [ ] + 4 -> 2 [ ] + 5 -> 2 [ ] + 6 -> 1 [ ] + 9 -> 6 [ ] + 8 -> 6 [ ] + 8 -> 9 [ ] + 7 -> 6 [ ] + 7 -> 8 [ ] + 10 -> 1 [ ] + 11 -> 10 [ ] + 12 -> 10 [ ] +} diff --git a/crates/app/src/commands/ci.rs b/crates/app/src/commands/ci.rs index a3f48cf1c5e..a8c38d74a6e 100644 --- a/crates/app/src/commands/ci.rs +++ b/crates/app/src/commands/ci.rs @@ -9,7 +9,7 @@ use moon_action_graph::{ActionGraph, RunRequirements}; use moon_affected::{DownstreamScope, UpstreamScope}; use moon_common::path::WorkspaceRelativePathBuf; use moon_console::Console; -use moon_task::{Target, TargetLocator}; +use moon_task::TargetLocator; use moon_workspace_graph::WorkspaceGraph; use rustc_hash::FxHashSet; use starbase::AppResult; @@ -17,14 +17,14 @@ use starbase_styles::color; use std::sync::Arc; use tracing::instrument; -type TargetList = Vec; +type TargetList = Vec; const HEADING_PARALLELISM: &str = "Parallelism and distribution"; #[derive(Args, Clone, Debug)] pub struct CiArgs { - #[arg(help = "List of targets (scope:task) to run")] - targets: Vec, + #[arg(help = "List of targets to run")] + targets: Vec, #[arg(long, help = "Base branch, commit, or revision to compare against")] base: Option, @@ -70,16 +70,14 @@ impl CiConsole { } pub fn print_targets(&self, targets: &TargetList) -> miette::Result<()> { - let mut targets_to_print = targets.clone(); + let mut targets_to_print = targets + .iter() + .map(|t| format!(" {}", color::label(t.as_str()))) + .collect::>(); + targets_to_print.sort(); - self.write_line( - targets_to_print - .iter() - .map(|t| format!(" {}", color::label(&t.id))) - .collect::>() - .join("\n"), - ) + self.write_line(targets_to_print.join("\n")) } } @@ -151,7 +149,7 @@ async fn gather_potential_targets( if args.targets.is_empty() { for task in workspace_graph.get_tasks()? { - targets.push(task.target.clone()); + targets.push(TargetLocator::Qualified(task.target.clone())); } } else { targets.extend(args.targets.clone()); @@ -218,11 +216,7 @@ async fn generate_action_graph( ci: true, ci_check: true, dependents: true, - target_locators: FxHashSet::from_iter( - targets - .iter() - .map(|target| TargetLocator::Qualified(target.to_owned())), - ), + target_locators: FxHashSet::from_iter(targets.clone()), ..Default::default() })?; diff --git a/crates/target/src/target_locator.rs b/crates/target/src/target_locator.rs index 2eb40b7055f..3cbdc9fb144 100644 --- a/crates/target/src/target_locator.rs +++ b/crates/target/src/target_locator.rs @@ -1,18 +1,34 @@ -use crate::target::Target; +use crate::{target::Target, TargetScope}; use moon_common::Id; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use std::str::FromStr; #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub enum TargetLocator { - Qualified(Target), // scope:task_id - TaskFromWorkingDir(Id), // task_id + // proj-*:task_id, proj:task-* + GlobMatch { + original: String, + scope: Option, + project_glob: Option, + task_glob: String, + }, + + // scope:task_id + Qualified(Target), + + // task_id + TaskFromWorkingDir(Id), } impl TargetLocator { pub fn as_str(&self) -> &str { self.as_ref() } + + #[tracing::instrument(name = "parse_target_locator")] + pub fn parse(value: &str) -> miette::Result { + Self::from_str(value) + } } impl AsRef for TargetLocator { @@ -24,6 +40,7 @@ impl AsRef for TargetLocator { impl AsRef for TargetLocator { fn as_ref(&self) -> &str { match self { + Self::GlobMatch { original, .. } => original.as_str(), Self::Qualified(target) => target.as_str(), Self::TaskFromWorkingDir(id) => id.as_str(), } @@ -43,11 +60,33 @@ impl FromStr for TargetLocator { type Err = miette::Report; fn from_str(value: &str) -> Result { - Ok(if value.contains(':') { - TargetLocator::Qualified(Target::parse(value)?) + if value.contains(':') { + if is_glob(value) { + let (base_scope, base_id) = value.split_once(':').unwrap(); + let mut scope = None; + let mut project_glob = None; + + match base_scope { + "" | "*" | "**" | "**/*" => scope = Some(TargetScope::All), + "~" => scope = Some(TargetScope::OwnSelf), + "^" => scope = Some(TargetScope::Deps), + inner => { + project_glob = Some(inner.to_owned()); + } + }; + + Ok(TargetLocator::GlobMatch { + original: value.to_owned(), + scope, + project_glob, + task_glob: base_id.to_owned(), + }) + } else { + Ok(TargetLocator::Qualified(Target::parse(value)?)) + } } else { - TargetLocator::TaskFromWorkingDir(Id::new(value)?) - }) + Ok(TargetLocator::TaskFromWorkingDir(Id::new(value)?)) + } } } @@ -70,3 +109,7 @@ impl Serialize for TargetLocator { serializer.serialize_str(self.as_str()) } } + +fn is_glob(value: &str) -> bool { + value.contains(['*', '?', '[', ']', '{', '}', '!']) +} diff --git a/crates/target/tests/target_locator_test.rs b/crates/target/tests/target_locator_test.rs new file mode 100644 index 00000000000..445fc29c780 --- /dev/null +++ b/crates/target/tests/target_locator_test.rs @@ -0,0 +1,167 @@ +use moon_common::Id; +use moon_target::*; + +mod target_locator { + use super::*; + + mod glob { + use super::*; + + #[test] + fn all_scope() { + assert_eq!( + TargetLocator::parse(":build-*").unwrap(), + TargetLocator::GlobMatch { + original: String::from(":build-*"), + scope: Some(TargetScope::All), + project_glob: None, + task_glob: String::from("build-*"), + } + ); + + assert_eq!( + TargetLocator::parse("*:build").unwrap(), + TargetLocator::GlobMatch { + original: String::from("*:build"), + scope: Some(TargetScope::All), + project_glob: None, + task_glob: String::from("build"), + } + ); + } + + #[test] + fn deps_scope() { + assert_eq!( + TargetLocator::parse("^:build-*").unwrap(), + TargetLocator::GlobMatch { + original: String::from("^:build-*"), + scope: Some(TargetScope::Deps), + project_glob: None, + task_glob: String::from("build-*"), + } + ); + } + + #[test] + fn self_scope() { + assert_eq!( + TargetLocator::parse("~:build-*").unwrap(), + TargetLocator::GlobMatch { + original: String::from("~:build-*"), + scope: Some(TargetScope::OwnSelf), + project_glob: None, + task_glob: String::from("build-*"), + } + ); + } + + #[test] + fn tag_scope() { + assert_eq!( + TargetLocator::parse("#tag:build-*").unwrap(), + TargetLocator::GlobMatch { + original: String::from("#tag:build-*"), + scope: None, + project_glob: Some(String::from("#tag")), + task_glob: String::from("build-*"), + } + ); + + assert_eq!( + TargetLocator::parse("#tag-*:build-*").unwrap(), + TargetLocator::GlobMatch { + original: String::from("#tag-*:build-*"), + scope: None, + project_glob: Some(String::from("#tag-*")), + task_glob: String::from("build-*"), + } + ); + } + + #[test] + fn project_scope() { + assert_eq!( + TargetLocator::parse("project:build-*").unwrap(), + TargetLocator::GlobMatch { + original: String::from("project:build-*"), + scope: None, + project_glob: Some(String::from("project")), + task_glob: String::from("build-*"), + } + ); + + assert_eq!( + TargetLocator::parse("proj-*:build-*").unwrap(), + TargetLocator::GlobMatch { + original: String::from("proj-*:build-*"), + scope: None, + project_glob: Some(String::from("proj-*")), + task_glob: String::from("build-*"), + } + ); + } + } + + mod target { + use super::*; + + #[test] + #[should_panic(expected = "Invalid target $:build")] + fn errors_invalid() { + TargetLocator::parse("$:build").unwrap(); + } + + #[test] + fn all_scope() { + assert_eq!( + TargetLocator::parse(":build").unwrap(), + TargetLocator::Qualified(Target::parse(":build").unwrap()) + ); + } + + #[test] + fn deps_scope() { + assert_eq!( + TargetLocator::parse("^:build").unwrap(), + TargetLocator::Qualified(Target::parse("^:build").unwrap()) + ); + } + + #[test] + fn self_scope() { + assert_eq!( + TargetLocator::parse("~:build").unwrap(), + TargetLocator::Qualified(Target::parse("~:build").unwrap()) + ); + } + + #[test] + fn tag_scope() { + assert_eq!( + TargetLocator::parse("#tag:build").unwrap(), + TargetLocator::Qualified(Target::parse("#tag:build").unwrap()) + ); + } + + #[test] + fn project_scope() { + assert_eq!( + TargetLocator::parse("project:build").unwrap(), + TargetLocator::Qualified(Target::parse("project:build").unwrap()) + ); + } + } + + mod cwd { + use super::*; + + #[test] + fn returns_task() { + assert_eq!( + TargetLocator::parse("build").unwrap(), + TargetLocator::TaskFromWorkingDir(Id::raw("build")) + ); + } + } +}