From 1b250d30fe34920d8e7229fea26d87180ef000ea Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Wed, 12 Jun 2024 11:00:55 -0700 Subject: [PATCH 1/4] chore(shim): make dynamic downloads opt in --- .../turborepo-lib/src/shim/local_turbo_config.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/turborepo-lib/src/shim/local_turbo_config.rs b/crates/turborepo-lib/src/shim/local_turbo_config.rs index 179ca3195bdc1..25ae00a4af108 100644 --- a/crates/turborepo-lib/src/shim/local_turbo_config.rs +++ b/crates/turborepo-lib/src/shim/local_turbo_config.rs @@ -3,6 +3,7 @@ use std::env; use tracing::debug; use turborepo_repository::{inference::RepoState, package_manager::PackageManager}; +const TURBO_DOWNLOAD_LOCAL_ENABLED: &str = "TURBO_DOWNLOAD_LOCAL_ENABLED"; const TURBO_DOWNLOAD_LOCAL_DISABLED: &str = "TURBO_DOWNLOAD_LOCAL_DISABLED"; /// Struct containing information about the desired local turbo version @@ -12,13 +13,18 @@ pub struct LocalTurboConfig { turbo_version: String, } +fn is_env_var_truthy(env_var: &str) -> bool { + env::var(env_var).map_or(false, |value| matches!(value.as_str(), "1" | "true")) +} + impl LocalTurboConfig { pub fn infer(repo_state: &RepoState) -> Option { - // Don't attempt a download if user has opted out - if env::var(TURBO_DOWNLOAD_LOCAL_DISABLED) - .map_or(false, |disable| matches!(disable.as_str(), "1" | "true")) + // TODO: once we have properly communicated this functionality we should make + // this opt-out. + if !is_env_var_truthy(TURBO_DOWNLOAD_LOCAL_ENABLED) + || is_env_var_truthy(TURBO_DOWNLOAD_LOCAL_DISABLED) { - debug!("downloading correct local version disabled"); + debug!("downloading correct local version not enabled"); return None; } let turbo_version = Self::turbo_version_from_lockfile(repo_state)?; From fb5913a8232d05b698ef0c61d8782e82681c7313 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Wed, 12 Jun 2024 11:42:05 -0700 Subject: [PATCH 2/4] pr feedback --- crates/turborepo-lib/src/shim/local_turbo_config.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/turborepo-lib/src/shim/local_turbo_config.rs b/crates/turborepo-lib/src/shim/local_turbo_config.rs index 25ae00a4af108..aca1110bd60ad 100644 --- a/crates/turborepo-lib/src/shim/local_turbo_config.rs +++ b/crates/turborepo-lib/src/shim/local_turbo_config.rs @@ -13,17 +13,20 @@ pub struct LocalTurboConfig { turbo_version: String, } -fn is_env_var_truthy(env_var: &str) -> bool { - env::var(env_var).map_or(false, |value| matches!(value.as_str(), "1" | "true")) +fn is_env_var_truthy(env_var: &str) -> Option { + let value = env::var(env_var).ok()?; + match value.as_str() { + "1" | "true" => Some(true), + "0" | "false" => Some(false), + _ => None, + } } impl LocalTurboConfig { pub fn infer(repo_state: &RepoState) -> Option { // TODO: once we have properly communicated this functionality we should make // this opt-out. - if !is_env_var_truthy(TURBO_DOWNLOAD_LOCAL_ENABLED) - || is_env_var_truthy(TURBO_DOWNLOAD_LOCAL_DISABLED) - { + if !is_env_var_truthy(TURBO_DOWNLOAD_LOCAL_ENABLED).unwrap_or(false) { debug!("downloading correct local version not enabled"); return None; } From 0d3c6ce521d8e62f8743c31c698ff452778ecdcc Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Wed, 12 Jun 2024 11:45:16 -0700 Subject: [PATCH 3/4] chore: update integration tests --- crates/turborepo-lib/src/shim/local_turbo_config.rs | 1 - turborepo-tests/helpers/setup.sh | 2 +- turborepo-tests/helpers/setup_example_test.sh | 2 +- turborepo-tests/integration/tests/find-turbo/setup.sh | 2 +- .../integration/tests/lockfile-aware-caching/setup.sh | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/turborepo-lib/src/shim/local_turbo_config.rs b/crates/turborepo-lib/src/shim/local_turbo_config.rs index aca1110bd60ad..e5c5ef47bbe5d 100644 --- a/crates/turborepo-lib/src/shim/local_turbo_config.rs +++ b/crates/turborepo-lib/src/shim/local_turbo_config.rs @@ -4,7 +4,6 @@ use tracing::debug; use turborepo_repository::{inference::RepoState, package_manager::PackageManager}; const TURBO_DOWNLOAD_LOCAL_ENABLED: &str = "TURBO_DOWNLOAD_LOCAL_ENABLED"; -const TURBO_DOWNLOAD_LOCAL_DISABLED: &str = "TURBO_DOWNLOAD_LOCAL_DISABLED"; /// Struct containing information about the desired local turbo version /// according to lockfiles, package.jsons, and if all else fails turbo.json diff --git a/turborepo-tests/helpers/setup.sh b/turborepo-tests/helpers/setup.sh index 9e15edfa48032..c8580cd5ac7cb 100755 --- a/turborepo-tests/helpers/setup.sh +++ b/turborepo-tests/helpers/setup.sh @@ -12,5 +12,5 @@ fi # disable the first-run telemetry message export TURBO_TELEMETRY_MESSAGE_DISABLED=1 export TURBO_GLOBAL_WARNING_DISABLED=1 -export TURBO_DOWNLOAD_LOCAL_DISABLED=1 +export TURBO_DOWNLOAD_LOCAL_ENABLED=0 TURBO=${MONOREPO_ROOT_DIR}/target/debug/turbo${EXT} diff --git a/turborepo-tests/helpers/setup_example_test.sh b/turborepo-tests/helpers/setup_example_test.sh index f4fd970947d9b..f7445c0097a45 100644 --- a/turborepo-tests/helpers/setup_example_test.sh +++ b/turborepo-tests/helpers/setup_example_test.sh @@ -3,7 +3,7 @@ set -eo pipefail export TURBO_TELEMETRY_MESSAGE_DISABLED=1 -export TURBO_DOWNLOAD_LOCAL_DISABLED=1 +export TURBO_DOWNLOAD_LOCAL_ENABLED=0 # Start by figuring out which example we're testing and its package manager example_path=$1 diff --git a/turborepo-tests/integration/tests/find-turbo/setup.sh b/turborepo-tests/integration/tests/find-turbo/setup.sh index bafa637cf0d21..387b9c37e14d4 100644 --- a/turborepo-tests/integration/tests/find-turbo/setup.sh +++ b/turborepo-tests/integration/tests/find-turbo/setup.sh @@ -1,6 +1,6 @@ #!/bin/bash -export TURBO_DOWNLOAD_LOCAL_DISABLED=1 +export TURBO_DOWNLOAD_LOCAL_ENABLED=0 SCRIPT_DIR=$(dirname ${BASH_SOURCE[0]}) TARGET_DIR=$1 FIXTURE_DIR=$2 diff --git a/turborepo-tests/integration/tests/lockfile-aware-caching/setup.sh b/turborepo-tests/integration/tests/lockfile-aware-caching/setup.sh index 3280fe5e03e09..9a2f9ddd05cd8 100755 --- a/turborepo-tests/integration/tests/lockfile-aware-caching/setup.sh +++ b/turborepo-tests/integration/tests/lockfile-aware-caching/setup.sh @@ -1,7 +1,7 @@ #!/bin/bash export TURBO_GLOBAL_WARNING_DISABLED=1 -export TURBO_DOWNLOAD_LOCAL_DISABLED=1 +export TURBO_DOWNLOAD_LOCAL_ENABLED=0 SCRIPT_DIR=$(dirname ${BASH_SOURCE[0]}) TARGET_DIR=$1 FIXTURE_DIR=$2 From 459c4620caf52e68122808a769649c32b264e057 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Wed, 12 Jun 2024 13:31:39 -0700 Subject: [PATCH 4/4] chore: fix tests by mocking env --- .../src/shim/local_turbo_config.rs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/turborepo-lib/src/shim/local_turbo_config.rs b/crates/turborepo-lib/src/shim/local_turbo_config.rs index e5c5ef47bbe5d..735559d2a19ac 100644 --- a/crates/turborepo-lib/src/shim/local_turbo_config.rs +++ b/crates/turborepo-lib/src/shim/local_turbo_config.rs @@ -23,9 +23,14 @@ fn is_env_var_truthy(env_var: &str) -> Option { impl LocalTurboConfig { pub fn infer(repo_state: &RepoState) -> Option { + Self::infer_internal(repo_state, is_env_var_truthy(TURBO_DOWNLOAD_LOCAL_ENABLED)) + } + + // Used for testing when we want to manually set the controlling env var + fn infer_internal(repo_state: &RepoState, is_enabled: Option) -> Option { // TODO: once we have properly communicated this functionality we should make // this opt-out. - if !is_env_var_truthy(TURBO_DOWNLOAD_LOCAL_ENABLED).unwrap_or(false) { + if !is_enabled.unwrap_or(false) { debug!("downloading correct local version not enabled"); return None; } @@ -86,7 +91,7 @@ mod test { .unwrap(); assert_eq!( - LocalTurboConfig::infer(&repo), + LocalTurboConfig::infer_internal(&repo, Some(true)), Some(LocalTurboConfig { turbo_version: "2.0.3".into() }) @@ -111,7 +116,7 @@ mod test { .unwrap(); assert_eq!( - LocalTurboConfig::infer(&repo), + LocalTurboConfig::infer_internal(&repo, Some(true)), Some(LocalTurboConfig { turbo_version: "2.0.3".into() }) @@ -136,7 +141,7 @@ mod test { package_manager: Err(Error::MissingPackageManager), }; - assert_eq!(LocalTurboConfig::infer(&repo), None,); + assert_eq!(LocalTurboConfig::infer_internal(&repo, Some(true)), None,); } #[test] @@ -157,7 +162,7 @@ mod test { package_manager: Err(Error::MissingPackageManager), }; - assert_eq!(LocalTurboConfig::infer(&repo), None); + assert_eq!(LocalTurboConfig::infer_internal(&repo, Some(true)), None); } #[test] @@ -174,7 +179,7 @@ mod test { turbo_json .create_with_contents(include_bytes!("../../fixtures/local_config/turbo.v1.json")) .unwrap(); - assert_eq!(LocalTurboConfig::infer(&repo), None); + assert_eq!(LocalTurboConfig::infer_internal(&repo, Some(true)), None); } #[test] @@ -191,7 +196,7 @@ mod test { turbo_json .create_with_contents(include_bytes!("../../fixtures/local_config/turbo.v2.json")) .unwrap(); - assert_eq!(LocalTurboConfig::infer(&repo), None,); + assert_eq!(LocalTurboConfig::infer_internal(&repo, Some(true)), None,); } #[test] @@ -204,6 +209,6 @@ mod test { root_package_json: PackageJson::default(), package_manager: Err(Error::MissingPackageManager), }; - assert_eq!(LocalTurboConfig::infer(&repo), None,); + assert_eq!(LocalTurboConfig::infer_internal(&repo, Some(true)), None,); } }