-
Notifications
You must be signed in to change notification settings - Fork 282
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
Update Gradle config for ARM architectures, initially MacOS. #1100
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
I didn't review the functionality, but the build practices, LGTM
Sure, I'm looking forward to it!
…On Sun, Jan 15, 2023 at 2:12 PM Pete Bentley ***@***.***> wrote:
Thanks, Boris!
I forgot to mention in the commit message that this should mostly resolve
#1034 <#1034> and go some way
towards resolving #1051 <#1051>,
but there's still a long way to go in modernising the Gradle config and
I'll probably ping you during the week if that's oK.
—
Reply to this email directly, view it on GitHub
<#1100 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFWRIEAPCT4UQJC5GHXIP3WSPSRPANCNFSM6AAAAAATW6LVJM>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Those CI failures are real.... Not sure about the testJar one on Linux, will try and reproduce tomorrow, but the MacOS link error is because our CI config doesn't match the changes I made to compile for both x86 and ARM on MacOS. Will fix. |
Can't fix the Mac CI for this until upstream https://boringssl-review.googlesource.com/c/boringssl/+/56065 is resolved. |
Two more small commits to come:
|
Still to do: update build docs. Overall change: * Upgraded to Gradle 6.9.2 * Added explicit architectures (CPU+OS) to build for, i.e. don't assume everything is x86 and just worry about 32bit vs 64bit. * Explicitly drop 32bit Windows support, we effectively had done already anyway. * Building on either MacOS architecture will build for both arches, but only run tests for the current one. Builds and tests on: MacOS x86_64 MacOS ARM Linux x86_64 Windows x86_64
project.configurations.testRuntimeOnly isn't resolvable, use runtimeClasspath instead. Repro'd on MacOS but should fix Linux build.
Update NDK and cnake version. Don't forgt win32.
Much yak shaving: * BoringSSL HEAD requires a recent-ish cmake. * Conscrypt was using Android Gradle plugiun 4.x which defaults to cmake 10 and crashes with later versions * Upgrading to plugin version 7.x requires upgrading to Gradle 7.x This is a minimum viable upgrade to Gradle 7.5 plus plugin 7.4, i.e. just enough to build and remove most warnings. Some sub-projects are broken, but I believe they were broken before (e.g. conscrypt-android-platform) and there is still much scope for optimisation. This update is orthogonal to the ARM changes in google#1100 and once this lands I'll rebase that PR on top of it. Once google#1100 lands we can consider Gradle 8. :)
Much yak shaving: * BoringSSL HEAD requires a recent-ish cmake. * Conscrypt was using Android Gradle plugiun 4.x which defaults to cmake 3.10 and crashes with later versions * Upgrading to plugin version 7.x requires upgrading to Gradle 7.x This is a minimum viable upgrade to Gradle 7.5 plus plugin 7.4, i.e. just enough to build and remove most warnings. Some sub-projects are broken, but I believe they were broken before (e.g. conscrypt-android-platform) and there is still much scope for optimisation. This update is orthogonal to the ARM changes in google#1100 and once this lands I'll rebase that PR on top of it. Once google#1100 lands we can consider Gradle 8. :)
Due to overlapping changes with the Gradle version upgrade, it's non-trivial to rebase this PR against master so I have a new branch that merges the CLs from this change and ignores Gradle changes which I will upload presently. |
Is it possible to fix #858, too? |
We should definitely do that! From a release point of view the question would be what hardware baseline we support as I understand ARM is a bit more fragmented in terms of things like AES hardware support, but we should certainly add some build support ASAP and iterate from there. |
It would be great to have support for AWS Graviton processors. Thanks! |
+1 we are trying to build on AWS Graviton and is running in
|
Pete, do you have any news on this? Thanks! |
Still to do: update build docs.
Overall change:
Builds and tests on:
MacOS x86_64
MacOS ARM
Linux x86_64
Windows x86_64