RFC: deprecation of cargo-apk/ndk-build in favor of xbuild
dvc94ch opened this issue · 12 comments
Moving this into separate issue to increase discoverability.
- 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.
- 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.
- 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;
- 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 :)
- 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".
Originally posted by @dvc94ch in #238 (comment)
cc cargo-apk contributors/maintainers: @msiglreith @francesca64 @katyo @JasperDeSutter
that would be the first step, really looking forward to get your feedback on this.
Overall I'm quite happy with this:
- Awesome to see automatic downloading of SDK tools, we built a similar thing over the weekend but having it integrated directly is great;
- Not sure how everyone feels about downloading repacked NDKs/SDKs from your repo instead of the "official" version from the Android repo (even if it is straightforward to check how the CI put it together);
- Perhaps I'd like to use the installed NDK on my system instead of redownloading, not sure if that's already configurable;
- Indeed running in the problem of native code not compiling, sent a PR to fix the
sysrootbut it may need some more polishing; - Also missing
libc++_shared.soas you mentioned below, that'll just need to be added; - Great to see logs for the current process and run appear directly!;
- Would be awesome if it could work with
-v colortoo :); - Could use some more verbose logging, in particular what target
Build rustis running for, and what device is deployed to (just to make sure cmdline arguments come through correctly).
And that's where I'm at now. Will need to fix that libc++_shared.so dependency and then we'll see how it looks from there but I think this is as good/close as it gets!
There are a couple of differences between
cargo-apkandx --platform android:2.1
cargo-apkuses readelf to automatically add third party libraries to apk. We could generalize it using llvm-objdump to make it work cross platform.
We'll need to port that indeed to make Android library inclusion work.
2.2 C++ has not been tested, think it's just a matter of including the libc++
As mentioned above, needing libc++_shared.so indeed.
2.3 cmake has not been tested, but think we agreed previously that cmake should fix any issues instead of us working around it
I don't think we discussed cmake before, but we're not a fan of it in our project for requiring every user to install cmake. We've even replaced it with cc in one of our dependencies to be a bit faster and leaner (though that is of course a complete mess, since the project at hand has all their configuration in CMake which we either "replicated" in Rust or just hardcoded 🤮).
2.4 manifest is parsed from the
manifest.ymlandroid: 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)
Sounds like serde coming in clutch, we can just move our original manifest structure from ndk-build in here? Someone just needs to update it with recent PRs.
2.5 ndk-debug is not supported. I have a work in progress
x lldbcommand which should be cross platform. this is on my todo list.
cargo apk gdb was missing symbols for me too, hope this solution works a bit better. If it can somehow integrate with vscode lldb (ie automatically attach or whatever, when ran from the vscode terminal) that'd be amazing, but I have no idea if it's feasible.
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
gradleinstead of a prebaked tool likexorcargo-apk.
I don't use any of this, don't remember if cargo-apk supported it even? Let's focus on app bundles etc first.
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.
Yeah as mentioned above it'd be great if:
- The NDK from official sources can be used;
- A pre-installed NDK (through env vars) can be used instead of redownloading it - maybe even depending on a configured version in
manifest.yml; - It seems
x doctorshows system tools which can also originate from one of the Android sources (adb), perhaps they should be included and loaded from this repackaged/detected "NDK"?
- 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;
- 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 :)
1. is done, and I started with the first bugfix already 😛
- 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)orcargo.use_xwin(path: &Path)orcargo.use_ios_sdk(path: &Path).
Let's deprecate ndk-build too then.
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.rstocargo-subcommand. Another possibility would be to foldcargo-subcommandandxcli/src/cargo/mod.rsinto a cargo utility crate inside thexmonorepo, as I expectxto be the main "consumer".
Is it xcli or xbuild btw 😉?
Importing cargo-subcommand history into the x monorepo sounds like a good idea, yes (if you think there's a good use for it). Perhaps we need to merge or recreate open PRs first though.
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.
We'll need to port that indeed to make Android library inclusion work.
Opened an issue rust-mobile/xbuild#47
A pre-installed NDK (through env vars) can be used instead of redownloading it - maybe even depending on a configured version in manifest.yml;
sounds good
It seems x doctor shows system tools which can also originate from one of the Android sources (adb), perhaps they should be included and loaded from this repackaged/detected "NDK"?
maybe, although most linux distros, brew and choco have a way to install adb natively. I think this one is fine to just pick up from PATH. you need to use the system package manager for installing clang/llvm, openjdk, libimobiledevice anyway, although maybe some day someone builds a tool in rust that works for android, ios and desktop.
Is it xcli or xbuild btw?
Was a last minute name change because xcli was already taken on crates.io
It seems x doctor shows system tools which can also originate from one of the Android sources (adb), perhaps they should be included and loaded from this repackaged/detected "NDK"?
maybe, although most linux distros,
brewandchocohave a way to installadbnatively. I think this one is fine to just pick up fromPATH. you need to use the system package manager for installing clang/llvm, openjdk, libimobiledevice anyway, although maybe some day someone builds a tool in rust that works for android, ios and desktop.
For Windows we'll use clang/llvm from the Android NDK I suppose? Will be much easier to bundle adb there too, instead of having to document how to install adb manually or through one of the many different package managers that exist for Windows nowadays 😞
Is it xcli or xbuild btw?
Was a last minute name change because xcli was already taken on crates.io
Shall we update the folder name too? Was a bit confusing to find xbuild under xcli, I thought that was just a helper crate at first 🙂
Not sure if x should become a package manager. When it comes to setting up rust/clang/llvm, I think since rustup distributes most of it already, adding clang wouldn't be that far fetched. Like brew on macos choco is the defacto windows package manager, I think we can rely on it.
Shall we update the folder name too? Was a bit confusing to find xbuild under xcli, I thought that was just a helper crate at first slightly_smiling_face
Contacted the owner of the squated x crate. Still hopeful we might get a better crate name.
X does not need to become a package manager, it is already repackaging/bundling pieces from SDK/NDK and adb could come with that. I personally have no-one in my circle using choco nor knowing about it much, I guess its popularity depends on the field?
Here's hoping we can take over the x name (not even xcli) 😁
well it's the package manager preinstalled on github action runners (actually that's how I first learned about it. it's just a presumption that it's popular based on it's use by github), which means it's on every windows machine I use 🤷♂️
@MarijnS95 haven't played around with it yet, but there seem to be a few rust adb implementations. Maybe it's not that much work to get rid of the adb dependency [0][1][2]. I'd prefer to look into this approach first.
Justed tested the raqote example from the repository and the experience was quite nice so far, as it mostly worked out of the box (even on Windows!) - including automatic log output 👍
For now my only suggestion would be to re-use as much as possible of the prebuilt tools included in the ndk/sdk on Android, but that's already point of prev. discussion. Having to install llvm/adb (+ other tools?)/choco along with manually putting them into the path env variables on Windows can be annoying and error prone in case of version conflicts.
@MarijnS95 at some point we should pull the trigger on this. what are your current feelings towards rm -rf cargo-apk ndk-build?
That's fine with me, but you can't complain if you don't get help maintaining it.
@dvc94ch I won't complain about needing help, but receiving a review is always appreciated. I do need help reviewing the non-deprecated ndk/ndk-sys crates though 😉
Also keep in mind winit is still using cargo-apk in the CI, CC @rib and @msiglreith.