From 9be10657a0600db9cb73592a539eadb7cf369717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Mi=C5=9Btal?= <37044072+szymmis@users.noreply.github.com> Date: Tue, 24 Oct 2023 15:35:06 +0200 Subject: [PATCH 1/3] Make profile args take precedence over env --- scarb/src/bin/scarb/args.rs | 8 ++++++-- scarb/tests/profiles.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/scarb/src/bin/scarb/args.rs b/scarb/src/bin/scarb/args.rs index 5172479a7..107debe2a 100644 --- a/scarb/src/bin/scarb/args.rs +++ b/scarb/src/bin/scarb/args.rs @@ -3,6 +3,7 @@ //! CLI arguments datastructures. use std::collections::BTreeMap; +use std::env; use std::ffi::OsString; use anyhow::Result; @@ -346,7 +347,7 @@ pub struct GitRefGroup { #[group(multiple = false)] pub struct ProfileSpec { /// Specify profile to use by name. - #[arg(short = 'P', long, env = "SCARB_PROFILE")] + #[arg(short = 'P', long)] pub profile: Option, /// Use release profile. #[arg(long, hide_short_help = true)] @@ -365,7 +366,10 @@ impl ProfileSpec { profile: Some(profile), .. } => Profile::new(profile.clone())?, - _ => Profile::default(), + _ => match env::var_os("SCARB_PROFILE") { + Some(profile) => Profile::new(profile.to_string_lossy().into())?, + None => Profile::default(), + }, }) } } diff --git a/scarb/tests/profiles.rs b/scarb/tests/profiles.rs index 70d843608..0b4bd2261 100644 --- a/scarb/tests/profiles.rs +++ b/scarb/tests/profiles.rs @@ -240,6 +240,33 @@ fn cannot_choose_not_existing_profile() { .stdout_matches("error: workspace `[..]` has no profile `custom`\n"); } +#[test] +fn can_use_short_profile_args_in_scripts() { + let t = TempDir::new().unwrap(); + ProjectBuilder::start() + .name("hello") + .manifest_extra( + r#" + [scripts] + build-dev = "scarb --dev build" + build-release = "scarb --release build" + "#, + ) + .build(&t); + + Scarb::quick_snapbox() + .args(["run", "build-dev"]) + .current_dir(&t) + .assert() + .success(); + + Scarb::quick_snapbox() + .args(["run", "build-release"]) + .current_dir(&t) + .assert() + .success(); +} + #[test] fn sierra_replace_ids_defaults_true_in_dev() { let t = TempDir::new().unwrap(); From 087e0711bd83e24875154e29682a7fee699eef86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Mi=C5=9Btal?= <37044072+szymmis@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:49:11 +0200 Subject: [PATCH 2/3] Allow multiple arsg from profile group --- scarb/src/bin/scarb/args.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/scarb/src/bin/scarb/args.rs b/scarb/src/bin/scarb/args.rs index 107debe2a..b14544513 100644 --- a/scarb/src/bin/scarb/args.rs +++ b/scarb/src/bin/scarb/args.rs @@ -3,7 +3,6 @@ //! CLI arguments datastructures. use std::collections::BTreeMap; -use std::env; use std::ffi::OsString; use anyhow::Result; @@ -344,10 +343,10 @@ pub struct GitRefGroup { /// Profile specifier. #[derive(Parser, Clone, Debug)] -#[group(multiple = false)] +#[group(multiple = true)] pub struct ProfileSpec { /// Specify profile to use by name. - #[arg(short = 'P', long)] + #[arg(short = 'P', long, env = "SCARB_PROFILE")] pub profile: Option, /// Use release profile. #[arg(long, hide_short_help = true)] @@ -366,10 +365,7 @@ impl ProfileSpec { profile: Some(profile), .. } => Profile::new(profile.clone())?, - _ => match env::var_os("SCARB_PROFILE") { - Some(profile) => Profile::new(profile.to_string_lossy().into())?, - None => Profile::default(), - }, + _ => Profile::default(), }) } } From b6f8a5fe5fad25795e3ce6c974e2af8ed2d826fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Mi=C5=9Btal?= <37044072+szymmis@users.noreply.github.com> Date: Wed, 25 Oct 2023 12:00:30 +0200 Subject: [PATCH 3/3] Create sepearate group for `--dev` and `--profile` --- scarb/src/bin/scarb/args.rs | 4 +-- scarb/tests/profiles.rs | 71 +++++++++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/scarb/src/bin/scarb/args.rs b/scarb/src/bin/scarb/args.rs index b14544513..dff3c22e9 100644 --- a/scarb/src/bin/scarb/args.rs +++ b/scarb/src/bin/scarb/args.rs @@ -349,10 +349,10 @@ pub struct ProfileSpec { #[arg(short = 'P', long, env = "SCARB_PROFILE")] pub profile: Option, /// Use release profile. - #[arg(long, hide_short_help = true)] + #[arg(long, hide_short_help = true, group = "ProfileShortcuts")] pub release: bool, /// Use dev profile. - #[arg(long, hide_short_help = true)] + #[arg(long, hide_short_help = true, group = "ProfileShortcuts")] pub dev: bool, } diff --git a/scarb/tests/profiles.rs b/scarb/tests/profiles.rs index 0b4bd2261..e3553de44 100644 --- a/scarb/tests/profiles.rs +++ b/scarb/tests/profiles.rs @@ -132,19 +132,19 @@ fn can_choose_release_by_name() { } #[test] -fn cannot_choose_both_release_and_by_name() { +fn cannot_choose_both_release_and_dev() { let t = TempDir::new().unwrap(); ProjectBuilder::start().name("hello").build(&t); Scarb::quick_snapbox() - .args(["--release", "--profile", "dev","metadata", "--format-version", "1"]) + .args(["--release", "--dev", "metadata", "--format-version", "1"]) .current_dir(&t) .assert() .failure() .stderr_matches(indoc! {r#" - error: the argument '--release' cannot be used with '--profile ' + error: the argument '--release' cannot be used with '--dev' - Usage: scarb[..] --release --global-cache-dir --global-config-dir + Usage: scarb --release [..] For more information, try '--help'. "#}); @@ -241,30 +241,71 @@ fn cannot_choose_not_existing_profile() { } #[test] -fn can_use_short_profile_args_in_scripts() { +fn shortcuts_precede_profile_arg() { + let t = TempDir::new().unwrap(); + ProjectBuilder::start().name("hello").build(&t); + + let metadata = Scarb::quick_snapbox() + .args([ + "--json", + "--release", + "--profile", + "dev", + "metadata", + "--format-version", + "1", + ]) + .current_dir(&t) + .stdout_json::(); + + assert_eq!(metadata.current_profile, "release".to_string()); +} + +#[test] +fn shortcuts_precede_profile_env() { + let t = TempDir::new().unwrap(); + ProjectBuilder::start().name("hello").build(&t); + + let metadata = Scarb::quick_snapbox() + .env("SCARB_PROFILE", "release") + .args(["--json", "--dev", "metadata", "--format-version", "1"]) + .current_dir(&t) + .stdout_json::(); + + assert_eq!(metadata.current_profile, "dev".to_string()); +} + +#[test] +fn can_use_shortcuts_in_scripts() { let t = TempDir::new().unwrap(); ProjectBuilder::start() .name("hello") .manifest_extra( r#" [scripts] - build-dev = "scarb --dev build" - build-release = "scarb --release build" + script-release = "scarb --json --release metadata --format-version 1" + script = "scarb --json metadata --format-version 1" + + [profile.custom] "#, ) .build(&t); - Scarb::quick_snapbox() - .args(["run", "build-dev"]) + let metadata = Scarb::quick_snapbox() + .env("SCARB_PROFILE", "custom") + .args(["run", "script-release"]) .current_dir(&t) - .assert() - .success(); + .stdout_json::(); - Scarb::quick_snapbox() - .args(["run", "build-release"]) + assert_eq!(metadata.current_profile, "release".to_string()); + + let metadata = Scarb::quick_snapbox() + .env("SCARB_PROFILE", "custom") + .args(["run", "script"]) .current_dir(&t) - .assert() - .success(); + .stdout_json::(); + + assert_eq!(metadata.current_profile, "custom".to_string()); } #[test]