From 2e42a346ad64db36ff05b0a76249ad76e5842eab Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Fri, 2 Jun 2023 12:51:49 -0700 Subject: [PATCH 1/4] Work on stripping anyhow from package manager detection --- crates/turborepo-lib/src/lib.rs | 2 + .../turborepo-lib/src/package_manager/mod.rs | 111 ++++++++++++------ 2 files changed, 78 insertions(+), 35 deletions(-) diff --git a/crates/turborepo-lib/src/lib.rs b/crates/turborepo-lib/src/lib.rs index 324134a1d41c0..ebec7236037e8 100644 --- a/crates/turborepo-lib/src/lib.rs +++ b/crates/turborepo-lib/src/lib.rs @@ -1,5 +1,7 @@ #![feature(assert_matches)] #![feature(box_patterns)] +#![feature(error_generic_member_access)] +#![feature(provide_any)] mod child; mod cli; diff --git a/crates/turborepo-lib/src/package_manager/mod.rs b/crates/turborepo-lib/src/package_manager/mod.rs index 30f4cd5de4b58..8cd944c1a932d 100644 --- a/crates/turborepo-lib/src/package_manager/mod.rs +++ b/crates/turborepo-lib/src/package_manager/mod.rs @@ -3,14 +3,17 @@ mod pnpm; mod yarn; use std::{ - fmt, fs, + backtrace, + fmt::{self, Display}, + fs, path::{Path, PathBuf}, }; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Result as AnyhowResult}; use itertools::{Either, Itertools}; use regex::Regex; use serde::{Deserialize, Serialize}; +use thiserror::Error; use turbopath::AbsoluteSystemPath; use wax::{Any, Glob, Pattern}; @@ -127,7 +130,7 @@ impl Globs { }) } - pub fn test(&self, root: &Path, target: PathBuf) -> Result { + pub fn test(&self, root: &Path, target: PathBuf) -> anyhow::Result { let search_value = target .strip_prefix(root)? .to_str() @@ -140,30 +143,68 @@ impl Globs { } } +#[derive(Debug, Error)] +struct MissingWorkspaceError { + package_manager: PackageManager, +} + +impl Display for MissingWorkspaceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let err = match self.package_manager { + PackageManager::Pnpm | PackageManager::Pnpm6 => { + "pnpm-workspace.yaml: no packages found. Turborepo requires pnpm workspaces and \ + thus packages to be defined in the root pnpm-workspace.yaml" + } + PackageManager::Yarn | PackageManager::Berry => { + "package.json: no workspaces found. Turborepo requires Yarn workspaces to be \ + defined in the root package.json" + } + PackageManager::Npm => { + "package.json: no workspaces found. Turborepo requires npm workspaces to be \ + defined in the root package.json" + } + }; + write!(f, "{}", err) + } +} + +impl From<&PackageManager> for MissingWorkspaceError { + fn from(value: &PackageManager) -> Self { + Self { + package_manager: value.clone(), + } + } +} + +#[derive(Debug, Error)] +pub enum Error { + #[error("io error: {0}")] + Io(#[from] std::io::Error, #[backtrace] backtrace::Backtrace), + #[error(transparent)] + Workspace(#[from] MissingWorkspaceError), + #[error("yaml parsing error: {0}")] + ParsingYaml(#[from] serde_yaml::Error, #[backtrace] backtrace::Backtrace), + #[error("json parsing error: {0}")] + ParsingJson(#[from] serde_json::Error, #[backtrace] backtrace::Backtrace), + #[error("globbing error: {0}")] + Wax(#[from] wax::BuildError, #[backtrace] backtrace::Backtrace), + #[error(transparent)] + Other(#[from] anyhow::Error), +} + impl PackageManager { - /// Returns a list of globs for the package workspace. - /// NOTE: We return a `Vec` instead of a `GlobSet` because we - /// may need to iterate through these globs and a `GlobSet` doesn't allow - /// that. - /// - /// # Arguments - /// - /// * `root_path`: - /// - /// returns: Result, Error> - /// - /// # Examples - /// - /// ``` - /// ``` - pub fn get_workspace_globs(&self, root_path: &AbsoluteSystemPath) -> Result> { + /// Returns the set of globs for the workspace. + pub fn get_workspace_globs( + &self, + root_path: &AbsoluteSystemPath, + ) -> std::result::Result { let globs = match self { PackageManager::Pnpm | PackageManager::Pnpm6 => { let workspace_yaml = fs::read_to_string(root_path.join_component("pnpm-workspace.yaml"))?; let pnpm_workspace: PnpmWorkspace = serde_yaml::from_str(&workspace_yaml)?; if pnpm_workspace.packages.is_empty() { - return Ok(None); + return Err(MissingWorkspaceError::from(self).into()); } else { pnpm_workspace.packages } @@ -174,7 +215,7 @@ impl PackageManager { let package_json: PackageJsonWorkspaces = serde_json::from_str(&package_json_text)?; if package_json.workspaces.as_ref().is_empty() { - return Ok(None); + return Err(MissingWorkspaceError::from(self).into()); } else { package_json.workspaces.into() } @@ -189,13 +230,14 @@ impl PackageManager { } }); - match Globs::new(inclusions, exclusions) { - Ok(globs) => Ok(Some(globs)), - Err(err) => Err(anyhow!("Error building globs: {}", err)), - } + let globs = Globs::new(inclusions, exclusions)?; + Ok(globs) } - pub fn get_package_manager(base: &CommandBase, pkg: Option<&PackageJson>) -> Result { + pub fn get_package_manager( + base: &CommandBase, + pkg: Option<&PackageJson>, + ) -> AnyhowResult { // We don't surface errors for `read_package_manager` as we can fall back to // `detect_package_manager` if let Some(package_json) = pkg { @@ -208,7 +250,7 @@ impl PackageManager { } // Attempts to read the package manager from the package.json - fn read_package_manager(pkg: &PackageJson) -> Result> { + fn read_package_manager(pkg: &PackageJson) -> AnyhowResult> { let Some(package_manager) = &pkg.package_manager else { return Ok(None) }; @@ -225,11 +267,11 @@ impl PackageManager { Ok(manager) } - fn detect_package_manager(base: &CommandBase) -> Result { + fn detect_package_manager(base: &CommandBase) -> AnyhowResult { let mut detected_package_managers = PnpmDetector::new(&base.repo_root) .chain(NpmDetector::new(&base.repo_root)) .chain(YarnDetector::new(&base.repo_root)) - .collect::>>()?; + .collect::>>()?; match detected_package_managers.len() { 0 => { @@ -251,7 +293,7 @@ impl PackageManager { } } - pub(crate) fn parse_package_manager_string(manager: &str) -> Result<(&str, &str)> { + pub(crate) fn parse_package_manager_string(manager: &str) -> AnyhowResult<(&str, &str)> { let package_manager_pattern = Regex::new(r"(?Pnpm|pnpm|yarn)@(?P\d+\.\d+\.\d+(-.+)?)")?; if let Some(captures) = package_manager_pattern.captures(manager) { @@ -368,7 +410,7 @@ mod tests { } #[test] - fn test_read_package_manager() -> Result<()> { + fn test_read_package_manager() -> AnyhowResult<()> { let mut package_json = PackageJson { package_manager: Some("npm@8.19.4".to_string()), }; @@ -395,7 +437,7 @@ mod tests { } #[test] - fn test_detect_multiple_package_managers() -> Result<()> { + fn test_detect_multiple_package_managers() -> AnyhowResult<()> { let repo_root = tempdir()?; let repo_root_path = AbsoluteSystemPathBuf::new(repo_root.path())?; let base = CommandBase::new( @@ -436,7 +478,6 @@ mod tests { let package_manager = PackageManager::Npm; let globs = package_manager .get_workspace_globs(&with_yarn) - .unwrap() .unwrap(); let expected = Globs::new(vec!["apps/*", "packages/*"], vec![]).unwrap(); @@ -449,7 +490,7 @@ mod tests { globs: Globs, root: PathBuf, target: PathBuf, - output: Result, + output: AnyhowResult, } let tests = [TestCase { @@ -468,7 +509,7 @@ mod tests { } #[test] - fn test_nested_workspace_globs() -> Result<()> { + fn test_nested_workspace_globs() -> AnyhowResult<()> { let top_level: PackageJsonWorkspaces = serde_json::from_str("{ \"workspaces\": [\"packages/**\"]}")?; assert_eq!(top_level.workspaces.as_ref(), vec!["packages/**"]); From 678d77599225df1d48585098194de2546bdd6098 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Fri, 2 Jun 2023 14:13:25 -0700 Subject: [PATCH 2/4] Remove a lot of anyhow --- crates/turborepo-lib/src/execution_state.rs | 2 +- .../turborepo-lib/src/package_manager/mod.rs | 111 +++++++++++------- .../turborepo-lib/src/package_manager/npm.rs | 23 ++-- .../turborepo-lib/src/package_manager/pnpm.rs | 26 ++-- .../turborepo-lib/src/package_manager/yarn.rs | 34 ++---- crates/turborepo-lib/src/shim.rs | 19 +-- 6 files changed, 108 insertions(+), 107 deletions(-) diff --git a/crates/turborepo-lib/src/execution_state.rs b/crates/turborepo-lib/src/execution_state.rs index f2bb3852082ec..38b85deeee952 100644 --- a/crates/turborepo-lib/src/execution_state.rs +++ b/crates/turborepo-lib/src/execution_state.rs @@ -32,7 +32,7 @@ impl<'a> TryFrom<&'a CommandBase> for ExecutionState<'a> { PackageJson::load(&base.repo_root.join_component("package.json")).ok(); let package_manager = - PackageManager::get_package_manager(base, root_package_json.as_ref())?; + PackageManager::get_package_manager(&base.repo_root, root_package_json.as_ref())?; trace!("Found {} as package manager", package_manager); let repo_config = base.repo_config()?; diff --git a/crates/turborepo-lib/src/package_manager/mod.rs b/crates/turborepo-lib/src/package_manager/mod.rs index 8cd944c1a932d..4a1b675f73f1a 100644 --- a/crates/turborepo-lib/src/package_manager/mod.rs +++ b/crates/turborepo-lib/src/package_manager/mod.rs @@ -6,7 +6,7 @@ use std::{ backtrace, fmt::{self, Display}, fs, - path::{Path, PathBuf}, + path::PathBuf, }; use anyhow::{anyhow, Result as AnyhowResult}; @@ -18,10 +18,9 @@ use turbopath::AbsoluteSystemPath; use wax::{Any, Glob, Pattern}; use crate::{ - commands::CommandBase, package_json::PackageJson, package_manager::{npm::NpmDetector, pnpm::PnpmDetector, yarn::YarnDetector}, - ui::UNDERLINE, + ui::{UI, UNDERLINE}, }; #[derive(Debug, Deserialize)] @@ -130,7 +129,7 @@ impl Globs { }) } - pub fn test(&self, root: &Path, target: PathBuf) -> anyhow::Result { + pub fn test(&self, root: &AbsoluteSystemPath, target: PathBuf) -> AnyhowResult { let search_value = target .strip_prefix(root)? .to_str() @@ -144,10 +143,33 @@ impl Globs { } #[derive(Debug, Error)] -struct MissingWorkspaceError { +pub struct MissingWorkspaceError { package_manager: PackageManager, } +#[derive(Debug, Error)] +pub struct NoPackageManager; + +impl NoPackageManager { + pub fn ui_display(&self, ui: &UI) -> String { + let url = + ui.apply(UNDERLINE.apply_to("https://nodejs.org/api/packages.html#packagemanager")); + format!( + "We did not find a package manager specified in your root package.json. Please set \ + the \"packageManager\" property in your root package.json ({url}) or run `npx \ + @turbo/codemod add-package-manager` in the root of your monorepo." + ) + } +} + +impl Display for NoPackageManager { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "We did not find a package manager specified in your root package.json. \ + Please set the \"packageManager\" property in your root package.json (https://nodejs.org/api/packages.html#packagemanager) \ + or run `npx @turbo/codemod add-package-manager` in the root of your monorepo.") + } +} + impl Display for MissingWorkspaceError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let err = match self.package_manager { @@ -190,6 +212,17 @@ pub enum Error { Wax(#[from] wax::BuildError, #[backtrace] backtrace::Backtrace), #[error(transparent)] Other(#[from] anyhow::Error), + #[error(transparent)] + NoPackageManager(#[from] NoPackageManager), + #[error("We detected multiple package managers in your repository: {}. Please remove one \ + of them.", managers.join(", "))] + MultiplePackageManagers { managers: Vec }, + #[error(transparent)] + Semver(#[from] node_semver::SemverError), + #[error(transparent)] + Which(#[from] which::Error), + #[error("invalid utf8: {0}")] + Utf8Error(#[from] std::string::FromUtf8Error), } impl PackageManager { @@ -235,9 +268,9 @@ impl PackageManager { } pub fn get_package_manager( - base: &CommandBase, + repo_root: &AbsoluteSystemPath, pkg: Option<&PackageJson>, - ) -> AnyhowResult { + ) -> Result { // We don't surface errors for `read_package_manager` as we can fall back to // `detect_package_manager` if let Some(package_json) = pkg { @@ -246,7 +279,7 @@ impl PackageManager { } } - Self::detect_package_manager(base) + Self::detect_package_manager(repo_root) } // Attempts to read the package manager from the package.json @@ -267,29 +300,22 @@ impl PackageManager { Ok(manager) } - fn detect_package_manager(base: &CommandBase) -> AnyhowResult { - let mut detected_package_managers = PnpmDetector::new(&base.repo_root) - .chain(NpmDetector::new(&base.repo_root)) - .chain(YarnDetector::new(&base.repo_root)) - .collect::>>()?; + fn detect_package_manager(repo_root: &AbsoluteSystemPath) -> Result { + let mut detected_package_managers = PnpmDetector::new(repo_root) + .chain(NpmDetector::new(repo_root)) + .chain(YarnDetector::new(repo_root)) + .collect::, Error>>()?; match detected_package_managers.len() { - 0 => { - let url = base.ui.apply( - UNDERLINE.apply_to("https://nodejs.org/api/packages.html#packagemanager"), - ); - Err(anyhow!( - "We did not find a package manager specified in your root package.json. \ - Please set the \"packageManager\" property in your root package.json ({url}) \ - or run `npx @turbo/codemod add-package-manager` in the root of your monorepo." - )) - } + 0 => Err(NoPackageManager.into()), 1 => Ok(detected_package_managers.pop().unwrap()), - _ => Err(anyhow!( - "We detected multiple package managers in your repository: {}. Please remove one \ - of them.", - detected_package_managers.into_iter().join(", ") - )), + _ => { + let managers = detected_package_managers + .iter() + .map(|mgr| mgr.to_string()) + .collect(); + Err(Error::MultiplePackageManagers { managers }) + } } } @@ -319,7 +345,6 @@ mod tests { use turbopath::AbsoluteSystemPathBuf; use super::*; - use crate::{get_version, ui::UI, Args}; struct TestCase { name: String, @@ -440,19 +465,14 @@ mod tests { fn test_detect_multiple_package_managers() -> AnyhowResult<()> { let repo_root = tempdir()?; let repo_root_path = AbsoluteSystemPathBuf::new(repo_root.path())?; - let base = CommandBase::new( - Args::default(), - repo_root_path, - get_version(), - UI::new(true), - )?; let package_lock_json_path = repo_root.path().join(npm::LOCKFILE); File::create(&package_lock_json_path)?; let pnpm_lock_path = repo_root.path().join(pnpm::LOCKFILE); File::create(pnpm_lock_path)?; - let error = PackageManager::detect_package_manager(&base).unwrap_err(); + let error = + PackageManager::detect_package_manager(repo_root_path.as_absolute_path()).unwrap_err(); assert_eq!( error.to_string(), "We detected multiple package managers in your repository: pnpm, npm. Please remove \ @@ -461,7 +481,8 @@ mod tests { fs::remove_file(&package_lock_json_path)?; - let package_manager = PackageManager::detect_package_manager(&base)?; + let package_manager = + PackageManager::detect_package_manager(repo_root_path.as_absolute_path())?; assert_eq!(package_manager, PackageManager::Pnpm); Ok(()) @@ -488,15 +509,25 @@ mod tests { fn test_globs_test() { struct TestCase { globs: Globs, - root: PathBuf, + root: AbsoluteSystemPathBuf, target: PathBuf, output: AnyhowResult, } + #[cfg(unix)] + let root = AbsoluteSystemPathBuf::new("/a/b/c").unwrap(); + #[cfg(windows)] + let root = AbsoluteSystemPathBuf::new("C:\\a\\b\\c").unwrap(); + + #[cfg(unix)] + let target = PathBuf::from("/a/b/c/d/e/f"); + #[cfg(windows)] + let target = PathBuf::from("C:\\a\\b\\c\\d\\e\\f"); + let tests = [TestCase { globs: Globs::new(vec!["d/**".to_string()], vec![]).unwrap(), - root: PathBuf::from("/a/b/c"), - target: PathBuf::from("/a/b/c/d/e/f"), + root, + target, output: Ok(true), }]; diff --git a/crates/turborepo-lib/src/package_manager/npm.rs b/crates/turborepo-lib/src/package_manager/npm.rs index c3fd98bb1982a..5bf8c87b80c1b 100644 --- a/crates/turborepo-lib/src/package_manager/npm.rs +++ b/crates/turborepo-lib/src/package_manager/npm.rs @@ -1,17 +1,16 @@ -use anyhow::Result; -use turbopath::AbsoluteSystemPathBuf; +use turbopath::AbsoluteSystemPath; -use crate::package_manager::PackageManager; +use crate::package_manager::{Error, PackageManager}; pub const LOCKFILE: &str = "package-lock.json"; pub struct NpmDetector<'a> { - repo_root: &'a AbsoluteSystemPathBuf, + repo_root: &'a AbsoluteSystemPath, found: bool, } impl<'a> NpmDetector<'a> { - pub fn new(repo_root: &'a AbsoluteSystemPathBuf) -> Self { + pub fn new(repo_root: &'a AbsoluteSystemPath) -> Self { Self { repo_root, found: false, @@ -20,7 +19,7 @@ impl<'a> NpmDetector<'a> { } impl<'a> Iterator for NpmDetector<'a> { - type Item = Result; + type Item = Result; fn next(&mut self) -> Option { if self.found { @@ -47,24 +46,16 @@ mod tests { use turbopath::AbsoluteSystemPathBuf; use super::LOCKFILE; - use crate::{ - commands::CommandBase, get_version, package_manager::PackageManager, ui::UI, Args, - }; + use crate::package_manager::PackageManager; #[test] fn test_detect_npm() -> Result<()> { let repo_root = tempdir()?; let repo_root_path = AbsoluteSystemPathBuf::new(repo_root.path())?; - let mut base = CommandBase::new( - Args::default(), - repo_root_path, - get_version(), - UI::new(true), - )?; let lockfile_path = repo_root.path().join(LOCKFILE); File::create(&lockfile_path)?; - let package_manager = PackageManager::detect_package_manager(&mut base)?; + let package_manager = PackageManager::detect_package_manager(&repo_root_path)?; assert_eq!(package_manager, PackageManager::Npm); Ok(()) diff --git a/crates/turborepo-lib/src/package_manager/pnpm.rs b/crates/turborepo-lib/src/package_manager/pnpm.rs index b8f37469e4117..e16375402e869 100644 --- a/crates/turborepo-lib/src/package_manager/pnpm.rs +++ b/crates/turborepo-lib/src/package_manager/pnpm.rs @@ -1,25 +1,24 @@ -use anyhow::Result; use node_semver::{Range, Version}; -use turbopath::AbsoluteSystemPathBuf; +use turbopath::AbsoluteSystemPath; -use crate::package_manager::PackageManager; +use crate::package_manager::{Error, PackageManager}; pub const LOCKFILE: &str = "pnpm-lock.yaml"; pub struct PnpmDetector<'a> { found: bool, - repo_root: &'a AbsoluteSystemPathBuf, + repo_root: &'a AbsoluteSystemPath, } impl<'a> PnpmDetector<'a> { - pub fn new(repo_root: &'a AbsoluteSystemPathBuf) -> Self { + pub fn new(repo_root: &'a AbsoluteSystemPath) -> Self { Self { repo_root, found: false, } } - pub fn detect_pnpm6_or_pnpm(version: &Version) -> Result { + pub fn detect_pnpm6_or_pnpm(version: &Version) -> Result { let pnpm6_constraint: Range = "<7.0.0".parse()?; if pnpm6_constraint.satisfies(version) { Ok(PackageManager::Pnpm6) @@ -30,7 +29,7 @@ impl<'a> PnpmDetector<'a> { } impl<'a> Iterator for PnpmDetector<'a> { - type Item = Result; + type Item = Result; fn next(&mut self) -> Option { if self.found { @@ -53,24 +52,15 @@ mod tests { use turbopath::AbsoluteSystemPathBuf; use super::LOCKFILE; - use crate::{ - commands::CommandBase, get_version, package_manager::PackageManager, ui::UI, Args, - }; + use crate::package_manager::PackageManager; #[test] fn test_detect_pnpm() -> Result<()> { let repo_root = tempdir()?; let repo_root_path = AbsoluteSystemPathBuf::new(repo_root.path())?; - let mut base = CommandBase::new( - Args::default(), - repo_root_path, - get_version(), - UI::new(true), - )?; - let lockfile_path = repo_root.path().join(LOCKFILE); File::create(&lockfile_path)?; - let package_manager = PackageManager::detect_package_manager(&mut base)?; + let package_manager = PackageManager::detect_package_manager(&repo_root_path)?; assert_eq!(package_manager, PackageManager::Pnpm); Ok(()) diff --git a/crates/turborepo-lib/src/package_manager/yarn.rs b/crates/turborepo-lib/src/package_manager/yarn.rs index a6a91c5366075..6aa67304ffe62 100644 --- a/crates/turborepo-lib/src/package_manager/yarn.rs +++ b/crates/turborepo-lib/src/package_manager/yarn.rs @@ -1,23 +1,22 @@ use std::process::Command; -use anyhow::Result; use node_semver::{Range, Version}; -use turbopath::AbsoluteSystemPathBuf; +use turbopath::AbsoluteSystemPath; use which::which; -use crate::package_manager::PackageManager; +use crate::package_manager::{Error, PackageManager}; pub const LOCKFILE: &str = "yarn.lock"; pub struct YarnDetector<'a> { - repo_root: &'a AbsoluteSystemPathBuf, + repo_root: &'a AbsoluteSystemPath, // For testing purposes version_override: Option, found: bool, } impl<'a> YarnDetector<'a> { - pub fn new(repo_root: &'a AbsoluteSystemPathBuf) -> Self { + pub fn new(repo_root: &'a AbsoluteSystemPath) -> Self { Self { repo_root, version_override: None, @@ -30,7 +29,7 @@ impl<'a> YarnDetector<'a> { self.version_override = Some(version); } - fn get_yarn_version(&self) -> Result { + fn get_yarn_version(&self) -> Result { if let Some(version) = &self.version_override { return Ok(version.clone()); } @@ -44,7 +43,7 @@ impl<'a> YarnDetector<'a> { Ok(yarn_version_output.trim().parse()?) } - pub fn detect_berry_or_yarn(version: &Version) -> Result { + pub fn detect_berry_or_yarn(version: &Version) -> Result { let berry_constraint: Range = ">=2.0.0-0".parse()?; if berry_constraint.satisfies(version) { Ok(PackageManager::Berry) @@ -55,7 +54,7 @@ impl<'a> YarnDetector<'a> { } impl<'a> Iterator for YarnDetector<'a> { - type Item = Result; + type Item = Result; fn next(&mut self) -> Option { if self.found { @@ -85,35 +84,22 @@ mod tests { use turbopath::AbsoluteSystemPathBuf; use super::LOCKFILE; - use crate::{ - commands::CommandBase, - get_version, - package_manager::{yarn::YarnDetector, PackageManager}, - ui::UI, - Args, - }; + use crate::package_manager::{yarn::YarnDetector, PackageManager}; #[test] fn test_detect_yarn() -> Result<()> { let repo_root = tempdir()?; let repo_root_path = AbsoluteSystemPathBuf::new(repo_root.path())?; - let base = CommandBase::new( - Args::default(), - repo_root_path, - get_version(), - UI::new(true), - )?; let yarn_lock_path = repo_root.path().join(LOCKFILE); File::create(&yarn_lock_path)?; - let absolute_repo_root = AbsoluteSystemPathBuf::new(base.repo_root)?; - let mut detector = YarnDetector::new(&absolute_repo_root); + let mut detector = YarnDetector::new(&repo_root_path); detector.set_version_override("1.22.10".parse()?); let package_manager = detector.next().unwrap()?; assert_eq!(package_manager, PackageManager::Yarn); - let mut detector = YarnDetector::new(&absolute_repo_root); + let mut detector = YarnDetector::new(&repo_root_path); detector.set_version_override("2.22.10".parse()?); let package_manager = detector.next().unwrap()?; assert_eq!(package_manager, PackageManager::Berry); diff --git a/crates/turborepo-lib/src/shim.rs b/crates/turborepo-lib/src/shim.rs index 8772506cc3dad..c90737b3856ed 100644 --- a/crates/turborepo-lib/src/shim.rs +++ b/crates/turborepo-lib/src/shim.rs @@ -448,7 +448,7 @@ impl InferInfo { pub fn is_workspace_root_of(&self, target_path: &Path) -> bool { match &self.workspace_globs { Some(globs) => globs - .test(self.path.as_path(), target_path.to_path_buf()) + .test(&self.path, target_path.to_path_buf()) .unwrap_or(false), None => false, } @@ -472,13 +472,16 @@ impl RepoState { // FIXME: This should be based upon detecting the pacakage manager. // However, we don't have that functionality implemented in Rust yet. // PackageManager::detect(path).get_workspace_globs().unwrap_or(None) - let workspace_globs = PackageManager::Pnpm - .get_workspace_globs(path) - .unwrap_or_else(|_| { - PackageManager::Npm - .get_workspace_globs(path) - .unwrap_or(None) - }); + let workspace_globs = PackageManager::get_package_manager(reference_dir, None) + .and_then(|mgr| mgr.get_workspace_globs(reference_dir)) + .ok(); + // let workspace_globs = PackageManager::Pnpm + // .get_workspace_globs(path) + // .unwrap_or_else(|_| { + // PackageManager::Npm + // .get_workspace_globs(path) + // .unwrap_or(None) + // }); Some(InferInfo { path: path.to_owned(), From e7473232ef8a2a8dba4ea095b0f6e82780b6e919 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Fri, 2 Jun 2023 15:24:11 -0700 Subject: [PATCH 3/4] Remove anyhow and commandbase from package_manager --- Cargo.lock | 25 +++++++ crates/turborepo-lib/Cargo.toml | 1 + .../turborepo-lib/src/package_manager/mod.rs | 71 ++++++++++--------- crates/turborepo-lib/src/shim.rs | 23 ++---- crates/turborepo-paths/Cargo.toml | 1 + .../src/anchored_system_path_buf.rs | 7 ++ 6 files changed, 77 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc07b26a4afe6..295637400934d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3496,6 +3496,29 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf36173d4167ed999940f804952e6b08197cae5ad5d572eb4db150ce8ad5d58f" +[[package]] +name = "lazy-regex" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ff63c423c68ea6814b7da9e88ce585f793c87ddd9e78f646970891769c8235d4" +dependencies = [ + "lazy-regex-proc_macros", + "once_cell", + "regex", +] + +[[package]] +name = "lazy-regex-proc_macros" +version = "2.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8edfc11b8f56ce85e207e62ea21557cfa09bb24a8f6b04ae181b086ff8611c22" +dependencies = [ + "proc-macro2", + "quote", + "regex", + "syn 1.0.109", +] + [[package]] name = "lazy_static" version = "1.4.0" @@ -9380,6 +9403,7 @@ dependencies = [ "path-slash", "serde", "thiserror", + "wax", ] [[package]] @@ -9477,6 +9501,7 @@ dependencies = [ "indicatif", "is-terminal", "itertools", + "lazy-regex", "lazy_static", "libc", "node-semver", diff --git a/crates/turborepo-lib/Cargo.toml b/crates/turborepo-lib/Cargo.toml index bb3eee815a4db..b35d0dee4a0cd 100644 --- a/crates/turborepo-lib/Cargo.toml +++ b/crates/turborepo-lib/Cargo.toml @@ -85,6 +85,7 @@ url = "2.3.1" const_format = "0.2.30" go-parse-duration = "0.1.1" is-terminal = "0.4.7" +lazy-regex = "2.5.0" node-semver = "2.1.0" num_cpus = "1.15.0" owo-colors.workspace = true diff --git a/crates/turborepo-lib/src/package_manager/mod.rs b/crates/turborepo-lib/src/package_manager/mod.rs index 4a1b675f73f1a..6f3ab803b43dd 100644 --- a/crates/turborepo-lib/src/package_manager/mod.rs +++ b/crates/turborepo-lib/src/package_manager/mod.rs @@ -6,11 +6,10 @@ use std::{ backtrace, fmt::{self, Display}, fs, - path::PathBuf, }; -use anyhow::{anyhow, Result as AnyhowResult}; use itertools::{Either, Itertools}; +use lazy_regex::{lazy_regex, Lazy}; use regex::Regex; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -129,14 +128,15 @@ impl Globs { }) } - pub fn test(&self, root: &AbsoluteSystemPath, target: PathBuf) -> AnyhowResult { - let search_value = target - .strip_prefix(root)? - .to_str() - .ok_or_else(|| anyhow!("The relative path is not UTF8."))?; + pub fn test( + &self, + root: &AbsoluteSystemPath, + target: &AbsoluteSystemPath, + ) -> Result { + let search_value = root.anchor(target)?; - let includes = self.inclusions.is_match(search_value); - let excludes = self.exclusions.is_match(search_value); + let includes = self.inclusions.is_match(&search_value); + let excludes = self.exclusions.is_match(&search_value); Ok(includes && !excludes) } @@ -151,6 +151,8 @@ pub struct MissingWorkspaceError { pub struct NoPackageManager; impl NoPackageManager { + // TODO: determine how to thread through user-friendly error message and apply + // our UI pub fn ui_display(&self, ui: &UI) -> String { let url = ui.apply(UNDERLINE.apply_to("https://nodejs.org/api/packages.html#packagemanager")); @@ -223,8 +225,17 @@ pub enum Error { Which(#[from] which::Error), #[error("invalid utf8: {0}")] Utf8Error(#[from] std::string::FromUtf8Error), + #[error(transparent)] + Path(#[from] turbopath::PathError), + #[error( + "We could not parse the packageManager field in package.json, expected: {0}, received: {1}" + )] + InvalidPackageManager(String, String), } +static PACKAGE_MANAGER_PATTERN: Lazy = + lazy_regex!(r"(?Pnpm|pnpm|yarn)@(?P\d+\.\d+\.\d+(-.+)?)"); + impl PackageManager { /// Returns the set of globs for the workspace. pub fn get_workspace_globs( @@ -283,7 +294,7 @@ impl PackageManager { } // Attempts to read the package manager from the package.json - fn read_package_manager(pkg: &PackageJson) -> AnyhowResult> { + fn read_package_manager(pkg: &PackageJson) -> Result, Error> { let Some(package_manager) = &pkg.package_manager else { return Ok(None) }; @@ -319,19 +330,15 @@ impl PackageManager { } } - pub(crate) fn parse_package_manager_string(manager: &str) -> AnyhowResult<(&str, &str)> { - let package_manager_pattern = - Regex::new(r"(?Pnpm|pnpm|yarn)@(?P\d+\.\d+\.\d+(-.+)?)")?; - if let Some(captures) = package_manager_pattern.captures(manager) { + pub(crate) fn parse_package_manager_string(manager: &str) -> Result<(&str, &str), Error> { + if let Some(captures) = PACKAGE_MANAGER_PATTERN.captures(manager) { let manager = captures.name("manager").unwrap().as_str(); let version = captures.name("version").unwrap().as_str(); Ok((manager, version)) } else { - Err(anyhow!( - "We could not parse packageManager field in package.json, expected: {}, received: \ - {}", - package_manager_pattern, - manager + Err(Error::InvalidPackageManager( + PACKAGE_MANAGER_PATTERN.to_string(), + manager.to_string(), )) } } @@ -435,7 +442,7 @@ mod tests { } #[test] - fn test_read_package_manager() -> AnyhowResult<()> { + fn test_read_package_manager() -> Result<(), Error> { let mut package_json = PackageJson { package_manager: Some("npm@8.19.4".to_string()), }; @@ -462,7 +469,7 @@ mod tests { } #[test] - fn test_detect_multiple_package_managers() -> AnyhowResult<()> { + fn test_detect_multiple_package_managers() -> Result<(), Error> { let repo_root = tempdir()?; let repo_root_path = AbsoluteSystemPathBuf::new(repo_root.path())?; @@ -471,8 +478,7 @@ mod tests { let pnpm_lock_path = repo_root.path().join(pnpm::LOCKFILE); File::create(pnpm_lock_path)?; - let error = - PackageManager::detect_package_manager(repo_root_path.as_absolute_path()).unwrap_err(); + let error = PackageManager::detect_package_manager(&repo_root_path).unwrap_err(); assert_eq!( error.to_string(), "We detected multiple package managers in your repository: pnpm, npm. Please remove \ @@ -481,8 +487,7 @@ mod tests { fs::remove_file(&package_lock_json_path)?; - let package_manager = - PackageManager::detect_package_manager(repo_root_path.as_absolute_path())?; + let package_manager = PackageManager::detect_package_manager(&repo_root_path)?; assert_eq!(package_manager, PackageManager::Pnpm); Ok(()) @@ -497,9 +502,7 @@ mod tests { .unwrap(); let with_yarn = repo_root.join_components(&["examples", "with-yarn"]); let package_manager = PackageManager::Npm; - let globs = package_manager - .get_workspace_globs(&with_yarn) - .unwrap(); + let globs = package_manager.get_workspace_globs(&with_yarn).unwrap(); let expected = Globs::new(vec!["apps/*", "packages/*"], vec![]).unwrap(); assert_eq!(globs, expected); @@ -510,8 +513,8 @@ mod tests { struct TestCase { globs: Globs, root: AbsoluteSystemPathBuf, - target: PathBuf, - output: AnyhowResult, + target: AbsoluteSystemPathBuf, + output: Result, } #[cfg(unix)] @@ -520,9 +523,9 @@ mod tests { let root = AbsoluteSystemPathBuf::new("C:\\a\\b\\c").unwrap(); #[cfg(unix)] - let target = PathBuf::from("/a/b/c/d/e/f"); + let target = AbsoluteSystemPathBuf::new("/a/b/c/d/e/f").unwrap(); #[cfg(windows)] - let target = PathBuf::from("C:\\a\\b\\c\\d\\e\\f"); + let target = AbsoluteSystemPathBuf::new("C:\\a\\b\\c\\d\\e\\f").unwrap(); let tests = [TestCase { globs: Globs::new(vec!["d/**".to_string()], vec![]).unwrap(), @@ -532,7 +535,7 @@ mod tests { }]; for test in tests { - match test.globs.test(&test.root, test.target) { + match test.globs.test(&test.root, &test.target) { Ok(value) => assert_eq!(value, test.output.unwrap()), Err(value) => assert_eq!(value.to_string(), test.output.unwrap_err().to_string()), }; @@ -540,7 +543,7 @@ mod tests { } #[test] - fn test_nested_workspace_globs() -> AnyhowResult<()> { + fn test_nested_workspace_globs() -> Result<(), Error> { let top_level: PackageJsonWorkspaces = serde_json::from_str("{ \"workspaces\": [\"packages/**\"]}")?; assert_eq!(top_level.workspaces.as_ref(), vec!["packages/**"]); diff --git a/crates/turborepo-lib/src/shim.rs b/crates/turborepo-lib/src/shim.rs index c90737b3856ed..d167f62f99354 100644 --- a/crates/turborepo-lib/src/shim.rs +++ b/crates/turborepo-lib/src/shim.rs @@ -445,11 +445,9 @@ impl InferInfo { info.has_turbo_json } - pub fn is_workspace_root_of(&self, target_path: &Path) -> bool { + pub fn is_workspace_root_of(&self, target_path: &AbsoluteSystemPath) -> bool { match &self.workspace_globs { - Some(globs) => globs - .test(&self.path, target_path.to_path_buf()) - .unwrap_or(false), + Some(globs) => globs.test(&self.path, target_path).unwrap_or(false), None => false, } } @@ -469,19 +467,10 @@ impl RepoState { return None; } - // FIXME: This should be based upon detecting the pacakage manager. - // However, we don't have that functionality implemented in Rust yet. - // PackageManager::detect(path).get_workspace_globs().unwrap_or(None) - let workspace_globs = PackageManager::get_package_manager(reference_dir, None) - .and_then(|mgr| mgr.get_workspace_globs(reference_dir)) + // FIXME: We should save this package manager that we detected + let workspace_globs = PackageManager::get_package_manager(path, None) + .and_then(|mgr| mgr.get_workspace_globs(path)) .ok(); - // let workspace_globs = PackageManager::Pnpm - // .get_workspace_globs(path) - // .unwrap_or_else(|_| { - // PackageManager::Npm - // .get_workspace_globs(path) - // .unwrap_or(None) - // }); Some(InferInfo { path: path.to_owned(), @@ -558,7 +547,7 @@ impl RepoState { // Failing that we just choose the closest. } else { for ancestor_infer in check_roots { - if ancestor_infer.is_workspace_root_of(current.path.as_path()) { + if ancestor_infer.is_workspace_root_of(¤t.path) { let local_turbo_state = LocalTurboState::infer(ancestor_infer.path.as_path()); return Ok(Self { diff --git a/crates/turborepo-paths/Cargo.toml b/crates/turborepo-paths/Cargo.toml index 5d4951549bd48..528763529512b 100644 --- a/crates/turborepo-paths/Cargo.toml +++ b/crates/turborepo-paths/Cargo.toml @@ -14,6 +14,7 @@ path-slash = "0.2.1" # TODO: Make this a crate feature serde = { workspace = true } thiserror = { workspace = true } +wax = { workspace = true } [dev-dependencies] anyhow = { workspace = true } diff --git a/crates/turborepo-paths/src/anchored_system_path_buf.rs b/crates/turborepo-paths/src/anchored_system_path_buf.rs index 7b4eaf39b33f5..f96fc3904f01b 100644 --- a/crates/turborepo-paths/src/anchored_system_path_buf.rs +++ b/crates/turborepo-paths/src/anchored_system_path_buf.rs @@ -20,6 +20,13 @@ impl TryFrom<&Path> for AnchoredSystemPathBuf { } } +// TODO: perhaps we ought to be converting to a unix path? +impl<'a> Into> for &'a AnchoredSystemPathBuf { + fn into(self) -> wax::CandidatePath<'a> { + self.as_path().into() + } +} + impl AnchoredSystemPathBuf { pub fn new( root: impl AsRef, From 665be554807eec3f06bf8936a27169eb29a8423f Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 5 Jun 2023 09:43:49 -0700 Subject: [PATCH 4/4] Update crates/turborepo-lib/src/package_manager/mod.rs Co-authored-by: Nicholas Yang --- crates/turborepo-lib/src/package_manager/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/turborepo-lib/src/package_manager/mod.rs b/crates/turborepo-lib/src/package_manager/mod.rs index 6f3ab803b43dd..302999999d5fa 100644 --- a/crates/turborepo-lib/src/package_manager/mod.rs +++ b/crates/turborepo-lib/src/package_manager/mod.rs @@ -180,7 +180,7 @@ impl Display for MissingWorkspaceError { thus packages to be defined in the root pnpm-workspace.yaml" } PackageManager::Yarn | PackageManager::Berry => { - "package.json: no workspaces found. Turborepo requires Yarn workspaces to be \ + "package.json: no workspaces found. Turborepo requires yarn workspaces to be \ defined in the root package.json" } PackageManager::Npm => {