Revert "Add args: Args back to Ndk subcommand to see them in -h"

This reverts commit e2b9db4235138ee7e24d2c57c797a6c85698e0b2, merged as part of 419df14 ("cargo-apk: Reimplement "default" (`--`) subcommand trailing args for `cargo` (#363)").
rust-mobile · Dec 12, 2022 · 98b3516 · 98b3516
1 parent d90f71e
commit 98b3516
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 54 deletions.
diff --git a/cargo-apk/CHANGELOG.md b/cargo-apk/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Unreleased
 
+- Removed known-arg parsing from `cargo apk --` to not make argument flags/values get lost, see also [#375](https://github.com/rust-windowing/android-ndk-rs/issues/375). ([#377](https://github.com/rust-windowing/android-ndk-rs/pull/377))
+
 # 0.9.6 (2022-11-23)
 
 - Profile signing information can now be specified via the `CARGO_APK_<PROFILE>_KEYSTORE` and `CARGO_APK_<PROFILE>_KEYSTORE_PASSWORD` environment variables. The environment variables take precedence over signing information in the cargo manifest. Both environment variables are required except in the case of the `dev` profile, which will fall back to the default password if `CARGO_APK_DEV_KEYSTORE_PASSWORD` is not set. ([#358](https://github.com/rust-windowing/android-ndk-rs/pull/358))

diff --git a/cargo-apk/src/main.rs b/cargo-apk/src/main.rs
@@ -49,13 +49,10 @@ enum ApkSubCmd {
         /// `cargo` subcommand to run
         cargo_cmd: String,
 
-        // This struct will be filled up later by arguments that are intermixed
-        // with unknown args and ended up in `cargo_args` below.
-        #[clap(flatten)]
-        args: Args,
-
         /// Arguments passed to cargo. Some arguments will be used to configure
         /// the environment similar to other `cargo apk` commands
+        // TODO: This enum variant should parse into `Args` as soon as `clap` supports
+        // parsing only unrecognized args into a side-buffer.
         #[clap(trailing_var_arg = true, allow_hyphen_values = true)]
         cargo_args: Vec<String>,
     },
@@ -77,13 +74,13 @@ enum ApkSubCmd {
     Version,
 }
 
-fn split_apk_and_cargo_args(mut args: Args, input: Vec<String>) -> (Args, Vec<String>) {
+fn split_apk_and_cargo_args(input: Vec<String>) -> (Args, Vec<String>) {
     // Clap doesn't support parsing unknown args properly:
     // https://github.com/clap-rs/clap/issues/1404
     // https://github.com/clap-rs/clap/issues/4498
     // Introspect the `Args` struct and extract every known arg, and whether it takes a value. Use
     // this information to separate out known args from unknown args, and re-parse all the known
-    // args into an existing `args: Args` struct instance.
+    // args into an `Args` struct.
 
     let known_args_taking_value = Args::command()
         .get_arguments()
@@ -128,7 +125,7 @@ fn split_apk_and_cargo_args(mut args: Args, input: Vec<String>) -> (Args, Vec<St
     let m = Args::command()
         .no_binary_name(true)
         .get_matches_from(&split_args.apk_args);
-    args.update_from_arg_matches(&m).unwrap();
+    let args = Args::from_arg_matches(&m).unwrap();
     (args, split_args.cargo_args)
 }
 
@@ -152,10 +149,9 @@ fn main() -> anyhow::Result<()> {
         }
         ApkSubCmd::Ndk {
             cargo_cmd,
-            args,
             cargo_args,
         } => {
-            let (args, cargo_args) = split_apk_and_cargo_args(args, cargo_args);
+            let (args, cargo_args) = split_apk_and_cargo_args(cargo_args);
 
             let cmd = Subcommand::new(args.subcommand_args)?;
             let builder = ApkBuilder::from_subcommand(&cmd, args.device)?;
@@ -182,11 +178,11 @@ fn main() -> anyhow::Result<()> {
 
 #[test]
 fn test_split_apk_and_cargo_args() {
-    // Set up a default because cargo-subcommand doesn't derive/implement Default
+    // Set up a default because cargo-subcommand doesn't derive a default
     let args_default = Args::parse_from(std::iter::empty::<&str>());
 
     assert_eq!(
-        split_apk_and_cargo_args(args_default.clone(), vec!["--quiet".to_string()]),
+        split_apk_and_cargo_args(vec!["--quiet".to_string()]),
         (
             Args {
                 subcommand_args: cargo_subcommand::Args {
@@ -200,10 +196,7 @@ fn test_split_apk_and_cargo_args() {
     );
 
     assert_eq!(
-        split_apk_and_cargo_args(
-            args_default.clone(),
-            vec!["unrecognized".to_string(), "--quiet".to_string()]
-        ),
+        split_apk_and_cargo_args(vec!["unrecognized".to_string(), "--quiet".to_string()]),
         (
             Args {
                 subcommand_args: cargo_subcommand::Args {
@@ -217,10 +210,7 @@ fn test_split_apk_and_cargo_args() {
     );
 
     assert_eq!(
-        split_apk_and_cargo_args(
-            args_default.clone(),
-            vec!["--unrecognized".to_string(), "--quiet".to_string()]
-        ),
+        split_apk_and_cargo_args(vec!["--unrecognized".to_string(), "--quiet".to_string()]),
         (
             Args {
                 subcommand_args: cargo_subcommand::Args {
@@ -234,10 +224,7 @@ fn test_split_apk_and_cargo_args() {
     );
 
     assert_eq!(
-        split_apk_and_cargo_args(
-            args_default.clone(),
-            vec!["-p".to_string(), "foo".to_string()]
-        ),
+        split_apk_and_cargo_args(vec!["-p".to_string(), "foo".to_string()]),
         (
             Args {
                 subcommand_args: cargo_subcommand::Args {
@@ -251,15 +238,12 @@ fn test_split_apk_and_cargo_args() {
     );
 
     assert_eq!(
-        split_apk_and_cargo_args(
-            args_default.clone(),
-            vec![
-                "-p".to_string(),
-                "foo".to_string(),
-                "--unrecognized".to_string(),
-                "--quiet".to_string()
-            ]
-        ),
+        split_apk_and_cargo_args(vec![
+            "-p".to_string(),
+            "foo".to_string(),
+            "--unrecognized".to_string(),
+            "--quiet".to_string()
+        ]),
         (
             Args {
                 subcommand_args: cargo_subcommand::Args {
@@ -274,40 +258,34 @@ fn test_split_apk_and_cargo_args() {
     );
 
     assert_eq!(
-        split_apk_and_cargo_args(
-            args_default.clone(),
-            vec![
-                "--no-deps".to_string(),
-                "-p".to_string(),
-                "foo".to_string(),
-                "--unrecognized".to_string(),
-                "--quiet".to_string()
-            ]
-        ),
+        split_apk_and_cargo_args(vec![
+            "--no-deps".to_string(),
+            "-p".to_string(),
+            "foo".to_string(),
+            "--unrecognized".to_string(),
+            "--quiet".to_string()
+        ]),
         (
             Args {
                 subcommand_args: cargo_subcommand::Args {
                     quiet: true,
                     package: vec!["foo".to_string()],
                     ..args_default.subcommand_args.clone()
                 },
-                ..args_default.clone()
+                ..args_default
             },
             vec!["--no-deps".to_string(), "--unrecognized".to_string()]
         )
     );
 
     assert_eq!(
-        split_apk_and_cargo_args(
-            args_default.clone(),
-            vec![
-                "--no-deps".to_string(),
-                "--device".to_string(),
-                "adb:test".to_string(),
-                "--unrecognized".to_string(),
-                "--quiet".to_string()
-            ]
-        ),
+        split_apk_and_cargo_args(vec![
+            "--no-deps".to_string(),
+            "--device".to_string(),
+            "adb:test".to_string(),
+            "--unrecognized".to_string(),
+            "--quiet".to_string()
+        ]),
         (
             Args {
                 subcommand_args: cargo_subcommand::Args {