From cb6062380eee794b130d63b2d5f0c9c1ac1ffe45 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 21 Oct 2024 21:01:50 -0400 Subject: [PATCH 01/32] feat(task): dependencies --- Cargo.lock | 2 - Cargo.toml | 3 + cli/tools/task.rs | 268 +++++++++++++++++++++++++++------------------- 3 files changed, 160 insertions(+), 113 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd77a331334cb0..40e85560ac2885 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1392,8 +1392,6 @@ dependencies = [ [[package]] name = "deno_config" version = "0.37.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5900bfb37538d83b19ba0b157cdc785770e38422ee4632411e3bd3d90ac0f537" dependencies = [ "anyhow", "deno_package_json", diff --git a/Cargo.toml b/Cargo.toml index 7d5c507cae9685..8b7970306464c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -326,3 +326,6 @@ opt-level = 3 opt-level = 3 [profile.release.package.zstd-sys] opt-level = 3 + +[patch.crates-io] +deno_config = { path = "../deno_config" } diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 502b09d2c2ca76..d772d5b438eda8 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -16,7 +16,10 @@ use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::futures::stream::futures_unordered; +use deno_core::futures::StreamExt; use deno_path_util::normalize_path; +use deno_runtime::deno_node::NodeResolver; use deno_task_shell::ShellCommand; use crate::args::CliOptions; @@ -71,132 +74,163 @@ pub async fn execute_script( let node_resolver = factory.node_resolver().await?; let env_vars = task_runner::real_env_vars(); - match tasks_config.task(task_name) { - Some((dir_url, task_or_script)) => match task_or_script { - TaskOrScript::Task(_tasks, script) => { - let cwd = match task_flags.cwd { - Some(path) => canonicalize_path(&PathBuf::from(path)) - .context("failed canonicalizing --cwd")?, - None => normalize_path(dir_url.to_file_path().unwrap()), - }; + let task_runner = TaskRunner { + tasks_config, + task_flags: &task_flags, + npm_resolver: npm_resolver.as_ref(), + node_resolver: node_resolver.as_ref(), + env_vars, + cli_options, + }; + task_runner.run_task(task_name).await +} - let custom_commands = task_runner::resolve_custom_commands( - npm_resolver.as_ref(), - node_resolver, - )?; - run_task(RunTaskOptions { - task_name, - script, - cwd: &cwd, - env_vars, - custom_commands, - npm_resolver: npm_resolver.as_ref(), - cli_options, - }) - .await - } - TaskOrScript::Script(scripts, _script) => { - // ensure the npm packages are installed if using a managed resolver - if let Some(npm_resolver) = npm_resolver.as_managed() { - npm_resolver.ensure_top_level_package_json_install().await?; - } +struct RunSingleOptions<'a> { + task_name: &'a str, + script: &'a str, + cwd: &'a Path, + custom_commands: HashMap>, +} - let cwd = match task_flags.cwd { - Some(path) => canonicalize_path(&PathBuf::from(path))?, - None => normalize_path(dir_url.to_file_path().unwrap()), - }; +struct TaskRunner<'a> { + tasks_config: WorkspaceTasksConfig, + task_flags: &'a TaskFlags, + npm_resolver: &'a dyn CliNpmResolver, + node_resolver: &'a NodeResolver, + env_vars: HashMap, + cli_options: &'a CliOptions, +} - // At this point we already checked if the task name exists in package.json. - // We can therefore check for "pre" and "post" scripts too, since we're only - // dealing with package.json here and not deno.json - let task_names = vec![ - format!("pre{}", task_name), - task_name.clone(), - format!("post{}", task_name), - ]; - let custom_commands = task_runner::resolve_custom_commands( - npm_resolver.as_ref(), - node_resolver, - )?; - for task_name in &task_names { - if let Some(script) = scripts.get(task_name) { - let exit_code = run_task(RunTaskOptions { - task_name, - script, - cwd: &cwd, - env_vars: env_vars.clone(), - custom_commands: custom_commands.clone(), - npm_resolver: npm_resolver.as_ref(), - cli_options, - }) - .await?; +impl<'a> TaskRunner<'a> { + async fn run_task( + &self, + task_name: &String, + ) -> Result { + match self.tasks_config.task(task_name) { + Some((dir_url, task_or_script)) => match task_or_script { + TaskOrScript::Task(_tasks, definition) => { + let mut futures_unordered = + futures_unordered::FuturesUnordered::new(); + for dep in &definition.dependencies { + let dep = dep.clone(); + futures_unordered.push(async move { self.run_task(&dep).await }) + } + while let Some(result) = futures_unordered.next().await { + let exit_code = result?; if exit_code > 0 { return Ok(exit_code); } } + let cwd = match &self.task_flags.cwd { + Some(path) => canonicalize_path(&PathBuf::from(path)) + .context("failed canonicalizing --cwd")?, + None => normalize_path(dir_url.to_file_path().unwrap()), + }; + + let custom_commands = task_runner::resolve_custom_commands( + self.npm_resolver, + self.node_resolver, + )?; + self + .run_single(RunSingleOptions { + task_name, + script: &definition.command, + cwd: &cwd, + custom_commands, + }) + .await } + TaskOrScript::Script(scripts, _script) => { + // ensure the npm packages are installed if using a managed resolver + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + } - Ok(0) - } - }, - None => { - if task_flags.is_run { - return Err(anyhow!("Task not found: {}", task_name)); - } - log::error!("Task not found: {}", task_name); - if log::log_enabled!(log::Level::Error) { - print_available_tasks( - &mut std::io::stderr(), - &cli_options.start_dir, - &tasks_config, - )?; + let cwd = match &self.task_flags.cwd { + Some(path) => canonicalize_path(&PathBuf::from(path))?, + None => normalize_path(dir_url.to_file_path().unwrap()), + }; + + // At this point we already checked if the task name exists in package.json. + // We can therefore check for "pre" and "post" scripts too, since we're only + // dealing with package.json here and not deno.json + let task_names = vec![ + format!("pre{}", task_name), + task_name.clone(), + format!("post{}", task_name), + ]; + let custom_commands = task_runner::resolve_custom_commands( + self.npm_resolver, + self.node_resolver, + )?; + for task_name in &task_names { + if let Some(script) = scripts.get(task_name) { + let exit_code = self + .run_single(RunSingleOptions { + task_name, + script, + cwd: &cwd, + custom_commands: custom_commands.clone(), + }) + .await?; + if exit_code > 0 { + return Ok(exit_code); + } + } + } + + Ok(0) + } + }, + None => { + if self.task_flags.is_run { + return Err(anyhow!("Task not found: {}", task_name)); + } + log::error!("Task not found: {}", task_name); + if log::log_enabled!(log::Level::Error) { + print_available_tasks( + &mut std::io::stderr(), + &self.cli_options.start_dir, + &self.tasks_config, + )?; + } + Ok(1) } - Ok(1) } } -} - -struct RunTaskOptions<'a> { - task_name: &'a str, - script: &'a str, - cwd: &'a Path, - env_vars: HashMap, - custom_commands: HashMap>, - npm_resolver: &'a dyn CliNpmResolver, - cli_options: &'a CliOptions, -} -async fn run_task(opts: RunTaskOptions<'_>) -> Result { - let RunTaskOptions { - task_name, - script, - cwd, - env_vars, - custom_commands, - npm_resolver, - cli_options, - } = opts; - - output_task( - opts.task_name, - &task_runner::get_script_with_args(script, cli_options.argv()), - ); - - Ok( - task_runner::run_task(task_runner::RunTaskOptions { + async fn run_single( + &self, + opts: RunSingleOptions<'_>, + ) -> Result { + let RunSingleOptions { task_name, script, cwd, - env_vars, custom_commands, - init_cwd: opts.cli_options.initial_cwd(), - argv: cli_options.argv(), - root_node_modules_dir: npm_resolver.root_node_modules_path(), - stdio: None, - }) - .await? - .exit_code, - ) + } = opts; + + output_task( + opts.task_name, + &task_runner::get_script_with_args(script, self.cli_options.argv()), + ); + + Ok( + task_runner::run_task(task_runner::RunTaskOptions { + task_name, + script, + cwd, + env_vars: self.env_vars.clone(), + custom_commands, + init_cwd: self.cli_options.initial_cwd(), + argv: self.cli_options.argv(), + root_node_modules_dir: self.npm_resolver.root_node_modules_path(), + stdio: None, + }) + .await? + .exit_code, + ) + } } fn output_task(task_name: &str, script: &str) { @@ -252,7 +286,19 @@ fn print_available_tasks( && config.folder_url == *workspace_dir.workspace.root_dir().as_ref(); config.tasks.iter().map(move |(k, v)| { - (is_root, false, (k, Cow::Owned(Task::Definition(v.clone())))) + ( + is_root, + false, + ( + k, + Cow::Owned(Task::Definition( + deno_config::deno_json::TaskDefinition { + command: v.clone(), + dependencies: vec![], + }, + )), + ), + ) }) }) .into_iter() @@ -292,7 +338,7 @@ fn print_available_tasks( )?; } } - writeln!(writer, " {definition}")?; + writeln!(writer, " {}", definition.command)?; } } } From 7f2e2bde7771820fcf1eb39c270cca914c6dc413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Nov 2024 17:19:22 +0100 Subject: [PATCH 02/32] support for description --- cli/tools/task.rs | 153 +++++++++++++++++++++++++--------------------- 1 file changed, 82 insertions(+), 71 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index d772d5b438eda8..43002d872f04f8 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -256,80 +256,91 @@ fn print_available_tasks( " {}", colors::red("No tasks found in configuration file") )?; - } else { - let mut seen_task_names = - HashSet::with_capacity(tasks_config.tasks_count()); - for maybe_config in [&tasks_config.member, &tasks_config.root] { - let Some(config) = maybe_config else { - continue; - }; - for (is_root, is_deno, (key, task)) in config - .deno_json - .as_ref() - .map(|config| { - let is_root = !is_cwd_root_dir - && config.folder_url - == *workspace_dir.workspace.root_dir().as_ref(); - config - .tasks - .iter() - .map(move |(k, t)| (is_root, true, (k, Cow::Borrowed(t)))) - }) - .into_iter() - .flatten() - .chain( - config - .package_json - .as_ref() - .map(|config| { - let is_root = !is_cwd_root_dir - && config.folder_url - == *workspace_dir.workspace.root_dir().as_ref(); - config.tasks.iter().map(move |(k, v)| { + return Ok(()); + } + + let mut seen_task_names = HashSet::with_capacity(tasks_config.tasks_count()); + for maybe_config in [&tasks_config.member, &tasks_config.root] { + let Some(config) = maybe_config else { + continue; + }; + for (is_root, is_deno, (key, task)) in config + .deno_json + .as_ref() + .map(|config| { + let is_root = !is_cwd_root_dir + && config.folder_url == *workspace_dir.workspace.root_dir().as_ref(); + config + .tasks + .iter() + .map(move |(k, t)| (is_root, true, (k, Cow::Borrowed(t)))) + }) + .into_iter() + .flatten() + .chain( + config + .package_json + .as_ref() + .map(|config| { + let is_root = !is_cwd_root_dir + && config.folder_url + == *workspace_dir.workspace.root_dir().as_ref(); + config.tasks.iter().map(move |(k, v)| { + ( + is_root, + false, ( - is_root, - false, - ( - k, - Cow::Owned(Task::Definition( - deno_config::deno_json::TaskDefinition { - command: v.clone(), - dependencies: vec![], - }, - )), - ), - ) - }) + k, + Cow::Owned(Task::Definition( + deno_config::deno_json::TaskDefinition { + command: v.clone(), + dependencies: vec![], + description: None, + }, + )), + ), + ) }) - .into_iter() - .flatten(), - ) - { - if !seen_task_names.insert(key) { - continue; // already seen - } - writeln!( - writer, - "- {}{}", - colors::cyan(key), - if is_root { - if is_deno { - format!(" {}", colors::italic_gray("(workspace)")) - } else { - format!(" {}", colors::italic_gray("(workspace package.json)")) - } - } else if is_deno { - "".to_string() + }) + .into_iter() + .flatten(), + ) + { + if !seen_task_names.insert(key) { + continue; // already seen + } + writeln!( + writer, + "- {}{}", + colors::cyan(key), + if is_root { + if is_deno { + format!(" {}", colors::italic_gray("(workspace)")) } else { - format!(" {}", colors::italic_gray("(package.json)")) + format!(" {}", colors::italic_gray("(workspace package.json)")) } - )?; - let definition = match task.as_ref() { - Task::Definition(definition) => definition, - Task::Commented { definition, .. } => definition, - }; - if let Task::Commented { comments, .. } = task.as_ref() { - let slash_slash = colors::italic_gray("//"); + } else if is_deno { + "".to_string() + } else { + format!(" {}", colors::italic_gray("(package.json)")) + } + )?; + let definition = match task.as_ref() { + Task::Definition(definition) => definition, + Task::Commented { definition, .. } => definition, + }; + let slash_slash = colors::italic_gray("//"); + match task.as_ref() { + Task::Definition(definition) => { + if let Some(description) = &definition.description { + writeln!( + writer, + " {slash_slash} {}", + colors::italic_gray(description) + )?; + } + } + Task::Commented { comments, .. } => { for comment in comments { writeln!( writer, @@ -338,8 +349,8 @@ fn print_available_tasks( )?; } } - writeln!(writer, " {}", definition.command)?; } + writeln!(writer, " {}", definition.command)?; } } From 8d02b99702b4a4de79d6de59bc8f1d2a776f5190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Nov 2024 23:06:40 +0100 Subject: [PATCH 03/32] simplify --- cli/args/mod.rs | 8 +- cli/lsp/language_server.rs | 5 +- cli/tools/task.rs | 182 +++++++++++++++++-------------------- 3 files changed, 85 insertions(+), 110 deletions(-) diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 50a37b3346e753..318a6ca76b1068 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -868,12 +868,8 @@ impl CliOptions { } else { &[] }; - let config_parse_options = deno_config::deno_json::ConfigParseOptions { - include_task_comments: matches!( - flags.subcommand, - DenoSubcommand::Task(..) - ), - }; + let config_parse_options = + deno_config::deno_json::ConfigParseOptions::default(); let discover_pkg_json = flags.config_flag != ConfigFlag::Disabled && !flags.no_npm && !has_flag_env_var("DENO_NO_PACKAGE_JSON"); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index b2bd72416ac8c7..24f53480d20671 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -3629,9 +3629,8 @@ impl Inner { deno_json_cache: None, pkg_json_cache: None, workspace_cache: None, - config_parse_options: deno_config::deno_json::ConfigParseOptions { - include_task_comments: false, - }, + config_parse_options: + deno_config::deno_json::ConfigParseOptions::default(), additional_config_file_names: &[], discover_pkg_json: !has_flag_env_var("DENO_NO_PACKAGE_JSON"), maybe_vendor_override: if force_global_cache { diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 43002d872f04f8..2f5840ae153e78 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -1,6 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use std::borrow::Cow; use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; @@ -8,7 +7,7 @@ use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; -use deno_config::deno_json::Task; +use deno_config::workspace::TaskDefinition; use deno_config::workspace::TaskOrScript; use deno_config::workspace::WorkspaceDirectory; use deno_config::workspace::WorkspaceTasksConfig; @@ -51,23 +50,18 @@ pub async fn execute_script( v == "1" }) .unwrap_or(false); - let tasks_config = start_dir.to_tasks_config()?; - let tasks_config = if force_use_pkg_json { - tasks_config.with_only_pkg_json() - } else { - tasks_config - }; + let mut tasks_config = start_dir.to_tasks_config()?; + if force_use_pkg_json { + tasks_config = tasks_config.with_only_pkg_json() + } - let task_name = match &task_flags.task { - Some(task) => task, - None => { - print_available_tasks( - &mut std::io::stdout(), - &cli_options.start_dir, - &tasks_config, - )?; - return Ok(0); - } + let Some(task_name) = &task_flags.task else { + print_available_tasks( + &mut std::io::stdout(), + &cli_options.start_dir, + &tasks_config, + )?; + return Ok(0); }; let npm_resolver = factory.npm_resolver().await?; @@ -259,99 +253,85 @@ fn print_available_tasks( return Ok(()); } + struct AvailableTaskDescription { + is_root: bool, + is_deno: bool, + name: String, + task: TaskDefinition, + } let mut seen_task_names = HashSet::with_capacity(tasks_config.tasks_count()); + let mut task_descriptions = Vec::with_capacity(tasks_config.tasks_count()); + for maybe_config in [&tasks_config.member, &tasks_config.root] { let Some(config) = maybe_config else { continue; }; - for (is_root, is_deno, (key, task)) in config - .deno_json - .as_ref() - .map(|config| { - let is_root = !is_cwd_root_dir - && config.folder_url == *workspace_dir.workspace.root_dir().as_ref(); - config - .tasks - .iter() - .map(move |(k, t)| (is_root, true, (k, Cow::Borrowed(t)))) - }) - .into_iter() - .flatten() - .chain( - config - .package_json - .as_ref() - .map(|config| { - let is_root = !is_cwd_root_dir - && config.folder_url - == *workspace_dir.workspace.root_dir().as_ref(); - config.tasks.iter().map(move |(k, v)| { - ( - is_root, - false, - ( - k, - Cow::Owned(Task::Definition( - deno_config::deno_json::TaskDefinition { - command: v.clone(), - dependencies: vec![], - description: None, - }, - )), - ), - ) - }) - }) - .into_iter() - .flatten(), - ) - { - if !seen_task_names.insert(key) { - continue; // already seen - } - writeln!( - writer, - "- {}{}", - colors::cyan(key), - if is_root { - if is_deno { - format!(" {}", colors::italic_gray("(workspace)")) - } else { - format!(" {}", colors::italic_gray("(workspace package.json)")) - } - } else if is_deno { - "".to_string() - } else { - format!(" {}", colors::italic_gray("(package.json)")) + + if let Some(config) = config.deno_json.as_ref() { + let is_root = !is_cwd_root_dir + && config.folder_url == *workspace_dir.workspace.root_dir().as_ref(); + + for (name, definition) in &config.tasks { + if !seen_task_names.insert(name) { + continue; // already seen } - )?; - let definition = match task.as_ref() { - Task::Definition(definition) => definition, - Task::Commented { definition, .. } => definition, - }; - let slash_slash = colors::italic_gray("//"); - match task.as_ref() { - Task::Definition(definition) => { - if let Some(description) = &definition.description { - writeln!( - writer, - " {slash_slash} {}", - colors::italic_gray(description) - )?; - } + task_descriptions.push(AvailableTaskDescription { + is_root, + is_deno: true, + name: name.to_string(), + task: definition.clone(), + }); + } + } + + if let Some(config) = config.package_json.as_ref() { + let is_root = !is_cwd_root_dir + && config.folder_url == *workspace_dir.workspace.root_dir().as_ref(); + for (name, script) in &config.tasks { + if !seen_task_names.insert(name) { + continue; // already seen } - Task::Commented { comments, .. } => { - for comment in comments { - writeln!( - writer, - " {slash_slash} {}", - colors::italic_gray(comment) - )?; - } + + task_descriptions.push(AvailableTaskDescription { + is_root, + is_deno: true, + name: name.to_string(), + task: deno_config::deno_json::TaskDefinition { + command: script.to_string(), + dependencies: vec![], + description: None, + }, + }); + } + } + } + + for desc in task_descriptions { + writeln!( + writer, + "- {}{}", + colors::cyan(desc.name), + if desc.is_root { + if desc.is_deno { + format!(" {}", colors::italic_gray("(workspace)")) + } else { + format!(" {}", colors::italic_gray("(workspace package.json)")) } + } else if desc.is_deno { + "".to_string() + } else { + format!(" {}", colors::italic_gray("(package.json)")) } - writeln!(writer, " {}", definition.command)?; + )?; + if let Some(description) = &desc.task.description { + let slash_slash = colors::italic_gray("//"); + writeln!( + writer, + " {slash_slash} {}", + colors::italic_gray(description) + )?; } + writeln!(writer, " {}", desc.task.command)?; } Ok(()) From 9e164660880a1eb69c914ca6ac160d2eece666a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Nov 2024 23:17:43 +0100 Subject: [PATCH 04/32] further refactor --- cli/tools/task.rs | 198 +++++++++++++++++++++++++--------------------- 1 file changed, 109 insertions(+), 89 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 2f5840ae153e78..f6158ce8499285 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -17,9 +17,11 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::futures::stream::futures_unordered; use deno_core::futures::StreamExt; +use deno_core::url::Url; use deno_path_util::normalize_path; use deno_runtime::deno_node::NodeResolver; use deno_task_shell::ShellCommand; +use indexmap::IndexMap; use crate::args::CliOptions; use crate::args::Flags; @@ -100,97 +102,115 @@ impl<'a> TaskRunner<'a> { &self, task_name: &String, ) -> Result { - match self.tasks_config.task(task_name) { - Some((dir_url, task_or_script)) => match task_or_script { - TaskOrScript::Task(_tasks, definition) => { - let mut futures_unordered = - futures_unordered::FuturesUnordered::new(); - for dep in &definition.dependencies { - let dep = dep.clone(); - futures_unordered.push(async move { self.run_task(&dep).await }) - } - while let Some(result) = futures_unordered.next().await { - let exit_code = result?; - if exit_code > 0 { - return Ok(exit_code); - } - } - let cwd = match &self.task_flags.cwd { - Some(path) => canonicalize_path(&PathBuf::from(path)) - .context("failed canonicalizing --cwd")?, - None => normalize_path(dir_url.to_file_path().unwrap()), - }; - - let custom_commands = task_runner::resolve_custom_commands( - self.npm_resolver, - self.node_resolver, - )?; - self - .run_single(RunSingleOptions { - task_name, - script: &definition.command, - cwd: &cwd, - custom_commands, - }) - .await - } - TaskOrScript::Script(scripts, _script) => { - // ensure the npm packages are installed if using a managed resolver - if let Some(npm_resolver) = self.npm_resolver.as_managed() { - npm_resolver.ensure_top_level_package_json_install().await?; - } - - let cwd = match &self.task_flags.cwd { - Some(path) => canonicalize_path(&PathBuf::from(path))?, - None => normalize_path(dir_url.to_file_path().unwrap()), - }; - - // At this point we already checked if the task name exists in package.json. - // We can therefore check for "pre" and "post" scripts too, since we're only - // dealing with package.json here and not deno.json - let task_names = vec![ - format!("pre{}", task_name), - task_name.clone(), - format!("post{}", task_name), - ]; - let custom_commands = task_runner::resolve_custom_commands( - self.npm_resolver, - self.node_resolver, - )?; - for task_name in &task_names { - if let Some(script) = scripts.get(task_name) { - let exit_code = self - .run_single(RunSingleOptions { - task_name, - script, - cwd: &cwd, - custom_commands: custom_commands.clone(), - }) - .await?; - if exit_code > 0 { - return Ok(exit_code); - } - } - } - - Ok(0) - } - }, - None => { - if self.task_flags.is_run { - return Err(anyhow!("Task not found: {}", task_name)); - } - log::error!("Task not found: {}", task_name); - if log::log_enabled!(log::Level::Error) { - print_available_tasks( - &mut std::io::stderr(), - &self.cli_options.start_dir, - &self.tasks_config, - )?; + let Some((dir_url, task_or_script)) = self.tasks_config.task(task_name) + else { + if self.task_flags.is_run { + return Err(anyhow!("Task not found: {}", task_name)); + } + + log::error!("Task not found: {}", task_name); + if log::log_enabled!(log::Level::Error) { + print_available_tasks( + &mut std::io::stderr(), + &self.cli_options.start_dir, + &self.tasks_config, + )?; + } + return Ok(1); + }; + + match task_or_script { + TaskOrScript::Task(_tasks, definition) => { + self.run_deno_task(dir_url, task_name, definition).await + } + TaskOrScript::Script(scripts, _script) => { + self.run_npm_script(dir_url, task_name, scripts).await + } + } + } + + async fn run_deno_task( + &self, + dir_url: &Url, + task_name: &String, + definition: &TaskDefinition, + ) -> Result { + let mut futures_unordered = futures_unordered::FuturesUnordered::new(); + for dep in &definition.dependencies { + let dep = dep.clone(); + futures_unordered.push(async move { self.run_task(&dep).await }) + } + while let Some(result) = futures_unordered.next().await { + let exit_code = result?; + if exit_code > 0 { + return Ok(exit_code); + } + } + let cwd = match &self.task_flags.cwd { + Some(path) => canonicalize_path(&PathBuf::from(path)) + .context("failed canonicalizing --cwd")?, + None => normalize_path(dir_url.to_file_path().unwrap()), + }; + + let custom_commands = task_runner::resolve_custom_commands( + self.npm_resolver, + self.node_resolver, + )?; + self + .run_single(RunSingleOptions { + task_name, + script: &definition.command, + cwd: &cwd, + custom_commands, + }) + .await + } + + async fn run_npm_script( + &self, + dir_url: &Url, + task_name: &String, + scripts: &IndexMap, + ) -> Result { + // ensure the npm packages are installed if using a managed resolver + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + } + + let cwd = match &self.task_flags.cwd { + Some(path) => canonicalize_path(&PathBuf::from(path))?, + None => normalize_path(dir_url.to_file_path().unwrap()), + }; + + // At this point we already checked if the task name exists in package.json. + // We can therefore check for "pre" and "post" scripts too, since we're only + // dealing with package.json here and not deno.json + let task_names = vec![ + format!("pre{}", task_name), + task_name.clone(), + format!("post{}", task_name), + ]; + let custom_commands = task_runner::resolve_custom_commands( + self.npm_resolver, + self.node_resolver, + )?; + for task_name in &task_names { + if let Some(script) = scripts.get(task_name) { + let exit_code = self + .run_single(RunSingleOptions { + task_name, + script, + cwd: &cwd, + custom_commands: custom_commands.clone(), + }) + .await?; + if exit_code > 0 { + return Ok(exit_code); } - Ok(1) } } + + Ok(0) } async fn run_single( @@ -294,7 +314,7 @@ fn print_available_tasks( task_descriptions.push(AvailableTaskDescription { is_root, - is_deno: true, + is_deno: false, name: name.to_string(), task: deno_config::deno_json::TaskDefinition { command: script.to_string(), From ba7086027233e38349052e63a19e5bf0025f364e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Nov 2024 23:19:10 +0100 Subject: [PATCH 05/32] add a todo --- cli/tools/task.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index f6158ce8499285..28a9c447e12c83 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -135,12 +135,14 @@ impl<'a> TaskRunner<'a> { task_name: &String, definition: &TaskDefinition, ) -> Result { - let mut futures_unordered = futures_unordered::FuturesUnordered::new(); + // TODO(bartlomieju): we might want to limit concurrency here - eg. `wireit` runs 2 tasks in parallel + // unless and env var is specified. + let mut dependency_tasks = futures_unordered::FuturesUnordered::new(); for dep in &definition.dependencies { let dep = dep.clone(); - futures_unordered.push(async move { self.run_task(&dep).await }) + dependency_tasks.push(async move { self.run_task(&dep).await }) } - while let Some(result) = futures_unordered.next().await { + while let Some(result) = dependency_tasks.next().await { let exit_code = result?; if exit_code > 0 { return Ok(exit_code); From b835b450fd88c2d7a4d5184dab6514a0bda49417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 12:55:33 +0100 Subject: [PATCH 06/32] add basic tests --- tests/specs/task/dependencies1/__test__.jsonc | 5 +++++ tests/specs/task/dependencies1/build1.js | 9 +++++++++ tests/specs/task/dependencies1/build2.js | 9 +++++++++ tests/specs/task/dependencies1/deno.json | 10 ++++++++++ tests/specs/task/dependencies1/run.js | 1 + tests/specs/task/dependencies1/run.out | 12 ++++++++++++ tests/specs/task/dependencies1/util.js | 5 +++++ tests/specs/task/dependencies2/__test__.jsonc | 5 +++++ tests/specs/task/dependencies2/build1.js | 9 +++++++++ tests/specs/task/dependencies2/build2.js | 9 +++++++++ tests/specs/task/dependencies2/deno.json | 13 +++++++++++++ tests/specs/task/dependencies2/run.js | 1 + tests/specs/task/dependencies2/run.out | 10 ++++++++++ tests/specs/task/dependencies2/util.js | 5 +++++ 14 files changed, 103 insertions(+) create mode 100644 tests/specs/task/dependencies1/__test__.jsonc create mode 100644 tests/specs/task/dependencies1/build1.js create mode 100644 tests/specs/task/dependencies1/build2.js create mode 100644 tests/specs/task/dependencies1/deno.json create mode 100644 tests/specs/task/dependencies1/run.js create mode 100644 tests/specs/task/dependencies1/run.out create mode 100644 tests/specs/task/dependencies1/util.js create mode 100644 tests/specs/task/dependencies2/__test__.jsonc create mode 100644 tests/specs/task/dependencies2/build1.js create mode 100644 tests/specs/task/dependencies2/build2.js create mode 100644 tests/specs/task/dependencies2/deno.json create mode 100644 tests/specs/task/dependencies2/run.js create mode 100644 tests/specs/task/dependencies2/run.out create mode 100644 tests/specs/task/dependencies2/util.js diff --git a/tests/specs/task/dependencies1/__test__.jsonc b/tests/specs/task/dependencies1/__test__.jsonc new file mode 100644 index 00000000000000..63de80ed217be3 --- /dev/null +++ b/tests/specs/task/dependencies1/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "tempDir": true, + "args": "task run", + "output": "run.out" +} diff --git a/tests/specs/task/dependencies1/build1.js b/tests/specs/task/dependencies1/build1.js new file mode 100644 index 00000000000000..e7a69126fd1821 --- /dev/null +++ b/tests/specs/task/dependencies1/build1.js @@ -0,0 +1,9 @@ +import { randomTimeout } from "./util.js"; + +console.log("Starting build1"); + +await randomTimeout(500, 750); +console.log("build1 performing more work..."); +await randomTimeout(500, 750); + +console.log("build1 finished"); \ No newline at end of file diff --git a/tests/specs/task/dependencies1/build2.js b/tests/specs/task/dependencies1/build2.js new file mode 100644 index 00000000000000..a733ae896ba6c1 --- /dev/null +++ b/tests/specs/task/dependencies1/build2.js @@ -0,0 +1,9 @@ +import { randomTimeout } from "./util.js"; + +console.log("Starting build2"); + +await randomTimeout(250, 750); +console.log("build2 performing more work..."); +await randomTimeout(250, 750); + +console.log("build2 finished"); \ No newline at end of file diff --git a/tests/specs/task/dependencies1/deno.json b/tests/specs/task/dependencies1/deno.json new file mode 100644 index 00000000000000..c0910bb692971d --- /dev/null +++ b/tests/specs/task/dependencies1/deno.json @@ -0,0 +1,10 @@ +{ + "tasks": { + "build1": "deno run build1.js", + "build2": "deno run build2.js", + "run": { + "command": "deno run run.js", + "dependencies": ["build1", "build2"] + } + } +} diff --git a/tests/specs/task/dependencies1/run.js b/tests/specs/task/dependencies1/run.js new file mode 100644 index 00000000000000..f457de6ab06309 --- /dev/null +++ b/tests/specs/task/dependencies1/run.js @@ -0,0 +1 @@ +console.log("run finished"); diff --git a/tests/specs/task/dependencies1/run.out b/tests/specs/task/dependencies1/run.out new file mode 100644 index 00000000000000..f15590f1b18d35 --- /dev/null +++ b/tests/specs/task/dependencies1/run.out @@ -0,0 +1,12 @@ +Task build1 deno run build1.js +Task build2 deno run build2.js +[UNORDERED_START] +Starting build2 +Starting build1 +build2 performing more work... +build1 performing more work... +build2 finished +build1 finished +[UNORDERED_END] +Task run deno run run.js +run finished diff --git a/tests/specs/task/dependencies1/util.js b/tests/specs/task/dependencies1/util.js new file mode 100644 index 00000000000000..8d2e7523f0f08a --- /dev/null +++ b/tests/specs/task/dependencies1/util.js @@ -0,0 +1,5 @@ + +export async function randomTimeout(min, max) { + const timeout = Math.floor(Math.random() * (max - min + 1) + min); + return new Promise(resolve => setTimeout(resolve, timeout)); +} \ No newline at end of file diff --git a/tests/specs/task/dependencies2/__test__.jsonc b/tests/specs/task/dependencies2/__test__.jsonc new file mode 100644 index 00000000000000..63de80ed217be3 --- /dev/null +++ b/tests/specs/task/dependencies2/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "tempDir": true, + "args": "task run", + "output": "run.out" +} diff --git a/tests/specs/task/dependencies2/build1.js b/tests/specs/task/dependencies2/build1.js new file mode 100644 index 00000000000000..e7a69126fd1821 --- /dev/null +++ b/tests/specs/task/dependencies2/build1.js @@ -0,0 +1,9 @@ +import { randomTimeout } from "./util.js"; + +console.log("Starting build1"); + +await randomTimeout(500, 750); +console.log("build1 performing more work..."); +await randomTimeout(500, 750); + +console.log("build1 finished"); \ No newline at end of file diff --git a/tests/specs/task/dependencies2/build2.js b/tests/specs/task/dependencies2/build2.js new file mode 100644 index 00000000000000..a733ae896ba6c1 --- /dev/null +++ b/tests/specs/task/dependencies2/build2.js @@ -0,0 +1,9 @@ +import { randomTimeout } from "./util.js"; + +console.log("Starting build2"); + +await randomTimeout(250, 750); +console.log("build2 performing more work..."); +await randomTimeout(250, 750); + +console.log("build2 finished"); \ No newline at end of file diff --git a/tests/specs/task/dependencies2/deno.json b/tests/specs/task/dependencies2/deno.json new file mode 100644 index 00000000000000..682b79b58eaf8e --- /dev/null +++ b/tests/specs/task/dependencies2/deno.json @@ -0,0 +1,13 @@ +{ + "tasks": { + "build1": "deno run build1.js", + "build2": { + "command": "deno run build2.js", + "dependencies": ["build1"] + }, + "run": { + "command": "deno run run.js", + "dependencies": ["build2"] + } + } +} diff --git a/tests/specs/task/dependencies2/run.js b/tests/specs/task/dependencies2/run.js new file mode 100644 index 00000000000000..f457de6ab06309 --- /dev/null +++ b/tests/specs/task/dependencies2/run.js @@ -0,0 +1 @@ +console.log("run finished"); diff --git a/tests/specs/task/dependencies2/run.out b/tests/specs/task/dependencies2/run.out new file mode 100644 index 00000000000000..e33547ffa7dcc4 --- /dev/null +++ b/tests/specs/task/dependencies2/run.out @@ -0,0 +1,10 @@ +Task build1 deno run build1.js +Starting build1 +build1 performing more work... +build1 finished +Task build2 deno run build2.js +Starting build2 +build2 performing more work... +build2 finished +Task run deno run run.js +run finished diff --git a/tests/specs/task/dependencies2/util.js b/tests/specs/task/dependencies2/util.js new file mode 100644 index 00000000000000..8d2e7523f0f08a --- /dev/null +++ b/tests/specs/task/dependencies2/util.js @@ -0,0 +1,5 @@ + +export async function randomTimeout(min, max) { + const timeout = Math.floor(Math.random() * (max - min + 1) + min); + return new Promise(resolve => setTimeout(resolve, timeout)); +} \ No newline at end of file From 9e206dbad9e289abe535d540b2012cc4d41dbea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 13:03:36 +0100 Subject: [PATCH 07/32] update tests --- tests/specs/task/dependencies/__test__.jsonc | 16 ++++++++++++++++ .../run.out => dependencies/basic1.out} | 6 +++--- tests/specs/task/dependencies/basic1/deno.json | 10 ++++++++++ .../run.out => dependencies/basic2.out} | 6 +++--- .../basic2}/deno.json | 6 +++--- .../{dependencies1 => dependencies}/build1.js | 0 .../{dependencies1 => dependencies}/build2.js | 0 .../task/{dependencies1 => dependencies}/run.js | 0 .../task/{dependencies1 => dependencies}/util.js | 0 tests/specs/task/dependencies1/__test__.jsonc | 5 ----- tests/specs/task/dependencies1/deno.json | 10 ---------- tests/specs/task/dependencies2/__test__.jsonc | 5 ----- tests/specs/task/dependencies2/build1.js | 9 --------- tests/specs/task/dependencies2/build2.js | 9 --------- tests/specs/task/dependencies2/run.js | 1 - tests/specs/task/dependencies2/util.js | 5 ----- 16 files changed, 35 insertions(+), 53 deletions(-) create mode 100644 tests/specs/task/dependencies/__test__.jsonc rename tests/specs/task/{dependencies1/run.out => dependencies/basic1.out} (64%) create mode 100644 tests/specs/task/dependencies/basic1/deno.json rename tests/specs/task/{dependencies2/run.out => dependencies/basic2.out} (59%) rename tests/specs/task/{dependencies2 => dependencies/basic2}/deno.json (51%) rename tests/specs/task/{dependencies1 => dependencies}/build1.js (100%) rename tests/specs/task/{dependencies1 => dependencies}/build2.js (100%) rename tests/specs/task/{dependencies1 => dependencies}/run.js (100%) rename tests/specs/task/{dependencies1 => dependencies}/util.js (100%) delete mode 100644 tests/specs/task/dependencies1/__test__.jsonc delete mode 100644 tests/specs/task/dependencies1/deno.json delete mode 100644 tests/specs/task/dependencies2/__test__.jsonc delete mode 100644 tests/specs/task/dependencies2/build1.js delete mode 100644 tests/specs/task/dependencies2/build2.js delete mode 100644 tests/specs/task/dependencies2/run.js delete mode 100644 tests/specs/task/dependencies2/util.js diff --git a/tests/specs/task/dependencies/__test__.jsonc b/tests/specs/task/dependencies/__test__.jsonc new file mode 100644 index 00000000000000..abcaf08df0a3d4 --- /dev/null +++ b/tests/specs/task/dependencies/__test__.jsonc @@ -0,0 +1,16 @@ +{ + "tests": { + "basic1": { + "cwd": "basic1", + "tempDir": true, + "args": "task run", + "output": "./basic1.out" + }, + "basic2": { + "cwd": "basic2", + "tempDir": true, + "args": "task run", + "output": "./basic2.out" + } + } +} diff --git a/tests/specs/task/dependencies1/run.out b/tests/specs/task/dependencies/basic1.out similarity index 64% rename from tests/specs/task/dependencies1/run.out rename to tests/specs/task/dependencies/basic1.out index f15590f1b18d35..e9443d3a7478c8 100644 --- a/tests/specs/task/dependencies1/run.out +++ b/tests/specs/task/dependencies/basic1.out @@ -1,5 +1,5 @@ -Task build1 deno run build1.js -Task build2 deno run build2.js +Task build1 deno run ../build1.js +Task build2 deno run ../build2.js [UNORDERED_START] Starting build2 Starting build1 @@ -8,5 +8,5 @@ build1 performing more work... build2 finished build1 finished [UNORDERED_END] -Task run deno run run.js +Task run deno run ../run.js run finished diff --git a/tests/specs/task/dependencies/basic1/deno.json b/tests/specs/task/dependencies/basic1/deno.json new file mode 100644 index 00000000000000..16bb9937e47047 --- /dev/null +++ b/tests/specs/task/dependencies/basic1/deno.json @@ -0,0 +1,10 @@ +{ + "tasks": { + "build1": "deno run ../build1.js", + "build2": "deno run ../build2.js", + "run": { + "command": "deno run ../run.js", + "dependencies": ["build1", "build2"] + } + } +} diff --git a/tests/specs/task/dependencies2/run.out b/tests/specs/task/dependencies/basic2.out similarity index 59% rename from tests/specs/task/dependencies2/run.out rename to tests/specs/task/dependencies/basic2.out index e33547ffa7dcc4..24ccd0eb03ace0 100644 --- a/tests/specs/task/dependencies2/run.out +++ b/tests/specs/task/dependencies/basic2.out @@ -1,10 +1,10 @@ -Task build1 deno run build1.js +Task build1 deno run ../build1.js Starting build1 build1 performing more work... build1 finished -Task build2 deno run build2.js +Task build2 deno run ../build2.js Starting build2 build2 performing more work... build2 finished -Task run deno run run.js +Task run deno run ../run.js run finished diff --git a/tests/specs/task/dependencies2/deno.json b/tests/specs/task/dependencies/basic2/deno.json similarity index 51% rename from tests/specs/task/dependencies2/deno.json rename to tests/specs/task/dependencies/basic2/deno.json index 682b79b58eaf8e..9a54926dd393a1 100644 --- a/tests/specs/task/dependencies2/deno.json +++ b/tests/specs/task/dependencies/basic2/deno.json @@ -1,12 +1,12 @@ { "tasks": { - "build1": "deno run build1.js", + "build1": "deno run ../build1.js", "build2": { - "command": "deno run build2.js", + "command": "deno run ../build2.js", "dependencies": ["build1"] }, "run": { - "command": "deno run run.js", + "command": "deno run ../run.js", "dependencies": ["build2"] } } diff --git a/tests/specs/task/dependencies1/build1.js b/tests/specs/task/dependencies/build1.js similarity index 100% rename from tests/specs/task/dependencies1/build1.js rename to tests/specs/task/dependencies/build1.js diff --git a/tests/specs/task/dependencies1/build2.js b/tests/specs/task/dependencies/build2.js similarity index 100% rename from tests/specs/task/dependencies1/build2.js rename to tests/specs/task/dependencies/build2.js diff --git a/tests/specs/task/dependencies1/run.js b/tests/specs/task/dependencies/run.js similarity index 100% rename from tests/specs/task/dependencies1/run.js rename to tests/specs/task/dependencies/run.js diff --git a/tests/specs/task/dependencies1/util.js b/tests/specs/task/dependencies/util.js similarity index 100% rename from tests/specs/task/dependencies1/util.js rename to tests/specs/task/dependencies/util.js diff --git a/tests/specs/task/dependencies1/__test__.jsonc b/tests/specs/task/dependencies1/__test__.jsonc deleted file mode 100644 index 63de80ed217be3..00000000000000 --- a/tests/specs/task/dependencies1/__test__.jsonc +++ /dev/null @@ -1,5 +0,0 @@ -{ - "tempDir": true, - "args": "task run", - "output": "run.out" -} diff --git a/tests/specs/task/dependencies1/deno.json b/tests/specs/task/dependencies1/deno.json deleted file mode 100644 index c0910bb692971d..00000000000000 --- a/tests/specs/task/dependencies1/deno.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "tasks": { - "build1": "deno run build1.js", - "build2": "deno run build2.js", - "run": { - "command": "deno run run.js", - "dependencies": ["build1", "build2"] - } - } -} diff --git a/tests/specs/task/dependencies2/__test__.jsonc b/tests/specs/task/dependencies2/__test__.jsonc deleted file mode 100644 index 63de80ed217be3..00000000000000 --- a/tests/specs/task/dependencies2/__test__.jsonc +++ /dev/null @@ -1,5 +0,0 @@ -{ - "tempDir": true, - "args": "task run", - "output": "run.out" -} diff --git a/tests/specs/task/dependencies2/build1.js b/tests/specs/task/dependencies2/build1.js deleted file mode 100644 index e7a69126fd1821..00000000000000 --- a/tests/specs/task/dependencies2/build1.js +++ /dev/null @@ -1,9 +0,0 @@ -import { randomTimeout } from "./util.js"; - -console.log("Starting build1"); - -await randomTimeout(500, 750); -console.log("build1 performing more work..."); -await randomTimeout(500, 750); - -console.log("build1 finished"); \ No newline at end of file diff --git a/tests/specs/task/dependencies2/build2.js b/tests/specs/task/dependencies2/build2.js deleted file mode 100644 index a733ae896ba6c1..00000000000000 --- a/tests/specs/task/dependencies2/build2.js +++ /dev/null @@ -1,9 +0,0 @@ -import { randomTimeout } from "./util.js"; - -console.log("Starting build2"); - -await randomTimeout(250, 750); -console.log("build2 performing more work..."); -await randomTimeout(250, 750); - -console.log("build2 finished"); \ No newline at end of file diff --git a/tests/specs/task/dependencies2/run.js b/tests/specs/task/dependencies2/run.js deleted file mode 100644 index f457de6ab06309..00000000000000 --- a/tests/specs/task/dependencies2/run.js +++ /dev/null @@ -1 +0,0 @@ -console.log("run finished"); diff --git a/tests/specs/task/dependencies2/util.js b/tests/specs/task/dependencies2/util.js deleted file mode 100644 index 8d2e7523f0f08a..00000000000000 --- a/tests/specs/task/dependencies2/util.js +++ /dev/null @@ -1,5 +0,0 @@ - -export async function randomTimeout(min, max) { - const timeout = Math.floor(Math.random() * (max - min + 1) + min); - return new Promise(resolve => setTimeout(resolve, timeout)); -} \ No newline at end of file From 8e2cb550c2db5311813035cf8ca4d05a58cb03d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 13:09:57 +0100 Subject: [PATCH 08/32] add cross package test --- tests/specs/task/dependencies/__test__.jsonc | 7 +++++++ tests/specs/task/dependencies/cross_package.out | 4 ++++ .../task/dependencies/cross_package/package1/deno.json | 8 ++++++++ .../task/dependencies/cross_package/package2/deno.json | 5 +++++ 4 files changed, 24 insertions(+) create mode 100644 tests/specs/task/dependencies/cross_package.out create mode 100644 tests/specs/task/dependencies/cross_package/package1/deno.json create mode 100644 tests/specs/task/dependencies/cross_package/package2/deno.json diff --git a/tests/specs/task/dependencies/__test__.jsonc b/tests/specs/task/dependencies/__test__.jsonc index abcaf08df0a3d4..6df21b681ac065 100644 --- a/tests/specs/task/dependencies/__test__.jsonc +++ b/tests/specs/task/dependencies/__test__.jsonc @@ -11,6 +11,13 @@ "tempDir": true, "args": "task run", "output": "./basic2.out" + }, + "cross_package": { + "cwd": "cross_package/package1", + "tempDir": true, + "args": "task run", + "output": "./cross_package.out", + "exitCode": 1 } } } diff --git a/tests/specs/task/dependencies/cross_package.out b/tests/specs/task/dependencies/cross_package.out new file mode 100644 index 00000000000000..1f96ed9b0f36ac --- /dev/null +++ b/tests/specs/task/dependencies/cross_package.out @@ -0,0 +1,4 @@ +Task not found: ../package2:run +Available tasks: +- run + deno run.js diff --git a/tests/specs/task/dependencies/cross_package/package1/deno.json b/tests/specs/task/dependencies/cross_package/package1/deno.json new file mode 100644 index 00000000000000..6684a1e2c4490f --- /dev/null +++ b/tests/specs/task/dependencies/cross_package/package1/deno.json @@ -0,0 +1,8 @@ +{ + "tasks": { + "run": { + "command": "deno run.js", + "dependencies": ["../package2:run"] + } + } +} diff --git a/tests/specs/task/dependencies/cross_package/package2/deno.json b/tests/specs/task/dependencies/cross_package/package2/deno.json new file mode 100644 index 00000000000000..e45ec398f61065 --- /dev/null +++ b/tests/specs/task/dependencies/cross_package/package2/deno.json @@ -0,0 +1,5 @@ +{ + "tasks": { + "run": "deno run.js" + } +} From e8a5459b4066174208a9ae97cc4f506a70e235cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 13:12:38 +0100 Subject: [PATCH 09/32] fmt and lint --- tests/specs/task/dependencies/build1.js | 2 +- tests/specs/task/dependencies/build2.js | 2 +- tests/specs/task/dependencies/util.js | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/specs/task/dependencies/build1.js b/tests/specs/task/dependencies/build1.js index e7a69126fd1821..d14fb401a31995 100644 --- a/tests/specs/task/dependencies/build1.js +++ b/tests/specs/task/dependencies/build1.js @@ -6,4 +6,4 @@ await randomTimeout(500, 750); console.log("build1 performing more work..."); await randomTimeout(500, 750); -console.log("build1 finished"); \ No newline at end of file +console.log("build1 finished"); diff --git a/tests/specs/task/dependencies/build2.js b/tests/specs/task/dependencies/build2.js index a733ae896ba6c1..3032a099ae31d9 100644 --- a/tests/specs/task/dependencies/build2.js +++ b/tests/specs/task/dependencies/build2.js @@ -6,4 +6,4 @@ await randomTimeout(250, 750); console.log("build2 performing more work..."); await randomTimeout(250, 750); -console.log("build2 finished"); \ No newline at end of file +console.log("build2 finished"); diff --git a/tests/specs/task/dependencies/util.js b/tests/specs/task/dependencies/util.js index 8d2e7523f0f08a..9579eb9c9fc86a 100644 --- a/tests/specs/task/dependencies/util.js +++ b/tests/specs/task/dependencies/util.js @@ -1,5 +1,4 @@ - export async function randomTimeout(min, max) { - const timeout = Math.floor(Math.random() * (max - min + 1) + min); - return new Promise(resolve => setTimeout(resolve, timeout)); -} \ No newline at end of file + const timeout = Math.floor(Math.random() * (max - min + 1) + min); + return new Promise((resolve) => setTimeout(resolve, timeout)); +} From 32b04fe86a14109620c5c84cee08a67ed7beb19c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 13:21:14 +0100 Subject: [PATCH 10/32] add failing test for 'diamond' dependencies --- tests/specs/task/dependencies/__test__.jsonc | 6 +++++ tests/specs/task/dependencies/diamond.out | 10 +++++++++ tests/specs/task/dependencies/diamond/a.js | 1 + tests/specs/task/dependencies/diamond/b.js | 1 + tests/specs/task/dependencies/diamond/c.js | 1 + tests/specs/task/dependencies/diamond/d.js | 1 + .../task/dependencies/diamond/deno.jsonc | 22 +++++++++++++++++++ 7 files changed, 42 insertions(+) create mode 100644 tests/specs/task/dependencies/diamond.out create mode 100644 tests/specs/task/dependencies/diamond/a.js create mode 100644 tests/specs/task/dependencies/diamond/b.js create mode 100644 tests/specs/task/dependencies/diamond/c.js create mode 100644 tests/specs/task/dependencies/diamond/d.js create mode 100644 tests/specs/task/dependencies/diamond/deno.jsonc diff --git a/tests/specs/task/dependencies/__test__.jsonc b/tests/specs/task/dependencies/__test__.jsonc index 6df21b681ac065..4959a47be68cce 100644 --- a/tests/specs/task/dependencies/__test__.jsonc +++ b/tests/specs/task/dependencies/__test__.jsonc @@ -18,6 +18,12 @@ "args": "task run", "output": "./cross_package.out", "exitCode": 1 + }, + "diamond": { + "cwd": "diamond", + "tempDir": true, + "args": "task a", + "output": "./diamond.out" } } } diff --git a/tests/specs/task/dependencies/diamond.out b/tests/specs/task/dependencies/diamond.out new file mode 100644 index 00000000000000..375f0d4a915575 --- /dev/null +++ b/tests/specs/task/dependencies/diamond.out @@ -0,0 +1,10 @@ +Task d deno run d.js +Running d +[UNORDERED_START] +Task c deno run c.js +Task b deno run b.js +Running b +Running c +[UNORDERED_END] +Task a deno run a.js +Running a diff --git a/tests/specs/task/dependencies/diamond/a.js b/tests/specs/task/dependencies/diamond/a.js new file mode 100644 index 00000000000000..688695558e9ffc --- /dev/null +++ b/tests/specs/task/dependencies/diamond/a.js @@ -0,0 +1 @@ +console.log("Running a"); diff --git a/tests/specs/task/dependencies/diamond/b.js b/tests/specs/task/dependencies/diamond/b.js new file mode 100644 index 00000000000000..ed1addf1a70e1d --- /dev/null +++ b/tests/specs/task/dependencies/diamond/b.js @@ -0,0 +1 @@ +console.log("Running b"); diff --git a/tests/specs/task/dependencies/diamond/c.js b/tests/specs/task/dependencies/diamond/c.js new file mode 100644 index 00000000000000..194d656be66e5a --- /dev/null +++ b/tests/specs/task/dependencies/diamond/c.js @@ -0,0 +1 @@ +console.log("Running c"); diff --git a/tests/specs/task/dependencies/diamond/d.js b/tests/specs/task/dependencies/diamond/d.js new file mode 100644 index 00000000000000..a9f231f83d9e3b --- /dev/null +++ b/tests/specs/task/dependencies/diamond/d.js @@ -0,0 +1 @@ +console.log("Running d"); diff --git a/tests/specs/task/dependencies/diamond/deno.jsonc b/tests/specs/task/dependencies/diamond/deno.jsonc new file mode 100644 index 00000000000000..07d0a917752d88 --- /dev/null +++ b/tests/specs/task/dependencies/diamond/deno.jsonc @@ -0,0 +1,22 @@ +{ + // a + // / \ + // b c + // \ / + // d + "tasks": { + "a": { + "command": "deno run a.js", + "dependencies": ["b", "c"] + }, + "b": { + "command": "deno run b.js", + "dependencies": ["d"] + }, + "c": { + "command": "deno run c.js", + "dependencies": ["d"] + }, + "d": "deno run d.js" + } +} From 52762f917f58dfc2152f03716eb6f9d67d368ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 14:39:34 +0100 Subject: [PATCH 11/32] update schema file --- cli/schemas/config-file.v1.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cli/schemas/config-file.v1.json b/cli/schemas/config-file.v1.json index 56a8090f9bb997..3ba803ef8c1a16 100644 --- a/cli/schemas/config-file.v1.json +++ b/cli/schemas/config-file.v1.json @@ -448,6 +448,13 @@ "type": "string", "required": true, "description": "The task to execute" + }, + "dependencies": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Tasks that should be executed before this task" } } } From ed367a2dc9f927e06d957871b74f9e4475b6e252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sun, 17 Nov 2024 23:47:02 +0100 Subject: [PATCH 12/32] change diamond dep test for now --- tests/specs/task/dependencies/diamond.out | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/specs/task/dependencies/diamond.out b/tests/specs/task/dependencies/diamond.out index 375f0d4a915575..ceb460bd20cc1e 100644 --- a/tests/specs/task/dependencies/diamond.out +++ b/tests/specs/task/dependencies/diamond.out @@ -1,4 +1,7 @@ +[# this is not really correct - this diamond dependency should only be run once, needs to be fixed in a follow up] Task d deno run d.js +Task d deno run d.js +Running d Running d [UNORDERED_START] Task c deno run c.js From 2e73474cccabc35d4d6d1e769025d2c34f8316ad Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 18 Nov 2024 16:14:02 +0100 Subject: [PATCH 13/32] feat: run task chains in topological order --- cli/tools/task.rs | 142 ++++++++++++++---- .../prefix_import_and_alias/__test__.jsonc | 7 + .../npm/prefix_import_and_alias/deno.json | 5 + .../npm/prefix_import_and_alias/main.out | 0 tests/specs/task/dependencies/__test__.jsonc | 20 +++ tests/specs/task/dependencies/basic1.out | 8 +- tests/specs/task/dependencies/cycle.out | 1 + tests/specs/task/dependencies/cycle/a.js | 1 + .../specs/task/dependencies/cycle/deno.jsonc | 8 + tests/specs/task/dependencies/cycle_2.out | 1 + tests/specs/task/dependencies/cycle_2/a.js | 1 + tests/specs/task/dependencies/cycle_2/b.js | 1 + .../task/dependencies/cycle_2/deno.jsonc | 12 ++ tests/specs/task/dependencies/diamond.out | 7 +- tests/specs/task/dependencies/diamond_big.out | 10 ++ .../specs/task/dependencies/diamond_big/a.js | 1 + .../specs/task/dependencies/diamond_big/b.js | 1 + .../specs/task/dependencies/diamond_big/c.js | 1 + .../specs/task/dependencies/diamond_big/d.js | 1 + .../task/dependencies/diamond_big/deno.jsonc | 28 ++++ .../specs/task/dependencies/diamond_big/e.js | 1 + 21 files changed, 213 insertions(+), 44 deletions(-) create mode 100644 tests/specs/npm/prefix_import_and_alias/__test__.jsonc create mode 100644 tests/specs/npm/prefix_import_and_alias/deno.json create mode 100644 tests/specs/npm/prefix_import_and_alias/main.out create mode 100644 tests/specs/task/dependencies/cycle.out create mode 100644 tests/specs/task/dependencies/cycle/a.js create mode 100644 tests/specs/task/dependencies/cycle/deno.jsonc create mode 100644 tests/specs/task/dependencies/cycle_2.out create mode 100644 tests/specs/task/dependencies/cycle_2/a.js create mode 100644 tests/specs/task/dependencies/cycle_2/b.js create mode 100644 tests/specs/task/dependencies/cycle_2/deno.jsonc create mode 100644 tests/specs/task/dependencies/diamond_big.out create mode 100644 tests/specs/task/dependencies/diamond_big/a.js create mode 100644 tests/specs/task/dependencies/diamond_big/b.js create mode 100644 tests/specs/task/dependencies/diamond_big/c.js create mode 100644 tests/specs/task/dependencies/diamond_big/d.js create mode 100644 tests/specs/task/dependencies/diamond_big/deno.jsonc create mode 100644 tests/specs/task/dependencies/diamond_big/e.js diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 98a0270f389ee2..9e916a4a0e5c12 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -15,8 +15,6 @@ use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; -use deno_core::futures::stream::futures_unordered; -use deno_core::futures::StreamExt; use deno_core::url::Url; use deno_path_util::normalize_path; use deno_runtime::deno_node::NodeResolver; @@ -78,7 +76,33 @@ pub async fn execute_script( env_vars, cli_options, }; - task_runner.run_task(task_name).await + + match task_runner.sort_tasks_topo(task_name) { + Ok(sorted) => { + return task_runner.run_tasks(sorted).await; + } + Err(err) => match err { + TaskError::NotFound(name) => { + if task_flags.is_run { + return Err(anyhow!("Task not found: {}", name)); + } + + log::error!("Task not found: {}", name); + if log::log_enabled!(log::Level::Error) { + print_available_tasks( + &mut std::io::stderr(), + &task_runner.cli_options.start_dir, + &task_runner.tasks_config, + )?; + } + return Ok(1); + } + TaskError::TaskDepCycle(name) => { + log::error!("Task cycle detected: {}", name); + return Ok(1); + } + }, + } } struct RunSingleOptions<'a> { @@ -88,6 +112,12 @@ struct RunSingleOptions<'a> { custom_commands: HashMap>, } +#[derive(Debug)] +enum TaskError { + NotFound(String), + TaskDepCycle(String), +} + struct TaskRunner<'a> { tasks_config: WorkspaceTasksConfig, task_flags: &'a TaskFlags, @@ -98,26 +128,30 @@ struct TaskRunner<'a> { } impl<'a> TaskRunner<'a> { - async fn run_task( + async fn run_tasks( &self, - task_name: &String, + task_names: Vec, ) -> Result { - let Some((dir_url, task_or_script)) = self.tasks_config.task(task_name) - else { - if self.task_flags.is_run { - return Err(anyhow!("Task not found: {}", task_name)); + // TODO(bartlomieju): we might want to limit concurrency here - eg. `wireit` runs 2 tasks in parallel + // unless and env var is specified. + // TODO(marvinhagemeister): Run in sequence for now as it's not safe + // to run all tasks in parallel. We can only run tasks in parallel where + // all dependencies have been executed prior to that. + for task_name in &task_names { + let exit_code = self.run_task(&task_name).await?; + if exit_code > 0 { + return Ok(exit_code); } + } - log::error!("Task not found: {}", task_name); - if log::log_enabled!(log::Level::Error) { - print_available_tasks( - &mut std::io::stderr(), - &self.cli_options.start_dir, - &self.tasks_config, - )?; - } - return Ok(1); - }; + Ok(0) + } + + async fn run_task( + &self, + task_name: &String, + ) -> Result { + let (dir_url, task_or_script) = self.get_task(task_name.as_str()).unwrap(); match task_or_script { TaskOrScript::Task(_tasks, definition) => { @@ -135,20 +169,6 @@ impl<'a> TaskRunner<'a> { task_name: &String, definition: &TaskDefinition, ) -> Result { - // TODO(bartlomieju): we might want to limit concurrency here - eg. `wireit` runs 2 tasks in parallel - // unless and env var is specified. - let mut dependency_tasks = futures_unordered::FuturesUnordered::new(); - for dep in &definition.dependencies { - let dep = dep.clone(); - dependency_tasks.push(async move { self.run_task(&dep).await }) - } - while let Some(result) = dependency_tasks.next().await { - let exit_code = result?; - if exit_code > 0 { - return Ok(exit_code); - } - } - let cwd = match &self.task_flags.cwd { Some(path) => canonicalize_path(&PathBuf::from(path)) .context("failed canonicalizing --cwd")?, @@ -248,6 +268,62 @@ impl<'a> TaskRunner<'a> { .exit_code, ) } + + fn get_task( + &self, + task_name: &str, + ) -> Result<(&Url, TaskOrScript), TaskError> { + let Some(result) = self.tasks_config.task(task_name) else { + return Err(TaskError::NotFound(task_name.to_string())); + }; + + Ok(result) + } + + fn sort_tasks_topo(&self, name: &str) -> Result, TaskError> { + let mut marked: Vec = vec![]; + let mut sorted: Vec = vec![]; + + self.sort_visit(name, &mut marked, &mut sorted)?; + + Ok(sorted) + } + + fn sort_visit( + &self, + name: &str, + marked: &mut Vec, + sorted: &mut Vec, + ) -> Result<(), TaskError> { + // Already sorted + if sorted.contains(&name.to_string()) { + return Ok(()); + } + + // Graph has a cycle + if marked.contains(&name.to_string()) { + return Err(TaskError::TaskDepCycle(name.to_string())); + } + + marked.push(name.to_string()); + + let Some(def) = self.tasks_config.task(name) else { + return Err(TaskError::NotFound(name.to_string())); + }; + + match def.1 { + TaskOrScript::Task(_, actual_def) => { + for dep in &actual_def.dependencies { + self.sort_visit(dep, marked, sorted)? + } + } + TaskOrScript::Script(_, name) => self.sort_visit(name, marked, sorted)?, + } + + sorted.push(name.to_string()); + + Ok(()) + } } fn output_task(task_name: &str, script: &str) { diff --git a/tests/specs/npm/prefix_import_and_alias/__test__.jsonc b/tests/specs/npm/prefix_import_and_alias/__test__.jsonc new file mode 100644 index 00000000000000..009ca54e3a1650 --- /dev/null +++ b/tests/specs/npm/prefix_import_and_alias/__test__.jsonc @@ -0,0 +1,7 @@ +{ + "tempDir": true, + "steps": [{ + "args": "run -A main.js", + "output": "main.out" + }] +} diff --git a/tests/specs/npm/prefix_import_and_alias/deno.json b/tests/specs/npm/prefix_import_and_alias/deno.json new file mode 100644 index 00000000000000..75fce807f0ef1f --- /dev/null +++ b/tests/specs/npm/prefix_import_and_alias/deno.json @@ -0,0 +1,5 @@ +{ + "imports": { + "@denotest/basic": "npm:@denotest/add@1.0.0" + } +} diff --git a/tests/specs/npm/prefix_import_and_alias/main.out b/tests/specs/npm/prefix_import_and_alias/main.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/task/dependencies/__test__.jsonc b/tests/specs/task/dependencies/__test__.jsonc index 4959a47be68cce..81d34a667fb375 100644 --- a/tests/specs/task/dependencies/__test__.jsonc +++ b/tests/specs/task/dependencies/__test__.jsonc @@ -24,6 +24,26 @@ "tempDir": true, "args": "task a", "output": "./diamond.out" + }, + "diamond_big": { + "cwd": "diamond_big", + "tempDir": true, + "args": "task a", + "output": "./diamond_big.out" + }, + "cycle": { + "cwd": "cycle", + "tempDir": true, + "output": "./cycle.out", + "args": "task a", + "exitCode": 1 + }, + "cycle_2": { + "cwd": "cycle_2", + "tempDir": true, + "args": "task a", + "output": "./cycle_2.out", + "exitCode": 1 } } } diff --git a/tests/specs/task/dependencies/basic1.out b/tests/specs/task/dependencies/basic1.out index e9443d3a7478c8..24ccd0eb03ace0 100644 --- a/tests/specs/task/dependencies/basic1.out +++ b/tests/specs/task/dependencies/basic1.out @@ -1,12 +1,10 @@ Task build1 deno run ../build1.js +Starting build1 +build1 performing more work... +build1 finished Task build2 deno run ../build2.js -[UNORDERED_START] Starting build2 -Starting build1 build2 performing more work... -build1 performing more work... build2 finished -build1 finished -[UNORDERED_END] Task run deno run ../run.js run finished diff --git a/tests/specs/task/dependencies/cycle.out b/tests/specs/task/dependencies/cycle.out new file mode 100644 index 00000000000000..d8674cb34ff169 --- /dev/null +++ b/tests/specs/task/dependencies/cycle.out @@ -0,0 +1 @@ +Task cycle detected: a diff --git a/tests/specs/task/dependencies/cycle/a.js b/tests/specs/task/dependencies/cycle/a.js new file mode 100644 index 00000000000000..688695558e9ffc --- /dev/null +++ b/tests/specs/task/dependencies/cycle/a.js @@ -0,0 +1 @@ +console.log("Running a"); diff --git a/tests/specs/task/dependencies/cycle/deno.jsonc b/tests/specs/task/dependencies/cycle/deno.jsonc new file mode 100644 index 00000000000000..31e67488cbd7c5 --- /dev/null +++ b/tests/specs/task/dependencies/cycle/deno.jsonc @@ -0,0 +1,8 @@ +{ + "tasks": { + "a": { + "command": "deno run a.js", + "dependencies": ["a"] + } + } +} diff --git a/tests/specs/task/dependencies/cycle_2.out b/tests/specs/task/dependencies/cycle_2.out new file mode 100644 index 00000000000000..d8674cb34ff169 --- /dev/null +++ b/tests/specs/task/dependencies/cycle_2.out @@ -0,0 +1 @@ +Task cycle detected: a diff --git a/tests/specs/task/dependencies/cycle_2/a.js b/tests/specs/task/dependencies/cycle_2/a.js new file mode 100644 index 00000000000000..688695558e9ffc --- /dev/null +++ b/tests/specs/task/dependencies/cycle_2/a.js @@ -0,0 +1 @@ +console.log("Running a"); diff --git a/tests/specs/task/dependencies/cycle_2/b.js b/tests/specs/task/dependencies/cycle_2/b.js new file mode 100644 index 00000000000000..ed1addf1a70e1d --- /dev/null +++ b/tests/specs/task/dependencies/cycle_2/b.js @@ -0,0 +1 @@ +console.log("Running b"); diff --git a/tests/specs/task/dependencies/cycle_2/deno.jsonc b/tests/specs/task/dependencies/cycle_2/deno.jsonc new file mode 100644 index 00000000000000..5a5d38ec9cd9e7 --- /dev/null +++ b/tests/specs/task/dependencies/cycle_2/deno.jsonc @@ -0,0 +1,12 @@ +{ + "tasks": { + "a": { + "command": "deno run a.js", + "dependencies": ["b"] + }, + "b": { + "command": "deno run b.js", + "dependencies": ["a"] + } + } +} diff --git a/tests/specs/task/dependencies/diamond.out b/tests/specs/task/dependencies/diamond.out index ceb460bd20cc1e..e36e608b650989 100644 --- a/tests/specs/task/dependencies/diamond.out +++ b/tests/specs/task/dependencies/diamond.out @@ -1,13 +1,8 @@ -[# this is not really correct - this diamond dependency should only be run once, needs to be fixed in a follow up] Task d deno run d.js -Task d deno run d.js -Running d Running d -[UNORDERED_START] -Task c deno run c.js Task b deno run b.js Running b +Task c deno run c.js Running c -[UNORDERED_END] Task a deno run a.js Running a diff --git a/tests/specs/task/dependencies/diamond_big.out b/tests/specs/task/dependencies/diamond_big.out new file mode 100644 index 00000000000000..805630df41d916 --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big.out @@ -0,0 +1,10 @@ +Task e deno run e.js +Running e +Task b deno run b.js +Running b +Task d deno run d.js +Running d +Task c deno run c.js +Running c +Task a deno run a.js +Running a diff --git a/tests/specs/task/dependencies/diamond_big/a.js b/tests/specs/task/dependencies/diamond_big/a.js new file mode 100644 index 00000000000000..688695558e9ffc --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big/a.js @@ -0,0 +1 @@ +console.log("Running a"); diff --git a/tests/specs/task/dependencies/diamond_big/b.js b/tests/specs/task/dependencies/diamond_big/b.js new file mode 100644 index 00000000000000..ed1addf1a70e1d --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big/b.js @@ -0,0 +1 @@ +console.log("Running b"); diff --git a/tests/specs/task/dependencies/diamond_big/c.js b/tests/specs/task/dependencies/diamond_big/c.js new file mode 100644 index 00000000000000..194d656be66e5a --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big/c.js @@ -0,0 +1 @@ +console.log("Running c"); diff --git a/tests/specs/task/dependencies/diamond_big/d.js b/tests/specs/task/dependencies/diamond_big/d.js new file mode 100644 index 00000000000000..a9f231f83d9e3b --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big/d.js @@ -0,0 +1 @@ +console.log("Running d"); diff --git a/tests/specs/task/dependencies/diamond_big/deno.jsonc b/tests/specs/task/dependencies/diamond_big/deno.jsonc new file mode 100644 index 00000000000000..28ea7f69546c90 --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big/deno.jsonc @@ -0,0 +1,28 @@ +{ + // a + // / \ + // b c + // | | + // | d + // \ / + // e + "tasks": { + "a": { + "command": "deno run a.js", + "dependencies": ["b", "c"] + }, + "b": { + "command": "deno run b.js", + "dependencies": ["e"] + }, + "c": { + "command": "deno run c.js", + "dependencies": ["d"] + }, + "d": { + "command": "deno run d.js", + "dependencies": ["e"] + }, + "e": "deno run e.js" + } +} diff --git a/tests/specs/task/dependencies/diamond_big/e.js b/tests/specs/task/dependencies/diamond_big/e.js new file mode 100644 index 00000000000000..b36066c3d7303f --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big/e.js @@ -0,0 +1 @@ +console.log("Running e"); From efd9c8084cc2da4003f7e0f521f386b3d25f4910 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 18 Nov 2024 16:18:36 +0100 Subject: [PATCH 14/32] fix: make clippy happy --- cli/tools/task.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 9e916a4a0e5c12..ee623920a1560f 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -78,9 +78,7 @@ pub async fn execute_script( }; match task_runner.sort_tasks_topo(task_name) { - Ok(sorted) => { - return task_runner.run_tasks(sorted).await; - } + Ok(sorted) => task_runner.run_tasks(sorted).await, Err(err) => match err { TaskError::NotFound(name) => { if task_flags.is_run { @@ -95,11 +93,11 @@ pub async fn execute_script( &task_runner.tasks_config, )?; } - return Ok(1); + Ok(1) } TaskError::TaskDepCycle(name) => { log::error!("Task cycle detected: {}", name); - return Ok(1); + Ok(1) } }, } @@ -138,7 +136,7 @@ impl<'a> TaskRunner<'a> { // to run all tasks in parallel. We can only run tasks in parallel where // all dependencies have been executed prior to that. for task_name in &task_names { - let exit_code = self.run_task(&task_name).await?; + let exit_code = self.run_task(task_name).await?; if exit_code > 0 { return Ok(exit_code); } From b54306bd49ed77b31798c12156c288792f153817 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 18 Nov 2024 21:32:40 +0100 Subject: [PATCH 15/32] fix: don't parse script value as task --- cli/tools/task.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index ee623920a1560f..08b6aff43c2199 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -315,7 +315,7 @@ impl<'a> TaskRunner<'a> { self.sort_visit(dep, marked, sorted)? } } - TaskOrScript::Script(_, name) => self.sort_visit(name, marked, sorted)?, + TaskOrScript::Script(..) => {} } sorted.push(name.to_string()); From b5978df3e1f6d4a300ff73e2b3c638e67470c5e4 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 18 Nov 2024 22:46:23 +0100 Subject: [PATCH 16/32] fix: remove accidentally committed fixture --- tests/specs/npm/prefix_import_and_alias/__test__.jsonc | 7 ------- tests/specs/npm/prefix_import_and_alias/deno.json | 5 ----- tests/specs/npm/prefix_import_and_alias/main.out | 0 3 files changed, 12 deletions(-) delete mode 100644 tests/specs/npm/prefix_import_and_alias/__test__.jsonc delete mode 100644 tests/specs/npm/prefix_import_and_alias/deno.json delete mode 100644 tests/specs/npm/prefix_import_and_alias/main.out diff --git a/tests/specs/npm/prefix_import_and_alias/__test__.jsonc b/tests/specs/npm/prefix_import_and_alias/__test__.jsonc deleted file mode 100644 index 009ca54e3a1650..00000000000000 --- a/tests/specs/npm/prefix_import_and_alias/__test__.jsonc +++ /dev/null @@ -1,7 +0,0 @@ -{ - "tempDir": true, - "steps": [{ - "args": "run -A main.js", - "output": "main.out" - }] -} diff --git a/tests/specs/npm/prefix_import_and_alias/deno.json b/tests/specs/npm/prefix_import_and_alias/deno.json deleted file mode 100644 index 75fce807f0ef1f..00000000000000 --- a/tests/specs/npm/prefix_import_and_alias/deno.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "imports": { - "@denotest/basic": "npm:@denotest/add@1.0.0" - } -} diff --git a/tests/specs/npm/prefix_import_and_alias/main.out b/tests/specs/npm/prefix_import_and_alias/main.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 From 89c5542a6e6cb5912702e3b8f50f9d50e6f440c6 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 19 Nov 2024 00:34:45 +0100 Subject: [PATCH 17/32] feat: parallelize tasks if possible --- cli/tools/task.rs | 74 +++++++++++++++++-- tests/specs/task/dependencies/basic1.out | 4 +- tests/specs/task/dependencies/diamond.out | 2 + tests/specs/task/dependencies/diamond_big.out | 2 + 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 08b6aff43c2199..a9b55ecd068860 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -15,6 +15,8 @@ use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::futures::stream::futures_unordered; +use deno_core::futures::StreamExt; use deno_core::url::Url; use deno_path_util::normalize_path; use deno_runtime::deno_node::NodeResolver; @@ -132,14 +134,76 @@ impl<'a> TaskRunner<'a> { ) -> Result { // TODO(bartlomieju): we might want to limit concurrency here - eg. `wireit` runs 2 tasks in parallel // unless and env var is specified. - // TODO(marvinhagemeister): Run in sequence for now as it's not safe - // to run all tasks in parallel. We can only run tasks in parallel where - // all dependencies have been executed prior to that. - for task_name in &task_names { - let exit_code = self.run_task(task_name).await?; + // TODO(marvinhagemeister): The fastest way to run this would be a + // work stealing queue that checks after each task completion which + // next task can be run. My rusts skills don't suffice to do that though. + // So for now this sorta does a breadth first approach. + + let mut completed: Vec = vec![]; + let mut running: Vec = vec![]; + + while completed.len() < task_names.len() { + let exit_code = self + .run_tasks_inner(&task_names, &mut completed, &mut running) + .await?; + + if exit_code > 0 { + return Ok(exit_code); + } + } + + Ok(0) + } + + async fn run_tasks_inner( + &self, + task_names: &Vec, + completed: &mut Vec, + running: &mut Vec, + ) -> Result { + let mut queue = futures_unordered::FuturesUnordered::new(); + + // Find all tasks that: + // 1. haven't been run already + // 2. that aren't being run at the moment + // 3. have no dependencies or all dependencies are completed + for name in task_names { + if !completed.contains(&name) && !running.contains(&name) { + let should_run = if let Ok((_, def)) = self.get_task(&name) { + match def { + TaskOrScript::Task(_, def) => { + def.dependencies.iter().all(|dep| completed.contains(&dep)) + } + TaskOrScript::Script(_, _) => true, + } + } else { + false + }; + + if should_run { + running.push(name.clone()); + let name = name.clone(); + queue.push(async move { + self + .run_task(&name) + .await + .map(|exit_code| (exit_code, name.clone())) + }); + } + } + } + + while let Some(result) = queue.next().await { + let (exit_code, name) = result?; if exit_code > 0 { return Ok(exit_code); } + + if let Some(idx) = running.iter().position(|item| item == &name) { + running.remove(idx); + } + + completed.push(name.clone()); } Ok(0) diff --git a/tests/specs/task/dependencies/basic1.out b/tests/specs/task/dependencies/basic1.out index 24ccd0eb03ace0..8c31d02b4efcf0 100644 --- a/tests/specs/task/dependencies/basic1.out +++ b/tests/specs/task/dependencies/basic1.out @@ -1,10 +1,12 @@ Task build1 deno run ../build1.js +Task build2 deno run ../build2.js +[UNORDERED_START] Starting build1 build1 performing more work... build1 finished -Task build2 deno run ../build2.js Starting build2 build2 performing more work... build2 finished +[UNORDERED_END] Task run deno run ../run.js run finished diff --git a/tests/specs/task/dependencies/diamond.out b/tests/specs/task/dependencies/diamond.out index e36e608b650989..75b06a35b2051c 100644 --- a/tests/specs/task/dependencies/diamond.out +++ b/tests/specs/task/dependencies/diamond.out @@ -1,8 +1,10 @@ Task d deno run d.js Running d +[UNORDERED_START] Task b deno run b.js Running b Task c deno run c.js Running c +[UNORDERED_END] Task a deno run a.js Running a diff --git a/tests/specs/task/dependencies/diamond_big.out b/tests/specs/task/dependencies/diamond_big.out index 805630df41d916..cc0ad337cf12ed 100644 --- a/tests/specs/task/dependencies/diamond_big.out +++ b/tests/specs/task/dependencies/diamond_big.out @@ -1,9 +1,11 @@ Task e deno run e.js Running e +[UNORDERED_START] Task b deno run b.js Running b Task d deno run d.js Running d +[UNORDERED_END] Task c deno run c.js Running c Task a deno run a.js From efbd227801ded1a3911845fd1d1998eb4bbbe65b Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 19 Nov 2024 00:37:20 +0100 Subject: [PATCH 18/32] fix: make clippy happy --- cli/tools/task.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index a9b55ecd068860..541b3c9cf5f4d1 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -168,11 +168,11 @@ impl<'a> TaskRunner<'a> { // 2. that aren't being run at the moment // 3. have no dependencies or all dependencies are completed for name in task_names { - if !completed.contains(&name) && !running.contains(&name) { - let should_run = if let Ok((_, def)) = self.get_task(&name) { + if !completed.contains(name) && !running.contains(name) { + let should_run = if let Ok((_, def)) = self.get_task(name) { match def { TaskOrScript::Task(_, def) => { - def.dependencies.iter().all(|dep| completed.contains(&dep)) + def.dependencies.iter().all(|dep| completed.contains(dep)) } TaskOrScript::Script(_, _) => true, } From d5c4dc6271e028f26322f28a497e8895e5bdd7c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 01:30:26 +0100 Subject: [PATCH 19/32] this doesn't work --- cli/tools/task.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 541b3c9cf5f4d1..925a8fd86a4b5a 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::collections::HashSet; +use std::num::NonZeroUsize; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; @@ -17,6 +18,7 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::futures::stream::futures_unordered; use deno_core::futures::StreamExt; +use deno_core::parking_lot::Mutex; use deno_core::url::Url; use deno_path_util::normalize_path; use deno_runtime::deno_node::NodeResolver; @@ -80,7 +82,7 @@ pub async fn execute_script( }; match task_runner.sort_tasks_topo(task_name) { - Ok(sorted) => task_runner.run_tasks(sorted).await, + Ok(sorted) => task_runner.run_tasks_in_parallel(sorted).await, Err(err) => match err { TaskError::NotFound(name) => { if task_flags.is_run { @@ -128,6 +130,84 @@ struct TaskRunner<'a> { } impl<'a> TaskRunner<'a> { + async fn run_tasks_in_parallel( + &self, + task_names: Vec, + ) -> Result { + let no_of_concurrent_tasks = + if let Ok(value) = std::env::var("DENO_JOBS") { + value.parse::().ok() + } else { + std::thread::available_parallelism().ok() + } + .unwrap_or_else(|| NonZeroUsize::new(2).unwrap()); + + let completed = + Arc::new(Mutex::new(HashSet::with_capacity(task_names.len()))); + let running = Arc::new(Mutex::new(HashSet::with_capacity( + no_of_concurrent_tasks.into(), + ))); + + let mut task_futures = Vec::with_capacity(task_names.len()); + + for name in task_names { + let completed = completed.clone(); + let running = running.clone(); + task_futures.push(async move { + if completed.lock().contains(&name) { + return None; + } + + if running.lock().contains(&name) { + return None; + } + + let should_run = if let Ok((_, def)) = self.get_task(&name) { + match def { + TaskOrScript::Task(_, def) => def + .dependencies + .iter() + .all(|dep| completed.lock().contains(dep)), + TaskOrScript::Script(_, _) => true, + } + } else { + false + }; + + if !should_run { + return None; + } + + running.lock().insert(name.clone()); + let name = name.clone(); + Some( + self + .run_task(&name) + .await + .map(|exit_code| (exit_code, name.clone())), + ) + }); + } + + let stream_of_futures = deno_core::futures::stream::iter(task_futures); + let mut buffered = stream_of_futures.buffered(10); + + while let Some(maybe_task_result) = buffered.next().await { + let Some(task_result) = maybe_task_result else { + continue; + }; + let (exit_code, name) = task_result?; + if exit_code > 0 { + return Ok(exit_code); + } + + running.lock().remove(&name); + completed.lock().insert(name.to_string()); + } + + Ok(0) + } + async fn run_tasks( &self, task_names: Vec, From 27d86a45e806d8234f07e0ee643c7e0ba5632dfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 02:44:28 +0100 Subject: [PATCH 20/32] wip --- cli/tools/task.rs | 333 +++++++++++++++++++++------------------------- 1 file changed, 155 insertions(+), 178 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 925a8fd86a4b5a..a0e0731b14ae71 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -2,9 +2,11 @@ use std::collections::HashMap; use std::collections::HashSet; +use std::future::Future; use std::num::NonZeroUsize; use std::path::Path; use std::path::PathBuf; +use std::pin::Pin; use std::rc::Rc; use std::sync::Arc; @@ -16,7 +18,9 @@ use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::futures::future::LocalBoxFuture; use deno_core::futures::stream::futures_unordered; +use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; use deno_core::parking_lot::Mutex; use deno_core::url::Url; @@ -72,17 +76,40 @@ pub async fn execute_script( let node_resolver = factory.node_resolver().await?; let env_vars = task_runner::real_env_vars(); - let task_runner = TaskRunner { - tasks_config, - task_flags: &task_flags, - npm_resolver: npm_resolver.as_ref(), - node_resolver: node_resolver.as_ref(), - env_vars, - cli_options, + let no_of_concurrent_tasks = if let Ok(value) = std::env::var("DENO_JOBS") { + value.parse::().ok() + } else { + std::thread::available_parallelism().ok() + } + .unwrap_or_else(|| NonZeroUsize::new(2).unwrap()); + let completed = Arc::new(Mutex::new(HashSet::with_capacity(8))); + let running = Arc::new(Mutex::new(HashSet::with_capacity( + no_of_concurrent_tasks.into(), + ))); + + let mut task_runner = TaskRunner { + config: TaskRunnerConfig { + tasks_config, + task_flags: &task_flags, + npm_resolver: npm_resolver.as_ref(), + node_resolver: node_resolver.as_ref(), + env_vars, + cli_options, + }, + executor: TaskRunnerExecutor { + concurrency: no_of_concurrent_tasks.into(), + completed, + running, + task_names: vec![], + queue: Default::default(), + }, }; match task_runner.sort_tasks_topo(task_name) { - Ok(sorted) => task_runner.run_tasks_in_parallel(sorted).await, + Ok(sorted) => { + task_runner.executor.task_names = sorted.clone(); + run_tasks_in_parallel(task_runner).await + } Err(err) => match err { TaskError::NotFound(name) => { if task_flags.is_run { @@ -93,13 +120,14 @@ pub async fn execute_script( if log::log_enabled!(log::Level::Error) { print_available_tasks( &mut std::io::stderr(), - &task_runner.cli_options.start_dir, - &task_runner.tasks_config, + &task_runner.config.cli_options.start_dir, + &task_runner.config.tasks_config, )?; } Ok(1) } TaskError::TaskDepCycle(name) => { + // TODO(bartlomieju): this should be improved to show the cycle log::error!("Task cycle detected: {}", name); Ok(1) } @@ -120,7 +148,7 @@ enum TaskError { TaskDepCycle(String), } -struct TaskRunner<'a> { +struct TaskRunnerConfig<'a> { tasks_config: WorkspaceTasksConfig, task_flags: &'a TaskFlags, npm_resolver: &'a dyn CliNpmResolver, @@ -129,171 +157,128 @@ struct TaskRunner<'a> { cli_options: &'a CliOptions, } -impl<'a> TaskRunner<'a> { - async fn run_tasks_in_parallel( +impl<'a> TaskRunnerConfig<'a> { + fn get_task( &self, - task_names: Vec, - ) -> Result { - let no_of_concurrent_tasks = - if let Ok(value) = std::env::var("DENO_JOBS") { - value.parse::().ok() - } else { - std::thread::available_parallelism().ok() - } - .unwrap_or_else(|| NonZeroUsize::new(2).unwrap()); - - let completed = - Arc::new(Mutex::new(HashSet::with_capacity(task_names.len()))); - let running = Arc::new(Mutex::new(HashSet::with_capacity( - no_of_concurrent_tasks.into(), - ))); - - let mut task_futures = Vec::with_capacity(task_names.len()); - - for name in task_names { - let completed = completed.clone(); - let running = running.clone(); - task_futures.push(async move { - if completed.lock().contains(&name) { - return None; - } + task_name: &str, + ) -> Result<(&Url, TaskOrScript), TaskError> { + let Some(result) = self.tasks_config.task(task_name) else { + return Err(TaskError::NotFound(task_name.to_string())); + }; - if running.lock().contains(&name) { - return None; - } + Ok(result) + } +} - let should_run = if let Ok((_, def)) = self.get_task(&name) { - match def { - TaskOrScript::Task(_, def) => def - .dependencies - .iter() - .all(|dep| completed.lock().contains(dep)), - TaskOrScript::Script(_, _) => true, - } - } else { - false - }; +struct TaskRunnerExecutor { + concurrency: usize, + completed: Arc>>, + running: Arc>>, + task_names: Vec, + queue: Arc< + Mutex< + futures_unordered::FuturesUnordered< + Pin< + Box< + dyn Future< + Output = Result<(i32, String), deno_core::anyhow::Error>, + >, + >, + >, + >, + >, + >, +} - if !should_run { - return None; - } +struct TaskRunner<'config> { + config: TaskRunnerConfig<'config>, + executor: TaskRunnerExecutor, +} - running.lock().insert(name.clone()); - let name = name.clone(); - Some( - self - .run_task(&name) - .await - .map(|exit_code| (exit_code, name.clone())), - ) - }); - } +async fn run_tasks_in_parallel<'config>( + task_runner: TaskRunner<'config>, +) -> Result { + task_runner.add_tasks(); - let stream_of_futures = deno_core::futures::stream::iter(task_futures); - let mut buffered = stream_of_futures.buffered(10); + loop { + if !task_runner.has_remaining_tasks() { + break; + } - while let Some(maybe_task_result) = buffered.next().await { - let Some(task_result) = maybe_task_result else { - continue; - }; - let (exit_code, name) = task_result?; - if exit_code > 0 { - return Ok(exit_code); - } + let Some(result) = task_runner.executor.queue.lock().next().await else { + // TODO(bartlomieju): not sure if this condition is correct + break; + }; - running.lock().remove(&name); - completed.lock().insert(name.to_string()); + let (exit_code, name) = result?; + if exit_code > 0 { + return Ok(exit_code); } - Ok(0) + task_runner.executor.running.lock().remove(&name); + task_runner.executor.completed.lock().insert(name); + task_runner.add_tasks(); } - async fn run_tasks( - &self, - task_names: Vec, - ) -> Result { - // TODO(bartlomieju): we might want to limit concurrency here - eg. `wireit` runs 2 tasks in parallel - // unless and env var is specified. - // TODO(marvinhagemeister): The fastest way to run this would be a - // work stealing queue that checks after each task completion which - // next task can be run. My rusts skills don't suffice to do that though. - // So for now this sorta does a breadth first approach. - - let mut completed: Vec = vec![]; - let mut running: Vec = vec![]; - - while completed.len() < task_names.len() { - let exit_code = self - .run_tasks_inner(&task_names, &mut completed, &mut running) - .await?; - - if exit_code > 0 { - return Ok(exit_code); - } - } + Ok(0) +} - Ok(0) +impl<'config> TaskRunner<'config> { + fn has_remaining_tasks(&self) -> bool { + self.executor.completed.lock().len() < self.executor.task_names.len() } - async fn run_tasks_inner( - &self, - task_names: &Vec, - completed: &mut Vec, - running: &mut Vec, - ) -> Result { - let mut queue = futures_unordered::FuturesUnordered::new(); - - // Find all tasks that: - // 1. haven't been run already - // 2. that aren't being run at the moment - // 3. have no dependencies or all dependencies are completed - for name in task_names { - if !completed.contains(name) && !running.contains(name) { - let should_run = if let Ok((_, def)) = self.get_task(name) { - match def { - TaskOrScript::Task(_, def) => { - def.dependencies.iter().all(|dep| completed.contains(dep)) - } - TaskOrScript::Script(_, _) => true, - } - } else { - false - }; - - if should_run { - running.push(name.clone()); - let name = name.clone(); - queue.push(async move { - self - .run_task(&name) - .await - .map(|exit_code| (exit_code, name.clone())) - }); - } + fn add_tasks(&self) { + for name in &self.executor.task_names { + if self.executor.completed.lock().contains(name) { + return; } - } - while let Some(result) = queue.next().await { - let (exit_code, name) = result?; - if exit_code > 0 { - return Ok(exit_code); + if self.executor.running.lock().contains(name) { + return; } - if let Some(idx) = running.iter().position(|item| item == &name) { - running.remove(idx); + let should_run = if let Ok((_, def)) = self.config.get_task(name) { + match def { + TaskOrScript::Task(_, def) => def + .dependencies + .iter() + .all(|dep| self.executor.completed.lock().contains(dep)), + TaskOrScript::Script(_, _) => true, + } + } else { + false + }; + + if !should_run { + return; } - completed.push(name.clone()); - } + self.executor.running.lock().insert(name.clone()); + let name = name.clone(); + let queue = self.executor.queue.lock(); + queue.push( + async move { + self + .run_task(&name) + .await + .map(|exit_code| (exit_code, name.clone())) + } + .boxed_local(), + ); - Ok(0) + if queue.len() >= self.executor.concurrency { + break; + } + } } async fn run_task( &self, task_name: &String, ) -> Result { - let (dir_url, task_or_script) = self.get_task(task_name.as_str()).unwrap(); + let (dir_url, task_or_script) = + self.config.get_task(task_name.as_str()).unwrap(); match task_or_script { TaskOrScript::Task(_tasks, definition) => { @@ -311,15 +296,15 @@ impl<'a> TaskRunner<'a> { task_name: &String, definition: &TaskDefinition, ) -> Result { - let cwd = match &self.task_flags.cwd { + let cwd = match &self.config.task_flags.cwd { Some(path) => canonicalize_path(&PathBuf::from(path)) .context("failed canonicalizing --cwd")?, None => normalize_path(dir_url.to_file_path().unwrap()), }; let custom_commands = task_runner::resolve_custom_commands( - self.npm_resolver, - self.node_resolver, + self.config.npm_resolver, + self.config.node_resolver, )?; self .run_single(RunSingleOptions { @@ -338,11 +323,11 @@ impl<'a> TaskRunner<'a> { scripts: &IndexMap, ) -> Result { // ensure the npm packages are installed if using a managed resolver - if let Some(npm_resolver) = self.npm_resolver.as_managed() { + if let Some(npm_resolver) = self.config.npm_resolver.as_managed() { npm_resolver.ensure_top_level_package_json_install().await?; } - let cwd = match &self.task_flags.cwd { + let cwd = match &self.config.task_flags.cwd { Some(path) => canonicalize_path(&PathBuf::from(path))?, None => normalize_path(dir_url.to_file_path().unwrap()), }; @@ -356,8 +341,8 @@ impl<'a> TaskRunner<'a> { format!("post{}", task_name), ]; let custom_commands = task_runner::resolve_custom_commands( - self.npm_resolver, - self.node_resolver, + self.config.npm_resolver, + self.config.node_resolver, )?; for task_name in &task_names { if let Some(script) = scripts.get(task_name) { @@ -391,7 +376,10 @@ impl<'a> TaskRunner<'a> { output_task( opts.task_name, - &task_runner::get_script_with_args(script, self.cli_options.argv()), + &task_runner::get_script_with_args( + script, + self.config.cli_options.argv(), + ), ); Ok( @@ -399,11 +387,14 @@ impl<'a> TaskRunner<'a> { task_name, script, cwd, - env_vars: self.env_vars.clone(), + env_vars: self.config.env_vars.clone(), custom_commands, - init_cwd: self.cli_options.initial_cwd(), - argv: self.cli_options.argv(), - root_node_modules_dir: self.npm_resolver.root_node_modules_path(), + init_cwd: self.config.cli_options.initial_cwd(), + argv: self.config.cli_options.argv(), + root_node_modules_dir: self + .config + .npm_resolver + .root_node_modules_path(), stdio: None, }) .await? @@ -411,17 +402,6 @@ impl<'a> TaskRunner<'a> { ) } - fn get_task( - &self, - task_name: &str, - ) -> Result<(&Url, TaskOrScript), TaskError> { - let Some(result) = self.tasks_config.task(task_name) else { - return Err(TaskError::NotFound(task_name.to_string())); - }; - - Ok(result) - } - fn sort_tasks_topo(&self, name: &str) -> Result, TaskError> { let mut marked: Vec = vec![]; let mut sorted: Vec = vec![]; @@ -449,17 +429,14 @@ impl<'a> TaskRunner<'a> { marked.push(name.to_string()); - let Some(def) = self.tasks_config.task(name) else { + let Some(def) = self.config.tasks_config.task(name) else { return Err(TaskError::NotFound(name.to_string())); }; - match def.1 { - TaskOrScript::Task(_, actual_def) => { - for dep in &actual_def.dependencies { - self.sort_visit(dep, marked, sorted)? - } + if let TaskOrScript::Task(_, actual_def) = def.1 { + for dep in &actual_def.dependencies { + self.sort_visit(dep, marked, sorted)? } - TaskOrScript::Script(..) => {} } sorted.push(name.to_string()); From b47c1304ac589bccf41d115e9ee40c3c5fcc3a5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 03:17:01 +0100 Subject: [PATCH 21/32] a --- cli/tools/task.rs | 199 ++++++++++++++++++++-------------------------- 1 file changed, 85 insertions(+), 114 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index a0e0731b14ae71..44bd5877a10d59 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -2,11 +2,9 @@ use std::collections::HashMap; use std::collections::HashSet; -use std::future::Future; use std::num::NonZeroUsize; use std::path::Path; use std::path::PathBuf; -use std::pin::Pin; use std::rc::Rc; use std::sync::Arc; @@ -88,27 +86,22 @@ pub async fn execute_script( ))); let mut task_runner = TaskRunner { - config: TaskRunnerConfig { - tasks_config, - task_flags: &task_flags, - npm_resolver: npm_resolver.as_ref(), - node_resolver: node_resolver.as_ref(), - env_vars, - cli_options, - }, - executor: TaskRunnerExecutor { - concurrency: no_of_concurrent_tasks.into(), - completed, - running, - task_names: vec![], - queue: Default::default(), - }, + tasks_config, + task_flags: &task_flags, + npm_resolver: npm_resolver.as_ref(), + node_resolver: node_resolver.as_ref(), + env_vars, + cli_options, + concurrency: no_of_concurrent_tasks.into(), + completed, + running, + task_names: vec![], }; match task_runner.sort_tasks_topo(task_name) { Ok(sorted) => { - task_runner.executor.task_names = sorted.clone(); - run_tasks_in_parallel(task_runner).await + task_runner.task_names = sorted.clone(); + task_runner.run_tasks_in_parallel().await } Err(err) => match err { TaskError::NotFound(name) => { @@ -120,8 +113,8 @@ pub async fn execute_script( if log::log_enabled!(log::Level::Error) { print_available_tasks( &mut std::io::stderr(), - &task_runner.config.cli_options.start_dir, - &task_runner.config.tasks_config, + &task_runner.cli_options.start_dir, + &task_runner.tasks_config, )?; } Ok(1) @@ -148,116 +141,104 @@ enum TaskError { TaskDepCycle(String), } -struct TaskRunnerConfig<'a> { +struct TaskRunner<'a> { tasks_config: WorkspaceTasksConfig, task_flags: &'a TaskFlags, npm_resolver: &'a dyn CliNpmResolver, node_resolver: &'a NodeResolver, env_vars: HashMap, cli_options: &'a CliOptions, -} - -impl<'a> TaskRunnerConfig<'a> { - fn get_task( - &self, - task_name: &str, - ) -> Result<(&Url, TaskOrScript), TaskError> { - let Some(result) = self.tasks_config.task(task_name) else { - return Err(TaskError::NotFound(task_name.to_string())); - }; - - Ok(result) - } -} - -struct TaskRunnerExecutor { concurrency: usize, completed: Arc>>, running: Arc>>, task_names: Vec, - queue: Arc< - Mutex< - futures_unordered::FuturesUnordered< - Pin< - Box< - dyn Future< - Output = Result<(i32, String), deno_core::anyhow::Error>, - >, - >, - >, - >, - >, - >, } -struct TaskRunner<'config> { - config: TaskRunnerConfig<'config>, - executor: TaskRunnerExecutor, -} +impl<'a> TaskRunner<'a> { + async fn run_tasks_in_parallel( + &self, + ) -> Result { + let mut queue = futures_unordered::FuturesUnordered::new(); -async fn run_tasks_in_parallel<'config>( - task_runner: TaskRunner<'config>, -) -> Result { - task_runner.add_tasks(); + loop { + // eprintln!("has remaining tasks {}", self.has_remaining_tasks()); + if !self.has_remaining_tasks() { + break; + } - loop { - if !task_runner.has_remaining_tasks() { - break; - } + while queue.len() < self.concurrency { + // eprintln!("queue len {} {} ", queue.len(), self.concurrency); + if let Some(task) = self.add_tasks() { + queue.push(task); + } else { + break; + } + } - let Some(result) = task_runner.executor.queue.lock().next().await else { - // TODO(bartlomieju): not sure if this condition is correct - break; - }; + let Some(result) = queue.next().await else { + // TODO(bartlomieju): not sure if this condition is correct + break; + }; + + let (exit_code, name) = result?; + if exit_code > 0 { + return Ok(exit_code); + } - let (exit_code, name) = result?; - if exit_code > 0 { - return Ok(exit_code); + self.running.lock().remove(&name); + self.completed.lock().insert(name); } - task_runner.executor.running.lock().remove(&name); - task_runner.executor.completed.lock().insert(name); - task_runner.add_tasks(); + Ok(0) } - Ok(0) -} + fn get_task( + &self, + task_name: &str, + ) -> Result<(&Url, TaskOrScript), TaskError> { + let Some(result) = self.tasks_config.task(task_name) else { + return Err(TaskError::NotFound(task_name.to_string())); + }; + + Ok(result) + } -impl<'config> TaskRunner<'config> { fn has_remaining_tasks(&self) -> bool { - self.executor.completed.lock().len() < self.executor.task_names.len() + self.completed.lock().len() < self.task_names.len() } - fn add_tasks(&self) { - for name in &self.executor.task_names { - if self.executor.completed.lock().contains(name) { - return; + fn add_tasks( + &'a self, + ) -> Option>> { + for name in &self.task_names { + if self.completed.lock().contains(name) { + continue; } - if self.executor.running.lock().contains(name) { - return; + if self.running.lock().contains(name) { + continue; } - let should_run = if let Ok((_, def)) = self.config.get_task(name) { + let should_run = if let Ok((_, def)) = self.get_task(name) { match def { TaskOrScript::Task(_, def) => def .dependencies .iter() - .all(|dep| self.executor.completed.lock().contains(dep)), + .all(|dep| self.completed.lock().contains(dep)), TaskOrScript::Script(_, _) => true, } } else { false }; + // TODO(bartlomieju): is this correct? Shouldn't it be `return None;`? if !should_run { - return; + continue; } - self.executor.running.lock().insert(name.clone()); + self.running.lock().insert(name.clone()); let name = name.clone(); - let queue = self.executor.queue.lock(); - queue.push( + return Some( async move { self .run_task(&name) @@ -266,19 +247,15 @@ impl<'config> TaskRunner<'config> { } .boxed_local(), ); - - if queue.len() >= self.executor.concurrency { - break; - } } + None } async fn run_task( &self, task_name: &String, ) -> Result { - let (dir_url, task_or_script) = - self.config.get_task(task_name.as_str()).unwrap(); + let (dir_url, task_or_script) = self.get_task(task_name.as_str()).unwrap(); match task_or_script { TaskOrScript::Task(_tasks, definition) => { @@ -296,15 +273,15 @@ impl<'config> TaskRunner<'config> { task_name: &String, definition: &TaskDefinition, ) -> Result { - let cwd = match &self.config.task_flags.cwd { + let cwd = match &self.task_flags.cwd { Some(path) => canonicalize_path(&PathBuf::from(path)) .context("failed canonicalizing --cwd")?, None => normalize_path(dir_url.to_file_path().unwrap()), }; let custom_commands = task_runner::resolve_custom_commands( - self.config.npm_resolver, - self.config.node_resolver, + self.npm_resolver, + self.node_resolver, )?; self .run_single(RunSingleOptions { @@ -323,11 +300,11 @@ impl<'config> TaskRunner<'config> { scripts: &IndexMap, ) -> Result { // ensure the npm packages are installed if using a managed resolver - if let Some(npm_resolver) = self.config.npm_resolver.as_managed() { + if let Some(npm_resolver) = self.npm_resolver.as_managed() { npm_resolver.ensure_top_level_package_json_install().await?; } - let cwd = match &self.config.task_flags.cwd { + let cwd = match &self.task_flags.cwd { Some(path) => canonicalize_path(&PathBuf::from(path))?, None => normalize_path(dir_url.to_file_path().unwrap()), }; @@ -341,8 +318,8 @@ impl<'config> TaskRunner<'config> { format!("post{}", task_name), ]; let custom_commands = task_runner::resolve_custom_commands( - self.config.npm_resolver, - self.config.node_resolver, + self.npm_resolver, + self.node_resolver, )?; for task_name in &task_names { if let Some(script) = scripts.get(task_name) { @@ -376,10 +353,7 @@ impl<'config> TaskRunner<'config> { output_task( opts.task_name, - &task_runner::get_script_with_args( - script, - self.config.cli_options.argv(), - ), + &task_runner::get_script_with_args(script, self.cli_options.argv()), ); Ok( @@ -387,14 +361,11 @@ impl<'config> TaskRunner<'config> { task_name, script, cwd, - env_vars: self.config.env_vars.clone(), + env_vars: self.env_vars.clone(), custom_commands, - init_cwd: self.config.cli_options.initial_cwd(), - argv: self.config.cli_options.argv(), - root_node_modules_dir: self - .config - .npm_resolver - .root_node_modules_path(), + init_cwd: self.cli_options.initial_cwd(), + argv: self.cli_options.argv(), + root_node_modules_dir: self.npm_resolver.root_node_modules_path(), stdio: None, }) .await? @@ -429,7 +400,7 @@ impl<'config> TaskRunner<'config> { marked.push(name.to_string()); - let Some(def) = self.config.tasks_config.task(name) else { + let Some(def) = self.tasks_config.task(name) else { return Err(TaskError::NotFound(name.to_string())); }; From 23178220910e4c70e92eb229c19842954f2b01c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 03:35:57 +0100 Subject: [PATCH 22/32] show which tasks we depend on --- cli/tools/task.rs | 8 ++++++++ tests/specs/task/dependencies/__test__.jsonc | 12 ++++++++++++ tests/specs/task/dependencies/cross_package.out | 1 + .../specs/task/dependencies/diamond_big_list.out | 15 +++++++++++++++ tests/specs/task/dependencies/diamond_list.out | 12 ++++++++++++ 5 files changed, 48 insertions(+) create mode 100644 tests/specs/task/dependencies/diamond_big_list.out create mode 100644 tests/specs/task/dependencies/diamond_list.out diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 44bd5877a10d59..9203e2c7f40c98 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -521,6 +521,14 @@ fn print_available_tasks( )?; } writeln!(writer, " {}", desc.task.command)?; + if !desc.task.dependencies.is_empty() { + writeln!( + writer, + " {} {}", + colors::gray("depends on:"), + colors::cyan(desc.task.dependencies.join(", ")) + )?; + } } Ok(()) diff --git a/tests/specs/task/dependencies/__test__.jsonc b/tests/specs/task/dependencies/__test__.jsonc index 81d34a667fb375..38d085d796cc2f 100644 --- a/tests/specs/task/dependencies/__test__.jsonc +++ b/tests/specs/task/dependencies/__test__.jsonc @@ -25,12 +25,24 @@ "args": "task a", "output": "./diamond.out" }, + "diamond_list": { + "cwd": "diamond", + "tempDir": true, + "args": "task", + "output": "./diamond_list.out" + }, "diamond_big": { "cwd": "diamond_big", "tempDir": true, "args": "task a", "output": "./diamond_big.out" }, + "diamond_big_list": { + "cwd": "diamond_big", + "tempDir": true, + "args": "task", + "output": "./diamond_big_list.out" + }, "cycle": { "cwd": "cycle", "tempDir": true, diff --git a/tests/specs/task/dependencies/cross_package.out b/tests/specs/task/dependencies/cross_package.out index 1f96ed9b0f36ac..642608f9fc03f6 100644 --- a/tests/specs/task/dependencies/cross_package.out +++ b/tests/specs/task/dependencies/cross_package.out @@ -2,3 +2,4 @@ Task not found: ../package2:run Available tasks: - run deno run.js + depends on: ../packaga2:run \ No newline at end of file diff --git a/tests/specs/task/dependencies/diamond_big_list.out b/tests/specs/task/dependencies/diamond_big_list.out new file mode 100644 index 00000000000000..c95bcd272d9147 --- /dev/null +++ b/tests/specs/task/dependencies/diamond_big_list.out @@ -0,0 +1,15 @@ +Available tasks: +- a + deno run a.js + depends on: b, c +- b + deno run b.js + depends on: e +- c + deno run c.js + depends on: d +- d + deno run d.js + depends on: e +- e + deno run e.js diff --git a/tests/specs/task/dependencies/diamond_list.out b/tests/specs/task/dependencies/diamond_list.out new file mode 100644 index 00000000000000..dfd725a4053429 --- /dev/null +++ b/tests/specs/task/dependencies/diamond_list.out @@ -0,0 +1,12 @@ +Available tasks: +- a + deno run a.js + depends on: b, c +- b + deno run b.js + depends on: d +- c + deno run c.js + depends on: d +- d + deno run d.js From f537a2b641be15fd13f6ab802d8e377e77a92c95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 03:37:09 +0100 Subject: [PATCH 23/32] remove a clone --- cli/tools/task.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 9203e2c7f40c98..c7693e4252f280 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -100,7 +100,7 @@ pub async fn execute_script( match task_runner.sort_tasks_topo(task_name) { Ok(sorted) => { - task_runner.task_names = sorted.clone(); + task_runner.task_names = sorted; task_runner.run_tasks_in_parallel().await } Err(err) => match err { From 8e4e7d18eef1fd90f3a96010a1a894b0294c547e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 03:38:41 +0100 Subject: [PATCH 24/32] cleanup --- cli/tools/task.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index c7693e4252f280..152afb5b0126e7 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -99,10 +99,7 @@ pub async fn execute_script( }; match task_runner.sort_tasks_topo(task_name) { - Ok(sorted) => { - task_runner.task_names = sorted; - task_runner.run_tasks_in_parallel().await - } + Ok(sorted) => task_runner.run_tasks_in_parallel(sorted).await, Err(err) => match err { TaskError::NotFound(name) => { if task_flags.is_run { @@ -156,18 +153,18 @@ struct TaskRunner<'a> { impl<'a> TaskRunner<'a> { async fn run_tasks_in_parallel( - &self, + &mut self, + task_names: Vec, ) -> Result { + self.task_names = task_names; let mut queue = futures_unordered::FuturesUnordered::new(); loop { - // eprintln!("has remaining tasks {}", self.has_remaining_tasks()); if !self.has_remaining_tasks() { break; } while queue.len() < self.concurrency { - // eprintln!("queue len {} {} ", queue.len(), self.concurrency); if let Some(task) = self.add_tasks() { queue.push(task); } else { From 903da223b46503f116ce4b178aca1541c2cceabc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 03:39:41 +0100 Subject: [PATCH 25/32] remove TODO --- cli/tools/task.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 152afb5b0126e7..588f765dc67693 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -172,8 +172,8 @@ impl<'a> TaskRunner<'a> { } } + // If queue is empty at this point, then there are no more tasks in the queue. let Some(result) = queue.next().await else { - // TODO(bartlomieju): not sure if this condition is correct break; }; From 52c2305a5d656b623131dfbcf5c6a06ddcce50a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 03:41:07 +0100 Subject: [PATCH 26/32] simplify a bit --- cli/tools/task.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 588f765dc67693..dfb1c2e4de4b66 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -165,7 +165,7 @@ impl<'a> TaskRunner<'a> { } while queue.len() < self.concurrency { - if let Some(task) = self.add_tasks() { + if let Some(task) = self.get_next_task() { queue.push(task); } else { break; @@ -204,15 +204,13 @@ impl<'a> TaskRunner<'a> { self.completed.lock().len() < self.task_names.len() } - fn add_tasks( + fn get_next_task( &'a self, ) -> Option>> { for name in &self.task_names { - if self.completed.lock().contains(name) { - continue; - } - - if self.running.lock().contains(name) { + if self.completed.lock().contains(name) + || self.running.lock().contains(name) + { continue; } From 9c60d6f13222e4e493f1d180b3dc5b8c6852da20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 03:49:20 +0100 Subject: [PATCH 27/32] fix a test --- cli/tools/task.rs | 6 +----- tests/specs/task/dependencies/cross_package.out | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index dfb1c2e4de4b66..2499459611f6d2 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -159,11 +159,7 @@ impl<'a> TaskRunner<'a> { self.task_names = task_names; let mut queue = futures_unordered::FuturesUnordered::new(); - loop { - if !self.has_remaining_tasks() { - break; - } - + while self.has_remaining_tasks() { while queue.len() < self.concurrency { if let Some(task) = self.get_next_task() { queue.push(task); diff --git a/tests/specs/task/dependencies/cross_package.out b/tests/specs/task/dependencies/cross_package.out index 642608f9fc03f6..140cf81993bc68 100644 --- a/tests/specs/task/dependencies/cross_package.out +++ b/tests/specs/task/dependencies/cross_package.out @@ -2,4 +2,4 @@ Task not found: ../package2:run Available tasks: - run deno run.js - depends on: ../packaga2:run \ No newline at end of file + depends on: ../package2:run \ No newline at end of file From 245e81356eadb2dcfcdc26b3b17baeb94e3f05a5 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 18 Nov 2024 21:56:18 -0500 Subject: [PATCH 28/32] Use refcell --- cli/tools/task.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 2499459611f6d2..b500d88b8d295c 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::cell::RefCell; use std::collections::HashMap; use std::collections::HashSet; use std::num::NonZeroUsize; @@ -20,7 +21,6 @@ use deno_core::futures::future::LocalBoxFuture; use deno_core::futures::stream::futures_unordered; use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; -use deno_core::parking_lot::Mutex; use deno_core::url::Url; use deno_path_util::normalize_path; use deno_runtime::deno_node::NodeResolver; @@ -80,10 +80,9 @@ pub async fn execute_script( std::thread::available_parallelism().ok() } .unwrap_or_else(|| NonZeroUsize::new(2).unwrap()); - let completed = Arc::new(Mutex::new(HashSet::with_capacity(8))); - let running = Arc::new(Mutex::new(HashSet::with_capacity( - no_of_concurrent_tasks.into(), - ))); + let completed = RefCell::new(HashSet::with_capacity(8)); + let running = + RefCell::new(HashSet::with_capacity(no_of_concurrent_tasks.into())); let mut task_runner = TaskRunner { tasks_config, @@ -146,8 +145,8 @@ struct TaskRunner<'a> { env_vars: HashMap, cli_options: &'a CliOptions, concurrency: usize, - completed: Arc>>, - running: Arc>>, + completed: RefCell>, + running: RefCell>, task_names: Vec, } @@ -178,8 +177,8 @@ impl<'a> TaskRunner<'a> { return Ok(exit_code); } - self.running.lock().remove(&name); - self.completed.lock().insert(name); + self.running.borrow_mut().remove(&name); + self.completed.borrow_mut().insert(name); } Ok(0) @@ -197,15 +196,15 @@ impl<'a> TaskRunner<'a> { } fn has_remaining_tasks(&self) -> bool { - self.completed.lock().len() < self.task_names.len() + self.completed.borrow().len() < self.task_names.len() } fn get_next_task( &'a self, ) -> Option>> { for name in &self.task_names { - if self.completed.lock().contains(name) - || self.running.lock().contains(name) + if self.completed.borrow().contains(name) + || self.running.borrow().contains(name) { continue; } @@ -215,7 +214,7 @@ impl<'a> TaskRunner<'a> { TaskOrScript::Task(_, def) => def .dependencies .iter() - .all(|dep| self.completed.lock().contains(dep)), + .all(|dep| self.completed.borrow().contains(dep)), TaskOrScript::Script(_, _) => true, } } else { @@ -227,7 +226,7 @@ impl<'a> TaskRunner<'a> { continue; } - self.running.lock().insert(name.clone()); + self.running.borrow_mut().insert(name.clone()); let name = name.clone(); return Some( async move { From b35e17a62f0a9881a69e5094968c7e0ca890aefe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 04:13:33 +0100 Subject: [PATCH 29/32] add missing newline --- tests/specs/task/dependencies/cross_package.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/specs/task/dependencies/cross_package.out b/tests/specs/task/dependencies/cross_package.out index 140cf81993bc68..a57f4de9ff6fe5 100644 --- a/tests/specs/task/dependencies/cross_package.out +++ b/tests/specs/task/dependencies/cross_package.out @@ -2,4 +2,4 @@ Task not found: ../package2:run Available tasks: - run deno run.js - depends on: ../package2:run \ No newline at end of file + depends on: ../package2:run From c7f8c4bd18f58f35ca1ddb117ed0fce73cb86016 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 18 Nov 2024 22:59:06 -0500 Subject: [PATCH 30/32] Show cycle detection, move state into method call, extract function for sorting tasks --- cli/tools/task.rs | 249 ++++++++++-------- tests/specs/task/dependencies/cycle.out | 2 +- tests/specs/task/dependencies/cycle_2.out | 2 +- tests/specs/task/dependencies/diamond_big.out | 3 +- .../specs/task/dependencies/diamond_big/b.js | 3 + 5 files changed, 145 insertions(+), 114 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index b500d88b8d295c..111b19501d448c 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -1,6 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use std::cell::RefCell; use std::collections::HashMap; use std::collections::HashSet; use std::num::NonZeroUsize; @@ -80,11 +79,8 @@ pub async fn execute_script( std::thread::available_parallelism().ok() } .unwrap_or_else(|| NonZeroUsize::new(2).unwrap()); - let completed = RefCell::new(HashSet::with_capacity(8)); - let running = - RefCell::new(HashSet::with_capacity(no_of_concurrent_tasks.into())); - let mut task_runner = TaskRunner { + let task_runner = TaskRunner { tasks_config, task_flags: &task_flags, npm_resolver: npm_resolver.as_ref(), @@ -92,36 +88,9 @@ pub async fn execute_script( env_vars, cli_options, concurrency: no_of_concurrent_tasks.into(), - completed, - running, - task_names: vec![], }; - match task_runner.sort_tasks_topo(task_name) { - Ok(sorted) => task_runner.run_tasks_in_parallel(sorted).await, - Err(err) => match err { - TaskError::NotFound(name) => { - if task_flags.is_run { - return Err(anyhow!("Task not found: {}", name)); - } - - log::error!("Task not found: {}", name); - if log::log_enabled!(log::Level::Error) { - print_available_tasks( - &mut std::io::stderr(), - &task_runner.cli_options.start_dir, - &task_runner.tasks_config, - )?; - } - Ok(1) - } - TaskError::TaskDepCycle(name) => { - // TODO(bartlomieju): this should be improved to show the cycle - log::error!("Task cycle detected: {}", name); - Ok(1) - } - }, - } + task_runner.run_task(task_name).await } struct RunSingleOptions<'a> { @@ -131,12 +100,6 @@ struct RunSingleOptions<'a> { custom_commands: HashMap>, } -#[derive(Debug)] -enum TaskError { - NotFound(String), - TaskDepCycle(String), -} - struct TaskRunner<'a> { tasks_config: WorkspaceTasksConfig, task_flags: &'a TaskFlags, @@ -145,22 +108,116 @@ struct TaskRunner<'a> { env_vars: HashMap, cli_options: &'a CliOptions, concurrency: usize, - completed: RefCell>, - running: RefCell>, - task_names: Vec, } impl<'a> TaskRunner<'a> { + pub async fn run_task( + &self, + task_name: &str, + ) -> Result { + match sort_tasks_topo(task_name, &self.tasks_config) { + Ok(sorted) => self.run_tasks_in_parallel(sorted).await, + Err(err) => match err { + TaskError::NotFound(name) => { + if self.task_flags.is_run { + return Err(anyhow!("Task not found: {}", name)); + } + + log::error!("Task not found: {}", name); + if log::log_enabled!(log::Level::Error) { + self.print_available_tasks()?; + } + Ok(1) + } + TaskError::TaskDepCycle { path } => { + log::error!("Task cycle detected: {}", path.join(" -> ")); + Ok(1) + } + }, + } + } + + pub fn print_available_tasks(&self) -> Result<(), std::io::Error> { + print_available_tasks( + &mut std::io::stderr(), + &self.cli_options.start_dir, + &self.tasks_config, + ) + } + async fn run_tasks_in_parallel( - &mut self, + &self, task_names: Vec, ) -> Result { - self.task_names = task_names; + struct PendingTasksContext { + completed: HashSet, + running: HashSet, + task_names: Vec, + } + + impl PendingTasksContext { + fn has_remaining_tasks(&self) -> bool { + self.completed.len() < self.task_names.len() + } + + fn mark_complete(&mut self, task_name: String) { + self.running.remove(&task_name); + self.completed.insert(task_name); + } + + fn get_next_task<'a>( + &mut self, + runner: &'a TaskRunner<'a>, + ) -> Option>> { + for name in &self.task_names { + if self.completed.contains(name) || self.running.contains(name) { + continue; + } + + let should_run = if let Ok((_, def)) = runner.get_task(name) { + match def { + TaskOrScript::Task(_, def) => def + .dependencies + .iter() + .all(|dep| self.completed.contains(dep)), + TaskOrScript::Script(_, _) => true, + } + } else { + false + }; + + // TODO(bartlomieju): is this correct? Shouldn't it be `return None;`? + if !should_run { + continue; + } + + self.running.insert(name.clone()); + let name = name.clone(); + return Some( + async move { + runner + .run_task_no_dependencies(&name) + .await + .map(|exit_code| (exit_code, name)) + } + .boxed_local(), + ); + } + None + } + } + + let mut context = PendingTasksContext { + completed: HashSet::with_capacity(task_names.len()), + running: HashSet::with_capacity(self.concurrency), + task_names, + }; + let mut queue = futures_unordered::FuturesUnordered::new(); - while self.has_remaining_tasks() { + while context.has_remaining_tasks() { while queue.len() < self.concurrency { - if let Some(task) = self.get_next_task() { + if let Some(task) = context.get_next_task(self) { queue.push(task); } else { break; @@ -169,6 +226,7 @@ impl<'a> TaskRunner<'a> { // If queue is empty at this point, then there are no more tasks in the queue. let Some(result) = queue.next().await else { + debug_assert_eq!(context.task_names.len(), 0); break; }; @@ -177,8 +235,7 @@ impl<'a> TaskRunner<'a> { return Ok(exit_code); } - self.running.borrow_mut().remove(&name); - self.completed.borrow_mut().insert(name); + context.mark_complete(name); } Ok(0) @@ -195,53 +252,7 @@ impl<'a> TaskRunner<'a> { Ok(result) } - fn has_remaining_tasks(&self) -> bool { - self.completed.borrow().len() < self.task_names.len() - } - - fn get_next_task( - &'a self, - ) -> Option>> { - for name in &self.task_names { - if self.completed.borrow().contains(name) - || self.running.borrow().contains(name) - { - continue; - } - - let should_run = if let Ok((_, def)) = self.get_task(name) { - match def { - TaskOrScript::Task(_, def) => def - .dependencies - .iter() - .all(|dep| self.completed.borrow().contains(dep)), - TaskOrScript::Script(_, _) => true, - } - } else { - false - }; - - // TODO(bartlomieju): is this correct? Shouldn't it be `return None;`? - if !should_run { - continue; - } - - self.running.borrow_mut().insert(name.clone()); - let name = name.clone(); - return Some( - async move { - self - .run_task(&name) - .await - .map(|exit_code| (exit_code, name.clone())) - } - .boxed_local(), - ); - } - None - } - - async fn run_task( + async fn run_task_no_dependencies( &self, task_name: &String, ) -> Result { @@ -362,41 +373,50 @@ impl<'a> TaskRunner<'a> { .exit_code, ) } +} - fn sort_tasks_topo(&self, name: &str) -> Result, TaskError> { - let mut marked: Vec = vec![]; - let mut sorted: Vec = vec![]; - - self.sort_visit(name, &mut marked, &mut sorted)?; - - Ok(sorted) - } +#[derive(Debug)] +enum TaskError { + NotFound(String), + TaskDepCycle { path: Vec }, +} - fn sort_visit( - &self, - name: &str, +fn sort_tasks_topo( + name: &str, + task_config: &WorkspaceTasksConfig, +) -> Result, TaskError> { + fn sort_visit<'a>( + name: &'a str, marked: &mut Vec, sorted: &mut Vec, + tasks_config: &'a WorkspaceTasksConfig, + // todo(dsherret): one directional graph would be more efficient than vec + mut path: Vec<&'a str>, ) -> Result<(), TaskError> { // Already sorted - if sorted.contains(&name.to_string()) { + if sorted.iter().any(|sorted_name| sorted_name == name) { return Ok(()); } // Graph has a cycle - if marked.contains(&name.to_string()) { - return Err(TaskError::TaskDepCycle(name.to_string())); + if marked.iter().any(|marked_name| marked_name == name) { + path.push(name); + return Err(TaskError::TaskDepCycle { + path: path.iter().map(|s| s.to_string()).collect(), + }); } marked.push(name.to_string()); - let Some(def) = self.tasks_config.task(name) else { + let Some(def) = tasks_config.task(name) else { return Err(TaskError::NotFound(name.to_string())); }; if let TaskOrScript::Task(_, actual_def) = def.1 { for dep in &actual_def.dependencies { - self.sort_visit(dep, marked, sorted)? + let mut path = path.clone(); + path.push(name); + sort_visit(dep, marked, sorted, tasks_config, path)? } } @@ -404,6 +424,13 @@ impl<'a> TaskRunner<'a> { Ok(()) } + + let mut marked: Vec = vec![]; + let mut sorted: Vec = vec![]; + + sort_visit(name, &mut marked, &mut sorted, task_config, Vec::new())?; + + Ok(sorted) } fn output_task(task_name: &str, script: &str) { diff --git a/tests/specs/task/dependencies/cycle.out b/tests/specs/task/dependencies/cycle.out index d8674cb34ff169..33352b0bbc5f2c 100644 --- a/tests/specs/task/dependencies/cycle.out +++ b/tests/specs/task/dependencies/cycle.out @@ -1 +1 @@ -Task cycle detected: a +Task cycle detected: a -> a diff --git a/tests/specs/task/dependencies/cycle_2.out b/tests/specs/task/dependencies/cycle_2.out index d8674cb34ff169..89ef04a00ba922 100644 --- a/tests/specs/task/dependencies/cycle_2.out +++ b/tests/specs/task/dependencies/cycle_2.out @@ -1 +1 @@ -Task cycle detected: a +Task cycle detected: a -> b -> a diff --git a/tests/specs/task/dependencies/diamond_big.out b/tests/specs/task/dependencies/diamond_big.out index cc0ad337cf12ed..f0b827b0da7291 100644 --- a/tests/specs/task/dependencies/diamond_big.out +++ b/tests/specs/task/dependencies/diamond_big.out @@ -5,8 +5,9 @@ Task b deno run b.js Running b Task d deno run d.js Running d -[UNORDERED_END] Task c deno run c.js Running c +Finished b +[UNORDERED_END] Task a deno run a.js Running a diff --git a/tests/specs/task/dependencies/diamond_big/b.js b/tests/specs/task/dependencies/diamond_big/b.js index ed1addf1a70e1d..4b00ef56923cf9 100644 --- a/tests/specs/task/dependencies/diamond_big/b.js +++ b/tests/specs/task/dependencies/diamond_big/b.js @@ -1 +1,4 @@ console.log("Running b"); +setTimeout(() => { + console.log("Finished b"); +}, 10); From 45017a5b4683c5899827502c9cfb2e2a06d8475a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 18 Nov 2024 23:08:18 -0500 Subject: [PATCH 31/32] Remove marked now that we keep track of the path --- cli/tools/task.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 111b19501d448c..8a214b69771070 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -387,11 +387,9 @@ fn sort_tasks_topo( ) -> Result, TaskError> { fn sort_visit<'a>( name: &'a str, - marked: &mut Vec, sorted: &mut Vec, - tasks_config: &'a WorkspaceTasksConfig, - // todo(dsherret): one directional graph would be more efficient than vec mut path: Vec<&'a str>, + tasks_config: &'a WorkspaceTasksConfig, ) -> Result<(), TaskError> { // Already sorted if sorted.iter().any(|sorted_name| sorted_name == name) { @@ -399,15 +397,13 @@ fn sort_tasks_topo( } // Graph has a cycle - if marked.iter().any(|marked_name| marked_name == name) { + if path.contains(&name) { path.push(name); return Err(TaskError::TaskDepCycle { path: path.iter().map(|s| s.to_string()).collect(), }); } - marked.push(name.to_string()); - let Some(def) = tasks_config.task(name) else { return Err(TaskError::NotFound(name.to_string())); }; @@ -416,7 +412,7 @@ fn sort_tasks_topo( for dep in &actual_def.dependencies { let mut path = path.clone(); path.push(name); - sort_visit(dep, marked, sorted, tasks_config, path)? + sort_visit(dep, sorted, path, tasks_config)? } } @@ -425,10 +421,9 @@ fn sort_tasks_topo( Ok(()) } - let mut marked: Vec = vec![]; let mut sorted: Vec = vec![]; - sort_visit(name, &mut marked, &mut sorted, task_config, Vec::new())?; + sort_visit(name, &mut sorted, Vec::new(), task_config)?; Ok(sorted) } From f00edf1d549022dca84ccc5e7ef449f6f08eedac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 19 Nov 2024 13:08:36 +0100 Subject: [PATCH 32/32] remove todo --- cli/tools/task.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 8a214b69771070..682dbf814d384c 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -186,7 +186,6 @@ impl<'a> TaskRunner<'a> { false }; - // TODO(bartlomieju): is this correct? Shouldn't it be `return None;`? if !should_run { continue; }