Anki-Android-Backend icon indicating copy to clipboard operation
Anki-Android-Backend copied to clipboard

Simplify build setup process

Open voczi opened this issue 1 year ago • 8 comments

Hi! I made these changes to, hopefully, make it easier for people to build the project. I've ran this on Windows and it's worked fine so far. Please let me know if there's any issues with the changes. One thing I am unsure about is whether or not the workflows will work properly after this.

voczi avatar Mar 25 '24 13:03 voczi

Please provide an explanation of why we should change the existing working code. What advantages does this new approach have? What trouble did you have with the existing setup?

dae avatar Mar 26 '24 07:03 dae

Please provide an explanation of why we should change the existing working code. What advantages does this new approach have? What trouble did you have with the existing setup?

The troubles I had with the existing setup was that there were a few hard-coded restrictions set in build_rust for reasons I didn't quite understand. Mostly related to multi-arch builds. The main purpose of these changes is to remove those restrictions and let the user configure the build process themselves. To be fair, running this through Gradle isn't necessary and the options could be set as environment variables instead. So unsure if the main build approach, as documented in the README, really should be switched to building through Gradle instead.

voczi avatar Mar 26 '24 07:03 voczi

Multi-arch builds are restricted to macOS, because you can only legally target macOS from macOS.

The current approach tries to keep the Rust build process independent from Gradle. That avoids any startup time, and makes things easier to debug. Gradle is only required later, when taking the Rust outputs and bundling them into an .aar/.jar.

dae avatar Mar 26 '24 07:03 dae

Yep, that makes sense. But the current Rust code throws a panic when trying to do a multi-arch build, because the multi-arch setting for tests (Robolectric) and regular builds (JNI) is not separated. Anyhow, the code I proposed avoids throwing a panic altogether. Maybe the panic can still be kept, indicating a wrong combination; tests.multi_arch+non-macOS=panic (?) For the other part, I can agree on that Gradle made it harder to debug the cargo builds.

So maybe the Gradle part, of these changes, isn't quite necessary. But should the environment variables be kept for better configuration of the builds?

voczi avatar Mar 26 '24 07:03 voczi

If the user asks for a multi-arch build when it's not possible, I think stopping is the best course of action. In practice, multi-arch builds are only going to be done in CI (or when testing the build process on a Mac machine) - it's not something that's going to get used during regular development.

better configuration of the builds

Sorry, could you explain this better? We have a working build process at the moment. What advantages will these changes bring?

dae avatar Mar 26 '24 07:03 dae

This was more intended for improving local builds, rather than for the automated ones. In my case, I wanted to create a local multi-arch build so I can run any changes on my personal phone (arm64) too.

voczi avatar Mar 26 '24 07:03 voczi

Ok, I see. This was not something I tried to support, as once the code has been tested locally in the emulator, it can be pushed to CI and the artifacts for other platforms can be downloaded from the CI assets.

If you still want the ability to do this locally, a PR that makes the minimum changes necessary to accomplish this would be good. Maybe the introduction of an env var that controls the desired android platform? If not set, it could keep defaulting to the current arch.

dae avatar Mar 26 '24 08:03 dae

That, also, sounds good. I'll convert this to a draft for now and work on it later. Thank you!

voczi avatar Mar 26 '24 11:03 voczi