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

Modernize Gradle build #183

Merged
merged 12 commits into from Mar 16, 2021
Merged

Modernize Gradle build #183

merged 12 commits into from Mar 16, 2021

Conversation

melix
Copy link
Contributor

@melix melix commented Mar 13, 2021

This PR implements a number of improvements in the Gradle build.
It's largely a code cleanup, making sure that the build scripts of each project are declarative only. All build logic has been moved in its own subproject.

It was quite hard to follow what you've done before, but I think I managed to reproduce the previous behavior, in particular with where the artifacts are built (outside of the normal build directories).

I have migrated everything to the lazy APIs and I also made use of the new Maven Publish plugin. A side effect is that the core project now publishes javadocs and sources. By the way, the javadocs are full of errors which would need to be fixed.

I don't know how to produce docs for the Android projects, since I'm not an expert, but there was a lot of commented out code in the build that I just removed: for external folks like me it doesn't really make sense to have commented out code: either the code is useful in which case it shouldn't be commented out, but potentially activated by some flags, or it should be removed.

Here's a build scan after calling dist with this PR: https://scans.gradle.com/s/lst5evsi2rr3k

And the resulting output directory layout:

image

Please make sure to review the generated artifacts as I may have made some mistakes in the transition. I'm happy to answer questions on the new build layout if needed.

- Make use of a build logic directory for build logic, so that it's not
mixed into the build scripts themselves.
- Upgrade to use the Maven Publish plugin.
- Publish sources+javadocs for the "core" component.
- Make use of lazy APIs.
Make use of the `plugins` block instead.
Commented out repositories have been removed as this is a code
smell: if there's some configuration it should be explicit.
@google-cla
Copy link

google-cla bot commented Mar 13, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@camaelon camaelon self-requested a review March 16, 2021 01:15
Copy link
Collaborator

@camaelon camaelon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot -- I was able to patch it locally and it all seems to work :) -- the test on github fail I believe due to the jcenter resolution issues. Note that the old structure you had to replicate ... is not really necessary. This is a remnant of when we were publishing the artifacts via the Android Studio SDK Manager.

The only thing we need nowadays is to be able to publish to maven (and to mavenLocal for local tests/development) -- this might help simplify this CL.

constraintlayout/core/build.gradle Outdated Show resolved Hide resolved
constraintlayout/core/build.gradle Outdated Show resolved Hide resolved
@camaelon
Copy link
Collaborator

Oh and for javadoc -- not surprised to see some errors, I don't think we ever ran it on constraintlayout-core. The one we actually need are for the android libs, but that's another can of worm that's beyond this CL :)

@melix
Copy link
Contributor Author

melix commented Mar 16, 2021

The only thing we need nowadays is to be able to publish to maven (and to mavenLocal for local tests/development) -- this might help simplify this CL.

Indeed it's much simpler, so I can get rid of all the special setup! If I get you right, you need:

  • a way to publish to mavenLocal(): done
  • a way to publish to a local test repository: done
  • a way to publish to an external repository: technically no difference with providing a local URI

If so I can get rid of all the custom logic to put artifacts in different build directories. How does that sound?

@camaelon
Copy link
Collaborator

sounds good to me :)

@melix
Copy link
Contributor Author

melix commented Mar 16, 2021

Last question: if I remove the custom logic, what is the custom properties file (zip) used for? Should I remove it too? If not where should it be put?

So that we can run `./gradlew build` without running out of memory.
This commit gets rid of all the "old" build specifics to only use
standard mechanisms. In particular, the build directories are now
the standard ones.

Publishing location can be configured just by passing the
   -Prepo=....

parameter. For example:

./gradlew publish -Prepo=/tmp/my/repo
@melix
Copy link
Contributor Author

melix commented Mar 16, 2021

I have removed all custom code, including the special zip file. If it has to be reintroduced just let me know what it's supposed to do and where it should be produced.

This commit adds a daemon option to workaround an AGP bug causing
flakiness when processing the R resource.

javax.xml.parsers.SAXParserFactory=com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl

Ref: https://issuetracker.google.com/issues/175338398

This should be reverted once the bug is fixed.
@camaelon
Copy link
Collaborator

camaelon commented Mar 16, 2021

Nice! For the zip file, the way it works currently is that we are using it for our internal publication process; the zip contains the maven info+artifacts directly, so gradlew dist generate a zip like that:
core/constraintlayout-core-1.0.0-beta01.jar.md5
core/constraintlayout-core-1.0.0-beta01.pom.md5
core/constraintlayout-core-1.0.0-beta01.pom.sha1
core/source.properties
core/constraintlayout-core-1.0.0-beta01.jar
core/constraintlayout-core-1.0.0-beta01.jar.sha1
core/constraintlayout-core-1.0.0-beta01.pom

Let me try your latest changes, reading the doc on the publication process the intermediate directory structure doesn't seem to matter for the zip, as long as it contains pom files, and files at the same level adhere to maven naming convention. So simply zipping the result of what you now have ought to be enough.

@melix
Copy link
Contributor Author

melix commented Mar 16, 2021

Ok maybe I wasn't clear. In the previous version I created the zip, with the properties file inside, etc. Then you said that you don't need the custom setup anymore, in particular with the build directory layout. So my question is: should I restore the zip task, and if I do, where do you expect the zip file to be put?

@camaelon
Copy link
Collaborator

it would be nice to create a zip as it simplifies the process when publishing to maven.google.com, but if that's annoying to do we could skip that. It doesn't really matter where the zips end up to be (right now they are in out/dist); else from the doc the internal directory hierarchy inside the zip doesn't seem to matter either.

@melix
Copy link
Contributor Author

melix commented Mar 16, 2021

Done, I reintroduced a dist task which generates the zips into build/dist.

@camaelon
Copy link
Collaborator

looks good! many thanks!

@camaelon camaelon self-assigned this Mar 16, 2021
@camaelon camaelon merged commit b681ec2 into androidx:main Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants