From 2a15d319c5edae674fcaa156a7de5d0ba4caeef5 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Thu, 12 Sep 2024 11:48:02 -0400 Subject: [PATCH 01/10] chore(turbo_json): split out single package mode --- crates/turborepo-lib/src/engine/builder.rs | 12 +--- .../src/package_changes_watcher.rs | 9 +-- crates/turborepo-lib/src/run/builder.rs | 15 +++-- crates/turborepo-lib/src/turbo_json/mod.rs | 55 ++++++++++--------- 4 files changed, 40 insertions(+), 51 deletions(-) diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index 00aaf64a6b47a..22fe8055795fe 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -483,11 +483,6 @@ impl<'a> EngineBuilder<'a> { } fn load_turbo_json(&self, workspace: &PackageName) -> Result { - let package_json = self.package_graph.package_json(workspace).ok_or_else(|| { - Error::MissingPackageJson { - workspace: workspace.clone(), - } - })?; let workspace_dir = self.package_graph .package_dir(workspace) @@ -498,12 +493,7 @@ impl<'a> EngineBuilder<'a> { .repo_root .resolve(workspace_dir) .join_component(CONFIG_FILE); - Ok(TurboJson::load( - self.repo_root, - &workspace_turbo_json, - package_json, - self.is_single, - )?) + Ok(TurboJson::load(self.repo_root, &workspace_turbo_json)?) } } diff --git a/crates/turborepo-lib/src/package_changes_watcher.rs b/crates/turborepo-lib/src/package_changes_watcher.rs index deb74acc5d199..0488d68d5c25a 100644 --- a/crates/turborepo-lib/src/package_changes_watcher.rs +++ b/crates/turborepo-lib/src/package_changes_watcher.rs @@ -162,13 +162,8 @@ impl Subscriber { return None; }; - let root_turbo_json = TurboJson::load( - &self.repo_root, - &self.repo_root.join_component(CONFIG_FILE), - &root_package_json, - false, - ) - .ok(); + let root_turbo_json = + TurboJson::load(&self.repo_root, &self.repo_root.join_component(CONFIG_FILE)).ok(); let gitignore_path = self.repo_root.join_component(".gitignore"); let (root_gitignore, _) = Gitignore::new(&gitignore_path); diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index 04641f93df66c..7fcd542a05a9d 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -363,12 +363,15 @@ impl RunBuilder { .load_turbo_json(&self.root_turbo_json_path) .map_or_else( || { - TurboJson::load( - &self.repo_root, - &self.root_turbo_json_path, - &root_package_json, - is_single_package, - ) + if is_single_package { + TurboJson::from_root_package_json( + &self.repo_root, + &self.root_turbo_json_path, + &root_package_json, + ) + } else { + TurboJson::load(&self.repo_root, &self.root_turbo_json_path) + } }, Result::Ok, )?; diff --git a/crates/turborepo-lib/src/turbo_json/mod.rs b/crates/turborepo-lib/src/turbo_json/mod.rs index e84b0cf6073c1..6551539d39809 100644 --- a/crates/turborepo-lib/src/turbo_json/mod.rs +++ b/crates/turborepo-lib/src/turbo_json/mod.rs @@ -548,35 +548,37 @@ impl TryFrom for TurboJson { } impl TurboJson { - /// Loads turbo.json by reading the file at `dir` and optionally combining - /// with synthesized information from the provided package.json + /// Loads turbo.json by reading the file at `turbo_json_path` pub fn load( repo_root: &AbsoluteSystemPath, turbo_json_path: &AbsoluteSystemPath, - root_package_json: &PackageJson, - include_synthesized_from_root_package_json: bool, ) -> Result { - let turbo_from_files = Self::read(repo_root, turbo_json_path); - - let mut turbo_json = match (include_synthesized_from_root_package_json, turbo_from_files) { + match Self::read(repo_root, turbo_json_path) { // If the file didn't exist, throw a custom error here instead of propagating - (false, Err(Error::Io(_))) => return Err(Error::NoTurboJSON), + Err(Error::Io(_)) => Err(Error::NoTurboJSON), // There was an error, and we don't have any chance of recovering // because we aren't synthesizing anything - (false, Err(e)) => return Err(e), + Err(e) => Err(e), // We're not synthesizing anything and there was no error, we're done - (false, Ok(turbo)) => return Ok(turbo), - // turbo.json doesn't exist, but we're going try to synthesize something - (true, Err(Error::Io(_))) => TurboJson::default(), - // some other happened, we can't recover - (true, Err(e)) => return Err(e), + Ok(turbo) => Ok(turbo), + } + } + + /// Loads turbo.json by reading the file at `turbo_json_path` and combining + /// with synthesized task definitions from the provided package.json + pub fn from_root_package_json( + repo_root: &AbsoluteSystemPath, + turbo_json_path: &AbsoluteSystemPath, + root_package_json: &PackageJson, + ) -> Result { + let mut turbo_json = match Self::read(repo_root, turbo_json_path) { // we're synthesizing, but we have a starting point // Note: this will have to change to support task inference in a monorepo // for now, we're going to error on any "root" tasks and turn non-root tasks into root // tasks - (true, Ok(mut turbo_from_files)) => { + Ok(mut turbo_json) => { let mut pipeline = Pipeline::default(); - for (task_name, task_definition) in turbo_from_files.tasks { + for (task_name, task_definition) in turbo_json.tasks { if task_name.is_package_task() { let (span, text) = task_definition.span_and_text("turbo.json"); @@ -590,9 +592,15 @@ impl TurboJson { pipeline.insert(task_name.into_root_task(), task_definition); } - turbo_from_files.tasks = pipeline; + turbo_json.tasks = pipeline; - turbo_from_files + turbo_json + } + // turbo.json doesn't exist, but we're going try to synthesize something + Err(Error::Io(_)) => TurboJson::default(), + // some other happened, we can't recover + Err(e) => { + return Err(e); } }; @@ -781,16 +789,10 @@ mod tests { expected_turbo_json: TurboJson, ) -> Result<()> { let root_dir = tempdir()?; - let root_package_json = PackageJson::default(); let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; fs::write(repo_root.join_component("turbo.json"), turbo_json_content)?; - let mut turbo_json = TurboJson::load( - repo_root, - &repo_root.join_component(CONFIG_FILE), - &root_package_json, - false, - )?; + let mut turbo_json = TurboJson::load(repo_root, &repo_root.join_component(CONFIG_FILE))?; turbo_json.text = None; turbo_json.path = None; @@ -859,11 +861,10 @@ mod tests { fs::write(repo_root.join_component("turbo.json"), content)?; } - let mut turbo_json = TurboJson::load( + let mut turbo_json = TurboJson::from_root_package_json( repo_root, &repo_root.join_component(CONFIG_FILE), &root_package_json, - true, )?; turbo_json.text = None; turbo_json.path = None; From a9ba58f37c80b65a3747343771ff2655d0158259 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Thu, 12 Sep 2024 14:27:46 -0400 Subject: [PATCH 02/10] chore(turbo_json): move loading to loader struct --- crates/turborepo-lib/src/engine/builder.rs | 68 +++-- .../src/package_changes_watcher.rs | 7 +- crates/turborepo-lib/src/run/builder.rs | 35 ++- crates/turborepo-lib/src/turbo_json/loader.rs | 241 ++++++++++++++++++ crates/turborepo-lib/src/turbo_json/mod.rs | 199 +-------------- 5 files changed, 321 insertions(+), 229 deletions(-) create mode 100644 crates/turborepo-lib/src/turbo_json/loader.rs diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index 22fe8055795fe..a369efda584bc 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -15,7 +15,7 @@ use crate::{ task_graph::TaskDefinition, turbo_json::{ validate_extends, validate_no_package_task_syntax, RawTaskDefinition, TurboJson, - CONFIG_FILE, + TurboJsonLoader, CONFIG_FILE, }, }; @@ -95,6 +95,7 @@ pub enum Error { pub struct EngineBuilder<'a> { repo_root: &'a AbsoluteSystemPath, package_graph: &'a PackageGraph, + turbo_json_loader: TurboJsonLoader, is_single: bool, turbo_jsons: Option>, workspaces: Vec, @@ -107,11 +108,13 @@ impl<'a> EngineBuilder<'a> { pub fn new( repo_root: &'a AbsoluteSystemPath, package_graph: &'a PackageGraph, + turbo_json_loader: TurboJsonLoader, is_single: bool, ) -> Self { Self { repo_root, package_graph, + turbo_json_loader, is_single, turbo_jsons: None, workspaces: Vec::new(), @@ -274,13 +277,12 @@ impl<'a> EngineBuilder<'a> { task_id: task_id.to_string(), }); } - let raw_task_definition = RawTaskDefinition::from_iter(self.task_definition_chain( + + let task_definition = self.task_definition( &mut turbo_jsons, &task_id, &task_id.as_non_workspace_task_name(), - )?); - - let task_definition = TaskDefinition::try_from(raw_task_definition)?; + )?; // Skip this iteration of the loop if we've already seen this taskID if visited.contains(task_id.as_inner()) { @@ -403,6 +405,21 @@ impl<'a> EngineBuilder<'a> { } } + fn task_definition( + &self, + turbo_jsons: &mut HashMap, + task_id: &Spanned, + task_name: &TaskName, + ) -> Result { + let raw_task_definition = RawTaskDefinition::from_iter(self.task_definition_chain( + turbo_jsons, + task_id, + task_name, + )?); + + Ok(TaskDefinition::try_from(raw_task_definition)?) + } + fn task_definition_chain( &self, turbo_jsons: &mut HashMap, @@ -493,7 +510,7 @@ impl<'a> EngineBuilder<'a> { .repo_root .resolve(workspace_dir) .join_component(CONFIG_FILE); - Ok(TurboJson::load(self.repo_root, &workspace_turbo_json)?) + Ok(self.turbo_json_loader.load(&workspace_turbo_json)?) } } @@ -653,7 +670,8 @@ mod test { "c" => ["a", "b"] }, ); - let engine_builder = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine_builder = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(vec![].into_iter().collect())); let a_turbo_json = repo_root.join_components(&["packages", "a", "turbo.json"]); @@ -725,7 +743,8 @@ mod test { ] .into_iter() .collect(); - let engine_builder = EngineBuilder::new(&repo_root, &package_graph, false); + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine_builder = EngineBuilder::new(&repo_root, &package_graph, loader, false); let task_name = TaskName::from(task_name); let task_id = TaskId::try_from(task_id).unwrap(); @@ -795,7 +814,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("test")))) .with_workspaces(vec![ @@ -852,7 +872,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("test")))) .with_workspaces(vec![PackageName::from("app2")]) @@ -891,7 +912,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("special")))) .with_workspaces(vec![PackageName::from("app1"), PackageName::from("libA")]) @@ -929,7 +951,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(vec![ Spanned::new(TaskName::from("build")), @@ -982,7 +1005,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) @@ -1025,7 +1049,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) @@ -1060,7 +1085,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) @@ -1105,7 +1131,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) @@ -1144,7 +1171,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) .with_tasks(Some(Spanned::new(TaskName::from("test")))) @@ -1191,7 +1219,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) .with_tasks(Some(Spanned::new(TaskName::from("build")))) @@ -1230,7 +1259,8 @@ mod test { )] .into_iter() .collect(); - let engine = EngineBuilder::new(&repo_root, &package_graph, false) + let loader = TurboJsonLoader::workspace(repo_root.clone()); + let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) .with_tasks(Some(Spanned::new(TaskName::from("build")))) diff --git a/crates/turborepo-lib/src/package_changes_watcher.rs b/crates/turborepo-lib/src/package_changes_watcher.rs index 0488d68d5c25a..93ba26db593d4 100644 --- a/crates/turborepo-lib/src/package_changes_watcher.rs +++ b/crates/turborepo-lib/src/package_changes_watcher.rs @@ -21,7 +21,7 @@ use turborepo_repository::{ }; use turborepo_scm::package_deps::GitHashes; -use crate::turbo_json::{TurboJson, CONFIG_FILE}; +use crate::turbo_json::{TurboJson, TurboJsonLoader, CONFIG_FILE}; #[derive(Clone)] pub enum PackageChangeEvent { @@ -162,8 +162,9 @@ impl Subscriber { return None; }; - let root_turbo_json = - TurboJson::load(&self.repo_root, &self.repo_root.join_component(CONFIG_FILE)).ok(); + let root_turbo_json = TurboJsonLoader::workspace(self.repo_root.clone()) + .load(&self.repo_root.join_component(CONFIG_FILE)) + .ok(); let gitignore_path = self.repo_root.join_component(".gitignore"); let (root_gitignore, _) = Gitignore::new(&gitignore_path); diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index 7fcd542a05a9d..9461f277c1e63 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -45,7 +45,7 @@ use crate::{ run::{scope, task_access::TaskAccess, task_id::TaskName, Error, Run, RunCache}, shim::TurboState, signal::{SignalHandler, SignalSubscriber}, - turbo_json::{TurboJson, UIMode}, + turbo_json::{TurboJson, TurboJsonLoader, UIMode}, DaemonConnector, }; @@ -359,20 +359,15 @@ impl RunBuilder { let task_access = TaskAccess::new(self.repo_root.clone(), async_cache.clone(), &scm); task_access.restore_config().await; + let turbo_json_loader = if is_single_package { + TurboJsonLoader::single_package(self.repo_root.clone(), root_package_json.clone()) + } else { + TurboJsonLoader::workspace(self.repo_root.clone()) + }; let root_turbo_json = task_access .load_turbo_json(&self.root_turbo_json_path) .map_or_else( - || { - if is_single_package { - TurboJson::from_root_package_json( - &self.repo_root, - &self.root_turbo_json_path, - &root_package_json, - ) - } else { - TurboJson::load(&self.repo_root, &self.root_turbo_json_path) - } - }, + || turbo_json_loader.load(&self.root_turbo_json_path), Result::Ok, )?; @@ -387,11 +382,21 @@ impl RunBuilder { )?; let env_at_execution_start = EnvironmentVariableMap::infer(); - let mut engine = self.build_engine(&pkg_dep_graph, &root_turbo_json, &filtered_pkgs)?; + let mut engine = self.build_engine( + &pkg_dep_graph, + &root_turbo_json, + &filtered_pkgs, + turbo_json_loader.clone(), + )?; if self.opts.run_opts.parallel { pkg_dep_graph.remove_package_dependencies(); - engine = self.build_engine(&pkg_dep_graph, &root_turbo_json, &filtered_pkgs)?; + engine = self.build_engine( + &pkg_dep_graph, + &root_turbo_json, + &filtered_pkgs, + turbo_json_loader, + )?; } let color_selector = ColorSelector::default(); @@ -439,10 +444,12 @@ impl RunBuilder { pkg_dep_graph: &PackageGraph, root_turbo_json: &TurboJson, filtered_pkgs: &HashSet, + turbo_json_loader: TurboJsonLoader, ) -> Result { let mut engine = EngineBuilder::new( &self.repo_root, pkg_dep_graph, + turbo_json_loader, self.opts.run_opts.single_package, ) .with_root_tasks(root_turbo_json.tasks.keys().cloned()) diff --git a/crates/turborepo-lib/src/turbo_json/loader.rs b/crates/turborepo-lib/src/turbo_json/loader.rs new file mode 100644 index 0000000000000..058e78e41dde3 --- /dev/null +++ b/crates/turborepo-lib/src/turbo_json/loader.rs @@ -0,0 +1,241 @@ +use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf}; +use turborepo_errors::Spanned; +use turborepo_repository::package_json::PackageJson; + +use super::{Pipeline, RawTaskDefinition, TurboJson}; +use crate::{config::Error, run::task_id::TaskName}; + +/// Structure for loading TurboJson structures. +/// Depending on the strategy used, TurboJson might not correspond to +/// `turbo.json` file. +#[derive(Debug, Clone)] +pub struct TurboJsonLoader { + repo_root: AbsoluteSystemPathBuf, + strategy: Strategy, +} + +#[derive(Debug, Clone)] +enum Strategy { + SinglePackage { package_json: PackageJson }, + Workspace, +} + +impl TurboJsonLoader { + /// Create a loader that will load turbo.json files throughout the workspace + pub fn workspace(repo_root: AbsoluteSystemPathBuf) -> Self { + Self { + repo_root, + strategy: Strategy::Workspace, + } + } + + /// Create a loader that will load a root turbo.json or synthesize one if + /// the file doesn't exist + pub fn single_package(repo_root: AbsoluteSystemPathBuf, package_json: PackageJson) -> Self { + Self { + repo_root, + strategy: Strategy::SinglePackage { package_json }, + } + } + + pub fn load(&self, path: &AbsoluteSystemPath) -> Result { + match &self.strategy { + Strategy::SinglePackage { package_json } => { + load_from_root_package_json(&self.repo_root, path, package_json) + } + Strategy::Workspace => load_from_file(&self.repo_root, path), + } + } +} + +fn load_from_file( + repo_root: &AbsoluteSystemPath, + turbo_json_path: &AbsoluteSystemPath, +) -> Result { + match TurboJson::read(repo_root, turbo_json_path) { + // If the file didn't exist, throw a custom error here instead of propagating + Err(Error::Io(_)) => Err(Error::NoTurboJSON), + // There was an error, and we don't have any chance of recovering + // because we aren't synthesizing anything + Err(e) => Err(e), + // We're not synthesizing anything and there was no error, we're done + Ok(turbo) => Ok(turbo), + } +} + +fn load_from_root_package_json( + repo_root: &AbsoluteSystemPath, + turbo_json_path: &AbsoluteSystemPath, + root_package_json: &PackageJson, +) -> Result { + let mut turbo_json = match TurboJson::read(repo_root, turbo_json_path) { + // we're synthesizing, but we have a starting point + // Note: this will have to change to support task inference in a monorepo + // for now, we're going to error on any "root" tasks and turn non-root tasks into root + // tasks + Ok(mut turbo_json) => { + let mut pipeline = Pipeline::default(); + for (task_name, task_definition) in turbo_json.tasks { + if task_name.is_package_task() { + let (span, text) = task_definition.span_and_text("turbo.json"); + + return Err(Error::PackageTaskInSinglePackageMode { + task_id: task_name.to_string(), + span, + text, + }); + } + + pipeline.insert(task_name.into_root_task(), task_definition); + } + + turbo_json.tasks = pipeline; + + turbo_json + } + // turbo.json doesn't exist, but we're going try to synthesize something + Err(Error::Io(_)) => TurboJson::default(), + // some other happened, we can't recover + Err(e) => { + return Err(e); + } + }; + + // TODO: Add location info from package.json + for script_name in root_package_json.scripts.keys() { + let task_name = TaskName::from(script_name.as_str()); + if !turbo_json.has_task(&task_name) { + let task_name = task_name.into_root_task(); + // Explicitly set cache to Some(false) in this definition + // so we can pretend it was set on purpose. That way it + // won't get clobbered by the merge function. + turbo_json.tasks.insert( + task_name, + Spanned::new(RawTaskDefinition { + cache: Some(Spanned::new(false)), + ..RawTaskDefinition::default() + }), + ); + } + } + + Ok(turbo_json) +} + +#[cfg(test)] +mod test { + use std::fs; + + use anyhow::Result; + use tempfile::tempdir; + use test_case::test_case; + + use super::*; + use crate::turbo_json::CONFIG_FILE; + + #[test_case(r"{}", TurboJson::default() ; "empty")] + #[test_case(r#"{ "globalDependencies": ["tsconfig.json", "jest.config.js"] }"#, + TurboJson { + global_deps: vec!["jest.config.js".to_string(), "tsconfig.json".to_string()], + ..TurboJson::default() + } + ; "global dependencies (sorted)")] + #[test_case(r#"{ "globalPassThroughEnv": ["GITHUB_TOKEN", "AWS_SECRET_KEY"] }"#, + TurboJson { + global_pass_through_env: Some(vec!["AWS_SECRET_KEY".to_string(), "GITHUB_TOKEN".to_string()]), + ..TurboJson::default() + } + )] + #[test_case(r#"{ "//": "A comment"}"#, TurboJson::default() ; "faux comment")] + #[test_case(r#"{ "//": "A comment", "//": "Another comment" }"#, TurboJson::default() ; "two faux comments")] + fn test_get_root_turbo_no_synthesizing( + turbo_json_content: &str, + expected_turbo_json: TurboJson, + ) -> Result<()> { + let root_dir = tempdir()?; + let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; + fs::write(repo_root.join_component("turbo.json"), turbo_json_content)?; + let loader = TurboJsonLoader::workspace(repo_root.to_owned()); + + let mut turbo_json = loader.load(&repo_root.join_component(CONFIG_FILE))?; + + turbo_json.text = None; + turbo_json.path = None; + assert_eq!(turbo_json, expected_turbo_json); + + Ok(()) + } + + #[test_case( + None, + PackageJson { + scripts: [("build".to_string(), Spanned::new("echo build".to_string()))].into_iter().collect(), + ..PackageJson::default() + }, + TurboJson { + tasks: Pipeline([( + "//#build".into(), + Spanned::new(RawTaskDefinition { + cache: Some(Spanned::new(false)), + ..RawTaskDefinition::default() + }) + )].into_iter().collect() + ), + ..TurboJson::default() + } + )] + #[test_case( + Some(r#"{ + "tasks": { + "build": { + "cache": true + } + } + }"#), + PackageJson { + scripts: [("test".to_string(), Spanned::new("echo test".to_string()))].into_iter().collect(), + ..PackageJson::default() + }, + TurboJson { + tasks: Pipeline([( + "//#build".into(), + Spanned::new(RawTaskDefinition { + cache: Some(Spanned::new(true).with_range(81..85)), + ..RawTaskDefinition::default() + }).with_range(50..103) + ), + ( + "//#test".into(), + Spanned::new(RawTaskDefinition { + cache: Some(Spanned::new(false)), + ..RawTaskDefinition::default() + }) + )].into_iter().collect()), + ..TurboJson::default() + } + )] + fn test_get_root_turbo_with_synthesizing( + turbo_json_content: Option<&str>, + root_package_json: PackageJson, + expected_turbo_json: TurboJson, + ) -> Result<()> { + let root_dir = tempdir()?; + let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; + + if let Some(content) = turbo_json_content { + fs::write(repo_root.join_component("turbo.json"), content)?; + } + + let loader = TurboJsonLoader::single_package(repo_root.to_owned(), root_package_json); + let mut turbo_json = loader.load(&repo_root.join_component(CONFIG_FILE))?; + turbo_json.text = None; + turbo_json.path = None; + for (_, task_definition) in turbo_json.tasks.iter_mut() { + task_definition.path = None; + task_definition.text = None; + } + assert_eq!(turbo_json, expected_turbo_json); + + Ok(()) + } +} diff --git a/crates/turborepo-lib/src/turbo_json/mod.rs b/crates/turborepo-lib/src/turbo_json/mod.rs index 6551539d39809..17513629aca71 100644 --- a/crates/turborepo-lib/src/turbo_json/mod.rs +++ b/crates/turborepo-lib/src/turbo_json/mod.rs @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize}; use struct_iterable::Iterable; use turbopath::AbsoluteSystemPath; use turborepo_errors::Spanned; -use turborepo_repository::{package_graph::ROOT_PKG_NAME, package_json::PackageJson}; +use turborepo_repository::package_graph::ROOT_PKG_NAME; use turborepo_unescape::UnescapedString; use crate::{ @@ -25,8 +25,11 @@ use crate::{ task_graph::{TaskDefinition, TaskOutputs}, }; +mod loader; pub mod parser; +pub use loader::TurboJsonLoader; + #[derive(Serialize, Deserialize, Debug, Default, PartialEq, Clone, Deserializable)] #[serde(rename_all = "camelCase")] pub struct SpacesJson { @@ -548,83 +551,6 @@ impl TryFrom for TurboJson { } impl TurboJson { - /// Loads turbo.json by reading the file at `turbo_json_path` - pub fn load( - repo_root: &AbsoluteSystemPath, - turbo_json_path: &AbsoluteSystemPath, - ) -> Result { - match Self::read(repo_root, turbo_json_path) { - // If the file didn't exist, throw a custom error here instead of propagating - Err(Error::Io(_)) => Err(Error::NoTurboJSON), - // There was an error, and we don't have any chance of recovering - // because we aren't synthesizing anything - Err(e) => Err(e), - // We're not synthesizing anything and there was no error, we're done - Ok(turbo) => Ok(turbo), - } - } - - /// Loads turbo.json by reading the file at `turbo_json_path` and combining - /// with synthesized task definitions from the provided package.json - pub fn from_root_package_json( - repo_root: &AbsoluteSystemPath, - turbo_json_path: &AbsoluteSystemPath, - root_package_json: &PackageJson, - ) -> Result { - let mut turbo_json = match Self::read(repo_root, turbo_json_path) { - // we're synthesizing, but we have a starting point - // Note: this will have to change to support task inference in a monorepo - // for now, we're going to error on any "root" tasks and turn non-root tasks into root - // tasks - Ok(mut turbo_json) => { - let mut pipeline = Pipeline::default(); - for (task_name, task_definition) in turbo_json.tasks { - if task_name.is_package_task() { - let (span, text) = task_definition.span_and_text("turbo.json"); - - return Err(Error::PackageTaskInSinglePackageMode { - task_id: task_name.to_string(), - span, - text, - }); - } - - pipeline.insert(task_name.into_root_task(), task_definition); - } - - turbo_json.tasks = pipeline; - - turbo_json - } - // turbo.json doesn't exist, but we're going try to synthesize something - Err(Error::Io(_)) => TurboJson::default(), - // some other happened, we can't recover - Err(e) => { - return Err(e); - } - }; - - // TODO: Add location info from package.json - for script_name in root_package_json.scripts.keys() { - let task_name = TaskName::from(script_name.as_str()); - if !turbo_json.has_task(&task_name) { - let task_name = task_name.into_root_task(); - // Explicitly set cache to Some(false) in this definition - // so we can pretend it was set on purpose. That way it - // won't get clobbered by the merge function. - turbo_json.tasks.insert( - task_name, - Spanned::new(RawTaskDefinition { - cache: Some(Spanned::new(false)), - ..RawTaskDefinition::default() - }), - ); - } - } - - Ok(turbo_json) - } - fn has_task(&self, task_name: &TaskName) -> bool { for key in self.tasks.keys() { if key == task_name || (key.task() == task_name.task() && !task_name.is_package_task()) @@ -748,135 +674,22 @@ fn gather_env_vars( #[cfg(test)] mod tests { - use std::fs; - use anyhow::Result; use biome_deserialize::json::deserialize_from_json_str; use biome_json_parser::JsonParserOptions; use pretty_assertions::assert_eq; use serde_json::json; - use tempfile::tempdir; use test_case::test_case; - use turbopath::AbsoluteSystemPath; - use turborepo_repository::package_json::PackageJson; use turborepo_unescape::UnescapedString; - use super::{Pipeline, RawTurboJson, Spanned, UIMode}; + use super::{RawTurboJson, Spanned, UIMode}; use crate::{ cli::OutputLogsMode, run::task_id::TaskName, task_graph::{TaskDefinition, TaskOutputs}, - turbo_json::{RawTaskDefinition, TurboJson, CONFIG_FILE}, + turbo_json::RawTaskDefinition, }; - #[test_case(r"{}", TurboJson::default() ; "empty")] - #[test_case(r#"{ "globalDependencies": ["tsconfig.json", "jest.config.js"] }"#, - TurboJson { - global_deps: vec!["jest.config.js".to_string(), "tsconfig.json".to_string()], - ..TurboJson::default() - } - ; "global dependencies (sorted)")] - #[test_case(r#"{ "globalPassThroughEnv": ["GITHUB_TOKEN", "AWS_SECRET_KEY"] }"#, - TurboJson { - global_pass_through_env: Some(vec!["AWS_SECRET_KEY".to_string(), "GITHUB_TOKEN".to_string()]), - ..TurboJson::default() - } - )] - #[test_case(r#"{ "//": "A comment"}"#, TurboJson::default() ; "faux comment")] - #[test_case(r#"{ "//": "A comment", "//": "Another comment" }"#, TurboJson::default() ; "two faux comments")] - fn test_get_root_turbo_no_synthesizing( - turbo_json_content: &str, - expected_turbo_json: TurboJson, - ) -> Result<()> { - let root_dir = tempdir()?; - let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; - fs::write(repo_root.join_component("turbo.json"), turbo_json_content)?; - - let mut turbo_json = TurboJson::load(repo_root, &repo_root.join_component(CONFIG_FILE))?; - - turbo_json.text = None; - turbo_json.path = None; - assert_eq!(turbo_json, expected_turbo_json); - - Ok(()) - } - - #[test_case( - None, - PackageJson { - scripts: [("build".to_string(), Spanned::new("echo build".to_string()))].into_iter().collect(), - ..PackageJson::default() - }, - TurboJson { - tasks: Pipeline([( - "//#build".into(), - Spanned::new(RawTaskDefinition { - cache: Some(Spanned::new(false)), - ..RawTaskDefinition::default() - }) - )].into_iter().collect() - ), - ..TurboJson::default() - } - )] - #[test_case( - Some(r#"{ - "tasks": { - "build": { - "cache": true - } - } - }"#), - PackageJson { - scripts: [("test".to_string(), Spanned::new("echo test".to_string()))].into_iter().collect(), - ..PackageJson::default() - }, - TurboJson { - tasks: Pipeline([( - "//#build".into(), - Spanned::new(RawTaskDefinition { - cache: Some(Spanned::new(true).with_range(81..85)), - ..RawTaskDefinition::default() - }).with_range(50..103) - ), - ( - "//#test".into(), - Spanned::new(RawTaskDefinition { - cache: Some(Spanned::new(false)), - ..RawTaskDefinition::default() - }) - )].into_iter().collect()), - ..TurboJson::default() - } - )] - fn test_get_root_turbo_with_synthesizing( - turbo_json_content: Option<&str>, - root_package_json: PackageJson, - expected_turbo_json: TurboJson, - ) -> Result<()> { - let root_dir = tempdir()?; - let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; - - if let Some(content) = turbo_json_content { - fs::write(repo_root.join_component("turbo.json"), content)?; - } - - let mut turbo_json = TurboJson::from_root_package_json( - repo_root, - &repo_root.join_component(CONFIG_FILE), - &root_package_json, - )?; - turbo_json.text = None; - turbo_json.path = None; - for (_, task_definition) in turbo_json.tasks.iter_mut() { - task_definition.path = None; - task_definition.text = None; - } - assert_eq!(turbo_json, expected_turbo_json); - - Ok(()) - } - #[test_case( "{}", RawTaskDefinition::default(), From 3d2c541a3eee0af561dd91ecc89438658a83977d Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Thu, 12 Sep 2024 16:27:20 -0400 Subject: [PATCH 03/10] chore(task_access): move task access turbo.json loading into loader --- crates/turborepo-lib/src/run/builder.rs | 12 +- crates/turborepo-lib/src/run/task_access.rs | 23 +--- crates/turborepo-lib/src/turbo_json/loader.rs | 103 +++++++++++++++++- 3 files changed, 107 insertions(+), 31 deletions(-) diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index 9461f277c1e63..b36c75986193e 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -359,17 +359,15 @@ impl RunBuilder { let task_access = TaskAccess::new(self.repo_root.clone(), async_cache.clone(), &scm); task_access.restore_config().await; - let turbo_json_loader = if is_single_package { + let turbo_json_loader = if task_access.is_enabled() { + TurboJsonLoader::task_access(self.repo_root.clone(), root_package_json.clone()) + } else if is_single_package { TurboJsonLoader::single_package(self.repo_root.clone(), root_package_json.clone()) } else { TurboJsonLoader::workspace(self.repo_root.clone()) }; - let root_turbo_json = task_access - .load_turbo_json(&self.root_turbo_json_path) - .map_or_else( - || turbo_json_loader.load(&self.root_turbo_json_path), - Result::Ok, - )?; + + let root_turbo_json = turbo_json_loader.load(&self.root_turbo_json_path)?; pkg_dep_graph.validate()?; diff --git a/crates/turborepo-lib/src/run/task_access.rs b/crates/turborepo-lib/src/run/task_access.rs index 65fa374b9c26b..c6fc7798e3275 100644 --- a/crates/turborepo-lib/src/run/task_access.rs +++ b/crates/turborepo-lib/src/run/task_access.rs @@ -7,13 +7,13 @@ use std::{ use serde::Deserialize; use tracing::{debug, error, warn}; -use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, PathRelation}; +use turbopath::{AbsoluteSystemPathBuf, PathRelation}; use turborepo_cache::AsyncCache; use turborepo_scm::SCM; use turborepo_unescape::UnescapedString; use super::ConfigCache; -use crate::{config::RawTurboJson, gitignore::ensure_turbo_is_gitignored, turbo_json::TurboJson}; +use crate::{config::RawTurboJson, gitignore::ensure_turbo_is_gitignored}; // Environment variable key that will be used to enable, and set the expected // trace location @@ -245,25 +245,6 @@ impl TaskAccess { } } - /// Attempt to load a task traced turbo.json - pub fn load_turbo_json(&self, root_turbo_json_path: &AbsoluteSystemPath) -> Option { - if !self.enabled { - return None; - } - let trace_json_path = self.repo_root.join_components(&TASK_ACCESS_CONFIG_PATH); - let turbo_from_trace = TurboJson::read(&self.repo_root, &trace_json_path); - - // check the zero config case (turbo trace file, but no turbo.json file) - if let Ok(turbo_from_trace) = turbo_from_trace { - if !root_turbo_json_path.exists() { - debug!("Using turbo.json synthesized from trace file"); - return Some(turbo_from_trace); - } - } - - None - } - async fn to_file(&self) -> Result<(), ToFileError> { // if task access tracing is not enabled, we don't need to do anything if !self.is_enabled() { diff --git a/crates/turborepo-lib/src/turbo_json/loader.rs b/crates/turborepo-lib/src/turbo_json/loader.rs index 058e78e41dde3..0d5b30e5d04db 100644 --- a/crates/turborepo-lib/src/turbo_json/loader.rs +++ b/crates/turborepo-lib/src/turbo_json/loader.rs @@ -1,9 +1,13 @@ +use tracing::debug; use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf}; use turborepo_errors::Spanned; use turborepo_repository::package_json::PackageJson; use super::{Pipeline, RawTaskDefinition, TurboJson}; -use crate::{config::Error, run::task_id::TaskName}; +use crate::{ + config::Error, + run::{task_access::TASK_ACCESS_CONFIG_PATH, task_id::TaskName}, +}; /// Structure for loading TurboJson structures. /// Depending on the strategy used, TurboJson might not correspond to @@ -18,6 +22,7 @@ pub struct TurboJsonLoader { enum Strategy { SinglePackage { package_json: PackageJson }, Workspace, + TaskAccess { package_json: PackageJson }, } impl TurboJsonLoader { @@ -38,12 +43,24 @@ impl TurboJsonLoader { } } + /// Create a loader that will load a root turbo.json or synthesize one if + /// the file doesn't exist + pub fn task_access(repo_root: AbsoluteSystemPathBuf, package_json: PackageJson) -> Self { + Self { + repo_root, + strategy: Strategy::TaskAccess { package_json }, + } + } + pub fn load(&self, path: &AbsoluteSystemPath) -> Result { match &self.strategy { Strategy::SinglePackage { package_json } => { load_from_root_package_json(&self.repo_root, path, package_json) } Strategy::Workspace => load_from_file(&self.repo_root, path), + Strategy::TaskAccess { package_json } => { + load_task_access_trace_turbo_json(&self.repo_root, path, package_json) + } } } } @@ -122,16 +139,34 @@ fn load_from_root_package_json( Ok(turbo_json) } +fn load_task_access_trace_turbo_json( + repo_root: &AbsoluteSystemPath, + turbo_json_path: &AbsoluteSystemPath, + root_package_json: &PackageJson, +) -> Result { + let trace_json_path = repo_root.join_components(&TASK_ACCESS_CONFIG_PATH); + let turbo_from_trace = TurboJson::read(repo_root, &trace_json_path); + + // check the zero config case (turbo trace file, but no turbo.json file) + if let Ok(turbo_from_trace) = turbo_from_trace { + if !turbo_json_path.exists() { + debug!("Using turbo.json synthesized from trace file"); + return Ok(turbo_from_trace); + } + } + load_from_root_package_json(repo_root, turbo_json_path, root_package_json) +} + #[cfg(test)] mod test { - use std::fs; + use std::{collections::BTreeMap, fs}; use anyhow::Result; use tempfile::tempdir; use test_case::test_case; use super::*; - use crate::turbo_json::CONFIG_FILE; + use crate::{task_graph::TaskDefinition, turbo_json::CONFIG_FILE}; #[test_case(r"{}", TurboJson::default() ; "empty")] #[test_case(r#"{ "globalDependencies": ["tsconfig.json", "jest.config.js"] }"#, @@ -238,4 +273,66 @@ mod test { Ok(()) } + + #[test_case( + Some(r#"{ "tasks": {"//#build": {"env": ["SPECIAL_VAR"]}} }"#), + Some(r#"{ "tasks": {"build": {"env": ["EXPLICIT_VAR"]}} }"#), + TaskDefinition { env: vec!["EXPLICIT_VAR".to_string()], .. Default::default() } + ; "both present")] + #[test_case( + None, + Some(r#"{ "tasks": {"build": {"env": ["EXPLICIT_VAR"]}} }"#), + TaskDefinition { env: vec!["EXPLICIT_VAR".to_string()], .. Default::default() } + ; "no trace")] + #[test_case( + Some(r#"{ "tasks": {"//#build": {"env": ["SPECIAL_VAR"]}} }"#), + None, + TaskDefinition { env: vec!["SPECIAL_VAR".to_string()], .. Default::default() } + ; "no turbo.json")] + #[test_case( + None, + None, + TaskDefinition { cache: false, .. Default::default() } + ; "both missing")] + fn test_task_access_loading( + trace_contents: Option<&str>, + turbo_json_content: Option<&str>, + expected_root_build: TaskDefinition, + ) -> Result<()> { + let root_dir = tempdir()?; + let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; + + if let Some(content) = turbo_json_content { + repo_root + .join_component(CONFIG_FILE) + .create_with_contents(content.as_bytes())?; + } + if let Some(content) = trace_contents { + let trace_path = repo_root.join_components(&TASK_ACCESS_CONFIG_PATH); + trace_path.ensure_dir()?; + trace_path.create_with_contents(content.as_bytes())?; + } + + let mut scripts = BTreeMap::new(); + scripts.insert("build".into(), Spanned::new("echo building".into())); + let root_package_json = PackageJson { + scripts, + ..Default::default() + }; + + let loader = TurboJsonLoader::task_access(repo_root.to_owned(), root_package_json); + let turbo_json = loader.load(&repo_root.join_component(CONFIG_FILE))?; + let root_build = turbo_json + .tasks + .get(&TaskName::from("//#build")) + .expect("root build should always exist") + .as_inner(); + + assert_eq!( + expected_root_build, + TaskDefinition::try_from(root_build.clone())? + ); + + Ok(()) + } } From e3c9430def135e4f0a72e896d27ec0a8415fb877 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 13 Sep 2024 11:59:55 -0400 Subject: [PATCH 04/10] chore(turbo_json): change turbo.json loading to be package based instead of path based --- crates/turborepo-lib/src/config/mod.rs | 3 + crates/turborepo-lib/src/engine/builder.rs | 143 +++++++------ .../src/package_changes_watcher.rs | 23 +- crates/turborepo-lib/src/run/builder.rs | 23 +- crates/turborepo-lib/src/turbo_json/loader.rs | 199 +++++++++++++++--- crates/turborepo-lib/src/turbo_json/mod.rs | 2 +- 6 files changed, 285 insertions(+), 108 deletions(-) diff --git a/crates/turborepo-lib/src/config/mod.rs b/crates/turborepo-lib/src/config/mod.rs index c059333421446..24cf7111bc750 100644 --- a/crates/turborepo-lib/src/config/mod.rs +++ b/crates/turborepo-lib/src/config/mod.rs @@ -17,6 +17,7 @@ use thiserror::Error; use turbo_json::TurboJsonReader; use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf}; use turborepo_errors::TURBO_SITE; +use turborepo_repository::package_graph::PackageName; pub use crate::turbo_json::{RawTurboJson, UIMode}; use crate::{ @@ -179,6 +180,8 @@ pub enum Error { #[source_code] text: NamedSource, }, + #[error("Cannot load turbo.json for in {0} single package mode")] + InvalidTurboJsonLoad(PackageName), } const DEFAULT_API_URL: &str = "https://vercel.com/api"; diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index a369efda584bc..0caf671afd265 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -15,7 +15,7 @@ use crate::{ task_graph::TaskDefinition, turbo_json::{ validate_extends, validate_no_package_task_syntax, RawTaskDefinition, TurboJson, - TurboJsonLoader, CONFIG_FILE, + TurboJsonLoader, }, }; @@ -493,25 +493,11 @@ impl<'a> EngineBuilder<'a> { workspace: &PackageName, ) -> Result, Error> { if turbo_jsons.get(workspace).is_none() { - let json = self.load_turbo_json(workspace)?; + let json = self.turbo_json_loader.load(workspace)?; turbo_jsons.insert(workspace.clone(), json); } Ok(turbo_jsons.get(workspace)) } - - fn load_turbo_json(&self, workspace: &PackageName) -> Result { - let workspace_dir = - self.package_graph - .package_dir(workspace) - .ok_or_else(|| Error::MissingPackageJson { - workspace: workspace.clone(), - })?; - let workspace_turbo_json = self - .repo_root - .resolve(workspace_dir) - .join_component(CONFIG_FILE); - Ok(self.turbo_json_loader.load(&workspace_turbo_json)?) - } } impl Error { @@ -555,7 +541,10 @@ mod test { }; use super::*; - use crate::{engine::TaskNode, turbo_json::RawTurboJson}; + use crate::{ + engine::TaskNode, + turbo_json::{package_turbo_jsons, RawTurboJson}, + }; // Only used to prevent package graph construction from attempting to read // lockfile from disk @@ -657,42 +646,6 @@ mod test { .unwrap() } - #[test] - fn test_turbo_json_loading() { - let repo_root_dir = TempDir::with_prefix("repo").unwrap(); - let repo_root = AbsoluteSystemPathBuf::new(repo_root_dir.path().to_str().unwrap()).unwrap(); - let package_graph = mock_package_graph( - &repo_root, - package_jsons! { - repo_root, - "a" => [], - "b" => [], - "c" => ["a", "b"] - }, - ); - let loader = TurboJsonLoader::workspace(repo_root.clone()); - let engine_builder = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(vec![].into_iter().collect())); - - let a_turbo_json = repo_root.join_components(&["packages", "a", "turbo.json"]); - a_turbo_json.ensure_dir().unwrap(); - - let result = engine_builder.load_turbo_json(&PackageName::from("a")); - assert!( - result.is_err() && result.unwrap_err().is_missing_turbo_json(), - "expected parsing to fail with missing turbo.json" - ); - - a_turbo_json - .create_with_contents(r#"{"tasks": {"build": {}}}"#) - .unwrap(); - - let turbo_json = engine_builder - .load_turbo_json(&PackageName::from("a")) - .unwrap(); - assert_eq!(turbo_json.tasks.len(), 1); - } - fn turbo_json(value: serde_json::Value) -> TurboJson { let json_text = serde_json::to_string(&value).unwrap(); let raw = RawTurboJson::parse(&json_text, "").unwrap(); @@ -743,7 +696,12 @@ mod test { ] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine_builder = EngineBuilder::new(&repo_root, &package_graph, loader, false); let task_name = TaskName::from(task_name); let task_id = TaskId::try_from(task_id).unwrap(); @@ -814,7 +772,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("test")))) @@ -872,7 +835,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("test")))) @@ -912,7 +880,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("special")))) @@ -951,7 +924,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(vec![ @@ -1005,7 +983,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) @@ -1049,7 +1032,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) @@ -1085,7 +1073,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) @@ -1131,7 +1124,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) @@ -1171,7 +1169,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) @@ -1219,7 +1222,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) @@ -1259,7 +1267,12 @@ mod test { )] .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.clone()); + let package_turbo_jsons = package_turbo_jsons( + &repo_root, + repo_root.join_component("turbo.json"), + package_graph.packages(), + ); + let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) diff --git a/crates/turborepo-lib/src/package_changes_watcher.rs b/crates/turborepo-lib/src/package_changes_watcher.rs index 93ba26db593d4..94be766b469b1 100644 --- a/crates/turborepo-lib/src/package_changes_watcher.rs +++ b/crates/turborepo-lib/src/package_changes_watcher.rs @@ -21,7 +21,7 @@ use turborepo_repository::{ }; use turborepo_scm::package_deps::GitHashes; -use crate::turbo_json::{TurboJson, TurboJsonLoader, CONFIG_FILE}; +use crate::turbo_json::{package_turbo_jsons, TurboJson, TurboJsonLoader, CONFIG_FILE}; #[derive(Clone)] pub enum PackageChangeEvent { @@ -161,14 +161,6 @@ impl Subscriber { tracing::debug!("no package.json found, package watcher not available"); return None; }; - - let root_turbo_json = TurboJsonLoader::workspace(self.repo_root.clone()) - .load(&self.repo_root.join_component(CONFIG_FILE)) - .ok(); - - let gitignore_path = self.repo_root.join_component(".gitignore"); - let (root_gitignore, _) = Gitignore::new(&gitignore_path); - let Ok(pkg_dep_graph) = PackageGraphBuilder::new(&self.repo_root, root_package_json) .build() .await @@ -177,6 +169,19 @@ impl Subscriber { return None; }; + let package_turbo_jsons = package_turbo_jsons( + &self.repo_root, + self.repo_root.join_component(CONFIG_FILE), + pkg_dep_graph.packages(), + ); + let root_turbo_json = + TurboJsonLoader::workspace(self.repo_root.clone(), package_turbo_jsons) + .load(&PackageName::Root) + .ok(); + + let gitignore_path = self.repo_root.join_component(".gitignore"); + let (root_gitignore, _) = Gitignore::new(&gitignore_path); + Some(( RepoState { root_turbo_json, diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index b36c75986193e..fe2bea0922927 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -45,7 +45,7 @@ use crate::{ run::{scope, task_access::TaskAccess, task_id::TaskName, Error, Run, RunCache}, shim::TurboState, signal::{SignalHandler, SignalSubscriber}, - turbo_json::{TurboJson, TurboJsonLoader, UIMode}, + turbo_json::{package_turbo_jsons, TurboJson, TurboJsonLoader, UIMode}, DaemonConnector, }; @@ -360,14 +360,27 @@ impl RunBuilder { task_access.restore_config().await; let turbo_json_loader = if task_access.is_enabled() { - TurboJsonLoader::task_access(self.repo_root.clone(), root_package_json.clone()) + TurboJsonLoader::task_access( + self.repo_root.clone(), + self.root_turbo_json_path.clone(), + root_package_json.clone(), + ) } else if is_single_package { - TurboJsonLoader::single_package(self.repo_root.clone(), root_package_json.clone()) + TurboJsonLoader::single_package( + self.repo_root.clone(), + self.root_turbo_json_path.clone(), + root_package_json.clone(), + ) } else { - TurboJsonLoader::workspace(self.repo_root.clone()) + let package_turbo_jsons = package_turbo_jsons( + &self.repo_root, + self.root_turbo_json_path.clone(), + pkg_dep_graph.packages(), + ); + TurboJsonLoader::workspace(self.repo_root.clone(), package_turbo_jsons) }; - let root_turbo_json = turbo_json_loader.load(&self.root_turbo_json_path)?; + let root_turbo_json = turbo_json_loader.load(&PackageName::Root)?; pkg_dep_graph.validate()?; diff --git a/crates/turborepo-lib/src/turbo_json/loader.rs b/crates/turborepo-lib/src/turbo_json/loader.rs index 0d5b30e5d04db..42511c4096cc8 100644 --- a/crates/turborepo-lib/src/turbo_json/loader.rs +++ b/crates/turborepo-lib/src/turbo_json/loader.rs @@ -1,9 +1,14 @@ +use std::collections::HashMap; + use tracing::debug; use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf}; use turborepo_errors::Spanned; -use turborepo_repository::package_json::PackageJson; +use turborepo_repository::{ + package_graph::{PackageInfo, PackageName}, + package_json::PackageJson, +}; -use super::{Pipeline, RawTaskDefinition, TurboJson}; +use super::{Pipeline, RawTaskDefinition, TurboJson, CONFIG_FILE}; use crate::{ config::Error, run::{task_access::TASK_ACCESS_CONFIG_PATH, task_id::TaskName}, @@ -20,51 +25,122 @@ pub struct TurboJsonLoader { #[derive(Debug, Clone)] enum Strategy { - SinglePackage { package_json: PackageJson }, - Workspace, - TaskAccess { package_json: PackageJson }, + SinglePackage { + root_turbo_json: AbsoluteSystemPathBuf, + package_json: PackageJson, + }, + Workspace { + // Map of package names to their package specific turbo.json + packages: HashMap, + }, + TaskAccess { + root_turbo_json: AbsoluteSystemPathBuf, + package_json: PackageJson, + }, } impl TurboJsonLoader { /// Create a loader that will load turbo.json files throughout the workspace - pub fn workspace(repo_root: AbsoluteSystemPathBuf) -> Self { + pub fn workspace( + repo_root: AbsoluteSystemPathBuf, + packages: HashMap, + ) -> Self { Self { repo_root, - strategy: Strategy::Workspace, + strategy: Strategy::Workspace { packages }, } } /// Create a loader that will load a root turbo.json or synthesize one if /// the file doesn't exist - pub fn single_package(repo_root: AbsoluteSystemPathBuf, package_json: PackageJson) -> Self { + pub fn single_package( + repo_root: AbsoluteSystemPathBuf, + root_turbo_json: AbsoluteSystemPathBuf, + package_json: PackageJson, + ) -> Self { Self { repo_root, - strategy: Strategy::SinglePackage { package_json }, + strategy: Strategy::SinglePackage { + root_turbo_json, + package_json, + }, } } /// Create a loader that will load a root turbo.json or synthesize one if /// the file doesn't exist - pub fn task_access(repo_root: AbsoluteSystemPathBuf, package_json: PackageJson) -> Self { + pub fn task_access( + repo_root: AbsoluteSystemPathBuf, + root_turbo_json: AbsoluteSystemPathBuf, + package_json: PackageJson, + ) -> Self { Self { repo_root, - strategy: Strategy::TaskAccess { package_json }, + strategy: Strategy::TaskAccess { + root_turbo_json, + package_json, + }, } } - pub fn load(&self, path: &AbsoluteSystemPath) -> Result { + /// Load a turbo.json for a given package + pub fn load(&self, package: &PackageName) -> Result { match &self.strategy { - Strategy::SinglePackage { package_json } => { - load_from_root_package_json(&self.repo_root, path, package_json) + Strategy::SinglePackage { + package_json, + root_turbo_json, + } => { + if !matches!(package, PackageName::Root) { + Err(Error::InvalidTurboJsonLoad(package.clone())) + } else { + load_from_root_package_json(&self.repo_root, root_turbo_json, package_json) + } } - Strategy::Workspace => load_from_file(&self.repo_root, path), - Strategy::TaskAccess { package_json } => { - load_task_access_trace_turbo_json(&self.repo_root, path, package_json) + Strategy::Workspace { packages } => { + let path = packages.get(package).ok_or_else(|| Error::NoTurboJSON)?; + load_from_file(&self.repo_root, path) + } + Strategy::TaskAccess { + package_json, + root_turbo_json, + } => { + if !matches!(package, PackageName::Root) { + Err(Error::InvalidTurboJsonLoad(package.clone())) + } else { + load_task_access_trace_turbo_json( + &self.repo_root, + root_turbo_json, + package_json, + ) + } } } } } +/// Map all packages in the package graph to their turbo.json path +pub fn package_turbo_jsons<'a>( + repo_root: &AbsoluteSystemPath, + root_turbo_json_path: AbsoluteSystemPathBuf, + packages: impl Iterator, +) -> HashMap { + let mut package_turbo_jsons = HashMap::new(); + package_turbo_jsons.insert(PackageName::Root, root_turbo_json_path); + package_turbo_jsons.extend(packages.filter_map(|(pkg, info)| { + if pkg == &PackageName::Root { + None + } else { + Some(( + pkg.clone(), + repo_root + .resolve(info.package_path()) + .join_component(CONFIG_FILE), + )) + } + })); + package_turbo_jsons +} + fn load_from_file( repo_root: &AbsoluteSystemPath, turbo_json_path: &AbsoluteSystemPath, @@ -189,10 +265,16 @@ mod test { ) -> Result<()> { let root_dir = tempdir()?; let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; - fs::write(repo_root.join_component("turbo.json"), turbo_json_content)?; - let loader = TurboJsonLoader::workspace(repo_root.to_owned()); + let root_turbo_json = repo_root.join_component("turbo.json"); + fs::write(&root_turbo_json, turbo_json_content)?; + let loader = TurboJsonLoader::workspace( + repo_root.to_owned(), + vec![(PackageName::Root, root_turbo_json)] + .into_iter() + .collect(), + ); - let mut turbo_json = loader.load(&repo_root.join_component(CONFIG_FILE))?; + let mut turbo_json = loader.load(&PackageName::Root)?; turbo_json.text = None; turbo_json.path = None; @@ -256,13 +338,18 @@ mod test { ) -> Result<()> { let root_dir = tempdir()?; let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; + let root_turbo_json = repo_root.join_component(CONFIG_FILE); if let Some(content) = turbo_json_content { - fs::write(repo_root.join_component("turbo.json"), content)?; + fs::write(&root_turbo_json, content)?; } - let loader = TurboJsonLoader::single_package(repo_root.to_owned(), root_package_json); - let mut turbo_json = loader.load(&repo_root.join_component(CONFIG_FILE))?; + let loader = TurboJsonLoader::single_package( + repo_root.to_owned(), + root_turbo_json, + root_package_json, + ); + let mut turbo_json = loader.load(&PackageName::Root)?; turbo_json.text = None; turbo_json.path = None; for (_, task_definition) in turbo_json.tasks.iter_mut() { @@ -301,11 +388,10 @@ mod test { ) -> Result<()> { let root_dir = tempdir()?; let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; + let root_turbo_json = repo_root.join_component(CONFIG_FILE); if let Some(content) = turbo_json_content { - repo_root - .join_component(CONFIG_FILE) - .create_with_contents(content.as_bytes())?; + root_turbo_json.create_with_contents(content.as_bytes())?; } if let Some(content) = trace_contents { let trace_path = repo_root.join_components(&TASK_ACCESS_CONFIG_PATH); @@ -320,8 +406,9 @@ mod test { ..Default::default() }; - let loader = TurboJsonLoader::task_access(repo_root.to_owned(), root_package_json); - let turbo_json = loader.load(&repo_root.join_component(CONFIG_FILE))?; + let loader = + TurboJsonLoader::task_access(repo_root.to_owned(), root_turbo_json, root_package_json); + let turbo_json = loader.load(&PackageName::Root)?; let root_build = turbo_json .tasks .get(&TaskName::from("//#build")) @@ -335,4 +422,60 @@ mod test { Ok(()) } + + #[test] + fn test_single_package_loading_non_root() { + let junk_path = AbsoluteSystemPath::new(if cfg!(windows) { + "C:\\never\\loaded" + } else { + "/never/loaded" + }) + .unwrap(); + let non_root = PackageName::from("some-pkg"); + let single_loader = TurboJsonLoader::single_package( + junk_path.to_owned(), + junk_path.to_owned(), + PackageJson::default(), + ); + let task_access_loader = TurboJsonLoader::task_access( + junk_path.to_owned(), + junk_path.to_owned(), + PackageJson::default(), + ); + + for loader in [single_loader, task_access_loader] { + let result = loader.load(&non_root); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + matches!(err, Error::InvalidTurboJsonLoad(_)), + "expected {err} to be no turbo json" + ); + } + } + + #[test] + fn test_workspace_turbo_json_loading() { + let root_dir = tempdir().unwrap(); + let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path()).unwrap(); + let a_turbo_json = repo_root.join_components(&["packages", "a", "turbo.json"]); + a_turbo_json.ensure_dir().unwrap(); + let turbo_jsons = vec![(PackageName::from("a"), a_turbo_json.clone())] + .into_iter() + .collect(); + + let loader = TurboJsonLoader::workspace(repo_root.to_owned(), turbo_jsons); + let result = loader.load(&PackageName::from("a")); + assert!( + matches!(result.unwrap_err(), Error::NoTurboJSON), + "expected parsing to fail with missing turbo.json" + ); + + a_turbo_json + .create_with_contents(r#"{"tasks": {"build": {}}}"#) + .unwrap(); + + let turbo_json = loader.load(&PackageName::from("a")).unwrap(); + assert_eq!(turbo_json.tasks.len(), 1); + } } diff --git a/crates/turborepo-lib/src/turbo_json/mod.rs b/crates/turborepo-lib/src/turbo_json/mod.rs index 17513629aca71..d28e3343d0660 100644 --- a/crates/turborepo-lib/src/turbo_json/mod.rs +++ b/crates/turborepo-lib/src/turbo_json/mod.rs @@ -28,7 +28,7 @@ use crate::{ mod loader; pub mod parser; -pub use loader::TurboJsonLoader; +pub use loader::{package_turbo_jsons, TurboJsonLoader}; #[derive(Serialize, Deserialize, Debug, Default, PartialEq, Clone, Deserializable)] #[serde(rename_all = "camelCase")] From 349042d5a1b4b55397f682aac7ed5da6c59caff4 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 13 Sep 2024 13:28:43 -0400 Subject: [PATCH 05/10] chore(engine): move turbo.json caching to loader --- crates/turborepo-lib/src/engine/builder.rs | 205 +++++------------- .../src/package_changes_watcher.rs | 3 +- crates/turborepo-lib/src/run/builder.rs | 9 +- crates/turborepo-lib/src/turbo_json/loader.rs | 68 +++++- 4 files changed, 115 insertions(+), 170 deletions(-) diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index 0caf671afd265..5bfa437266148 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -14,8 +14,7 @@ use crate::{ run::task_id::{TaskId, TaskName}, task_graph::TaskDefinition, turbo_json::{ - validate_extends, validate_no_package_task_syntax, RawTaskDefinition, TurboJson, - TurboJsonLoader, + validate_extends, validate_no_package_task_syntax, RawTaskDefinition, TurboJsonLoader, }, }; @@ -95,9 +94,8 @@ pub enum Error { pub struct EngineBuilder<'a> { repo_root: &'a AbsoluteSystemPath, package_graph: &'a PackageGraph, - turbo_json_loader: TurboJsonLoader, + turbo_json_loader: Option, is_single: bool, - turbo_jsons: Option>, workspaces: Vec, tasks: Vec>>, root_enabled_tasks: HashSet>, @@ -114,9 +112,8 @@ impl<'a> EngineBuilder<'a> { Self { repo_root, package_graph, - turbo_json_loader, + turbo_json_loader: Some(turbo_json_loader), is_single, - turbo_jsons: None, workspaces: Vec::new(), tasks: Vec::new(), root_enabled_tasks: HashSet::new(), @@ -124,14 +121,6 @@ impl<'a> EngineBuilder<'a> { } } - pub fn with_turbo_jsons( - mut self, - turbo_jsons: Option>, - ) -> Self { - self.turbo_jsons = turbo_jsons; - self - } - pub fn with_tasks_only(mut self, tasks_only: bool) -> Self { self.tasks_only = tasks_only; self @@ -189,7 +178,10 @@ impl<'a> EngineBuilder<'a> { return Ok(Engine::default().seal()); } - let mut turbo_jsons = self.turbo_jsons.take().unwrap_or_default(); + let mut turbo_json_loader = self + .turbo_json_loader + .take() + .expect("engine builder cannot be constructed without TurboJsonLoader"); let mut missing_tasks: HashMap<&TaskName<'_>, Spanned<()>> = HashMap::from_iter(self.tasks.iter().map(|spanned| spanned.as_ref().split())); let mut traversal_queue = VecDeque::with_capacity(1); @@ -198,7 +190,7 @@ impl<'a> EngineBuilder<'a> { .task_id() .unwrap_or_else(|| TaskId::new(workspace.as_ref(), task.task())); - if self.has_task_definition(&mut turbo_jsons, workspace, task, &task_id)? { + if Self::has_task_definition(&mut turbo_json_loader, workspace, task, &task_id)? { missing_tasks.remove(task.as_inner()); // Even if a task definition was found, we _only_ want to add it as an entry @@ -279,7 +271,7 @@ impl<'a> EngineBuilder<'a> { } let task_definition = self.task_definition( - &mut turbo_jsons, + &mut turbo_json_loader, &task_id, &task_id.as_non_workspace_task_name(), )?; @@ -372,25 +364,29 @@ impl<'a> EngineBuilder<'a> { // Helper methods used when building the engine fn has_task_definition( - &self, - turbo_jsons: &mut HashMap, + loader: &mut TurboJsonLoader, workspace: &PackageName, task_name: &TaskName<'static>, task_id: &TaskId, ) -> Result { - let turbo_json = self - .turbo_json(turbo_jsons, workspace) + let turbo_json = loader + .load(workspace) // If there was no turbo.json in the workspace, fallback to the root turbo.json - .or_else(|e| { - if e.is_missing_turbo_json() && !matches!(workspace, PackageName::Root) { - Ok(None) - } else { - Err(e) - } - })?; + .map_or_else( + |e| { + if matches!(e, config::Error::NoTurboJSON) + && !matches!(workspace, PackageName::Root) + { + Ok(None) + } else { + Err(e) + } + }, + |v| Ok(Some(v)), + )?; let Some(turbo_json) = turbo_json else { - return self.has_task_definition(turbo_jsons, &PackageName::Root, task_name, task_id); + return Self::has_task_definition(loader, &PackageName::Root, task_name, task_id); }; let task_id_as_name = task_id.as_task_name(); @@ -399,7 +395,7 @@ impl<'a> EngineBuilder<'a> { { Ok(true) } else if !matches!(workspace, PackageName::Root) { - self.has_task_definition(turbo_jsons, &PackageName::Root, task_name, task_id) + Self::has_task_definition(loader, &PackageName::Root, task_name, task_id) } else { Ok(false) } @@ -407,12 +403,12 @@ impl<'a> EngineBuilder<'a> { fn task_definition( &self, - turbo_jsons: &mut HashMap, + turbo_json_loader: &mut TurboJsonLoader, task_id: &Spanned, task_name: &TaskName, ) -> Result { let raw_task_definition = RawTaskDefinition::from_iter(self.task_definition_chain( - turbo_jsons, + turbo_json_loader, task_id, task_name, )?); @@ -422,15 +418,13 @@ impl<'a> EngineBuilder<'a> { fn task_definition_chain( &self, - turbo_jsons: &mut HashMap, + turbo_json_loader: &mut TurboJsonLoader, task_id: &Spanned, task_name: &TaskName, ) -> Result, Error> { let mut task_definitions = Vec::new(); - let root_turbo_json = self - .turbo_json(turbo_jsons, &PackageName::Root)? - .ok_or(Error::Config(crate::config::Error::NoTurboJSON))?; + let root_turbo_json = turbo_json_loader.load(&PackageName::Root)?; if let Some(root_definition) = root_turbo_json.task(task_id, task_name) { task_definitions.push(root_definition) @@ -451,8 +445,8 @@ impl<'a> EngineBuilder<'a> { } if task_id.package() != ROOT_PKG_NAME { - match self.turbo_json(turbo_jsons, &PackageName::from(task_id.package())) { - Ok(Some(workspace_json)) => { + match turbo_json_loader.load(&PackageName::from(task_id.package())) { + Ok(workspace_json) => { let validation_errors = workspace_json .validate(&[validate_no_package_task_syntax, validate_extends]); if !validation_errors.is_empty() { @@ -465,11 +459,9 @@ impl<'a> EngineBuilder<'a> { task_definitions.push(workspace_def.value.clone()); } } - Ok(None) => (), - // swallow the error where the config file doesn't exist, but bubble up other things - Err(e) if e.is_missing_turbo_json() => (), + Err(config::Error::NoTurboJSON) => (), Err(e) => { - return Err(e); + return Err(e.into()); } } } @@ -486,18 +478,6 @@ impl<'a> EngineBuilder<'a> { Ok(task_definitions) } - - fn turbo_json<'b>( - &self, - turbo_jsons: &'b mut HashMap, - workspace: &PackageName, - ) -> Result, Error> { - if turbo_jsons.get(workspace).is_none() { - let json = self.turbo_json_loader.load(workspace)?; - turbo_jsons.insert(workspace.clone(), json); - } - Ok(turbo_jsons.get(workspace)) - } } impl Error { @@ -543,7 +523,7 @@ mod test { use super::*; use crate::{ engine::TaskNode, - turbo_json::{package_turbo_jsons, RawTurboJson}, + turbo_json::{RawTurboJson, TurboJson}, }; // Only used to prevent package graph construction from attempting to read @@ -663,18 +643,7 @@ mod test { task_id: &'static str, expected: bool, ) { - let repo_root_dir = TempDir::with_prefix("repo").unwrap(); - let repo_root = AbsoluteSystemPathBuf::new(repo_root_dir.path().to_str().unwrap()).unwrap(); - let package_graph = mock_package_graph( - &repo_root, - package_jsons! { - repo_root, - "a" => [], - "b" => [], - "c" => ["a", "b"] - }, - ); - let mut turbo_jsons = vec![ + let turbo_jsons = vec![ ( PackageName::Root, turbo_json(json!({ @@ -696,19 +665,13 @@ mod test { ] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); - let engine_builder = EngineBuilder::new(&repo_root, &package_graph, loader, false); + let mut loader = TurboJsonLoader::noop(turbo_jsons); let task_name = TaskName::from(task_name); let task_id = TaskId::try_from(task_id).unwrap(); - let has_def = engine_builder - .has_task_definition(&mut turbo_jsons, &workspace, &task_name, &task_id) - .unwrap(); + let has_def = + EngineBuilder::has_task_definition(&mut loader, &workspace, &task_name, &task_id) + .unwrap(); assert_eq!(has_def, expected); } @@ -772,14 +735,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("test")))) .with_workspaces(vec![ PackageName::from("a"), @@ -835,14 +792,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("test")))) .with_workspaces(vec![PackageName::from("app2")]) .build() @@ -880,14 +831,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("special")))) .with_workspaces(vec![PackageName::from("app1"), PackageName::from("libA")]) .build() @@ -924,14 +869,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(vec![ Spanned::new(TaskName::from("build")), Spanned::new(TaskName::from("test")), @@ -983,14 +922,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) .with_root_tasks(vec![ @@ -1032,14 +965,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) .with_root_tasks(vec![TaskName::from("libA#build"), TaskName::from("build")]) @@ -1073,14 +1000,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) .with_root_tasks(vec![ @@ -1124,14 +1045,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("app1")]) .with_root_tasks(vec![ @@ -1169,14 +1084,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) .with_tasks(Some(Spanned::new(TaskName::from("test")))) .with_workspaces(vec![ @@ -1222,14 +1131,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("b")]) @@ -1267,14 +1170,8 @@ mod test { )] .into_iter() .collect(); - let package_turbo_jsons = package_turbo_jsons( - &repo_root, - repo_root.join_component("turbo.json"), - package_graph.packages(), - ); - let loader = TurboJsonLoader::workspace(repo_root.clone(), package_turbo_jsons); + let loader = TurboJsonLoader::noop(turbo_jsons); let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false) - .with_turbo_jsons(Some(turbo_jsons)) .with_tasks_only(true) .with_tasks(Some(Spanned::new(TaskName::from("build")))) .with_workspaces(vec![PackageName::from("b")]) diff --git a/crates/turborepo-lib/src/package_changes_watcher.rs b/crates/turborepo-lib/src/package_changes_watcher.rs index 94be766b469b1..d883200edfaf5 100644 --- a/crates/turborepo-lib/src/package_changes_watcher.rs +++ b/crates/turborepo-lib/src/package_changes_watcher.rs @@ -177,7 +177,8 @@ impl Subscriber { let root_turbo_json = TurboJsonLoader::workspace(self.repo_root.clone(), package_turbo_jsons) .load(&PackageName::Root) - .ok(); + .ok() + .cloned(); let gitignore_path = self.repo_root.join_component(".gitignore"); let (root_gitignore, _) = Gitignore::new(&gitignore_path); diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index fe2bea0922927..0b333d97244d3 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -359,7 +359,7 @@ impl RunBuilder { let task_access = TaskAccess::new(self.repo_root.clone(), async_cache.clone(), &scm); task_access.restore_config().await; - let turbo_json_loader = if task_access.is_enabled() { + let mut turbo_json_loader = if task_access.is_enabled() { TurboJsonLoader::task_access( self.repo_root.clone(), self.root_turbo_json_path.clone(), @@ -380,7 +380,7 @@ impl RunBuilder { TurboJsonLoader::workspace(self.repo_root.clone(), package_turbo_jsons) }; - let root_turbo_json = turbo_json_loader.load(&PackageName::Root)?; + let root_turbo_json = turbo_json_loader.load(&PackageName::Root)?.clone(); pkg_dep_graph.validate()?; @@ -464,11 +464,6 @@ impl RunBuilder { self.opts.run_opts.single_package, ) .with_root_tasks(root_turbo_json.tasks.keys().cloned()) - .with_turbo_jsons(Some( - Some((PackageName::Root, root_turbo_json.clone())) - .into_iter() - .collect(), - )) .with_tasks_only(self.opts.run_opts.only) .with_workspaces(filtered_pkgs.clone().into_iter().collect()) .with_tasks(self.opts.run_opts.tasks.iter().map(|task| { diff --git a/crates/turborepo-lib/src/turbo_json/loader.rs b/crates/turborepo-lib/src/turbo_json/loader.rs index 42511c4096cc8..f9806fe5f7161 100644 --- a/crates/turborepo-lib/src/turbo_json/loader.rs +++ b/crates/turborepo-lib/src/turbo_json/loader.rs @@ -20,6 +20,7 @@ use crate::{ #[derive(Debug, Clone)] pub struct TurboJsonLoader { repo_root: AbsoluteSystemPathBuf, + cache: HashMap, strategy: Strategy, } @@ -37,6 +38,7 @@ enum Strategy { root_turbo_json: AbsoluteSystemPathBuf, package_json: PackageJson, }, + Noop, } impl TurboJsonLoader { @@ -47,6 +49,7 @@ impl TurboJsonLoader { ) -> Self { Self { repo_root, + cache: HashMap::new(), strategy: Strategy::Workspace { packages }, } } @@ -60,6 +63,7 @@ impl TurboJsonLoader { ) -> Self { Self { repo_root, + cache: HashMap::new(), strategy: Strategy::SinglePackage { root_turbo_json, package_json, @@ -76,6 +80,7 @@ impl TurboJsonLoader { ) -> Self { Self { repo_root, + cache: HashMap::new(), strategy: Strategy::TaskAccess { root_turbo_json, package_json, @@ -83,8 +88,33 @@ impl TurboJsonLoader { } } + /// Create a loader that will only return provided turbo.jsons and will + /// never hit the file system. + /// Primarily intended for testing + pub fn noop(turbo_jsons: HashMap) -> Self { + Self { + // This never gets read from so we populate it with + repo_root: AbsoluteSystemPath::new(if cfg!(windows) { "C:\\" } else { "/" }) + .expect("wasn't able to create absolute system path") + .to_owned(), + cache: turbo_jsons, + strategy: Strategy::Noop, + } + } + /// Load a turbo.json for a given package - pub fn load(&self, package: &PackageName) -> Result { + pub fn load<'a>(&'a mut self, package: &PackageName) -> Result<&'a TurboJson, Error> { + if !self.cache.contains_key(package) { + let turbo_json = self.uncached_load(package)?; + self.cache.insert(package.clone(), turbo_json); + } + Ok(self + .cache + .get(package) + .expect("just inserted value for this key")) + } + + fn uncached_load(&self, package: &PackageName) -> Result { match &self.strategy { Strategy::SinglePackage { package_json, @@ -114,6 +144,7 @@ impl TurboJsonLoader { ) } } + Strategy::Noop => Err(Error::NoTurboJSON), } } } @@ -267,14 +298,14 @@ mod test { let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; let root_turbo_json = repo_root.join_component("turbo.json"); fs::write(&root_turbo_json, turbo_json_content)?; - let loader = TurboJsonLoader::workspace( + let mut loader = TurboJsonLoader::workspace( repo_root.to_owned(), vec![(PackageName::Root, root_turbo_json)] .into_iter() .collect(), ); - let mut turbo_json = loader.load(&PackageName::Root)?; + let mut turbo_json = loader.load(&PackageName::Root)?.clone(); turbo_json.text = None; turbo_json.path = None; @@ -344,12 +375,12 @@ mod test { fs::write(&root_turbo_json, content)?; } - let loader = TurboJsonLoader::single_package( + let mut loader = TurboJsonLoader::single_package( repo_root.to_owned(), root_turbo_json, root_package_json, ); - let mut turbo_json = loader.load(&PackageName::Root)?; + let mut turbo_json = loader.load(&PackageName::Root)?.clone(); turbo_json.text = None; turbo_json.path = None; for (_, task_definition) in turbo_json.tasks.iter_mut() { @@ -406,7 +437,7 @@ mod test { ..Default::default() }; - let loader = + let mut loader = TurboJsonLoader::task_access(repo_root.to_owned(), root_turbo_json, root_package_json); let turbo_json = loader.load(&PackageName::Root)?; let root_build = turbo_json @@ -443,7 +474,7 @@ mod test { PackageJson::default(), ); - for loader in [single_loader, task_access_loader] { + for mut loader in [single_loader, task_access_loader] { let result = loader.load(&non_root); assert!(result.is_err()); let err = result.unwrap_err(); @@ -464,7 +495,7 @@ mod test { .into_iter() .collect(); - let loader = TurboJsonLoader::workspace(repo_root.to_owned(), turbo_jsons); + let mut loader = TurboJsonLoader::workspace(repo_root.to_owned(), turbo_jsons); let result = loader.load(&PackageName::from("a")); assert!( matches!(result.unwrap_err(), Error::NoTurboJSON), @@ -478,4 +509,25 @@ mod test { let turbo_json = loader.load(&PackageName::from("a")).unwrap(); assert_eq!(turbo_json.tasks.len(), 1); } + + #[test] + fn test_turbo_json_caching() { + let root_dir = tempdir().unwrap(); + let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path()).unwrap(); + let a_turbo_json = repo_root.join_components(&["packages", "a", "turbo.json"]); + a_turbo_json.ensure_dir().unwrap(); + let turbo_jsons = vec![(PackageName::from("a"), a_turbo_json.clone())] + .into_iter() + .collect(); + + let mut loader = TurboJsonLoader::workspace(repo_root.to_owned(), turbo_jsons); + a_turbo_json + .create_with_contents(r#"{"tasks": {"build": {}}}"#) + .unwrap(); + + let turbo_json = loader.load(&PackageName::from("a")).unwrap(); + assert_eq!(turbo_json.tasks.len(), 1); + a_turbo_json.remove().unwrap(); + assert!(loader.load(&PackageName::from("a")).is_ok()); + } } From e723c3028c8084c420a29177ccd00d272efc9702 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 16 Sep 2024 09:24:54 -0400 Subject: [PATCH 06/10] feat(turbo_json): add loader for monorepo without turbo.json --- crates/turborepo-lib/src/turbo_json/loader.rs | 127 ++++++++++++++++++ crates/turborepo-lib/src/turbo_json/mod.rs | 2 +- 2 files changed, 128 insertions(+), 1 deletion(-) diff --git a/crates/turborepo-lib/src/turbo_json/loader.rs b/crates/turborepo-lib/src/turbo_json/loader.rs index f9806fe5f7161..c9e7976bd3a3a 100644 --- a/crates/turborepo-lib/src/turbo_json/loader.rs +++ b/crates/turborepo-lib/src/turbo_json/loader.rs @@ -34,6 +34,10 @@ enum Strategy { // Map of package names to their package specific turbo.json packages: HashMap, }, + WorkspaceNoTurboJson { + // Map of package names to their scripts + packages: HashMap>, + }, TaskAccess { root_turbo_json: AbsoluteSystemPathBuf, package_json: PackageJson, @@ -54,6 +58,19 @@ impl TurboJsonLoader { } } + /// Create a loader that will construct turbo.json structures based on + /// workspace `package.json`s. + pub fn workspace_no_turbo_json( + repo_root: AbsoluteSystemPathBuf, + packages: HashMap>, + ) -> Self { + Self { + repo_root, + cache: HashMap::new(), + strategy: Strategy::WorkspaceNoTurboJson { packages }, + } + } + /// Create a loader that will load a root turbo.json or synthesize one if /// the file doesn't exist pub fn single_package( @@ -130,6 +147,14 @@ impl TurboJsonLoader { let path = packages.get(package).ok_or_else(|| Error::NoTurboJSON)?; load_from_file(&self.repo_root, path) } + Strategy::WorkspaceNoTurboJson { packages } => { + let script_names = packages.get(package).ok_or(Error::NoTurboJSON)?; + if matches!(package, PackageName::Root) { + root_turbo_json_from_scripts(script_names) + } else { + workspace_turbo_json_from_scripts(script_names) + } + } Strategy::TaskAccess { package_json, root_turbo_json, @@ -172,6 +197,20 @@ pub fn package_turbo_jsons<'a>( package_turbo_jsons } +/// Map all packages in the package graph to their scripts +pub fn workspace_package_scripts<'a>( + packages: impl Iterator, +) -> HashMap> { + packages + .map(|(pkg, info)| { + ( + pkg.clone(), + info.package_json.scripts.keys().cloned().collect(), + ) + }) + .collect() +} + fn load_from_file( repo_root: &AbsoluteSystemPath, turbo_json_path: &AbsoluteSystemPath, @@ -246,6 +285,41 @@ fn load_from_root_package_json( Ok(turbo_json) } +fn root_turbo_json_from_scripts(scripts: &[String]) -> Result { + let mut turbo_json = TurboJson { + ..Default::default() + }; + for script in scripts { + let task_name = TaskName::from(script.as_str()).into_root_task(); + turbo_json.tasks.insert( + task_name, + Spanned::new(RawTaskDefinition { + cache: Some(Spanned::new(false)), + ..Default::default() + }), + ); + } + Ok(turbo_json) +} + +fn workspace_turbo_json_from_scripts(scripts: &[String]) -> Result { + let mut turbo_json = TurboJson { + extends: Spanned::new(vec!["//".to_owned()]), + ..Default::default() + }; + for script in scripts { + let task_name = TaskName::from(script.clone()); + turbo_json.tasks.insert( + task_name, + Spanned::new(RawTaskDefinition { + cache: Some(Spanned::new(false)), + ..Default::default() + }), + ); + } + Ok(turbo_json) +} + fn load_task_access_trace_turbo_json( repo_root: &AbsoluteSystemPath, turbo_json_path: &AbsoluteSystemPath, @@ -530,4 +604,57 @@ mod test { a_turbo_json.remove().unwrap(); assert!(loader.load(&PackageName::from("a")).is_ok()); } + + #[test] + fn test_no_turbo_json() { + let root_dir = tempdir().unwrap(); + let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path()).unwrap(); + + let mut loader = TurboJsonLoader::workspace_no_turbo_json( + repo_root.to_owned(), + vec![ + ( + PackageName::Root, + vec!["build".to_owned(), "lint".to_owned(), "test".to_owned()], + ), + ( + PackageName::from("pkg-a"), + vec!["build".to_owned(), "lint".to_owned(), "special".to_owned()], + ), + ] + .into_iter() + .collect(), + ); + + { + let root_json = loader.load(&PackageName::Root).unwrap(); + for task_name in ["//#build", "//#lint", "//#test"] { + if let Some(def) = root_json.tasks.get(&TaskName::from(task_name)) { + assert_eq!( + def.cache.as_ref().map(|cache| *cache.as_inner()), + Some(false) + ); + } else { + panic!("didn't find {task_name}"); + } + } + } + + { + let pkg_a_json = loader.load(&PackageName::from("pkg-a")).unwrap(); + for task_name in ["build", "lint", "special"] { + if let Some(def) = pkg_a_json.tasks.get(&TaskName::from(task_name)) { + assert_eq!( + def.cache.as_ref().map(|cache| *cache.as_inner()), + Some(false) + ); + } else { + panic!("didn't find {task_name}"); + } + } + } + // Should get no turbo.json error if package wasn't declared + let goose_err = loader.load(&PackageName::from("goose")).unwrap_err(); + assert!(matches!(goose_err, Error::NoTurboJSON)); + } } diff --git a/crates/turborepo-lib/src/turbo_json/mod.rs b/crates/turborepo-lib/src/turbo_json/mod.rs index d28e3343d0660..b3aebd6109bf0 100644 --- a/crates/turborepo-lib/src/turbo_json/mod.rs +++ b/crates/turborepo-lib/src/turbo_json/mod.rs @@ -28,7 +28,7 @@ use crate::{ mod loader; pub mod parser; -pub use loader::{package_turbo_jsons, TurboJsonLoader}; +pub use loader::{package_turbo_jsons, workspace_package_scripts, TurboJsonLoader}; #[derive(Serialize, Deserialize, Debug, Default, PartialEq, Clone, Deserializable)] #[serde(rename_all = "camelCase")] From cfc2c2dacb3c2c93761c59b106d53965dcb2736e Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 16 Sep 2024 10:27:46 -0400 Subject: [PATCH 07/10] feat: allow for using turbo without a turbo.json --- crates/turborepo-lib/src/cli/mod.rs | 2 + crates/turborepo-lib/src/commands/mod.rs | 1 + crates/turborepo-lib/src/config/env.rs | 7 +++ crates/turborepo-lib/src/config/mod.rs | 5 ++ crates/turborepo-lib/src/run/builder.rs | 15 +++++- .../monorepo_no_turbo_json/.gitignore | 3 ++ .../apps/my-app/.env.local | 0 .../apps/my-app/package.json | 10 ++++ .../fixtures/monorepo_no_turbo_json/bar.txt | 1 + .../fixtures/monorepo_no_turbo_json/foo.txt | 1 + .../monorepo_no_turbo_json/package.json | 11 ++++ .../packages/another/package.json | 4 ++ .../packages/util/package.json | 6 +++ .../tests/run/allow-no-root-turbo.t | 50 +++++++++++++++++++ 14 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/.gitignore create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/apps/my-app/.env.local create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/apps/my-app/package.json create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/bar.txt create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/foo.txt create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/package.json create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/another/package.json create mode 100644 turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/util/package.json create mode 100644 turborepo-tests/integration/tests/run/allow-no-root-turbo.t diff --git a/crates/turborepo-lib/src/cli/mod.rs b/crates/turborepo-lib/src/cli/mod.rs index 3d8a4fa1735b8..2f68ff217322c 100644 --- a/crates/turborepo-lib/src/cli/mod.rs +++ b/crates/turborepo-lib/src/cli/mod.rs @@ -222,6 +222,8 @@ pub struct Args { /// should be used. #[clap(long, global = true)] pub dangerously_disable_package_manager_check: bool, + #[clap(long = "experimental-allow-no-turbo-json", hide = true, global = true)] + pub allow_no_turbo_json: bool, /// Use the `turbo.json` located at the provided path instead of one at the /// root of the repository. #[clap(long, global = true)] diff --git a/crates/turborepo-lib/src/commands/mod.rs b/crates/turborepo-lib/src/commands/mod.rs index 857f5e31b111b..04f1568bc0df8 100644 --- a/crates/turborepo-lib/src/commands/mod.rs +++ b/crates/turborepo-lib/src/commands/mod.rs @@ -108,6 +108,7 @@ impl CommandBase { .and_then(|args| args.remote_cache_read_only()), ) .with_run_summary(self.args.run_args().and_then(|args| args.summarize())) + .with_allow_no_turbo_json(self.args.allow_no_turbo_json.then_some(true)) .build() } diff --git a/crates/turborepo-lib/src/config/env.rs b/crates/turborepo-lib/src/config/env.rs index f70987712b9fd..b09a08d20c00c 100644 --- a/crates/turborepo-lib/src/config/env.rs +++ b/crates/turborepo-lib/src/config/env.rs @@ -38,6 +38,7 @@ const TURBO_MAPPING: &[(&str, &str)] = [ ("turbo_remote_only", "remote_only"), ("turbo_remote_cache_read_only", "remote_cache_read_only"), ("turbo_run_summary", "run_summary"), + ("turbo_allow_no_turbo_json", "allow_no_turbo_json"), ] .as_slice(); @@ -86,6 +87,7 @@ impl ResolvedConfigurationOptions for EnvVars { let remote_only = self.truthy_value("remote_only").flatten(); let remote_cache_read_only = self.truthy_value("remote_cache_read_only").flatten(); let run_summary = self.truthy_value("run_summary").flatten(); + let allow_no_turbo_json = self.truthy_value("allow_no_turbo_json").flatten(); // Process timeout let timeout = self @@ -171,6 +173,7 @@ impl ResolvedConfigurationOptions for EnvVars { remote_only, remote_cache_read_only, run_summary, + allow_no_turbo_json, // Processed numbers timeout, @@ -317,6 +320,7 @@ mod test { env.insert("turbo_remote_only".into(), "1".into()); env.insert("turbo_remote_cache_read_only".into(), "1".into()); env.insert("turbo_run_summary".into(), "true".into()); + env.insert("turbo_allow_no_turbo_json".into(), "true".into()); let config = EnvVars::new(&env) .unwrap() @@ -328,6 +332,7 @@ mod test { assert!(config.remote_only()); assert!(config.remote_cache_read_only()); assert!(config.run_summary()); + assert!(config.allow_no_turbo_json()); assert_eq!(turbo_api, config.api_url.unwrap()); assert_eq!(turbo_login, config.login_url.unwrap()); assert_eq!(turbo_team, config.team_slug.unwrap()); @@ -365,6 +370,7 @@ mod test { env.insert("turbo_remote_only".into(), "".into()); env.insert("turbo_remote_cache_read_only".into(), "".into()); env.insert("turbo_run_summary".into(), "".into()); + env.insert("turbo_allow_no_turbo_json".into(), "".into()); let config = EnvVars::new(&env) .unwrap() @@ -387,6 +393,7 @@ mod test { assert!(!config.remote_only()); assert!(!config.remote_cache_read_only()); assert!(!config.run_summary()); + assert!(!config.allow_no_turbo_json()); } #[test] diff --git a/crates/turborepo-lib/src/config/mod.rs b/crates/turborepo-lib/src/config/mod.rs index 24cf7111bc750..d8011aea738ef 100644 --- a/crates/turborepo-lib/src/config/mod.rs +++ b/crates/turborepo-lib/src/config/mod.rs @@ -242,6 +242,7 @@ pub struct ConfigurationOptions { pub(crate) remote_only: Option, pub(crate) remote_cache_read_only: Option, pub(crate) run_summary: Option, + pub(crate) allow_no_turbo_json: Option, } #[derive(Default)] @@ -370,6 +371,10 @@ impl ConfigurationOptions { .clone() .unwrap_or_else(|| repo_root.join_component(CONFIG_FILE)) } + + pub fn allow_no_turbo_json(&self) -> bool { + self.allow_no_turbo_json.unwrap_or_default() + } } // Maps Some("") to None to emulate how Go handles empty strings diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index 0b333d97244d3..1dd91dff8ac3a 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -37,7 +37,7 @@ use { }; use crate::{ - cli::DryRunMode, + cli::{DryRunMode, EnvMode}, commands::CommandBase, engine::{Engine, EngineBuilder}, opts::Opts, @@ -45,7 +45,9 @@ use crate::{ run::{scope, task_access::TaskAccess, task_id::TaskName, Error, Run, RunCache}, shim::TurboState, signal::{SignalHandler, SignalSubscriber}, - turbo_json::{package_turbo_jsons, TurboJson, TurboJsonLoader, UIMode}, + turbo_json::{ + package_turbo_jsons, workspace_package_scripts, TurboJson, TurboJsonLoader, UIMode, + }, DaemonConnector, }; @@ -65,6 +67,7 @@ pub struct RunBuilder { entrypoint_packages: Option>, should_print_prelude_override: Option, allow_missing_package_manager: bool, + allow_no_turbo_json: bool, } impl RunBuilder { @@ -85,6 +88,7 @@ impl RunBuilder { (!cfg!(windows) || matches!(opts.run_opts.ui_mode, UIMode::Tui)), ); let root_turbo_json_path = config.root_turbo_json_path(&base.repo_root); + let allow_no_turbo_json = config.allow_no_turbo_json(); let CommandBase { repo_root, @@ -105,6 +109,7 @@ impl RunBuilder { should_print_prelude_override: None, allow_missing_package_manager, root_turbo_json_path, + allow_no_turbo_json, }) } @@ -371,6 +376,12 @@ impl RunBuilder { self.root_turbo_json_path.clone(), root_package_json.clone(), ) + } else if self.allow_no_turbo_json && !self.root_turbo_json_path.exists() { + let package_scripts = workspace_package_scripts(pkg_dep_graph.packages()); + // We explicitly set env mode to loose as otherwise no env vars will be passed + // to tasks. + self.opts.run_opts.env_mode = EnvMode::Loose; + TurboJsonLoader::workspace_no_turbo_json(self.repo_root.clone(), package_scripts) } else { let package_turbo_jsons = package_turbo_jsons( &self.repo_root, diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/.gitignore b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/.gitignore new file mode 100644 index 0000000000000..77af9fc60321d --- /dev/null +++ b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/.gitignore @@ -0,0 +1,3 @@ +node_modules/ +.turbo +.npmrc diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/apps/my-app/.env.local b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/apps/my-app/.env.local new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/apps/my-app/package.json b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/apps/my-app/package.json new file mode 100644 index 0000000000000..37523c1c8e838 --- /dev/null +++ b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/apps/my-app/package.json @@ -0,0 +1,10 @@ +{ + "name": "my-app", + "scripts": { + "build": "echo building", + "test": "echo $MY_VAR" + }, + "dependencies": { + "util": "*" + } +} diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/bar.txt b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/bar.txt new file mode 100644 index 0000000000000..5e849f85df5d1 --- /dev/null +++ b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/bar.txt @@ -0,0 +1 @@ +other file, not a global dependency diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/foo.txt b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/foo.txt new file mode 100644 index 0000000000000..eebae5f3ca7b5 --- /dev/null +++ b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/foo.txt @@ -0,0 +1 @@ +global dep! all tasks depend on this content! diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/package.json b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/package.json new file mode 100644 index 0000000000000..e86a3bf3c329b --- /dev/null +++ b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/package.json @@ -0,0 +1,11 @@ +{ + "name": "monorepo", + "scripts": { + "something": "turbo run build" + }, + "packageManager": "bower", + "workspaces": [ + "apps/**", + "packages/**" + ] +} diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/another/package.json b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/another/package.json new file mode 100644 index 0000000000000..e9e34ea52c154 --- /dev/null +++ b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/another/package.json @@ -0,0 +1,4 @@ +{ + "name": "another", + "scripts": {} +} diff --git a/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/util/package.json b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/util/package.json new file mode 100644 index 0000000000000..7309726a1df4e --- /dev/null +++ b/turborepo-tests/integration/fixtures/monorepo_no_turbo_json/packages/util/package.json @@ -0,0 +1,6 @@ +{ + "name": "util", + "scripts": { + "build": "echo building" + } +} diff --git a/turborepo-tests/integration/tests/run/allow-no-root-turbo.t b/turborepo-tests/integration/tests/run/allow-no-root-turbo.t new file mode 100644 index 0000000000000..4dfad7b546d18 --- /dev/null +++ b/turborepo-tests/integration/tests/run/allow-no-root-turbo.t @@ -0,0 +1,50 @@ +Setup + $ . ${TESTDIR}/../../../helpers/setup_integration_test.sh monorepo_no_turbo_json + +Run fails if not configured to allow missing turbo.json + $ ${TURBO} test + x Could not find turbo.json. + | Follow directions at https://turbo.build/repo/docs to create one + + [1] +Runs test tasks + $ MY_VAR=foo ${TURBO} test --experimental-allow-no-turbo-json + \xe2\x80\xa2 Packages in scope: another, my-app, util (esc) + \xe2\x80\xa2 Running test in 3 packages (esc) + \xe2\x80\xa2 Remote caching disabled (esc) + my-app:test: cache bypass, force executing ac1233e4c102becc + my-app:test: + my-app:test: > test + my-app:test: > echo $MY_VAR + my-app:test: + my-app:test: foo + + Tasks: 1 successful, 1 total + Cached: 0 cached, 1 total + Time:\s*[\.0-9]+m?s (re) + + + +Ensure caching is disabled + $ MY_VAR=foo ${TURBO} test --experimental-allow-no-turbo-json + \xe2\x80\xa2 Packages in scope: another, my-app, util (esc) + \xe2\x80\xa2 Running test in 3 packages (esc) + \xe2\x80\xa2 Remote caching disabled (esc) + my-app:test: cache bypass, force executing ac1233e4c102becc + my-app:test: + my-app:test: > test + my-app:test: > echo $MY_VAR + my-app:test: + my-app:test: foo + + Tasks: 1 successful, 1 total + Cached: 0 cached, 1 total + Time:\s*[\.0-9]+m?s (re) + +Finds all tasks based on scripts + $ TURBO_ALLOW_NO_TURBO_JSON=true ${TURBO} build test --dry=json | jq '.tasks | map(.taskId)| sort' + [ + "my-app#build", + "my-app#test", + "util#build" + ] From 3279a963fbaf6680aa4544a3aa7aa93cb5aaf047 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 16 Sep 2024 16:32:33 -0400 Subject: [PATCH 08/10] chore(engine): minor pr taste fixups --- crates/turborepo-lib/src/engine/builder.rs | 28 ++++++++++------------ 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index 5bfa437266148..484cce98974e5 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -369,23 +369,21 @@ impl<'a> EngineBuilder<'a> { task_name: &TaskName<'static>, task_id: &TaskId, ) -> Result { - let turbo_json = loader - .load(workspace) - // If there was no turbo.json in the workspace, fallback to the root turbo.json - .map_or_else( - |e| { - if matches!(e, config::Error::NoTurboJSON) - && !matches!(workspace, PackageName::Root) - { - Ok(None) - } else { - Err(e) - } - }, - |v| Ok(Some(v)), - )?; + let turbo_json = loader.load(workspace).map_or_else( + |err| { + if matches!(err, config::Error::NoTurboJSON) + && !matches!(workspace, PackageName::Root) + { + Ok(None) + } else { + Err(err) + } + }, + |turbo_json| Ok(Some(turbo_json)), + )?; let Some(turbo_json) = turbo_json else { + // If there was no turbo.json in the workspace, fallback to the root turbo.json return Self::has_task_definition(loader, &PackageName::Root, task_name, task_id); }; From 6ebb8efb09f6b89768559ffea59b2fe39dc4a2b3 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 17 Sep 2024 08:45:47 -0400 Subject: [PATCH 09/10] chore(turbo_json): take package iterator for workspace construction --- .../src/package_changes_watcher.rs | 16 ++-- crates/turborepo-lib/src/run/builder.rs | 17 ++-- crates/turborepo-lib/src/turbo_json/loader.rs | 78 +++++++++++-------- crates/turborepo-lib/src/turbo_json/mod.rs | 2 +- 4 files changed, 63 insertions(+), 50 deletions(-) diff --git a/crates/turborepo-lib/src/package_changes_watcher.rs b/crates/turborepo-lib/src/package_changes_watcher.rs index d883200edfaf5..d60ace1ffda93 100644 --- a/crates/turborepo-lib/src/package_changes_watcher.rs +++ b/crates/turborepo-lib/src/package_changes_watcher.rs @@ -21,7 +21,7 @@ use turborepo_repository::{ }; use turborepo_scm::package_deps::GitHashes; -use crate::turbo_json::{package_turbo_jsons, TurboJson, TurboJsonLoader, CONFIG_FILE}; +use crate::turbo_json::{TurboJson, TurboJsonLoader, CONFIG_FILE}; #[derive(Clone)] pub enum PackageChangeEvent { @@ -169,16 +169,14 @@ impl Subscriber { return None; }; - let package_turbo_jsons = package_turbo_jsons( - &self.repo_root, + let root_turbo_json = TurboJsonLoader::workspace( + self.repo_root.clone(), self.repo_root.join_component(CONFIG_FILE), pkg_dep_graph.packages(), - ); - let root_turbo_json = - TurboJsonLoader::workspace(self.repo_root.clone(), package_turbo_jsons) - .load(&PackageName::Root) - .ok() - .cloned(); + ) + .load(&PackageName::Root) + .ok() + .cloned(); let gitignore_path = self.repo_root.join_component(".gitignore"); let (root_gitignore, _) = Gitignore::new(&gitignore_path); diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index 1dd91dff8ac3a..5cf75b46acf45 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -45,9 +45,7 @@ use crate::{ run::{scope, task_access::TaskAccess, task_id::TaskName, Error, Run, RunCache}, shim::TurboState, signal::{SignalHandler, SignalSubscriber}, - turbo_json::{ - package_turbo_jsons, workspace_package_scripts, TurboJson, TurboJsonLoader, UIMode, - }, + turbo_json::{TurboJson, TurboJsonLoader, UIMode}, DaemonConnector, }; @@ -377,18 +375,19 @@ impl RunBuilder { root_package_json.clone(), ) } else if self.allow_no_turbo_json && !self.root_turbo_json_path.exists() { - let package_scripts = workspace_package_scripts(pkg_dep_graph.packages()); // We explicitly set env mode to loose as otherwise no env vars will be passed // to tasks. self.opts.run_opts.env_mode = EnvMode::Loose; - TurboJsonLoader::workspace_no_turbo_json(self.repo_root.clone(), package_scripts) + TurboJsonLoader::workspace_no_turbo_json( + self.repo_root.clone(), + pkg_dep_graph.packages(), + ) } else { - let package_turbo_jsons = package_turbo_jsons( - &self.repo_root, + TurboJsonLoader::workspace( + self.repo_root.clone(), self.root_turbo_json_path.clone(), pkg_dep_graph.packages(), - ); - TurboJsonLoader::workspace(self.repo_root.clone(), package_turbo_jsons) + ) }; let root_turbo_json = turbo_json_loader.load(&PackageName::Root)?.clone(); diff --git a/crates/turborepo-lib/src/turbo_json/loader.rs b/crates/turborepo-lib/src/turbo_json/loader.rs index c9e7976bd3a3a..6a7039e351a9f 100644 --- a/crates/turborepo-lib/src/turbo_json/loader.rs +++ b/crates/turborepo-lib/src/turbo_json/loader.rs @@ -47,10 +47,12 @@ enum Strategy { impl TurboJsonLoader { /// Create a loader that will load turbo.json files throughout the workspace - pub fn workspace( + pub fn workspace<'a>( repo_root: AbsoluteSystemPathBuf, - packages: HashMap, + root_turbo_json_path: AbsoluteSystemPathBuf, + packages: impl Iterator, ) -> Self { + let packages = package_turbo_jsons(&repo_root, root_turbo_json_path, packages); Self { repo_root, cache: HashMap::new(), @@ -60,10 +62,11 @@ impl TurboJsonLoader { /// Create a loader that will construct turbo.json structures based on /// workspace `package.json`s. - pub fn workspace_no_turbo_json( + pub fn workspace_no_turbo_json<'a>( repo_root: AbsoluteSystemPathBuf, - packages: HashMap>, + packages: impl Iterator, ) -> Self { + let packages = workspace_package_scripts(packages); Self { repo_root, cache: HashMap::new(), @@ -175,7 +178,7 @@ impl TurboJsonLoader { } /// Map all packages in the package graph to their turbo.json path -pub fn package_turbo_jsons<'a>( +fn package_turbo_jsons<'a>( repo_root: &AbsoluteSystemPath, root_turbo_json_path: AbsoluteSystemPathBuf, packages: impl Iterator, @@ -198,7 +201,7 @@ pub fn package_turbo_jsons<'a>( } /// Map all packages in the package graph to their scripts -pub fn workspace_package_scripts<'a>( +fn workspace_package_scripts<'a>( packages: impl Iterator, ) -> HashMap> { packages @@ -372,12 +375,15 @@ mod test { let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path())?; let root_turbo_json = repo_root.join_component("turbo.json"); fs::write(&root_turbo_json, turbo_json_content)?; - let mut loader = TurboJsonLoader::workspace( - repo_root.to_owned(), - vec![(PackageName::Root, root_turbo_json)] - .into_iter() - .collect(), - ); + let mut loader = TurboJsonLoader { + repo_root: repo_root.to_owned(), + cache: HashMap::new(), + strategy: Strategy::Workspace { + packages: vec![(PackageName::Root, root_turbo_json)] + .into_iter() + .collect(), + }, + }; let mut turbo_json = loader.load(&PackageName::Root)?.clone(); @@ -565,11 +571,15 @@ mod test { let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path()).unwrap(); let a_turbo_json = repo_root.join_components(&["packages", "a", "turbo.json"]); a_turbo_json.ensure_dir().unwrap(); - let turbo_jsons = vec![(PackageName::from("a"), a_turbo_json.clone())] + let packages = vec![(PackageName::from("a"), a_turbo_json.clone())] .into_iter() .collect(); - let mut loader = TurboJsonLoader::workspace(repo_root.to_owned(), turbo_jsons); + let mut loader = TurboJsonLoader { + repo_root: repo_root.to_owned(), + cache: HashMap::new(), + strategy: Strategy::Workspace { packages }, + }; let result = loader.load(&PackageName::from("a")); assert!( matches!(result.unwrap_err(), Error::NoTurboJSON), @@ -590,11 +600,15 @@ mod test { let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path()).unwrap(); let a_turbo_json = repo_root.join_components(&["packages", "a", "turbo.json"]); a_turbo_json.ensure_dir().unwrap(); - let turbo_jsons = vec![(PackageName::from("a"), a_turbo_json.clone())] + let packages = vec![(PackageName::from("a"), a_turbo_json.clone())] .into_iter() .collect(); - let mut loader = TurboJsonLoader::workspace(repo_root.to_owned(), turbo_jsons); + let mut loader = TurboJsonLoader { + repo_root: repo_root.to_owned(), + cache: HashMap::new(), + strategy: Strategy::Workspace { packages }, + }; a_turbo_json .create_with_contents(r#"{"tasks": {"build": {}}}"#) .unwrap(); @@ -609,22 +623,24 @@ mod test { fn test_no_turbo_json() { let root_dir = tempdir().unwrap(); let repo_root = AbsoluteSystemPath::from_std_path(root_dir.path()).unwrap(); + let packages = vec![ + ( + PackageName::Root, + vec!["build".to_owned(), "lint".to_owned(), "test".to_owned()], + ), + ( + PackageName::from("pkg-a"), + vec!["build".to_owned(), "lint".to_owned(), "special".to_owned()], + ), + ] + .into_iter() + .collect(); - let mut loader = TurboJsonLoader::workspace_no_turbo_json( - repo_root.to_owned(), - vec![ - ( - PackageName::Root, - vec!["build".to_owned(), "lint".to_owned(), "test".to_owned()], - ), - ( - PackageName::from("pkg-a"), - vec!["build".to_owned(), "lint".to_owned(), "special".to_owned()], - ), - ] - .into_iter() - .collect(), - ); + let mut loader = TurboJsonLoader { + repo_root: repo_root.to_owned(), + cache: HashMap::new(), + strategy: Strategy::WorkspaceNoTurboJson { packages }, + }; { let root_json = loader.load(&PackageName::Root).unwrap(); diff --git a/crates/turborepo-lib/src/turbo_json/mod.rs b/crates/turborepo-lib/src/turbo_json/mod.rs index b3aebd6109bf0..17513629aca71 100644 --- a/crates/turborepo-lib/src/turbo_json/mod.rs +++ b/crates/turborepo-lib/src/turbo_json/mod.rs @@ -28,7 +28,7 @@ use crate::{ mod loader; pub mod parser; -pub use loader::{package_turbo_jsons, workspace_package_scripts, TurboJsonLoader}; +pub use loader::TurboJsonLoader; #[derive(Serialize, Deserialize, Debug, Default, PartialEq, Clone, Deserializable)] #[serde(rename_all = "camelCase")] From 6af8473eb5d25ea34e52055b2547fe93d81988fb Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 17 Sep 2024 10:18:55 -0400 Subject: [PATCH 10/10] feat(env mode): allow for tasks to override the global env mode (#9157) ### Description In response to https://github.com/vercel/turborepo/pull/9149#discussion_r1761805304 this PR avoids mutating `opts` when building `Run` we now set env mode at the task level. A majority of this PR is adding the ability to set env mode at the task level. ### Testing Instructions Updated integration test (update needed as task hash changed since env mode is now set at the task level instead of the global level). --- crates/turborepo-lib/src/run/builder.rs | 5 +---- crates/turborepo-lib/src/run/summary/task.rs | 4 ++++ crates/turborepo-lib/src/task_graph/mod.rs | 6 +++++- crates/turborepo-lib/src/task_graph/visitor.rs | 2 +- crates/turborepo-lib/src/turbo_json/loader.rs | 3 +++ crates/turborepo-lib/src/turbo_json/mod.rs | 10 ++++++++++ .../integration/tests/run/allow-no-root-turbo.t | 4 ++-- 7 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/turborepo-lib/src/run/builder.rs b/crates/turborepo-lib/src/run/builder.rs index 5cf75b46acf45..aee20d56975b5 100644 --- a/crates/turborepo-lib/src/run/builder.rs +++ b/crates/turborepo-lib/src/run/builder.rs @@ -37,7 +37,7 @@ use { }; use crate::{ - cli::{DryRunMode, EnvMode}, + cli::DryRunMode, commands::CommandBase, engine::{Engine, EngineBuilder}, opts::Opts, @@ -375,9 +375,6 @@ impl RunBuilder { root_package_json.clone(), ) } else if self.allow_no_turbo_json && !self.root_turbo_json_path.exists() { - // We explicitly set env mode to loose as otherwise no env vars will be passed - // to tasks. - self.opts.run_opts.env_mode = EnvMode::Loose; TurboJsonLoader::workspace_no_turbo_json( self.repo_root.clone(), pkg_dep_graph.packages(), diff --git a/crates/turborepo-lib/src/run/summary/task.rs b/crates/turborepo-lib/src/run/summary/task.rs index 77aaa18c2689a..2f87ba15aef62 100644 --- a/crates/turborepo-lib/src/run/summary/task.rs +++ b/crates/turborepo-lib/src/run/summary/task.rs @@ -104,6 +104,8 @@ pub struct TaskSummaryTaskDefinition { env: Vec, pass_through_env: Option>, interactive: bool, + #[serde(skip_serializing_if = "Option::is_none")] + env_mode: Option, } #[derive(Debug, Serialize, Clone)] @@ -279,6 +281,7 @@ impl From for TaskSummaryTaskDefinition { output_logs, persistent, interactive, + env_mode, } = value; let mut outputs = inclusions; @@ -313,6 +316,7 @@ impl From for TaskSummaryTaskDefinition { interactive, env, pass_through_env, + env_mode, } } } diff --git a/crates/turborepo-lib/src/task_graph/mod.rs b/crates/turborepo-lib/src/task_graph/mod.rs index e1042db9d87f9..43cd452b296c6 100644 --- a/crates/turborepo-lib/src/task_graph/mod.rs +++ b/crates/turborepo-lib/src/task_graph/mod.rs @@ -9,7 +9,7 @@ use turborepo_errors::Spanned; pub use visitor::{Error as VisitorError, Visitor}; use crate::{ - cli::OutputLogsMode, + cli::{EnvMode, OutputLogsMode}, run::task_id::{TaskId, TaskName}, turbo_json::RawTaskDefinition, }; @@ -76,6 +76,9 @@ pub struct TaskDefinition { // Tasks that take stdin input cannot be cached as their outputs may depend on the // input. pub interactive: bool, + + // Override for global env mode setting + pub env_mode: Option, } impl Default for TaskDefinition { @@ -91,6 +94,7 @@ impl Default for TaskDefinition { output_logs: Default::default(), persistent: Default::default(), interactive: Default::default(), + env_mode: Default::default(), } } } diff --git a/crates/turborepo-lib/src/task_graph/visitor.rs b/crates/turborepo-lib/src/task_graph/visitor.rs index 4c37eaa77d943..f2313e4ad9584 100644 --- a/crates/turborepo-lib/src/task_graph/visitor.rs +++ b/crates/turborepo-lib/src/task_graph/visitor.rs @@ -219,7 +219,7 @@ impl<'a> Visitor<'a> { .task_definition(&info) .ok_or(Error::MissingDefinition)?; - let task_env_mode = self.global_env_mode; + let task_env_mode = task_definition.env_mode.unwrap_or(self.global_env_mode); package_task_event.track_env_mode(&task_env_mode.to_string()); let dependency_set = engine.dependencies(&info).ok_or(Error::MissingDefinition)?; diff --git a/crates/turborepo-lib/src/turbo_json/loader.rs b/crates/turborepo-lib/src/turbo_json/loader.rs index 6a7039e351a9f..b01fb054babfc 100644 --- a/crates/turborepo-lib/src/turbo_json/loader.rs +++ b/crates/turborepo-lib/src/turbo_json/loader.rs @@ -10,6 +10,7 @@ use turborepo_repository::{ use super::{Pipeline, RawTaskDefinition, TurboJson, CONFIG_FILE}; use crate::{ + cli::EnvMode, config::Error, run::{task_access::TASK_ACCESS_CONFIG_PATH, task_id::TaskName}, }; @@ -298,6 +299,7 @@ fn root_turbo_json_from_scripts(scripts: &[String]) -> Result task_name, Spanned::new(RawTaskDefinition { cache: Some(Spanned::new(false)), + env_mode: Some(EnvMode::Loose), ..Default::default() }), ); @@ -316,6 +318,7 @@ fn workspace_turbo_json_from_scripts(scripts: &[String]) -> Result>, #[serde(skip_serializing_if = "Option::is_none")] interactive: Option>, + // TODO: Remove this once we have the ability to load task definitions directly + // instead of deriving them from a TurboJson + #[serde(skip)] + env_mode: Option, } macro_rules! set_field { @@ -256,6 +260,7 @@ impl RawTaskDefinition { set_field!(self, other, env); set_field!(self, other, pass_through_env); set_field!(self, other, interactive); + set_field!(self, other, env_mode); } } @@ -404,6 +409,7 @@ impl TryFrom for TaskDefinition { output_logs: *raw_task.output_logs.unwrap_or_default(), persistent: *raw_task.persistent.unwrap_or_default(), interactive, + env_mode: raw_task.env_mode, }) } } @@ -726,6 +732,7 @@ mod tests { output_logs: Some(Spanned::new(OutputLogsMode::Full).with_range(246..252)), persistent: Some(Spanned::new(true).with_range(278..282)), interactive: Some(Spanned::new(true).with_range(309..313)), + env_mode: None, }, TaskDefinition { env: vec!["OS".to_string()], @@ -741,6 +748,7 @@ mod tests { topological_dependencies: vec![], persistent: true, interactive: true, + env_mode: None, } ; "full" )] @@ -765,6 +773,7 @@ mod tests { output_logs: Some(Spanned::new(OutputLogsMode::Full).with_range(279..285)), persistent: Some(Spanned::new(true).with_range(315..319)), interactive: None, + env_mode: None, }, TaskDefinition { env: vec!["OS".to_string()], @@ -780,6 +789,7 @@ mod tests { topological_dependencies: vec![], persistent: true, interactive: false, + env_mode: None, } ; "full (windows)" )] diff --git a/turborepo-tests/integration/tests/run/allow-no-root-turbo.t b/turborepo-tests/integration/tests/run/allow-no-root-turbo.t index 4dfad7b546d18..cb0ae8d7b1174 100644 --- a/turborepo-tests/integration/tests/run/allow-no-root-turbo.t +++ b/turborepo-tests/integration/tests/run/allow-no-root-turbo.t @@ -12,7 +12,7 @@ Runs test tasks \xe2\x80\xa2 Packages in scope: another, my-app, util (esc) \xe2\x80\xa2 Running test in 3 packages (esc) \xe2\x80\xa2 Remote caching disabled (esc) - my-app:test: cache bypass, force executing ac1233e4c102becc + my-app:test: cache bypass, force executing d80016a1a60c4c0a my-app:test: my-app:test: > test my-app:test: > echo $MY_VAR @@ -30,7 +30,7 @@ Ensure caching is disabled \xe2\x80\xa2 Packages in scope: another, my-app, util (esc) \xe2\x80\xa2 Running test in 3 packages (esc) \xe2\x80\xa2 Remote caching disabled (esc) - my-app:test: cache bypass, force executing ac1233e4c102becc + my-app:test: cache bypass, force executing d80016a1a60c4c0a my-app:test: my-app:test: > test my-app:test: > echo $MY_VAR