Upgrade to Gradle 5.0 #34263

alpar-t · 2018-10-03T11:30:29Z

Upgrade to Greadle 5.0
updates to comply with breaking changes in Groovy.
It seems like the latest version brought along by Gradle changes how the final keyword is interpreted,
unfortunately it can't see past branches so it doesn't work like ti would in java.

This is a work in progress to get the tests running. Will only merge when Gradle 5.0 is GA.

Release Notes: https://github.com/gradle/gradle/releases/tag/v5.0.0

elasticmachine · 2018-10-03T11:30:31Z

Pinging @elastic/es-core-infra

rjernst

I have one question, but I also don't think we should upgrade to a milestone release. This is like upgrading to a beta.

rjernst · 2018-10-03T17:37:24Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy

@@ -563,7 +563,7 @@ class ClusterFormationTasks {
    }

    static Task configureInstallPluginTask(String name, Project project, Task setup, NodeInfo node, String pluginName, String prefix) {
-        final FileCollection pluginZip;


why is final removed?

I will not merge the PR until there is a GA, just wanted to start running in CI.

Gradle bumps the Groovy version, which now complains that the final variable is changed.
It doesn't see past branches the same way java does. One more reason to switch to java.

alpar-t · 2018-10-18T07:20:40Z

@elasticmachine test this please

alpar-t · 2018-10-19T10:26:44Z

@elasticmachine test this please

Looks like Gradle changed the order in which things happen here. There were no guarantees on ordering before, the order just happened to be right.

alpar-t · 2018-12-03T09:54:48Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy

@@ -215,7 +215,7 @@ public class RestIntegTestTask extends DefaultTask {
     * @param project The project to add the copy task to
     * @param includePackagedTests true if the packaged tests should be copied, false otherwise
     */
-    static Task createCopyRestSpecTask(Project project, Provider<Boolean> includePackagedTests) {
+    Task createCopyRestSpecTask() {


The static method was called with the value of includePackagedTests in what happened to be the right order so that changes to the property were picked up in time and the right value was passed into the project.afterEvaluate ( which had no use, since the value was already picked up anyhow ).
This can't be static to make changes to the property reflected as they should. Gradle 5 seems to have changed the order in which some operations are executed. I wish they would have an option to randomize ordering of otherwise undefined sequences.

alpar-t · 2018-12-03T15:20:42Z

@rjernst Gradle 5.0 is GA and this is ready for review and merge now.

droberts195 · 2018-12-03T15:29:51Z

One thing to note is that in release manager Gradle will run with ml-cpp and found-elasticsearch-plugin in the elasticsearch-extra directory. Were there any particular things that needed changing as part of this PR that are also likely to need changing in the build.gradle files in those repos before Gradle 5 will work with them? I'm happy to make the ml-cpp changes if any are required, but it would be good to consider this before merging this PR because otherwise the unified snapshot will be broken in the meantime.

alpar-t · 2018-12-03T16:49:55Z

@droberts195 The best thing is to run with a build scan and look at deprecation messages.
The finally keyword changed how it works and I had to remove it from a bunch of places.
Other than that this was not special from minor version updates where things are ran in different order and this uncovers some explicit ordering that is missing. I'm also glad to help making those changes. I think the hadoop plugin is also affected ( /cc @jbaiera )

Needs to be merged before elastic/elasticsearch#34263 to prevent builds with ml-cpp in elasticsearch-extra from failing. Defining a custom wrapper task was incompatible with Gradle 5, so this is replaced with a modification to the built in wrapper task. The Gradle Wrapper is also upgraded to the latest version.

Needs to be merged before elastic/elasticsearch#34263 to prevent builds with ml-cpp in elasticsearch-extra from failing. Defining a custom wrapper task was incompatible with Gradle 5, so this is replaced with a modification to the built in wrapper task. The Gradle Wrapper is also upgraded to the latest version. Backport of #325

droberts195 · 2018-12-04T10:17:34Z

I can confirm that ml-cpp now works with Gradle 5

rjernst

LGTM

rjernst · 2018-12-04T16:26:16Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy

@@ -237,7 +237,7 @@ public class RestIntegTestTask extends DefaultTask {
        project.afterEvaluate {
            copyRestSpec.from({ project.zipTree(project.configurations.restSpec.singleFile) }) {
                include 'rest-api-spec/api/**'
-                if (includePackagedTests.get()) {


This reversion of using Property works here, because we are in an afterEvaluate. However, the original intention was to configure the copy spec outside of afterEvalute, so the Property would be needed. The reason the afterEvaluate still exists is because, for some reason I could not explain, the zipTree seems to always be evaluated when copyRestSpec.from is called, even though we pass a closure. I bring this up because I see a similar copy.from added below with a similar lazily evaluated zip, which I assume works, so maybe this afterEvaluate could be removed (in a followup), but then this would need to be a Property again.

alpar-t · 2018-12-05T12:06:02Z

gradlew in the found plugin actually uses the one from the ES repo. I checked to make sure Gradle 5 doesn't cause any issues there.

This pull request fixes the build.gradle file so that it works with the Gradle 5.0 update (#34263). It also makes use of the ignore: 404 header in REST tests so that previous snapshots are deleted as part of the test set up. This way each new test run with an external service should start with a fresh repository even if the previous test run failed and left snapshots in the repo. (This is a best effort as it does not fix a corrupted repository)

Upgrade to Gradle 5.0 Milestone 1

7d68fb3

alpar-t added >non-issue :Delivery/Build Build or test infrastructure WIP >upgrade v7.0.0 v6.5.0 labels Oct 3, 2018

rjernst reviewed Oct 3, 2018

View reviewed changes

alpar-t added 2 commits October 17, 2018 16:51

Merge branch 'master' into upgrade-gradle-5

62de13a

Add settings.gradle to unrelated project in the hierarchy

2d5d468

alpar-t force-pushed the upgrade-gradle-5 branch from dabae45 to 2d5d468 Compare October 18, 2018 06:13

alpar-t added 2 commits October 19, 2018 11:00

Merge remote-tracking branch 'origin/master' into upgrade-gradle-5

61b17bd

Merge branch 'master' into upgrade-gradle-5

de789ec

alpar-t added 2 commits October 22, 2018 11:00

Merge branch 'master' into upgrade-gradle-5

dc3dd59

Merge remote-tracking branch 'origin/master' into upgrade-gradle-5

701751d

colings86 added v6.6.0 and removed v6.5.0 labels Oct 25, 2018

alpar-t added 4 commits November 2, 2018 06:37

Upgrade to Grale 5.0 RC1

0b8a60d

Merge remote-tracking branch 'origin/master' into upgrade-gradle-5

5b52583

remove more finals

99f5c17

fix annotation processor

150d10d

alpar-t changed the title ~~Upgrade to Gradle 5.0 Milestone 1~~ Upgrade to Gradle 5.0 RC 1 Nov 2, 2018

alpar-t added 5 commits November 7, 2018 17:16

Fix ordering

6a48de6

Looks like Gradle changed the order in which things happen here. There were no guarantees on ordering before, the order just happened to be right.

Merge remote-tracking branch 'origin/master' into upgrade-gradle-5

11ea356

Merge remote-tracking branch 'origin/master' into upgrade-gradle-5

27e8dc9

Merge remote-tracking branch 'origin/master' into upgrade-gradle-5

7101aa2

bump scan plugin version

594dd6a

alpar-t added 6 commits November 30, 2018 16:44

Upgrade to 5.0 GA

a3daa30

remove redundant copy

2acae1a

Merge remote-tracking branch 'origin/master' into upgrade-gradle-5

e775327

Fix for copying YML tests

80123b4

Deal with deprecations

cd95f12

Resolve the configuration before copying

2d1005a

alpar-t commented Dec 3, 2018

View reviewed changes

Add dependency to resolve configurations

3054716

alpar-t removed the WIP label Dec 3, 2018

droberts195 mentioned this pull request Dec 3, 2018

[ML] Upgrade Gradle to v5.0 elastic/ml-cpp#325

Merged

droberts195 mentioned this pull request Dec 4, 2018

[ML][6.6] Upgrade Gradle to v5.0 elastic/ml-cpp#326

Merged

rjernst approved these changes Dec 4, 2018

View reviewed changes

alpar-t merged commit 59b0900 into elastic:master Dec 5, 2018

alpar-t deleted the upgrade-gradle-5 branch December 5, 2018 12:10

alpar-t added a commit that referenced this pull request Dec 5, 2018

Upgrade to Gradle 5.0 (#34263)

ddaf65d

tlrx mentioned this pull request Dec 6, 2018

[Tests] Fix third party tests with Gradle 5.0 #36302

Merged

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Gradle 5.0 #34263

Upgrade to Gradle 5.0 #34263

alpar-t commented Oct 3, 2018 •

edited

Loading

elasticmachine commented Oct 3, 2018

rjernst left a comment

rjernst Oct 3, 2018

alpar-t Oct 4, 2018

alpar-t commented Oct 18, 2018

alpar-t commented Oct 19, 2018

alpar-t Dec 3, 2018

alpar-t commented Dec 3, 2018

droberts195 commented Dec 3, 2018

alpar-t commented Dec 3, 2018

droberts195 commented Dec 4, 2018

rjernst left a comment

rjernst Dec 4, 2018

alpar-t commented Dec 5, 2018

Upgrade to Gradle 5.0 #34263

Upgrade to Gradle 5.0 #34263

Conversation

alpar-t commented Oct 3, 2018 • edited Loading

elasticmachine commented Oct 3, 2018

rjernst left a comment

Choose a reason for hiding this comment

rjernst Oct 3, 2018

Choose a reason for hiding this comment

alpar-t Oct 4, 2018

Choose a reason for hiding this comment

alpar-t commented Oct 18, 2018

alpar-t commented Oct 19, 2018

alpar-t Dec 3, 2018

Choose a reason for hiding this comment

alpar-t commented Dec 3, 2018

droberts195 commented Dec 3, 2018

alpar-t commented Dec 3, 2018

droberts195 commented Dec 4, 2018

rjernst left a comment

Choose a reason for hiding this comment

rjernst Dec 4, 2018

Choose a reason for hiding this comment

alpar-t commented Dec 5, 2018

alpar-t commented Oct 3, 2018 •

edited

Loading