From 5a56cf2e5bf51a9e189945c6794db58335a47e48 Mon Sep 17 00:00:00 2001 From: Nipunn Koorapati Date: Mon, 9 Aug 2021 10:01:04 -0700 Subject: [PATCH 1/2] Refactor echo cargo subcommand test helper into cargo-test-support --- crates/cargo-test-support/src/tools.rs | 21 ++++++++++++++++++- tests/testsuite/cargo_command.rs | 29 +++++++------------------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/crates/cargo-test-support/src/tools.rs b/crates/cargo-test-support/src/tools.rs index 14400fbff2d..54e29a43f76 100644 --- a/crates/cargo-test-support/src/tools.rs +++ b/crates/cargo-test-support/src/tools.rs @@ -1,6 +1,6 @@ //! Common executables that can be reused by various tests. -use crate::{basic_manifest, paths, project}; +use crate::{basic_manifest, paths, project, Project}; use lazy_static::lazy_static; use std::path::PathBuf; use std::sync::Mutex; @@ -38,3 +38,22 @@ pub fn echo_wrapper() -> PathBuf { *lock = Some(path.clone()); path } + +/// Returns a project which builds a cargo-echo simple subcommand +pub fn echo_subcommand() -> Project { + let p = project() + .at("cargo-echo") + .file("Cargo.toml", &basic_manifest("cargo-echo", "0.0.1")) + .file( + "src/main.rs", + r#" + fn main() { + let args: Vec<_> = ::std::env::args().skip(1).collect(); + println!("{}", args.join(" ")); + } + "#, + ) + .build(); + p.cargo("build").run(); + p +} diff --git a/tests/testsuite/cargo_command.rs b/tests/testsuite/cargo_command.rs index 574a6c94e78..ceaa4b12bd0 100644 --- a/tests/testsuite/cargo_command.rs +++ b/tests/testsuite/cargo_command.rs @@ -7,10 +7,11 @@ use std::path::{Path, PathBuf}; use std::process::Stdio; use std::str; -use cargo_test_support::cargo_process; -use cargo_test_support::paths; use cargo_test_support::registry::Package; -use cargo_test_support::{basic_bin_manifest, basic_manifest, cargo_exe, project, project_in_home}; +use cargo_test_support::tools::echo_subcommand; +use cargo_test_support::{ + basic_bin_manifest, cargo_exe, cargo_process, paths, project, project_in_home, +}; fn path() -> Vec { env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect() @@ -283,31 +284,17 @@ fn cargo_subcommand_env() { #[cargo_test] fn cargo_subcommand_args() { - let p = project() - .at("cargo-foo") - .file("Cargo.toml", &basic_manifest("cargo-foo", "0.0.1")) - .file( - "src/main.rs", - r#" - fn main() { - let args: Vec<_> = ::std::env::args().collect(); - println!("{}", args.join(" ")); - } - "#, - ) - .build(); - - p.cargo("build").run(); - let cargo_foo_bin = p.bin("cargo-foo"); + let p = echo_subcommand(); + let cargo_foo_bin = p.bin("cargo-echo"); assert!(cargo_foo_bin.is_file()); let mut path = path(); path.push(p.target_debug_dir()); let path = env::join_paths(path.iter()).unwrap(); - cargo_process("foo bar -v --help") + cargo_process("echo bar -v --help") .env("PATH", &path) - .with_stdout("[CWD]/cargo-foo/target/debug/cargo-foo[EXE] foo bar -v --help") + .with_stdout("echo bar -v --help") .run(); } From 28e1289eb17358afef67bf6cd784752368c525e5 Mon Sep 17 00:00:00 2001 From: Nipunn Koorapati Date: Thu, 5 Aug 2021 17:21:13 -0700 Subject: [PATCH 2/2] Teach cargo to failfast on recursive/corecursive aliases Eg. [alias] test-1 = test-2 test-2 = test-3 test-3 = test-1 Previously it would stack overflow It pulls out non controversial bits from from #9768 --- src/bin/cargo/cli.rs | 20 ++++++- tests/testsuite/cargo_alias_config.rs | 83 ++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 12346170111..5902fad2973 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -1,3 +1,4 @@ +use anyhow::anyhow; use cargo::core::{features, CliUnstable}; use cargo::{self, drop_print, drop_println, CliResult, Config}; use clap::{AppSettings, Arg, ArgMatches}; @@ -123,7 +124,7 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'", // Global args need to be extracted before expanding aliases because the // clap code for extracting a subcommand discards global options // (appearing before the subcommand). - let (expanded_args, global_args) = expand_aliases(config, args)?; + let (expanded_args, global_args) = expand_aliases(config, args, vec![])?; let (cmd, subcommand_args) = match expanded_args.subcommand() { (cmd, Some(args)) => (cmd, args), _ => { @@ -159,6 +160,7 @@ pub fn get_version_string(is_verbose: bool) -> String { fn expand_aliases( config: &mut Config, args: ArgMatches<'static>, + mut already_expanded: Vec, ) -> Result<(ArgMatches<'static>, GlobalArgs), CliError> { if let (cmd, Some(args)) = args.subcommand() { match ( @@ -197,7 +199,21 @@ fn expand_aliases( let new_args = cli() .setting(AppSettings::NoBinaryName) .get_matches_from_safe(alias)?; - let (expanded_args, _) = expand_aliases(config, new_args)?; + + let (new_cmd, _) = new_args.subcommand(); + already_expanded.push(cmd.to_string()); + if already_expanded.contains(&new_cmd.to_string()) { + // Crash if the aliases are corecursive / unresolvable + return Err(anyhow!( + "alias {} has unresolvable recursive definition: {} -> {}", + already_expanded[0], + already_expanded.join(" -> "), + new_cmd, + ) + .into()); + } + + let (expanded_args, _) = expand_aliases(config, new_args, already_expanded)?; return Ok((expanded_args, global_args)); } } diff --git a/tests/testsuite/cargo_alias_config.rs b/tests/testsuite/cargo_alias_config.rs index ba271624725..6e2804fd200 100644 --- a/tests/testsuite/cargo_alias_config.rs +++ b/tests/testsuite/cargo_alias_config.rs @@ -1,5 +1,8 @@ //! Tests for `[alias]` config command aliases. +use std::env; + +use cargo_test_support::tools::echo_subcommand; use cargo_test_support::{basic_bin_manifest, project}; #[cargo_test] @@ -50,7 +53,7 @@ fn alias_config() { } #[cargo_test] -fn recursive_alias() { +fn dependent_alias() { let p = project() .file("Cargo.toml", &basic_bin_manifest("foo")) .file("src/main.rs", "fn main() {}") @@ -73,6 +76,84 @@ fn recursive_alias() { .run(); } +#[cargo_test] +fn default_args_alias() { + let echo = echo_subcommand(); + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config", + r#" + [alias] + echo = "echo --flag1 --flag2" + test-1 = "echo" + build = "build --verbose" + "#, + ) + .build(); + + let mut paths: Vec<_> = env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect(); + paths.push(echo.target_debug_dir()); + let path = env::join_paths(paths).unwrap(); + + p.cargo("echo") + .env("PATH", &path) + .with_status(101) + .with_stderr("error: alias echo has unresolvable recursive definition: echo -> echo") + .run(); + + p.cargo("test-1") + .env("PATH", &path) + .with_status(101) + .with_stderr( + "error: alias test-1 has unresolvable recursive definition: test-1 -> echo -> echo", + ) + .run(); + + // Builtins are not expanded by rule + p.cargo("build") + .with_stderr( + "\ +[WARNING] user-defined alias `build` is ignored, because it is shadowed by a built-in command +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + +#[cargo_test] +fn corecursive_alias() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config", + r#" + [alias] + test-1 = "test-2 --flag1" + test-2 = "test-3 --flag2" + test-3 = "test-1 --flag3" + "#, + ) + .build(); + + p.cargo("test-1") + .with_status(101) + .with_stderr( + "error: alias test-1 has unresolvable recursive definition: test-1 -> test-2 -> test-3 -> test-1", + ) + .run(); + + p.cargo("test-2") + .with_status(101) + .with_stderr( + "error: alias test-2 has unresolvable recursive definition: test-2 -> test-3 -> test-1 -> test-2", + ) + .run(); +} + #[cargo_test] fn alias_list_test() { let p = project()