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

CORDA-1509 Configure and enable Gradle Build Cache #3908

Merged
merged 37 commits into from Sep 26, 2018

Conversation

Projects
None yet
3 participants
@josecoll
Contributor

josecoll commented Sep 6, 2018

Gradle configuration changes to support using Gradle Build Cache in a CI environment.

Usage of Gradle Build cache in Azure requires a patched version of Gradle to include TCP Keep-Alive, as per description and resolution described in gradle/gradle#6018.
This custom distribution has been uploaded to:
https://ci-artifactory.corda.r3cev.com/artifactory/downloads/org/gradle/gradle-4.9-keep-alive-bin.zip
This distribution is used by setting an additional Build Configuration step to set the gradle wrapper to use this version. See step 2 in https://ci-master.corda.r3cev.com/admin/editBuildRunners.html?id=buildType:Corda_Build_BuildGradleCache

These changes only take effect when the CI environment is correctly configured. Specifically the Build Configuration must define the following environment variable:
CI => must be set (to any value)
GRADLE_BUILD_CACHE_URL => must point to a Java VM running the Gradle Cache process.

and the build steps must include the following additional gradle command line parameters:
--build-cache (and optionally: --scan --stacktrace -Dorg.gradle.caching.debug=true)

@@ -91,6 +91,9 @@ buildscript {
maven {
url "$artifactory_contextUrl/corda-releases"
}
maven {
url 'https://plugins.gradle.org/m2/'

This comment has been minimized.

@chrisr3

chrisr3 Sep 6, 2018

Contributor

I think this maven {} repository is better replaced by

gradlePluginPortal()

This comment has been minimized.

@chrisr3

chrisr3 Sep 7, 2018

Contributor

Actually, you could just do this instead:

diff --git a/build.gradle b/build.gradle
diff --git a/build.gradle b/build.gradle
index b24135b5d4..6c7ba15642 100644
--- a/build.gradle
+++ b/build.gradle

@@ -121,6 +117,8 @@ plugins {
     // but the DSL has some restrictions e.g can't be used on the allprojects section. So we should revisit this if there are improvements in Gradle.
     // Version 1.0.2 of this plugin uses capsule:1.0.1
     id "us.kirchmeier.capsule" version "1.0.2"
+
+    id 'com.gradle.build-scan' version '1.16'
 }
 
 ext {

This would allow you to remove the extra repository entirely, along with the line adding the plugin to the classpath and the apply plugin: 'com.gradle.build-scan line.

@chrisr3

This comment has been minimized.

Contributor

chrisr3 commented Sep 6, 2018

And this (empty) file can be deleted too:

buildSrc/jarfilter/build.gradle
// Required to use Gradle build cache (until Gradle 5.0 is released with default value of "append" set to false)
// See https://github.com/gradle/gradle/issues/5269 and https://github.com/gradle/gradle/pull/6419
extensions.configure(TypeOf.typeOf(JacocoTaskExtension), { ex ->

This comment has been minimized.

@chrisr3

chrisr3 Sep 7, 2018

Contributor

Does this also work?

extensions.configure(TypeOf.typeOf(JacocoTaskExtension)) { ex ->
    ex.append = false
}

This comment has been minimized.

@josecoll

josecoll Sep 7, 2018

Contributor

yes.

@chrisr3

Some lingering lines about build-scan plugin.

@@ -109,6 +109,7 @@ buildscript {
classpath "net.i2p.crypto:eddsa:$eddsa_version" // Needed for ServiceIdentityGenerator in the build environment.
classpath "org.owasp:dependency-check-gradle:${dependency_checker_version}"
classpath "org.jfrog.buildinfo:build-info-extractor-gradle:$artifactory_plugin_version"
classpath 'com.gradle:build-scan-plugin:1.15.1'

This comment has been minimized.

@chrisr3

chrisr3 Sep 7, 2018

Contributor

You don't want this line any more because you're using the plugins DSL now.

@@ -128,6 +130,7 @@ apply plugin: 'com.github.ben-manes.versions'
apply plugin: 'net.corda.plugins.publish-utils'
apply plugin: 'maven-publish'
apply plugin: 'com.jfrog.artifactory'
apply plugin: 'com.gradle.build-scan'

This comment has been minimized.

@chrisr3

chrisr3 Sep 7, 2018

Contributor

You don't want this line any more either - you're using the plugins DSL instead.

@josecoll

This comment has been minimized.

Contributor

josecoll commented Sep 10, 2018

@chrisr3 response from Gradle forum:

@josecoll We probably won't be back-porting this feature to the 4.x series. We only do bugfix releases.
Is there something in particular which would block you from upgrading to 5.0 (i.e. do you have some deprecation messages in your build)?
@chrisr3

This comment has been minimized.

Contributor

chrisr3 commented Sep 10, 2018

The warning we're going to have trouble working around is:

Gradle does not allow passing null for the configuration action for CopySpec.from(). This behaviour has been deprecated and is scheduled to be removed in Gradle 5.0.
        at org.gradle.api.internal.file.copy.DefaultCopySpec.from(DefaultCopySpec.java:136)
        at org.gradle.api.tasks.AbstractCopyTask.from(AbstractCopyTask.java:324)
        at org.gradle.api.tasks.AbstractCopyTask$from.callCurrent(Unknown Source)
        at us.kirchmeier.capsule.task.Capsule.applyDefaultCapsuleSet(Capsule.groovy:150)
        at us.kirchmeier.capsule.task.Capsule.finalizeSettings(Capsule.groovy:125)

Because it's coming from the Capsule plugin, which seems to be unmaintained.

josecoll added some commits Jul 17, 2018

Generate dependency report and graph:
1) ./gradlew htmlDependencyReport
2) ./gradlew generateDependencyGraphCorda
Strip out all Jacoco references to prevent Gradle Build Cache error:
"Caching disabled for task ':<module>:test': 'JaCoCo agent configured with `append = true`' satisfied"
Revert jacoco back into jarFilter gradle build file.
Disable building deterministic modules (including jarFilter).
Echo URL of Gradle Build Repository.
Echo exit status of each Gradle build command.
Remove dependency graph generation plugin.
Align build cache settings across project and buildSrc gradle files.

@josecoll josecoll requested a review from Clintonio Sep 24, 2018

@josecoll

This comment has been minimized.

Contributor

josecoll commented Sep 25, 2018

@chrisr3 anything outstanding preventing you from approving this PR?

@chrisr3

Since you asked... 😈

@@ -0,0 +1,14 @@
// Gradle Build Cache configuration recommendation: https://docs.gradle.org/current/userguide/build_cache.html

This comment has been minimized.

@chrisr3

chrisr3 Sep 25, 2018

Contributor

Better:

ext {
    isCiServerBuildCacheURL = ...
    grdleBuildCacheURL = ...
}

This comment has been minimized.

@josecoll

josecoll Sep 25, 2018

Contributor

picky ;-)

This comment has been minimized.

@chrisr3

chrisr3 Sep 25, 2018

Contributor

🤓 But the ext.blah pattern is a nonsense that needs to die...!

@@ -1,2 +1,4 @@
rootProject.name = 'buildSrc'
include 'canonicalizer'
apply from: new File(settingsDir, '../buildCacheSettings.gradle')

This comment has been minimized.

@chrisr3

chrisr3 Sep 25, 2018

Contributor

Why are you using new File(settingsDir, ...) here? I would have thought that

apply from: 'buildCacheSettings.gradle

would be enough. According to the documentation, the file is assumed to be relative to the project directory.

@@ -60,6 +60,8 @@ include 'samples:cordapp-configuration'
include 'samples:network-verifier'
include 'serialization'
apply from: new File(settingsDir, 'buildCacheSettings.gradle')

This comment has been minimized.

@chrisr3

chrisr3 Sep 25, 2018

Contributor

Same comment as above.

@@ -0,0 +1,15 @@
// Gradle Build Cache configuration recommendation: https://docs.gradle.org/current/userguide/build_cache.html
ext {
isCiServer = System.getenv().containsKey("CI")

This comment has been minimized.

@chrisr3

chrisr3 Sep 25, 2018

Contributor

BTW, Java does have a System.getenv(String) API.

This comment has been minimized.

@josecoll

josecoll Sep 25, 2018

Contributor

in this case we only care if it exists (eg. want a boolean value, not its actual value).

This comment has been minimized.

@chrisr3

chrisr3 Sep 25, 2018

Contributor

System.getenv("CI") will return null if there is no such environment variable.

This comment has been minimized.

@josecoll

josecoll Sep 25, 2018

Contributor

It really makes no difference whether we check for NULL or a boolean.
Stop being so pedantic ... ;-)

This comment has been minimized.

@chrisr3

chrisr3 Sep 25, 2018

Contributor

Well, I'll let @Clintonio be the final arbiter 😉

This comment has been minimized.

@Clintonio

Clintonio Sep 25, 2018

Member

I don't mind either way here. The string constant "CI" here is a little bit too generic, I usually prefix any Corda specific envvar with CORDA_ to avoid collisions. Like CORDA_BINTRAY_USERNAME/CORDA_BINTRAY_PASSWORD, etc.

I won't block the review on it but I'd strongly recommend a prefix.

Delegating to Clinton.

@Clintonio

LGTM - simple, modular. Shame about the lack of a backport to gradle 4.10.x.

# Required to use custom patched Gradle 4.9 with KEEP-ALIVE timeout fix
# Currently built on Azure "gradle-build-client" machine at:
# export GRADLE_HOME=~/gradle/install
if [[ ! ${GRADLE_HOME} ]]; then

This comment has been minimized.

@Clintonio

Clintonio Sep 25, 2018

Member

Could we not make the educated guess of ~/.gradle here to preserve sensible default behaviour?

This comment has been minimized.

@josecoll

josecoll Sep 25, 2018

Contributor

Removed GRADLE_HOME as no longer needed.
Everything is driven via gradle wrapper.

@@ -0,0 +1,15 @@
// Gradle Build Cache configuration recommendation: https://docs.gradle.org/current/userguide/build_cache.html
ext {
isCiServer = System.getenv().containsKey("CI")

This comment has been minimized.

@Clintonio

Clintonio Sep 25, 2018

Member

I don't mind either way here. The string constant "CI" here is a little bit too generic, I usually prefix any Corda specific envvar with CORDA_ to avoid collisions. Like CORDA_BINTRAY_USERNAME/CORDA_BINTRAY_PASSWORD, etc.

I won't block the review on it but I'd strongly recommend a prefix.

@josecoll

This comment has been minimized.

Contributor

josecoll commented Sep 25, 2018

Thanks @Clintonio
Adjusted as per your final comments.

@josecoll josecoll merged commit 6f27898 into master Sep 26, 2018

4 checks passed

API Stability (Pull Requests) TeamCity build finished
Details
Build Pull Requests (Docs) TeamCity build finished
Details
Integration Tests (Pull Requests) TeamCity build finished
Details
Unit Tests (Pull Requests) TeamCity build finished
Details

roastario pushed a commit to roastario/corda that referenced this pull request Nov 8, 2018

CORDA-1509 Configure and enable Gradle Build Cache (corda#3908)
* Fix to enable gradle build caching of test runs.

* Configure gradle build caching to be enabled.

* Generate dependency report and graph:
1) ./gradlew htmlDependencyReport
2) ./gradlew generateDependencyGraphCorda

* Strip out all Jacoco references to prevent Gradle Build Cache error:
"Caching disabled for task ':<module>:test': 'JaCoCo agent configured with `append = true`' satisfied"

* Revert jacoco back into jarFilter gradle build file.
Disable building deterministic modules (including jarFilter).

* Added Gradle build scan plugin.

* Set file encoding to prevent incorrect Gradle build cache keys across different machines.
https://guides.gradle.org/using-build-cache/#system_file_encoding

* Apply gradle build cache settings to buildSrc.

* Added targetted gradle build tasks to leverage cache.

* Updated URL's of several different test Gradle Cache instances.

* Updated CI batch build scripts.

* Updated script perms to be executable.

* Added CI smoke tests batch script.

* Use TestDev Labs gradle cache.

* Echo URL of Gradle Build Repository.
Echo exit status of each Gradle build command.

* Use environment variables to define Gradle Build Cache usage and URL.

* Customisation through parameters.

* Remove dependency graph generation plugin.
Align build cache settings across project and buildSrc gradle files.

* Remove buildSrc gradle build cache config.

* Revert definition of gradle build cache variables back to settings.gradle.

* Fix incorrect path.

* Aligned gradle build cache configuration across buildSrc and project.

* Minor updates to test scope.

* Minor updates to test scope.

* Update scripts to use GRADLE_HOME

* Exit on unset GRADLE_HOME

* Remove duplication following rebase from master.

* Remove fine-grained build task scripts.

* Added back Jacoco reporting.

* Revert jdk8u-deterministic module.

* Incorporating changes from PR review feedback.

* Workaround Jacoco issue associated with Gradle Build Cache test task.

* Update init script.

* Remove redundant build-scan declarations.

* Updates from PR review feedback.

* Remove GRADLE_HOME as no longer needed - everything is driven via gradle wrapper.

* Use CORDA prefix in system environment variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment