From 09a8ece8eefbf7d3054a83631d1a0afe00d862f1 Mon Sep 17 00:00:00 2001 From: heisen-li Date: Fri, 31 May 2024 15:02:09 +0800 Subject: [PATCH 1/2] fix:Using --release/debug and --profile together becomes an error --- src/bin/cargo/commands/bench.rs | 2 +- src/bin/cargo/commands/clean.rs | 2 +- src/bin/cargo/commands/install.rs | 2 +- src/bin/cargo/commands/test.rs | 2 +- src/cargo/util/command_prelude.rs | 37 +++++++------------ tests/testsuite/check.rs | 13 +++++++ tests/testsuite/profile_custom.rs | 61 ++++++++++++------------------- 7 files changed, 54 insertions(+), 65 deletions(-) diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs index f6d8766570d..c79d7cb350d 100644 --- a/src/bin/cargo/commands/bench.rs +++ b/src/bin/cargo/commands/bench.rs @@ -63,7 +63,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { args.compile_options(gctx, CompileMode::Bench, Some(&ws), ProfileChecking::Custom)?; compile_opts.build_config.requested_profile = - args.get_profile_name(gctx, "bench", ProfileChecking::Custom)?; + args.get_profile_name("bench", ProfileChecking::Custom)?; let ops = TestOptions { no_run: args.flag("no-run"), diff --git a/src/bin/cargo/commands/clean.rs b/src/bin/cargo/commands/clean.rs index e358b967150..1764c0bca1c 100644 --- a/src/bin/cargo/commands/clean.rs +++ b/src/bin/cargo/commands/clean.rs @@ -146,7 +146,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { gctx, spec: values(args, "package"), targets: args.targets()?, - requested_profile: args.get_profile_name(gctx, "dev", ProfileChecking::Custom)?, + requested_profile: args.get_profile_name("dev", ProfileChecking::Custom)?, profile_specified: args.contains_id("profile") || args.flag("release"), doc: args.flag("doc"), dry_run: args.dry_run(), diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index e188d6a0d67..79e39c163f7 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -196,7 +196,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { )?; compile_opts.build_config.requested_profile = - args.get_profile_name(gctx, "release", ProfileChecking::Custom)?; + args.get_profile_name("release", ProfileChecking::Custom)?; if args.flag("list") { ops::install_list(root, gctx)?; diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index 1815ee62f9f..c2657a78f05 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -74,7 +74,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { args.compile_options(gctx, CompileMode::Test, Some(&ws), ProfileChecking::Custom)?; compile_opts.build_config.requested_profile = - args.get_profile_name(gctx, "test", ProfileChecking::Custom)?; + args.get_profile_name("test", ProfileChecking::Custom)?; // `TESTNAME` is actually an argument of the test binary, but it's // important, so we explicitly mention it and reconfigure. diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 63435b50eaa..11164b8409c 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -558,12 +558,19 @@ Run `{cmd}` to see possible targets." fn get_profile_name( &self, - gctx: &GlobalContext, default: &str, profile_checking: ProfileChecking, ) -> CargoResult { let specified_profile = self._value_of("profile"); + let err_message = |flag: &str| { + format!( + "\ +the `--{flag}` flag can not be specified with the `--profile` flag +Please remove one of the flags." + ) + }; + // Check for allowed legacy names. // This is an early exit, since it allows combination with `--release`. match (specified_profile, profile_checking) { @@ -572,39 +579,23 @@ Run `{cmd}` to see possible targets." // `cargo fix` and `cargo check` has legacy handling of this profile name | (Some(name @ "test"), ProfileChecking::LegacyTestOnly) => { if self.maybe_flag("release") { - gctx.shell().warn( - "the `--release` flag should not be specified with the `--profile` flag\n\ - The `--release` flag will be ignored.\n\ - This was historically accepted, but will become an error \ - in a future release." - )?; + bail!(err_message("release")); } return Ok(InternedString::new(name)); } _ => {} } - let conflict = |flag: &str, equiv: &str, specified: &str| -> anyhow::Error { - anyhow::format_err!( - "conflicting usage of --profile={} and --{flag}\n\ - The `--{flag}` flag is the same as `--profile={equiv}`.\n\ - Remove one flag or the other to continue.", - specified, - flag = flag, - equiv = equiv - ) - }; - let name = match ( self.maybe_flag("release"), self.maybe_flag("debug"), specified_profile, ) { (false, false, None) => default, - (true, _, None | Some("release")) => "release", - (true, _, Some(name)) => return Err(conflict("release", "release", name)), - (_, true, None | Some("dev")) => "dev", - (_, true, Some(name)) => return Err(conflict("debug", "dev", name)), + (true, _, None) => "release", + (true, _, Some(_)) => bail!(err_message("release")), + (_, true, None) => "dev", + (_, true, Some(_)) => bail!(err_message("debug")), // `doc` is separate from all the other reservations because // [profile.doc] was historically allowed, but is deprecated and // has no effect. To avoid potentially breaking projects, it is a @@ -710,7 +701,7 @@ Run `{cmd}` to see possible targets." mode, )?; build_config.message_format = message_format.unwrap_or(MessageFormat::Human); - build_config.requested_profile = self.get_profile_name(gctx, "dev", profile_checking)?; + build_config.requested_profile = self.get_profile_name("dev", profile_checking)?; build_config.build_plan = self.flag("build-plan"); build_config.unit_graph = self.flag("unit-graph"); build_config.future_incompat_report = self.flag("future-incompat-report"); diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index da8239fd748..ab135c7cb21 100644 --- a/tests/testsuite/check.rs +++ b/tests/testsuite/check.rs @@ -289,8 +289,21 @@ fn rustc_check() { // Verify compatible usage of --profile with --release, issue #7488 foo.cargo("rustc --profile check --release -- --emit=metadata") + .with_status(101) + .with_stderr( + "\ +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags.", + ) .run(); + foo.cargo("rustc --profile test --release -- --emit=metadata") + .with_status(101) + .with_stderr( + "\ +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags.", + ) .run(); } diff --git a/tests/testsuite/profile_custom.rs b/tests/testsuite/profile_custom.rs index e906d11ccf3..22481fabdeb 100644 --- a/tests/testsuite/profile_custom.rs +++ b/tests/testsuite/profile_custom.rs @@ -383,10 +383,8 @@ fn conflicting_usage() { .with_status(101) .with_stderr( "\ -error: conflicting usage of --profile=dev and --release -The `--release` flag is the same as `--profile=release`. -Remove one flag or the other to continue. -", +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); @@ -394,21 +392,17 @@ Remove one flag or the other to continue. .with_status(101) .with_stderr( "\ -error: conflicting usage of --profile=release and --debug -The `--debug` flag is the same as `--profile=dev`. -Remove one flag or the other to continue. -", +error: the `--debug` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); p.cargo("rustc --profile=dev --release") + .with_status(101) .with_stderr( "\ -warning: the `--release` flag should not be specified with the `--profile` flag -The `--release` flag will be ignored. -This was historically accepted, but will become an error in a future release. -[COMPILING] foo [..] -[FINISHED] `dev` profile [..] +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags. ", ) .run(); @@ -417,52 +411,45 @@ This was historically accepted, but will become an error in a future release. .with_status(101) .with_stderr( "\ -error: conflicting usage of --profile=dev and --release -The `--release` flag is the same as `--profile=release`. -Remove one flag or the other to continue. -", +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); p.cargo("check --profile=test --release") + .with_status(101) .with_stderr( "\ -warning: the `--release` flag should not be specified with the `--profile` flag -The `--release` flag will be ignored. -This was historically accepted, but will become an error in a future release. -[CHECKING] foo [..] -[FINISHED] `test` profile [..] -", +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); // This is OK since the two are the same. p.cargo("rustc --profile=release --release") + .with_status(101) .with_stderr( "\ -[COMPILING] foo [..] -[FINISHED] `release` profile [..] -", +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); p.cargo("build --profile=release --release") + .with_status(101) .with_stderr( "\ -[FINISHED] `release` profile [..] -", +error: the `--release` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); p.cargo("install --path . --profile=dev --debug") + .with_status(101) .with_stderr( "\ -[INSTALLING] foo [..] -[FINISHED] `dev` profile [..] -[INSTALLING] [..] -[INSTALLED] [..] -[WARNING] be sure to add [..] -", +error: the `--debug` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); @@ -470,10 +457,8 @@ This was historically accepted, but will become an error in a future release. .with_status(101) .with_stderr( "\ -error: conflicting usage of --profile=release and --debug -The `--debug` flag is the same as `--profile=dev`. -Remove one flag or the other to continue. -", +error: the `--debug` flag can not be specified with the `--profile` flag +Please remove one of the flags.", ) .run(); } From 5d8022c9e82490347a732834809106bdf8777616 Mon Sep 17 00:00:00 2001 From: heisen-li Date: Wed, 5 Jun 2024 15:36:06 +0800 Subject: [PATCH 2/2] Use clap's conflict support and keep some command tests --- src/bin/cargo/commands/install.rs | 11 +++-- src/cargo/util/command_prelude.rs | 21 +++------ tests/testsuite/check.rs | 18 +++++--- tests/testsuite/profile_custom.rs | 75 ++++++------------------------- 4 files changed, 40 insertions(+), 85 deletions(-) diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index 79e39c163f7..8aaaeb87b0e 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -85,10 +85,13 @@ pub fn cli() -> Command { ) .arg_features() .arg_parallel() - .arg(flag( - "debug", - "Build in debug mode (with the 'dev' profile) instead of release mode", - )) + .arg( + flag( + "debug", + "Build in debug mode (with the 'dev' profile) instead of release mode", + ) + .conflicts_with("profile"), + ) .arg_redundant_default_mode("release", "install", "debug") .arg_profile("Install artifacts with the specified profile") .arg_target_triple("Build for the target triple") diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 11164b8409c..f40dc85ac67 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -131,7 +131,12 @@ pub trait CommandExt: Sized { ) -> Self { let msg = format!("`--{default_mode}` is the default for `cargo {command}`; instead `--{supported_mode}` is supported"); let value_parser = UnknownArgumentValueParser::suggest(msg); - self._arg(flag(default_mode, "").value_parser(value_parser).hide(true)) + self._arg( + flag(default_mode, "") + .conflicts_with("profile") + .value_parser(value_parser) + .hide(true), + ) } fn arg_targets_all( @@ -226,6 +231,7 @@ pub trait CommandExt: Sized { self._arg( flag("release", release) .short('r') + .conflicts_with("profile") .help_heading(heading::COMPILATION_OPTIONS), ) } @@ -563,14 +569,6 @@ Run `{cmd}` to see possible targets." ) -> CargoResult { let specified_profile = self._value_of("profile"); - let err_message = |flag: &str| { - format!( - "\ -the `--{flag}` flag can not be specified with the `--profile` flag -Please remove one of the flags." - ) - }; - // Check for allowed legacy names. // This is an early exit, since it allows combination with `--release`. match (specified_profile, profile_checking) { @@ -578,9 +576,6 @@ Please remove one of the flags." (Some(name @ ("dev" | "test" | "bench" | "check")), ProfileChecking::LegacyRustc) // `cargo fix` and `cargo check` has legacy handling of this profile name | (Some(name @ "test"), ProfileChecking::LegacyTestOnly) => { - if self.maybe_flag("release") { - bail!(err_message("release")); - } return Ok(InternedString::new(name)); } _ => {} @@ -593,9 +588,7 @@ Please remove one of the flags." ) { (false, false, None) => default, (true, _, None) => "release", - (true, _, Some(_)) => bail!(err_message("release")), (_, true, None) => "dev", - (_, true, Some(_)) => bail!(err_message("debug")), // `doc` is separate from all the other reservations because // [profile.doc] was historically allowed, but is deprecated and // has no effect. To avoid potentially breaking projects, it is a diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index ab135c7cb21..54e11f5d74e 100644 --- a/tests/testsuite/check.rs +++ b/tests/testsuite/check.rs @@ -289,20 +289,26 @@ fn rustc_check() { // Verify compatible usage of --profile with --release, issue #7488 foo.cargo("rustc --profile check --release -- --emit=metadata") - .with_status(101) + .with_status(1) .with_stderr( "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags.", +[ERROR] the argument '--profile ' cannot be used with '--release' + +Usage: cargo[EXE] rustc --profile [ARGS]... + +For more information, try '--help'.", ) .run(); foo.cargo("rustc --profile test --release -- --emit=metadata") - .with_status(101) + .with_status(1) .with_stderr( "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags.", +[ERROR] the argument '--profile ' cannot be used with '--release' + +Usage: cargo[EXE] rustc --profile [ARGS]... + +For more information, try '--help'.", ) .run(); } diff --git a/tests/testsuite/profile_custom.rs b/tests/testsuite/profile_custom.rs index 22481fabdeb..d879507d0ca 100644 --- a/tests/testsuite/profile_custom.rs +++ b/tests/testsuite/profile_custom.rs @@ -380,85 +380,38 @@ fn conflicting_usage() { .build(); p.cargo("build --profile=dev --release") - .with_status(101) + .with_status(1) .with_stderr( "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags.", - ) - .run(); +[ERROR] the argument '--profile ' cannot be used with '--release' - p.cargo("install --profile=release --debug") - .with_status(101) - .with_stderr( - "\ -error: the `--debug` flag can not be specified with the `--profile` flag -Please remove one of the flags.", - ) - .run(); +Usage: cargo[EXE] build --profile - p.cargo("rustc --profile=dev --release") - .with_status(101) - .with_stderr( - "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags. -", +For more information, try '--help'.", ) .run(); - p.cargo("check --profile=dev --release") - .with_status(101) + p.cargo("install --profile=release --debug") + .with_status(1) .with_stderr( "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags.", - ) - .run(); +[ERROR] the argument '--profile ' cannot be used with '--debug' - p.cargo("check --profile=test --release") - .with_status(101) - .with_stderr( - "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags.", - ) - .run(); +Usage: cargo[EXE] install --profile [CRATE[@]]... - // This is OK since the two are the same. - p.cargo("rustc --profile=release --release") - .with_status(101) - .with_stderr( - "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags.", +For more information, try '--help'.", ) .run(); - p.cargo("build --profile=release --release") - .with_status(101) + p.cargo("check --profile=dev --release") + .with_status(1) .with_stderr( "\ -error: the `--release` flag can not be specified with the `--profile` flag -Please remove one of the flags.", - ) - .run(); +[ERROR] the argument '--profile ' cannot be used with '--release' - p.cargo("install --path . --profile=dev --debug") - .with_status(101) - .with_stderr( - "\ -error: the `--debug` flag can not be specified with the `--profile` flag -Please remove one of the flags.", - ) - .run(); +Usage: cargo[EXE] check --profile - p.cargo("install --path . --profile=release --debug") - .with_status(101) - .with_stderr( - "\ -error: the `--debug` flag can not be specified with the `--profile` flag -Please remove one of the flags.", +For more information, try '--help'.", ) .run(); }