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

Beginning the modernisation of the Android build system #2208

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ufidstudios-ch
Copy link

No description provided.

ufidstudios-ch and others added 6 commits March 1, 2020 11:44
Removed plugin based build system
Began work on migrating to gradle aware make.
Updated gradle version to 4.0.1
Removed old android sample app as it needs be replaced with new version once the library is building correctly.
Added new build.gradle files to link modules together and build via gradle and CMake.
Removed some outdated references to WIN32.
@MikeGitb
Copy link
Contributor

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

@ufidstudios-ch
Copy link
Author

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

@vinjn
Copy link
Contributor

vinjn commented Nov 26, 2020

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

@richardeakin richardeakin changed the title Beginning the modernisation of the build system Beginning the modernisation of the Android build system Nov 30, 2020
@richardeakin
Copy link
Collaborator

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.

@ufidstudios-ch
Copy link
Author

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?

@ufidstudios-ch
Copy link
Author

@ufidstudios-ch
Copy link
Author

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

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 2, 2020

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: #2199 (comment)) 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

@ufidstudios-ch
Copy link
Author

WRT CI, Bitrise have offered Cinder a free plan:

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

@ufidstudios-ch
Copy link
Author

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

@ufidstudios-ch
Copy link
Author

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

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 3, 2020

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

MikeGitb commented Dec 3, 2020

Continuing discussion from #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?

@ufidstudios-ch
Copy link
Author

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!

@ufidstudios-ch
Copy link
Author

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

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 3, 2020

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.

@ufidstudios-ch
Copy link
Author

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

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 3, 2020

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

@ufidstudios-ch
Copy link
Author

ufidstudios-ch commented Dec 3, 2020 via email

@ufidstudios-ch
Copy link
Author

ufidstudios-ch commented Dec 3, 2020 via email

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 3, 2020

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

@andrewfb
Copy link
Collaborator

andrewfb commented Dec 3, 2020

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.

@ufidstudios-ch
Copy link
Author

@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

@ufidstudios-ch
Copy link
Author

ufidstudios-ch commented Dec 4, 2020

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.

@ufidstudios-ch
Copy link
Author

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.

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 4, 2020

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

MikeGitb commented Dec 4, 2020

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

MikeGitb commented Dec 4, 2020

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

@richardeakin
Copy link
Collaborator

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

@PetrosKataras
Copy link
Contributor

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.

@ufidstudios-ch
Copy link
Author

ufidstudios-ch commented Dec 5, 2020 via email

@ufidstudios-ch
Copy link
Author

Does it really have to be cmake 3.10.2 instead of cmake 3.10?

Yes. Otherwise Android Studio either will try to use the patched earlier version, which will cause a bunch of non-standard behavior, or else use whatever other version is installed - if none, that will then depend on some external tool, e.g. CMake via snap, Visual Studio on Linux / Mac, Cmake via homebrew... lots of different options to support.

So it's better if Android just breaks immediately, because then it's easy to fix. What we don't want is for Android Studio to use the patched earlier version of CMake - this would be very bad because what happens is it uses the old toolchain, which is a highly customized and brittle mess of shell scripts.

If we're using > 3.10.2, however, it uses Gradle Aware Make, which is more more standardized, uses LLVM instead of GCC and will not do anything strange and Android specific.

The reason it's been so hard to get Android working in the past has been largely because of the patched CMake build system. I spent the last year working with clients who had spent literally millions of dollars on trying to get it working with their existing legacy codebases - believe me, it's better to have a build that takes a little longer to get working than a build that has a whole big tree of possible things that could go wrong.

Hope this clarifies things - I've had this discussion a lot in past, and usually people think I'm insane when I say to standardise on 3.10.2 or higher - but trust me, it's the best way to avoid a world of pain. :D

@richardeakin
Copy link
Collaborator

With cmake_minimum_required(VERSION 3.10) in there, why isn't Android Studio still using the vanilla 3.10.2 version that it ships 3.10.2 is > than 3.10.

I'm not fully engrossed in these changes or the cmake configuration these days since I've been working on some other things (and almost entirely using Visual Studio 2019), I think @PetrosKataras and @MikeGitb are both much more knowledgable than myself in this department. Though I believe we want to stick to cmake 3.10 as the minimum required version since this is all that is available on some linux distros (hence travis breaking).

That said, I still don't understand this issue, why Android Studio would complain about a minimum required version that is lower than the version of cmake it is using. For example, how do you use OpenCV (min cmake version 3.5.1) or GLM (min cmake version 3.2) with android studio?

I do hope we get to the bottom of it though, since it'd be great to have a modern android build system in finally. :)

@ufidstudios-ch
Copy link
Author

So this is a historic thing with the build system. Originally, Android had a seperate toolchain called the NDK, which was mostly made up of custom shell scripts, and a patched version of CMake, called nkd-build.

This was because, in order to build for android, CMake needed access to Android's build system, Gradle.

Love it or hate it, Gradle is required to build Android 99% of apps at the moment - for normal users, anyway - there's a few exceptional cases where it can be circumvented, if one has vast resources to make a custom build system, but we don't.

As you can imagine, this non-standard esoteric build system was difficult to use and time consuming and complex to integrate with existing projects - I know this because helping clients get their legacy C++ builds working on Android has been my main professional source of income for the past year or so.

There is a community developed custom gradle plugin which works with the older NDK versions, which is great, but it would be much better for the build to use as many standardized tools as possible.

Android Studio began supporting CMake in a standard, vanilla config at version 3.10.2. Exactly why is probably lost in the sands of time, but it's a baseline requirement if we are to use the standard CMake.

The biggest problem is that Windows devs don't want to let go of using Visual Studio, and will moan, complain and come up with all sorts of excuses and rationalizations as to why Android Studio is unnecessary / defective / badly designed / killing black metal. (that last one was a joke). I've seen millions of man hours wasted on this problem, when simply using Android Studio would have fixed the problem from day one. MSBuild generally handles fairly advanced CMake configs, and whilst it can happily coexist with Gradle when properly configured, Android Studio is a bit more delicate. The two can step on each others toes a little.

There's no reason that Visual Studio can't be used once the build system is up and running, but it we develop the Android version with Visual Studio / MSBuild and various different versions of CMake as a dependency, we will make life much harder for ourselves and and Android developers who try to use the framework. CMake errors are generally show-stoppers, and the average dev, when confronted with them, will often just give up.

I'm happy to explain all this stuff, and it's a skill I need to work on, so thank you for interrogating me on this - I often just know I'm right and get frustrated that people don't immediately agree with me and somehow know everything I know, so I apologize if I've come across as arrogant the last few days. I just want to get to cutting the code as soon as I can :)

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 7, 2020

@ufidstudios-ch : No one is suggesting to use visual studio for android development. We are using visual studio for devloping Windows (and sometimes Linux) apps not sure why this is so hard to understand.

Also, you keep going on about that the best build system for android is using cmake 3.10.2 No one is disputing that either and no one is suggesting you should use something different.

What I question is if it makes any functional differrence if the ROOT CMake List file in cinder has cmake_minimum_required(3.10) or cmake_minimum_required(3.10.2) as the first line. Nothing more, nothing less. We don't suggest to download or use a different version of cmake than 3.10.2 we don't suggest that you should configure your Andoid IDE differently. It is only about that single line in the CML file.

@richardeakin : FYI: Best I can tell, ubuntu 18.04 is shipped with cmake 3.10.2 (https://launchpad.net/ubuntu/bionic/+source/cmake), so that would be fine there. The CI failure on travis is comming from an MacOS machine.

@ufidstudios-ch
Copy link
Author

OK cool.

What I question is if make any functional differrence if the ROOT CMake List file in cinder has cmake_minimum_required(3.10) or cmake_minimum_required(3.10.2) as the first line. Nothing more nothing less.

My strong suspicion is that it will, but I would be happy to try both configurations in order to get a definitive answer.

@MikeGitb
Copy link
Contributor

MikeGitb commented Dec 7, 2020

@ufidstudios-ch : Ok, so I downloaded Android studio for windows and created a new "native c++" project.

Then I opened the automatically generated file App/cpp/CMakeLists.txt, which I assume would be the equivalent to cinders root CMakeLists.txt (CML) file, if you'd be using cinder in an Andoid project (again, never done this myself, but willing to learn).
There I addded the line message(WARNING "${CMAKE_VERSION}") such that the build console would show me, which version of cmake got actually invoked during build.
Then I changed the top line from cmake_minimum_required(VERSION 3.10.2) to cmake_minimum_required(VERSION 3.10)

Result: Just as I expected, the project used cmake 3.10.2:

Andoird-Native-Build

Now unless, I get step-by-step explanation of how to show that leaving cmake_minimum_required(VERSION 3.10) in the top cinder CML has any observable effect when building for android, thats as far as I'm willing to participate in this discussion. Personally I don't really care what cinder will end up with, because I don't use any dev environments that uses such an old cmake version anyway.

@ufidstudios-ch
Copy link
Author

Cool - it probably would only be a problem if you had another installation of CMake on the development machine -but it seems that's unlikely at this point. Let's leave it at 3.10 for the time being then, and if it causes issues on Windows we can address it further down the line.

@ufidstudios-ch
Copy link
Author

I'd just like to eliminate the potential for using the wrong version at the start, you know what I mean?

@richardeakin
Copy link
Collaborator

@ufidstudios-ch now that it seems there is a consensus, can you update your PR for cmake_minimum_required(VERSION 3.10) throughout. I believe that and a bit of user testing from the community are the major steps left in getting this merged.

@ufidstudios-ch
Copy link
Author

Sure, unfortunately a family friend has suddenly passed away so I can't do it today, but I'll get on it next week.

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

Successfully merging this pull request may close these issues.

None yet

6 participants