Skip to content

Commit

Permalink
chore: remove legacy filter flags (#7970)
Browse files Browse the repository at this point in the history 
Remove legacy filter flags now that they've been deprecated for quite
awhile.

Existing test suite passes. Updated tests that are still applicable and
removed those that aren't.
  • Loading branch information
@chris-olszewski
chris-olszewski committed May 10, 2024
1 parent cc565e8 commit 7130618
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 308 deletions.
150 changes: 21 additions & 129 deletions crates/turborepo-lib/src/cli/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,37 +688,30 @@ pub struct ExecutionArgs {
#[clap(short = 'F', long, group = "scope-filter-group")]
pub filter: Vec<String>,

/// DEPRECATED: Specify package(s) to act as entry
/// points for task execution. Supports globs.
#[clap(long, group = "scope-filter-group")]
pub scope: Vec<String>,

// ignore filters out files from scope and filter, so we require it here
// -----------------------
/// Files to ignore when calculating changed files from '--filter'.
/// Supports globs.
#[clap(long, requires = "scope-filter-group")]
pub ignore: Vec<String>,

// 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<String>,
/// Avoid saving task results to the cache. Useful for development/watch
/// tasks.
#[clap(long)]
pub no_cache: bool,

// include_dependencies only works with scope, so we require it here
// clap does not have negation flags such as --daemon and --no-daemon
// so we need to use a group to enforce that only one of them is set.
// we set the long name as [no-]daemon with an alias of daemon such
// that we can merge the help text together for both flags
// -----------------------
/// DEPRECATED: Include the dependencies of tasks in execution.
#[clap(long, requires = "scope")]
pub include_dependencies: bool,
#[clap(long = "daemon", group = "daemon-group")]
daemon: 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,
/// Force turbo to either use or not use the local daemon. If unset
/// turbo will use the default detection logic.
#[clap(long, group = "daemon-group")]
no_daemon: bool,

/// Set type of process output logging. Use "full" to show
/// all output. Use "hash-only" to show only turbo-computed
Expand Down Expand Up @@ -765,14 +758,14 @@ impl ExecutionArgs {
track_usage!(telemetry, self.framework_inference, |val: bool| !val);

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);
track_usage!(telemetry, self.only, |val| val);
track_usage!(telemetry, self.remote_only, |val| val);
track_usage!(telemetry, &self.cache_dir, 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);

if let Some(concurrency) = &self.concurrency {
Expand Down Expand Up @@ -808,10 +801,6 @@ impl ExecutionArgs {
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);
}
Expand Down Expand Up @@ -1110,7 +1099,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 { .. } => {
Expand Down Expand Up @@ -1334,39 +1322,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 { execution_args, .. }) = args.command.as_ref() {
if execution_args.since.is_some() {
warn_flag_removal("--since");
}
if !execution_args.scope.is_empty() {
warn_flag_removal("--scope");
}
if execution_args.include_dependencies {
warn_flag_removal("--include-dependencies");
}
if execution_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;
Expand Down Expand Up @@ -1870,22 +1825,6 @@ mod test {
} ;
"multiple ignores"
)]
#[test_case::test_case(
&["turbo", "run", "build", "--scope", "test", "--include-dependencies"],
Args {
command: Some(Command::Run {
execution_args: Box::new(ExecutionArgs {
tasks: vec!["build".to_string()],
include_dependencies: true,
scope: vec!["test".to_string()],
..get_default_execution_args()
}),
run_args: Box::new(get_default_run_args())
}),
..Args::default()
} ;
"include dependencies"
)]
#[test_case::test_case(
&["turbo", "run", "build", "--no-cache"],
Args {
Expand Down Expand Up @@ -1937,22 +1876,6 @@ mod test {
} ;
"daemon"
)]
#[test_case::test_case(
&["turbo", "run", "build", "--scope", "test", "--no-deps"],
Args {
command: Some(Command::Run {
execution_args: Box::new(ExecutionArgs {
tasks: vec!["build".to_string()],
scope: vec!["test".to_string()],
no_deps: true,
..get_default_execution_args()
}),
run_args: Box::new(get_default_run_args())
}),
..Args::default()
} ;
"no deps"
)]
#[test_case::test_case(
&["turbo", "run", "build", "--output-logs", "full"],
Args {
Expand Down Expand Up @@ -2184,38 +2107,7 @@ mod test {
"remote_only=false works"
)]
#[test_case::test_case(
&["turbo", "run", "build", "--scope", "foo", "--scope", "bar"],
Args {
command: Some(Command::Run {
execution_args: Box::new(ExecutionArgs {
tasks: vec!["build".to_string()],
scope: vec!["foo".to_string(), "bar".to_string()],
..get_default_execution_args()
}),
run_args: Box::new(get_default_run_args())
}),
..Args::default()
} ;
"scope"
)]
#[test_case::test_case(
&["turbo", "run", "build", "--scope", "test", "--since", "foo"],
Args {
command: Some(Command::Run {
run_args: Box::new(get_default_run_args()),
execution_args: Box::new(ExecutionArgs {
tasks: vec!["build".to_string()],
scope: vec!["test".to_string()],
since: Some("foo".to_string()),
..get_default_execution_args()
})
}),
..Args::default()
} ;
"scope and since"
)]
#[test_case::test_case(
&["turbo", "build"],
&["turbo", "build"],
Args {
execution_args: Some(ExecutionArgs {
tasks: vec!["build".to_string()],
Expand Down Expand Up @@ -2299,17 +2191,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) {
Expand Down
Loading

0 comments on commit 7130618

Please sign in to comment.