Cinder icon indicating copy to clipboard operation
Cinder copied to clipboard

Beginning the modernisation of the Android build system

Open alias-r-cummins opened this issue 5 years ago • 41 comments

alias-r-cummins avatar Nov 11 '20 20:11 alias-r-cummins

Just to make sure: This is specific to building for android - correct?

MikeGitb avatar Nov 12 '20 10:11 MikeGitb

Yes. This is to build for Android SDK 28 on Android, with NDK.

alias-r-cummins avatar Nov 12 '20 10:11 alias-r-cummins

What's the next step? Is this ready for review?

vinjn avatar Nov 26 '20 02:11 vinjn

This is great to see! Yes I think peer review will be necessary here, I don't personally have an android device to test on, this is best reviewed by those who have been using android on a regular basis.

Question - is the build.gradle file in the root directory a requirement, or is there a way for it to live at proj/android/build.gradle? Due to a growing number of supported platforms and IDEs, we made an effort to move all build / project files into those folders to clean up the root space, with the exception of CMakeLists.txt since that simplifies integration with many IDEs.

richardeakin avatar Dec 02 '20 03:12 richardeakin

This is failing because the CI tests are not providing a CMake 10.2 environment to build on. I have no idea how to resolve this.

Any idea who would be able to change the test criteria?

alias-r-cummins avatar Dec 02 '20 11:12 alias-r-cummins

See here:

https://travis-ci.org/github/cinder/Cinder/jobs/743028693

alias-r-cummins avatar Dec 02 '20 11:12 alias-r-cummins

@richardeakin With regard to testing on Android, everything can be done 100% in the cloud on emulators. Who would I speak to, maybe @andrewfb , about adding an Android CI pipeline? I'd recommend bitrise or buddybuild - pretty sure we can swing an open source license deal.

alias-r-cummins avatar Dec 02 '20 11:12 alias-r-cummins

Any idea who would be able to change the test criteria?

If you know how to efficiently install a newer cmake toolchain on macos (see my comment here: https://github.com/cinder/Cinder/pull/2199#issuecomment-686464461) I think you should just adapt the travis-ci file accrodingly.

That aside:

  1. Does it really have to be cmake 3.10.2 instead of cmake 3.10?
  2. As trivis will apparently stop the free CI service for open source projects (you only get a fixed amount of ci-time once, without automatic renewal), cinder might have to migrate the test infrastructure somewhere else anyway

MikeGitb avatar Dec 02 '20 12:12 MikeGitb

WRT CI, Bitrise have offered Cinder a free plan:

https://blog.bitrise.io/free-developer-plan-features-for-open-source-projects-on-bitrise

alias-r-cummins avatar Dec 03 '20 00:12 alias-r-cummins

CMake 3.10.2 comes with Android Studio on Mac, PC and Linux.

alias-r-cummins avatar Dec 03 '20 00:12 alias-r-cummins

Bitrise CI integration here, first build: https://app.bitrise.io/build/88e8d685acd39547#?tab=log

alias-r-cummins avatar Dec 03 '20 00:12 alias-r-cummins

CMake 3.10.2 comes with Android Studio on Mac, PC and Linux.

Sure, but that doesn't mean the cmake files need 3.10.2 - in particular the ones that aren't specific to android.

MikeGitb avatar Dec 03 '20 07:12 MikeGitb

Continuing discussion from https://github.com/cinder/Cinder/pull/2199

Right. But the object of this exercise is to make it as easy as possible for people who do.

Ideally, and Android developer wants to install Android Studio and nothing else, and be able to build Cinder from source, and deploy it to the Play Store. This is a normal use case.

Of course, but how is a cmake_minimum_required(VERSION 3.10.2) any better in that regard than a cmake_minimum_required(VERSION 3.10)?

Just out of interest, do you mainly write for Windows & Linux?

I'm exclusively writing for Linux and Windows.

With regard to toolchains - I don't know a single Android developer who doesn't use Android studio. It's theoretically possible with a lot of expensive and time consuming custom scripting, but this is best avoided if at all possible. We want Cinder for Android to work as easily as possible, out of the box.

You are aware though that not all users of "cmake cinder" are Android developers - right? If you make changes to the root CML, that affects people that have nothing to do with Android development what so ever.

Now again, if you raise the minimal required cmake version from 3.10.0 to 3.10.2 it doesn't make a difference for me and neither for people running stock Ubuntu 18.04 because they have cmake 3.10.2 too (provided they regularly update their packages). But apparently it would affect some mac-os developers or otherwise the CI wouldn't fail and at the same time, I don't see how Android Devs benefit from it, so why do it?

MikeGitb avatar Dec 03 '20 10:12 MikeGitb

Of course, but how is a cmake_minimum_required(VERSION 3.10.2) any better in that regard than a >cmake_minimum_required(VERSION 3.10)?

Obviously, I have no intention of breaking other people's builds - this is why I'm on a new, android specific branch.

I'm exclusively writing for Linux and Windows.

OK, great. What toolchain do you normally use on Linux?

I'm prioritizing the Linux support first, because generally if Linux works, it's easier to port that to Mac OS X and Windows as a next step.

You are aware though that not all users of "cmake cinder" are Android developers - right? If you make changes to the root CML, that affects people that have nothing to do with Android development what so ever.

Of course - which is why we're having this discussion and considering the benefits to Android developers of not having to use CMake 3.10.

But apparently it would affect some mac-os developers or otherwise the CI wouldn't fail and at the same time, I don't see how Android Devs benefit from it, so why do it?

Basically, because of changes to the NDK build system, it relies on a baseline version CMake 3.10.2, because Gradle requires it to compile the C++ and NDK side of things. The normal development workflow for Android devs it to just download the tools from Android Studio, and let it act as a package manager, compiler and emulator.

While it is theoretically possible to get Android Studio to use 3.10, it would involve an insane amount of customization which would quite quickly become outdated.

If it does break stuff on Mac OS, this is my problem as well - I intend to support Mac OS Android development too.

I hope this clarifies things a little!

alias-r-cummins avatar Dec 03 '20 11:12 alias-r-cummins

So, I am using the new bitrise open source integration in addition to the travis and appveyor - it has the benefit of being able to test both Android and iOS apps in the same pipeline, is easy to use and also means developers can avoid the expensive and time consuming process of buying OS X / iOS / Android devices - bitrise provide all this as a service.

Obviously,the build here is failing, but the question is why it's failing. As far as I can see, the tests are still referring to the boost headers. I'll make it my next task to get those updated. https://app.bitrise.io/build/88e8d685acd39547#?tab=log

alias-r-cummins avatar Dec 03 '20 11:12 alias-r-cummins

Of course - which is why we're having this discussion and considering the benefits to Android developers of not having to use CMake 3.10 .

Why would android devs have to use CMake 3.10 (I assume you mean 3.10.0 as opposed to 3.10.2), just because cmake_minimum_required(VERSION ...) is set to 3.10? It seems to me you are misunderstanding what that line does.

MikeGitb avatar Dec 03 '20 11:12 MikeGitb

If 3.10 is the minimum, we would have to install CMake twice.

alias-r-cummins avatar Dec 03 '20 13:12 alias-r-cummins

What? Why? What exactly do you think cmake_minimum_required(VERSION ...) does and why would you have to isntall cmake twice?

If 3.10 is the minimum you can use the script with any cmake version starting with 3.10.0 and up. That includes 3.10.2 from android studio as well as e.g. 3.18.2 (which is what I'm currently using on windows).

For reference: https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html

MikeGitb avatar Dec 03 '20 15:12 MikeGitb

I understand what the command does. But for simplicity, it's better to have a consistent build.

On Thu, 3 Dec 2020 at 15:21, MikeGitb [email protected] wrote:

What? Why? What exactly do you think cmake_minimum_required(VERSION ...) does and why would you have to isntall cmake twice?

If 3.10 is the minimum you can use the script with any cmake version starting with 3.10.0 and up. That includes 3.10.2 from android studio as well as e.g. 3.18.2 (which is what I'm currently using on windows).

For reference: https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cinder/Cinder/pull/2208#issuecomment-738074535, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOVTHNV4BS5AFAUVC6VOJ63SS6UIXANCNFSM4TSNTYHQ .

alias-r-cummins avatar Dec 03 '20 16:12 alias-r-cummins

I could simply add some Android specific #ifdefs, but that will only prolong the issue.

On Thu, 3 Dec 2020 at 15:21, MikeGitb [email protected] wrote:

What? Why? What exactly do you think cmake_minimum_required(VERSION ...) does and why would you have to isntall cmake twice?

If 3.10 is the minimum you can use the script with any cmake version starting with 3.10.0 and up. That includes 3.10.2 from android studio as well as e.g. 3.18.2 (which is what I'm currently using on windows).

For reference: https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cinder/Cinder/pull/2208#issuecomment-738074535, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOVTHNV4BS5AFAUVC6VOJ63SS6UIXANCNFSM4TSNTYHQ .

alias-r-cummins avatar Dec 03 '20 16:12 alias-r-cummins

I understand what the command does.

As you have claimed that you would have to install two versions of cmake, I don't believe you do. Again, what influence do you think cmake_minimum_required(VERSION 3.10.0) vs cmake_minimum_required(VERSION 3.10.2) has on the build when you are using any version of cmake greater or equal 3.10.2? Hint: The Answer is: "Exactly zero!"

But for simplicity, it's better to have a consistent build.

Why do you think there would be any inconsitency in the build? Inconsitency between what?

I could simply add some Android specific #ifdefs, but that will only prolong the issue.

Why exactly would you do this? What do you expect to achieve? What issue would it prolong? If you could answer my questions I might either be able to understand what the problem is, or explain to you what you are missunderstanding.

Also, are you really expecting any library's cmake file to reqire a minimal version of cmake of exatly 3.10.2, even if it would work perfectly fine with e.g. cmake 3.5? Take any arbitrary three cmake projects on github and chances are very high they have a different minimal requirement (maybe except 2.8.12).

MikeGitb avatar Dec 03 '20 17:12 MikeGitb

Thanks for everybody looking at this and for getting the ball rolling @ufidstudios-ch I wasn't aware of Bitrise option but that is a really promising direction and definitely simplifies maintaining this platform.

My own CMake experience is pretty marginal so I'm constantly having to search whenever I'm interacting with it. However as @MikeGitb mentioned, we're trying to standardize on 3.10 as our baseline, but it seems likely that would be a straightforward revision to this PR. Thanks to everyone looking at this - it's something I'd love to get updated and correct.

andrewfb avatar Dec 03 '20 19:12 andrewfb

@vinjn Sadly, no, this is not ready for review. I have set up a new CI integration which will allow us to test Mac, Windows, iOS and Android in a single pipeline. A quick walkthrough of the bitrise integration can be seen here: https://vimeo.com/487201299

alias-r-cummins avatar Dec 04 '20 14:12 alias-r-cummins

As you have claimed that you would have to install two versions of cmake, I don't believe you do. Again, what influence do you think cmake_minimum_required(VERSION 3.10.0) vs cmake_minimum_required(VERSION 3.10.2) has on the build when you are using any version of cmake greater or equal 3.10.2? Hint: The Answer is: "Exactly zero!"

OK. I may not be 100% correct on this and you're welcome to suggest changes, which I'd be happy to sign off on.

The reason I am using 10.0.2 is, as I've said, is that this is the minimum version supported by Android NDK 28 .Anything else will basically break Android. So, we need a solution. What I want is to break anything that will cause problems on Android, and work out what the strategy is to make them work. Android has an aggressively modernist compiler strategy, so some ancient GNU stuff may need to be dusted off and slightly adapted.

I'd be happy to share a pre-provisioned VMWare linux development image with you so we can agree on a recommended linux development environment for Android - I have strong opinions on this based on years of professional experience.

I'm also happy to accommodate any legacy compatibility requirements people may have, but I need to know exactly what they are. We can't support everything everywhere, but we can develop a strategy to make the transition as painless as possible.

alias-r-cummins avatar Dec 04 '20 14:12 alias-r-cummins

But for simplicity, it's better to have a consistent build.

Why do you think there would be any inconsitency in the build? Inconsitency between what?

If we have a team of developers working on an Android Cinder project, we want their IDEs to be as simple to set up as possible.

Android Studio is a complete and fully featured IDE - it ships with a build system, a package manager, emulators, a C++ compiler, CMake 3.10.2, Android NDK 28, and it should be a matter of downloading it and running the installer.

Once we start adding elaborate CMake configuration steps to the setup, we will alienate a large segment of the user base - remember that Android developers mainly use Java. However they can easily check the "NDK" box and have a working C++ compiler in 2 minutes. However if they then have to embark on a CMake oddessy, many will turn in fright and not use Cinder. This is my reasoning. You may disagree.

alias-r-cummins avatar Dec 04 '20 14:12 alias-r-cummins

Once we start adding elaborate CMake configuration steps to the setup, we will alienate a large segment of the user base - remember that Android developers mainly use Java. However if they then have to embark on a CMake oddessy, many will turn in fright and not use Cinder. This is my reasoning.

I have a feeling we ar running in circles here.

What "CMake configuration steps" exactly do you think would be necessary if the first line is cmake_minimum_required(VERSION 3.10) instead of 3.10.2 ? Please give me a single, concrete example of anything that a andoid dev using a freshly downloaded Android Studio would have to do differently.

FYI: Visual Studio ships with cmake 3.18 and I'm currently using it with libs requiring a minimal cmake version from anything between 2.8.12 to 3.15.0 without a single config change being necessary.

MikeGitb avatar Dec 04 '20 17:12 MikeGitb

You may disagree.

We are not discussion opionins here, not even technical tradeoffs, we are discussing technical facts:

  • You are claiming that having cmake_minimum_required(VERSION 3.10) at the beginning of the cmake script would somehow require android devs to perform some special steps to setup their project/dev environment that wouldn't be necessary if it was cmake_minimum_required(VERSION 3.10.2) (but you are not telling me what exactly that would be).
  • I'm telling you they don't.

One of us is right, the other is wrong.

I'd be happy to share a pre-provisioned VMWare linux development image with you so we can agree on a recommended linux development environment for Android - I have strong opinions on this based on years of professional experience.

I'm not sure, why you think we would disagree on the dev environment for Android. I've no Idea about android development and will certainly not make any recommendations on what to use. But by all means, feel free to share such an image together with some instruction on how to compile a cinder example for it. Then we can check both versions of the cmake script and see whose hypothesis is correct.

MikeGitb avatar Dec 04 '20 17:12 MikeGitb

The sad part ist that this change is really a small and insignificant part of the PR and the discussion is essentailly just a distraction. I hope it doesn't stop others from properly reviewing this PR and it getting merged eventually.

Sorry for derailing this thread

MikeGitb avatar Dec 04 '20 17:12 MikeGitb

To put it differently, will android studio complain if you leave it at cmake_minimum_required(VERSION 3.10), or does this still work?

richardeakin avatar Dec 05 '20 01:12 richardeakin

From a quick glance on it seems like Android Studio ships currently with a patched version of 3.6.2 and a vanilla version of 3.10.2.

If no CMake version is specified on the gradle file, version 3.10.2 will be used.

In all that, setting cmake_minimum_version to something else than 3.10.2 should only act as a hint for Android Studio, should not have any effect ( i.e version 3.10.2 will still be used even if you set 3.10.0 as a minimum as @MikeGitb has tried to explain ) and it could even be overwritten if desired by the user to use a completely different CMake version according to the info here

I think the fastest way to check this would be what Richie just suggested.

PetrosKataras avatar Dec 05 '20 08:12 PetrosKataras