From 53393f54a6c8711d63ae1512249476c0992a1b01 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Wed, 12 Jun 2024 10:55:07 -0700 Subject: [PATCH] fix: remove inferring turbo version from package.json or turbo.json (#8437) ### Description We're seeing failures when we try to invoke `turbo` via npx with a range instead of a fully resolved version. We'll try to get to the bottom of this, but in the meantime we'll avoid this codepath. For the most part this makes sense as users should have a lockfile. Guessing version based on package.json or turbo.json can still result in a version mismatch so restricting this behavior only when we're sure of the version the user wants makes sense. ### Testing Instructions Updated unit tests --- .../src/shim/local_turbo_config.rs | 77 +------------------ 1 file changed, 4 insertions(+), 73 deletions(-) diff --git a/crates/turborepo-lib/src/shim/local_turbo_config.rs b/crates/turborepo-lib/src/shim/local_turbo_config.rs index b0d716eabe18d..179ca3195bdc1 100644 --- a/crates/turborepo-lib/src/shim/local_turbo_config.rs +++ b/crates/turborepo-lib/src/shim/local_turbo_config.rs @@ -1,9 +1,5 @@ use std::env; -use jsonc_parser::{ - ast::{ObjectPropName, Value}, - parse_to_ast, -}; use tracing::debug; use turborepo_repository::{inference::RepoState, package_manager::PackageManager}; @@ -25,18 +21,7 @@ impl LocalTurboConfig { debug!("downloading correct local version disabled"); return None; } - let turbo_version = Self::turbo_version_from_lockfile(repo_state) - .or_else(|| { - debug!( - "No turbo version found in a lockfile. Attempting to read version from root \ - package.json" - ); - Self::turbo_version_from_package_json(repo_state) - }) - .or_else(|| { - debug!("No turbo version found in package.json. Checking if turbo.json is for v1"); - Self::turbo_version_from_turbo_json_schema(repo_state) - })?; + let turbo_version = Self::turbo_version_from_lockfile(repo_state)?; Some(Self { turbo_version }) } @@ -63,45 +48,6 @@ impl LocalTurboConfig { lockfile.turbo_version() }) } - - fn turbo_version_from_package_json(repo_state: &RepoState) -> Option { - let package_json = &repo_state.root_package_json; - // Look for turbo as a root dependency - package_json - .all_dependencies() - .find_map(|(name, version)| (name == "turbo").then(|| version.clone())) - } - - fn turbo_version_from_turbo_json_schema(repo_state: &RepoState) -> Option { - let turbo_json_path = repo_state.root.join_component("turbo.json"); - let turbo_json_contents = turbo_json_path.read_existing_to_string().ok().flatten()?; - // We explicitly do not use regular path for parsing turbo.json as that will - // fail if it sees unexpected keys. Future versions of turbo might add - // keys and we don't want to crash in that situation. - let turbo_json = parse_to_ast( - &turbo_json_contents, - &Default::default(), - &Default::default(), - ) - .ok()?; - - if let Value::Object(turbo_json) = turbo_json.value? { - let has_pipeline = turbo_json.properties.iter().any(|property| { - let ObjectPropName::String(name) = &property.name else { - return false; - }; - name.value == "pipeline" - }); - if has_pipeline { - // All we can determine is that the turbo.json is meant for a turbo v1 - return Some("^1".to_owned()); - } - } - // We do not check for the existence of `tasks` as it provides us no beneficial - // information. We're already a turbo 2 binary so we'll continue - // execution. - None - } } #[cfg(test)] @@ -182,12 +128,7 @@ mod test { package_manager: Err(Error::MissingPackageManager), }; - assert_eq!( - LocalTurboConfig::infer(&repo), - Some(LocalTurboConfig { - turbo_version: "^2.0.0".into() - }) - ); + assert_eq!(LocalTurboConfig::infer(&repo), None,); } #[test] @@ -208,12 +149,7 @@ mod test { package_manager: Err(Error::MissingPackageManager), }; - assert_eq!( - LocalTurboConfig::infer(&repo), - Some(LocalTurboConfig { - turbo_version: "^2.0.0".into() - }) - ); + assert_eq!(LocalTurboConfig::infer(&repo), None); } #[test] @@ -230,12 +166,7 @@ mod test { turbo_json .create_with_contents(include_bytes!("../../fixtures/local_config/turbo.v1.json")) .unwrap(); - assert_eq!( - LocalTurboConfig::infer(&repo), - Some(LocalTurboConfig { - turbo_version: "^1".into() - }) - ); + assert_eq!(LocalTurboConfig::infer(&repo), None); } #[test]