Update to cargo-subcommand 0.8.0 with clap argument parser

[dvc94ch]

Fixes #182
Fixes #217

[MarijnS95]

@dvc94ch Now that rust-mobile/cargo-subcommand#15 is in would you mind rebasing this?

[dvc94ch]

@MarijnS95 not sure it's worth it. I'd recommend giving x new helloworld && cd helloworld && x run a try. except ios all platforms should work out of the box, ios should catch up in the next few days now that I got an apple developer subscription. Would appreciate your thoughts on it, and what features you think are missing.

• [0] https://github.com/cloudpeers/x

[MarijnS95]

@dvc94ch Without even looking at the code or testing it out, I really dislike your approach of throwing the status quo out and starting over from scratch.

https://xkcd.com/927/

[dvc94ch]

it has a clearly different scope to cargo-apk. I don't think I'm throwing the status quo out. It doesn't replace ndk or ndk-glue or ndk-context. It's just a build/packaging tool that knows how to build flutter, rust and flutter/rust applications. I don't think extending the scope of cargo-apk would have been the right approach. It's also likely that cargo-apk still has utility in certain niches, especially if you only care about android game development.

[MarijnS95]

In fact, looking at your repo, I see a lot of familiar files. Files I've recently fixed bugs in, or contributed new features to, here in android-ndk-rs or cargo-subcommand.

I am just uncomfortable maintaining the same thing in two disconnected repos, and as such only feel ready to try out and migrate to x if we make a clear deprecation and archival plan for cargo-apk, here and now. ndk-build can stay if and only if it is used in x, otherwise I see no reason to keep the maintenance burden up for that too.

(Fwiw why bring ndk/ndk-glue/ndk-context up, when this issue is clearly only about cargo-apk/ndk-build/cargo-subcommand?)

[dvc94ch]

Files I've recently fixed bugs in, or contributed new features to, here in android-ndk-rs or cargo-subcommand.

I'm aware of that, and I appreciate it.

I see a lot of familiar files

before the renewed effort to maintain cargo-subcommand I did copy some parts. it shares very little or no code with cargo-apk or ndk-build other than some manifest structs apk/src/manifest.rs. that's because it takes a different approach. it implements aapt/apksigner/zipalign in rust more or less. A lot of care was taken to avoid dependencies on the environment, so I decided (maybe naively so) that implementing it in rust would mean that cargo install would just work on all platforms.

I really don't want to force you to use x if you don't like it :)

[dvc94ch]

Fwiw why bring ndk/ndk-glue/ndk-context up, when this issue is clearly only about cargo-apk/ndk-build/cargo-subcommand?

just for clarification, no particular reason.

anyway, I was asking about how to move forward, I'm happy to factor out shared components to decrease the maintainance burden or whatever. for various reasons it happened to turn out like this (it starting out as a research project). now there is a what I believe is a useful tool which I spent 3 months building. but I'm also fine going our separate ways.

[MarijnS95]

anyway, I was asking about how to move forward, I'm happy to factor out shared components to decrease the maintainance burden or whatever. for various reasons it happened to turn out like this (it starting out as a research project). now there is a what I believe is a useful tool which I spent 3 months building. but I'm also fine going our separate ways.

Fair enough, this had to start somewhere. I don't mind factoring out shared components as long x fully supersedes cargo-apk (and I'd love for cargo-ndk features to be possible with it too), and we deprecate it as a consequence. But do read on.

it implements aapt/apksigner/zipalign in rust more or less

I'm very happy about this. Setting those up and having a Java dependency (for at least keytool, but that isn't needed too often) has been a pain, while we want APK building to be as painless (and with the least possible dependencies outside of cargo) as possible.

I really don't want to force you to use x if you don't like it :)

I want to use x if it turns out to be good. What I absolutely don't want is for cargo-apk+ndk-build and x to coexist with significant enough feature overlap to compete, that's a waste of our development time. As such, I think to proceed we should:

1. I (and hopefully I'm not alone) should check out your tool and evaluate how much all the extra automation improves my workflow;
2. At the same time, evaluate what we're missing in contrast to cargo-apk, or just in general;
3. Set up a clear deprecation plan for cargo-apk. I don't want to maintain it and I don't think others should either, they should instead contribute to x;
4. I can perhaps take some of the burden of porting the bugfixes and features from here to x;
5. We deprecate and remove cargo-apk and ndk-build (caveat: we can also decide to keep ndk-build and slim it down to be useful for x and others).

I'm then not sure what to do with cargo-subcommand. Perhaps we need that crate pulled apart to have a separate library for the manifest format and parsing (perhaps including .cargo/config.toml). Some other tools already have/do that, doesn't a separate crate like it exist already?

[dvc94ch]

1. I (and hopefully I'm not alone) should check out your tool and evaluate how much all the extra automation improves my workflow;

that would be the first step, really looking forward to get your feedback on this.

2. At the same time, evaluate what we're missing in contrast to cargo-apk, or just in general;

There are a couple of differences between cargo-apk and x --platform android:

2.1 cargo-apk uses readelf to automatically add third party libraries to apk. We could generalize it using llvm-objdump to make it work cross platform.

2.2 C++ has not been tested, think it's just a matter of including the libc++

2.3 cmake has not been tested, but think we agreed previously that cmake should fix any issues instead of us working around it

2.4 manifest is parsed from the manifest.yml android: manifest: section. this is to enable the same workflow in flutter, rust and flutter/rust apps. should be mostly compatible (superset of existing manifest features converted from toml to yaml)

2.5 ndk-debug is not supported. I have a work in progress x lldb command which should be cross platform. this is on my todo list.

2.6 not all resource stuff is supported in the rust aapt port. mostly just allows setting a icon mipmap and launch screen, as flutter does it's own cross platform resource management. this is probably the biggest downside, but hardcore android devs likely use gradle instead of a prebaked tool like x or cargo-apk.

2.7 x currently repackages the android ndk (basically just takes the sysroot and leaves the rest). this means we currently only support the latest ndk. in the future we can see about supporting multiple ndk/sdk versions accross windows/macos/ios/android, at the moment you just get what we give you.

3. Set up a clear deprecation plan for cargo-apk. I don't want to maintain it and I don't think others should either, they should instead contribute to x;
4. I can perhaps take some of the burden of porting the bugfixes and features from here to x;

That would be cool, but let's start with 1 first :)

5. We deprecate and remove cargo-apk and ndk-build (caveat: we can also decide to keep ndk-build and slim it down to be useful for x and others).

By removing the reliance on shell/python scripts spread out through the ndk and doing everything in rust, ndk-build is not really that useful anymore. Instead there is a bit of code to handle each platform in various places without requiring much special handling for android. You can simply call cargo.use_ndk(path: &Path) or cargo.use_xwin(path: &Path) or cargo.use_ios_sdk(path: &Path).

I'm then not sure what to do with cargo-subcommand. Perhaps we need that crate pulled apart to have a separate library for the manifest format and parsing (perhaps including .cargo/config.toml). Some other tools already have/do that, doesn't a separate crate like it exist already?

Not sure about this one. Can probably move some of the code in xcli/src/cargo/mod.rs to cargo-subcommand. Another possibility would be to fold cargo-subcommand and xcli/src/cargo/mod.rs into a cargo utility crate inside the x monorepo, as I expect x to be the main "consumer".

[MarijnS95]

Reopening and resolving conflicts as I really like to get this cleanup/improvement in. Will re-review and dogfood this on my own working env for a while.

[MarijnS95]

@msiglreith PTAL. @dvc94ch also feel free to check what I made of your changes, if you have the time 😬

[msiglreith]

lgtm, checked it locally and functionality seems to be all there

[MarijnS95]

I'll push out a cargo-subcommand release tomorrow, and merge this while test-driving it locally for a few days. Will then probably release 0.10.0 or 0.9.5 some time next week.