Remove subprojects logic in favour of convention plugins. #1386
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
CLA added, but I think it needs an owner to re-run the check. As I have a bunch of changes planned here, perhaps someone would be so good as to grant me permissions to run CI on PR creation, which would allow better self-reviews? |
Should be a no-op for Gradle builds and definitely a no-op everywhere else. I didn't manage to test signing still works but will do at some point. No changes in the logic of what is applied to each project and I've kept the convention plugins in Groovy for now for ease of review. This is unfortunately a slightly large change but should be straightforward to review and unlocks the path to other, smaller, incremental gradle improvements. Planned next steps (in no particular order): * Move the NativeBuildInfo logic to a plugin and support ARM Linux. * Simplify boringssl and property file logic * Migrate to a recent Gradle * Use recent Gradle toolchain logic to simplify native builds * Simplify JVM task logic
prbprbprb
force-pushed
the
01_gradle_conventions
branch
from
9e0ce2c to
7e05c9e
Compare
|
Weird, I'm not seeing that failure on MacOS myself... No point reviewing this until I can repro and fix that! |
|
8f0c006 From the commit message: Note, this is fugly, but it's what we have right now. Once |
This was a latent bug all along. When Android SDK is present, android/build.gradle already contains this so openjdk was getting it that way. No idea why it was working on non-SDK builds though... Note, this is fugly, but it's what we have right now. Once this PR lands, I will do a smaller follow-up change which untangles the constants dependency a bit, and when we get to Gradle 9 we can migrate to "proper" C++ toolchains.
prbprbprb
force-pushed
the
01_gradle_conventions
branch
from
098757e to
8f0c006
Compare
|
Apologies for the multiple CI failures! Latest one is also puzzling as my local end-to-end test is to build and publish the uber jar locally, but I must have missed something. |
Missing boringssl convention plugin. Also needs explicit mavenLocal() repository, which we should refactor later to prevent any release accidents. Tidied up the boringssl convention plugin a bit while I was here.
|
Turns out I wasn't testing it as much as I thought I was :/ 42ab667 should fix uberjar publishing both locally (including the Github CI workflow and the script |
|
Woohoo, tests pass! |
Thanks for the review! This should be the only large one in the sequence, and it unblocks smaller self-contained future changes. I don't have perms to merge myself after approval (which is correct), so if someone could do the honours? Don't forget to squash when merging 😆 |
|
No worries. Thanks Pete. Yes, I was just waiting on some cross check internally. Thanks! |
|
Thanks! |
2 participants
Should be a no-op for Gradle builds and definitely a no-op everywhere else. I didn't manage to test signing still works but will do at some point.
No changes in the logic of what is applied to each project and I've kept the convention plugins in Groovy for now for ease of review.
This is unfortunately a slightly large change but should be straightforward to review and unlocks the path to other, smaller, incremental gradle improvements.
Planned next steps (in no particular order):