rust-mobile / ndk

Rust bindings to the Android NDK
Apache License 2.0
1.14k stars 112 forks source link

cargo-apk: Add `--device` arg to select adb device by serial #329

Closed Jasper-Bekkers closed 2 years ago

Jasper-Bekkers commented 2 years ago

This feels quite hacky, but it doesn't look like cargo_subcommand has adequate functionality to filter or add new arguments to some of the commands.

This adds support for a --device arg in both cargo apk run and cargo apk gdb to run on a specific device whenever one is present.

MarijnS95 commented 2 years ago

This feels quite hacky, but it doesn't look like cargo_subcommand has adequate functionality to filter or add new arguments to some of the commands.

This is exactly what |_, _| Ok(false) exists for (to filter out remaining arguments being passed to cargo). Granted that's a bit of an ugly API which is why we replaced it with clap in https://github.com/dvc94ch/cargo-subcommand/commit/6c77c51, allowing you to write the usual:

#[derive(Parser)]
struct CargoApkArgs {
    #[clap(flatten)]
    pub subcommand_args: cargo_subcommand::Args,
    /// Device serial to use, from `adb devices`
    #[clap(short, long)]
    pub device: Option<String>,
}

@dvc94ch Was there anything pending for cargo-subcommand that is blocking a release? Or were we pending for a rather substantial change here in cargo-apk to migrate to the new/improved API?

MarijnS95 commented 2 years ago

@dvc94ch Was there anything pending for cargo-subcommand that is blocking a release? Or were we pending for a rather substantial change here in cargo-apk to migrate to the new/improved API?

Found it, it got stale while we shifted to x: https://github.com/rust-windowing/android-ndk-rs/pull/238

dvc94ch commented 2 years ago

Hey @MarijnS95, sorry for being absent. Started a new job. Let me know if you need anything like repo access, if you send an email I'll be more likely to read it than if it's a gh notification.

MarijnS95 commented 2 years ago

@dvc94ch No worries, we already decided that PR would go stale in favour of X, though I might pick it up at some point to keep cargo-apk in a somewhat neat state.

I/we have also been super busy with not much time left for these tools, which doesn't really matter since they (both cargo-apk and xbuild, we use them both!) have been working out just fine for us besides the odd maintenance and usability improvements.

No need for anything else, I have repo access to both xbuld and android-ndk-rs, and even have publisher access for the latter (https://github.com/orgs/rust-windowing/teams/publishers) in case I need to yank a borked Android release (other than that, publishing is automated on GitHub :tada:).