From 47e3b8eeae87ae09e4686f3ce1214c964b512f1e Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 15 Apr 2024 11:56:33 -0700 Subject: [PATCH 1/3] chore: remove legacy filter flags --- crates/turborepo-lib/src/cli/mod.rs | 88 +------------------ crates/turborepo-lib/src/opts.rs | 128 +--------------------------- 2 files changed, 6 insertions(+), 210 deletions(-) diff --git a/crates/turborepo-lib/src/cli/mod.rs b/crates/turborepo-lib/src/cli/mod.rs index 063c3d9346074..beefa6c27ad30 100644 --- a/crates/turborepo-lib/src/cli/mod.rs +++ b/crates/turborepo-lib/src/cli/mod.rs @@ -685,11 +685,6 @@ pub struct RunArgs { #[clap(short = 'F', long, group = "scope-filter-group")] pub filter: Vec, - /// DEPRECATED: Specify package(s) to act as entry - /// points for task execution. Supports globs. - #[clap(long, group = "scope-filter-group")] - pub scope: Vec, - // ignore filters out files from scope and filter, so we require it here // ----------------------- /// Files to ignore when calculating changed files from '--filter'. @@ -697,26 +692,6 @@ pub struct RunArgs { #[clap(long, requires = "scope-filter-group")] pub ignore: Vec, - // since only works with scope, so we require it here - // ----------------------- - /// DEPRECATED: Limit/Set scope to changed packages - /// since a mergebase. This uses the git diff ${target_branch}... - /// mechanism to identify which packages have changed. - #[clap(long, requires = "scope")] - pub since: Option, - - // include_dependencies only works with scope, so we require it here - // ----------------------- - /// DEPRECATED: Include the dependencies of tasks in execution. - #[clap(long, requires = "scope")] - pub include_dependencies: bool, - - // no_deps only works with scope, so we require it here - // ----------------------- - /// DEPRECATED: Exclude dependent task consumers from execution. - #[clap(long, requires = "scope")] - pub no_deps: bool, - /// Avoid saving task results to the cache. Useful for development/watch /// tasks. #[clap(long)] @@ -829,9 +804,7 @@ impl RunArgs { // default to true track_usage!(telemetry, self.continue_execution, |val| val); - track_usage!(telemetry, self.include_dependencies, |val| val); track_usage!(telemetry, self.single_package, |val| val); - track_usage!(telemetry, self.no_deps, |val| val); track_usage!(telemetry, self.no_cache, |val| val); track_usage!(telemetry, self.daemon, |val| val); track_usage!(telemetry, self.no_daemon, |val| val); @@ -844,7 +817,6 @@ impl RunArgs { track_usage!(telemetry, &self.cache_dir, Option::is_some); track_usage!(telemetry, &self.profile, Option::is_some); track_usage!(telemetry, &self.force, Option::is_some); - track_usage!(telemetry, &self.since, Option::is_some); track_usage!(telemetry, &self.pkg_inference_root, Option::is_some); track_usage!(telemetry, &self.anon_profile, Option::is_some); track_usage!(telemetry, &self.summarize, Option::is_some); @@ -894,10 +866,6 @@ impl RunArgs { telemetry.track_arg_value("filter:length", self.filter.len(), EventType::NonSensitive); } - if !self.scope.is_empty() { - telemetry.track_arg_value("scope:length", self.scope.len(), EventType::NonSensitive); - } - if !self.ignore.is_empty() { telemetry.track_arg_value("ignore:length", self.ignore.len(), EventType::NonSensitive); } @@ -1685,19 +1653,6 @@ mod test { } ; "multiple ignores" )] - #[test_case::test_case( - &["turbo", "run", "build", "--scope", "test", "--include-dependencies"], - Args { - command: Some(Command::Run(Box::new(RunArgs { - tasks: vec!["build".to_string()], - include_dependencies: true, - scope: vec!["test".to_string()], - ..get_default_run_args() - }))), - ..Args::default() - } ; - "include dependencies" - )] #[test_case::test_case( &["turbo", "run", "build", "--no-cache"], Args { @@ -1731,19 +1686,6 @@ mod test { ..Args::default() } )] - #[test_case::test_case( - &["turbo", "run", "build", "--scope", "test", "--no-deps"], - Args { - command: Some(Command::Run(Box::new(RunArgs { - tasks: vec!["build".to_string()], - scope: vec!["test".to_string()], - no_deps: true, - ..get_default_run_args() - }))), - ..Args::default() - } ; - "no deps" - )] #[test_case::test_case( &["turbo", "run", "build", "--output-logs", "full"], Args { @@ -1914,30 +1856,6 @@ mod test { } ; "remote_only=false works" )] - #[test_case::test_case( - &["turbo", "run", "build", "--scope", "foo", "--scope", "bar"], - Args { - command: Some(Command::Run(Box::new(RunArgs { - tasks: vec!["build".to_string()], - scope: vec!["foo".to_string(), "bar".to_string()], - ..get_default_run_args() - }))), - ..Args::default() - } - )] - #[test_case::test_case( - &["turbo", "run", "build", "--scope", "test", "--since", "foo"], - Args { - command: Some(Command::Run(Box::new(RunArgs { - tasks: vec!["build".to_string()], - scope: vec!["test".to_string()], - since: Some("foo".to_string()), - ..get_default_run_args() - }))), - ..Args::default() - } ; - "scope and since" - )] #[test_case::test_case( &["turbo", "build"], Args { @@ -1983,17 +1901,17 @@ mod test { )] #[test_case::test_case( &["turbo", "run", "build", "--since", "foo"], - "the following required arguments were not provided" ; + "unexpected argument '--since' found" ; "since without filter or scope" )] #[test_case::test_case( &["turbo", "run", "build", "--include-dependencies"], - "the following required arguments were not provided" ; + "unexpected argument '--include-dependencies' found" ; "include-dependencies without filter or scope" )] #[test_case::test_case( &["turbo", "run", "build", "--no-deps"], - "the following required arguments were not provided" ; + "unexpected argument '--no-deps' found" ; "no-deps without filter or scope" )] fn test_parse_run_failures(args: &[&str], expected: &str) { diff --git a/crates/turborepo-lib/src/opts.rs b/crates/turborepo-lib/src/opts.rs index 1e90b4b87e387..dd9d56c1ccc36 100644 --- a/crates/turborepo-lib/src/opts.rs +++ b/crates/turborepo-lib/src/opts.rs @@ -46,11 +46,6 @@ impl Opts { cmd.push_str(pattern); } - for pattern in &self.scope_opts.legacy_filter.as_filter_pattern() { - cmd.push_str(" --filter="); - cmd.push_str(pattern); - } - if self.run_opts.parallel { cmd.push_str(" --parallel"); } @@ -262,54 +257,9 @@ impl From for ResolvedLogPrefix { } } -// LegacyFilter holds the options in use before the filter syntax. They have -// their own rules for how they are compiled into filter expressions. -#[derive(Debug, Default)] -pub struct LegacyFilter { - // include_dependencies is whether to include pkg.dependencies in execution (defaults to false) - include_dependencies: bool, - // skip_dependents is whether to skip dependent impacted consumers in execution (defaults to - // false) - skip_dependents: bool, - // entrypoints is a list of package entrypoints - entrypoints: Vec, - // since is the git ref used to calculate changed packages - since: Option, -} - -impl LegacyFilter { - pub fn as_filter_pattern(&self) -> Vec { - let prefix = if self.skip_dependents { "" } else { "..." }; - let suffix = if self.include_dependencies { "..." } else { "" }; - if self.entrypoints.is_empty() { - if let Some(since) = self.since.as_ref() { - vec![format!("{}[{}]{}", prefix, since, suffix)] - } else { - Vec::new() - } - } else { - let since = self - .since - .as_ref() - .map_or_else(String::new, |s| format!("...[{}]", s)); - self.entrypoints - .iter() - .map(|pattern| { - if pattern.starts_with('!') { - pattern.to_owned() - } else { - format!("{}{}{}{}", prefix, pattern, since, suffix) - } - }) - .collect() - } - } -} - #[derive(Debug)] pub struct ScopeOpts { pub pkg_inference_root: Option, - pub legacy_filter: LegacyFilter, pub global_deps: Vec, pub filter_patterns: Vec, pub ignore_patterns: Vec, @@ -325,16 +275,9 @@ impl<'a> TryFrom<&'a RunArgs> for ScopeOpts { .map(AnchoredSystemPathBuf::from_raw) .transpose()?; - let legacy_filter = LegacyFilter { - include_dependencies: args.include_dependencies, - skip_dependents: args.no_deps, - entrypoints: args.scope.clone(), - since: args.since.clone(), - }; Ok(Self { global_deps: args.global_deps.clone(), pkg_inference_root, - legacy_filter, filter_patterns: args.filter.clone(), ignore_patterns: args.ignore.clone(), }) @@ -363,11 +306,7 @@ impl RunOpts { impl ScopeOpts { pub fn get_filters(&self) -> Vec { - [ - self.filter_patterns.clone(), - self.legacy_filter.as_filter_pattern(), - ] - .concat() + self.filter_patterns.clone() } } @@ -376,36 +315,12 @@ mod test { use test_case::test_case; use turborepo_cache::CacheOpts; - use super::{LegacyFilter, RunOpts}; + use super::RunOpts; use crate::{ cli::DryRunMode, opts::{Opts, RunCacheOpts, ScopeOpts}, }; - #[test_case(LegacyFilter { - include_dependencies: true, - skip_dependents: false, - entrypoints: vec![], - since: Some("since".to_string()), - }, &["...[since]..."])] - #[test_case(LegacyFilter { - include_dependencies: false, - skip_dependents: true, - entrypoints: vec![], - since: Some("since".to_string()), - }, &["[since]"])] - #[test_case(LegacyFilter { - include_dependencies: false, - skip_dependents: true, - entrypoints: vec!["entry".to_string()], - since: Some("since".to_string()), - }, &["entry...[since]"])] - fn basic_legacy_filter_pattern(filter: LegacyFilter, expected: &[&str]) { - assert_eq!( - filter.as_filter_pattern(), - expected.iter().map(|s| s.to_string()).collect::>() - ) - } #[derive(Default)] struct TestCaseOpts { filter_patterns: Vec, @@ -415,7 +330,6 @@ mod test { parallel: bool, continue_on_error: bool, dry_run: Option, - legacy_filter: Option, } #[test_case(TestCaseOpts { @@ -443,47 +357,13 @@ mod test { )] #[test_case( TestCaseOpts { - legacy_filter: Some(LegacyFilter { - include_dependencies: false, - skip_dependents: true, - entrypoints: vec!["my-app".to_string()], - since: None, - }), - tasks: vec!["build".to_string()], - pass_through_args: vec!["-v".to_string(), "--foo=bar".to_string()], - ..Default::default() - }, - "turbo run build --filter=my-app -- -v --foo=bar" - )] - #[test_case( - TestCaseOpts { - legacy_filter: Some(LegacyFilter { - include_dependencies: false, - skip_dependents: true, - entrypoints: vec!["my-app".to_string()], - since: None, - }), - filter_patterns: vec!["other-app".to_string()], + filter_patterns: vec!["other-app".to_string(), "my-app".to_string()], tasks: vec!["build".to_string()], pass_through_args: vec!["-v".to_string(), "--foo=bar".to_string()], ..Default::default() }, "turbo run build --filter=other-app --filter=my-app -- -v --foo=bar" )] - #[test_case ( - TestCaseOpts { - legacy_filter: Some(LegacyFilter { - include_dependencies: true, - skip_dependents: false, - entrypoints: vec!["my-app".to_string()], - since: Some("some-ref".to_string()), - }), - filter_patterns: vec!["other-app".to_string()], - tasks: vec!["build".to_string()], - ..Default::default() - }, - "turbo run build --filter=other-app --filter=...my-app...[some-ref]..." - )] #[test_case ( TestCaseOpts { filter_patterns: vec!["my-app".to_string()], @@ -535,10 +415,8 @@ mod test { }; let cache_opts = CacheOpts::default(); let runcache_opts = RunCacheOpts::default(); - let legacy_filter = opts_input.legacy_filter.unwrap_or_default(); let scope_opts = ScopeOpts { pkg_inference_root: None, - legacy_filter, global_deps: vec![], filter_patterns: opts_input.filter_patterns, ignore_patterns: vec![], From a57a7f8fb41f4bfb5255b6761cdab543892051ad Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 15 Apr 2024 12:08:10 -0700 Subject: [PATCH 2/3] chore: update integration tests --- turborepo-tests/integration/tests/bad-flag.t | 2 +- .../integration/tests/conflicting-flags.t | 32 ++----------------- turborepo-tests/integration/tests/no-args.t | 8 ----- .../integration/tests/turbo-help.t | 16 ---------- 4 files changed, 4 insertions(+), 54 deletions(-) diff --git a/turborepo-tests/integration/tests/bad-flag.t b/turborepo-tests/integration/tests/bad-flag.t index 8588d75503c65..0fb9de1a3dcf8 100644 --- a/turborepo-tests/integration/tests/bad-flag.t +++ b/turborepo-tests/integration/tests/bad-flag.t @@ -19,7 +19,7 @@ Bad flag with an implied run command should display run flags tip: to pass '--bad-flag' as a value, use '-- --bad-flag' - Usage: turbo(\.exe)? <--cache-dir |--cache-workers |--concurrency |--continue|--dry-run []|--single-package|--filter |--force []|--framework-inference []|--global-deps |--graph []|--env-mode []|--ignore |--include-dependencies|--no-cache|--no-daemon|--no-deps|--output-logs |--log-order |--only|--parallel|--pkg-inference-root |--profile |--remote-only []|--scope |--since |--summarize []|--log-prefix |TASKS|PASS_THROUGH_ARGS|--experimental-space-id > (re) + Usage: turbo(\.exe)? <--cache-dir |--cache-workers |--concurrency |--continue|--dry-run []|--single-package|--filter |--force []|--framework-inference []|--global-deps |--graph []|--env-mode []|--ignore |--no-cache|--no-daemon|--output-logs |--log-order |--only|--parallel|--pkg-inference-root |--profile |--remote-only []|--summarize []|--log-prefix |TASKS|PASS_THROUGH_ARGS|--experimental-space-id > (re) For more information, try '--help'. diff --git a/turborepo-tests/integration/tests/conflicting-flags.t b/turborepo-tests/integration/tests/conflicting-flags.t index e293c65f8d198..8b587b27dd88c 100644 --- a/turborepo-tests/integration/tests/conflicting-flags.t +++ b/turborepo-tests/integration/tests/conflicting-flags.t @@ -8,38 +8,12 @@ Setup For more information, try '--help'. [1] - $ ${TURBO} run build --since main - ERROR the following required arguments were not provided: - --scope - - Usage: turbo(\.exe)? run --scope --since (re) - - For more information, try '--help'. - - [1] + $ ${TURBO} run build --ignore 'app/**' ERROR the following required arguments were not provided: - <--filter |--scope > - - Usage: turbo(\.exe)? run --ignore <--filter |--scope > (re) - - For more information, try '--help'. - - [1] - $ ${TURBO} run build --no-deps - ERROR the following required arguments were not provided: - --scope - - Usage: turbo(\.exe)? run --scope --no-deps (re) - - For more information, try '--help'. - - [1] - $ ${TURBO} run build --include-dependencies - ERROR the following required arguments were not provided: - --scope + <--filter > - Usage: turbo(\.exe)? run --scope --include-dependencies (re) + Usage: turbo(\.exe)? run --ignore <--filter > (re) For more information, try '--help'. diff --git a/turborepo-tests/integration/tests/no-args.t b/turborepo-tests/integration/tests/no-args.t index 152ac332f9d78..6be57317db936 100644 --- a/turborepo-tests/integration/tests/no-args.t +++ b/turborepo-tests/integration/tests/no-args.t @@ -65,16 +65,8 @@ Make sure exit code is 2 when no args are passed Environment variable mode. Use "loose" to pass the entire existing environment. Use "strict" to use an allowlist specified in turbo.json. Use "infer" to defer to existence of "passThroughEnv" or "globalPassThroughEnv" in turbo.json. (default infer) [default: infer] [possible values: infer, loose, strict] -F, --filter Use the given selector to specify package(s) to act as entry points. The syntax mirrors pnpm's syntax, and additional documentation and examples can be found in turbo's documentation https://turbo.build/repo/docs/reference/command-line-reference/run#--filter - --scope - DEPRECATED: Specify package(s) to act as entry points for task execution. Supports globs --ignore Files to ignore when calculating changed files from '--filter'. Supports globs - --since - DEPRECATED: Limit/Set scope to changed packages since a mergebase. This uses the git diff ${target_branch}... mechanism to identify which packages have changed - --include-dependencies - DEPRECATED: Include the dependencies of tasks in execution - --no-deps - DEPRECATED: Exclude dependent task consumers from execution --no-cache Avoid saving task results to the cache. Useful for development/watch tasks --daemon diff --git a/turborepo-tests/integration/tests/turbo-help.t b/turborepo-tests/integration/tests/turbo-help.t index 3619cf9a271fb..d0a01347b1d5c 100644 --- a/turborepo-tests/integration/tests/turbo-help.t +++ b/turborepo-tests/integration/tests/turbo-help.t @@ -65,16 +65,8 @@ Test help flag Environment variable mode. Use "loose" to pass the entire existing environment. Use "strict" to use an allowlist specified in turbo.json. Use "infer" to defer to existence of "passThroughEnv" or "globalPassThroughEnv" in turbo.json. (default infer) [default: infer] [possible values: infer, loose, strict] -F, --filter Use the given selector to specify package(s) to act as entry points. The syntax mirrors pnpm's syntax, and additional documentation and examples can be found in turbo's documentation https://turbo.build/repo/docs/reference/command-line-reference/run#--filter - --scope - DEPRECATED: Specify package(s) to act as entry points for task execution. Supports globs --ignore Files to ignore when calculating changed files from '--filter'. Supports globs - --since - DEPRECATED: Limit/Set scope to changed packages since a mergebase. This uses the git diff ${target_branch}... mechanism to identify which packages have changed - --include-dependencies - DEPRECATED: Include the dependencies of tasks in execution - --no-deps - DEPRECATED: Exclude dependent task consumers from execution --no-cache Avoid saving task results to the cache. Useful for development/watch tasks --daemon @@ -170,16 +162,8 @@ Test help flag Environment variable mode. Use "loose" to pass the entire existing environment. Use "strict" to use an allowlist specified in turbo.json. Use "infer" to defer to existence of "passThroughEnv" or "globalPassThroughEnv" in turbo.json. (default infer) [default: infer] [possible values: infer, loose, strict] -F, --filter Use the given selector to specify package(s) to act as entry points. The syntax mirrors pnpm's syntax, and additional documentation and examples can be found in turbo's documentation https://turbo.build/repo/docs/reference/command-line-reference/run#--filter - --scope - DEPRECATED: Specify package(s) to act as entry points for task execution. Supports globs --ignore Files to ignore when calculating changed files from '--filter'. Supports globs - --since - DEPRECATED: Limit/Set scope to changed packages since a mergebase. This uses the git diff ${target_branch}... mechanism to identify which packages have changed - --include-dependencies - DEPRECATED: Include the dependencies of tasks in execution - --no-deps - DEPRECATED: Exclude dependent task consumers from execution --no-cache Avoid saving task results to the cache. Useful for development/watch tasks --daemon From 13b2cf6cd500bf4fcf519b15ba97623591c13005 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 15 Apr 2024 14:46:35 -0700 Subject: [PATCH 3/3] fix: remove deprecated filter args warning --- crates/turborepo-lib/src/cli/mod.rs | 34 ----------------------------- 1 file changed, 34 deletions(-) diff --git a/crates/turborepo-lib/src/cli/mod.rs b/crates/turborepo-lib/src/cli/mod.rs index beefa6c27ad30..0cc7d4e7f9779 100644 --- a/crates/turborepo-lib/src/cli/mod.rs +++ b/crates/turborepo-lib/src/cli/mod.rs @@ -1020,7 +1020,6 @@ pub async fn run( root_telemetry.track_cpus(num_cpus::get()); // track args cli_args.track(&root_telemetry); - warn_all_deprecated_flags(&cli_args); let cli_result = match cli_args.command.as_ref().unwrap() { Command::Bin { .. } => { @@ -1231,39 +1230,6 @@ pub async fn run( cli_result } -fn warn_all_deprecated_flags(args: &Args) { - if args.trace.is_some() { - warn_flag_removal("--trace"); - } - - if args.heap.is_some() { - warn_flag_removal("--heap"); - } - - if args.cpu_profile.is_some() { - warn_flag_removal("--cpuprofile"); - } - - if let Some(Command::Run(run_args)) = args.command.as_ref() { - if run_args.since.is_some() { - warn_flag_removal("--since"); - } - if !run_args.scope.is_empty() { - warn_flag_removal("--scope"); - } - if run_args.include_dependencies { - warn_flag_removal("--include-dependencies"); - } - if run_args.no_deps { - warn_flag_removal("--no-deps"); - } - } -} - -fn warn_flag_removal(flag: &str) { - warn!("{flag} is deprecated and will be removed in 2.0"); -} - #[cfg(test)] mod test { use std::assert_matches::assert_matches;