Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Sep 7, 2022

Conversation

Jasper-Bekkers
Copy link
Contributor

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
Copy link
Member

MarijnS95 commented Sep 5, 2022

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 rust-mobile/cargo-subcommand@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
Copy link
Member

@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: #238

@MarijnS95 MarijnS95 merged commit 282af7d into rust-mobile:master Sep 7, 2022
@MarijnS95 MarijnS95 changed the title Add --device arg to be able to run on specific android devices cargo-apk: Add --device arg to select adb device by serial Sep 7, 2022
@dvc94ch
Copy link
Contributor

dvc94ch commented Sep 7, 2022

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
Copy link
Member

MarijnS95 commented Sep 7, 2022

@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 🎉).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants