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

Add Gradle 7 build job (and support Groovy 3) #214

Merged
merged 5 commits into from Apr 6, 2021
Merged

Add Gradle 7 build job (and support Groovy 3) #214

merged 5 commits into from Apr 6, 2021

Conversation

f4lco
Copy link
Collaborator

@f4lco f4lco commented Apr 1, 2021

Due to the recent improvements with regards to Gradle 7, I thought I might add a build job, too. The Gradle upgrade also implies using Groovy 3, which takes me to a major point: I think it is pointless for Gretty to do any effort in actively managing a specific version of Groovy. Instead, Gretty should rely on the Groovy version shipped with Gradle. I mean, Groovy and Gradle versions always co-evolve: I don't think its meaningful to lag behind Gradle's Groovy version, or to anticipate future changes of Gradle to the Groovy version. The added build shows that we can target Groovy 2 and 3 (and therefore Gradle 6.x and 7.x) just fine. This results in the following course of action:

  • Adding a Gradle 7.x build job
  • Pruning all occurrences of groovy_version
  • Opting in to the Groovy dependencies which are no longer part of localGroovy() in 7.x, see here, as Gretty needs it
  • Replacing almost all occurrences of jcenter, because some updated dependencies already discontinued publishing there

Questions:

  1. What about scripts like pluginScripts/gretty.plugin ? Will they require updated repositories, too?
  2. I don't like hiding dependency updates for Geb and Spock in the GitHub actions file, but for now, I don't know any better. Will we keep building for Gradle 6 and 7 in parallel? Or will we be creating new major releases / branches?
  3. When (2) is settled, we might consider using JUnit 5 for both Gradle 6 and 7. There is a separate "vintage" test engine which is able to pick up JUnit 4 tests while running on JUnit 5. That might replace the switch on Gradle 7 in the integration test plugin.

Fixes #193

@f4lco f4lco temporarily deployed to Bintray April 1, 2021 13:27 Inactive
@f4lco f4lco temporarily deployed to Bintray April 1, 2021 13:27 Inactive
@f4lco f4lco temporarily deployed to Bintray April 1, 2021 13:27 Inactive
@boris-petrov
Copy link
Member

Awesome work! Next time I see you not doing an A+ job, I'll ask you if you're feeling fine. 😄

I opened #193 exactly so we can discuss these things but you already have answers. :D

I think it is pointless for Gretty to do any effort in actively managing a specific version of Groovy.

I've always wondered why that is and guessed that there is some underlying reason that I don't understand. I guess there was none. So I definitely welcome this change.

Replacing almost all occurrences of jcenter

Why "almost"? What is left that is only there?

On your questions:

  1. Not sure. Is this part of the publishing mechanism? If so, I guess it will change when we migrate to Maven.

  2. I also wondered about the same thing when I opened the Groovy 3 issue. I also can't imagine a different way for now so this is fine I guess.

As for Gradle 6 and 7 - this PR makes me very happy and relieved that we can support both versions with the same code. So I see no reason to not do that. No need for branches or different Gretty versions. Once Gradle 6 becomes obsolete (I don't imagine this less than an year or two from now), we can drop it. Until then I guess lots of people will be stuck on it because of other plugins and/or their own build scripts.

  1. This concerns only Gretty's tests, right? If so, I don't personally mind which JUnit is used. I think Gradle until version 7 uses 4, does that change in 7? If so, we can be consistent. I don't know what has to change though.

Otherwise - perfect job, thanks!

@f4lco
Copy link
Collaborator Author

f4lco commented Apr 1, 2021

Replacing almost all occurrences of jcenter

Why "almost"? What is left that is only there?

  1. Not sure. Is this part of the publishing mechanism? If so, I guess it will change when we migrate to Maven.

I'll leave a comment #192. So this PR is concerned that Gretty's build does not use jcenter() as repository definiton. I was concerend with

repositories {
jcenter()
maven { url 'http://oss.jfrog.org/artifactory/oss-snapshot-local' }
}

for example. Fixing that can be easily in the scope of transitioning to Maven Central.

@boris-petrov
Copy link
Member

OK, I'm very happy with this, merge at will! :) Thanks again!

@f4lco f4lco temporarily deployed to Bintray April 1, 2021 15:34 Inactive
@f4lco f4lco temporarily deployed to Bintray April 1, 2021 15:34 Inactive
@f4lco f4lco temporarily deployed to Bintray April 1, 2021 15:34 Inactive
@f4lco f4lco temporarily deployed to Bintray April 1, 2021 15:34 Inactive
@f4lco f4lco temporarily deployed to Bintray April 1, 2021 15:34 Inactive
@f4lco f4lco temporarily deployed to Bintray April 1, 2021 15:34 Inactive
@chali
Copy link
Collaborator

chali commented Apr 1, 2021

I would highlight one thing which might not be very obvious if you plan to release a future version of gretty using Gradle 7 it might set a requirement that the consumer project has to use Gradle 7 too. Once the plugin switches to Gradle 7 all code is compiled against Groovy 3 which is not binary compatible with Groovy 2 so using the plugin in older projects will lead to failures.

I wanted to mention that since it is actually not that hard to migrate a plugin project to Gradle 7 but you might impose minimum version requirements by releasing with it. I think that will be important for 3.x release train.

Now it depends on your strategy of supported versions. There is a highly experimental feature in Gradle 7 that should allow feature variant release with plugin compiled against different Gradle/Groovy versions. I haven't explored it yet but I will need to use it for our nebula plugins.

@boris-petrov
Copy link
Member

@chali - thanks for chiming in! In that case is there any downside to building Gretty with Gradle 6 for the foreseeable future (only when releasing I mean. For the tests we could have Gradle 7, correct?)?

@chali
Copy link
Collaborator

chali commented Apr 2, 2021

I think that would be the safest approach for now. Tests running with both Gradle 6 and 7 and release made with Gradle 6. The downside is that it takes time to basically run your build twice and due to the different Gradle version there will be probably very few reused caches. You will not be able to benefit from Gradle 7 features but there are not that many at this point. 7.0 version is mostly "cleanup". I'm planning to look into the variant release in a few weeks so I can share how that works. In case you want to explore it too here are the docs

@f4lco f4lco mentioned this pull request Apr 6, 2021
@f4lco
Copy link
Collaborator Author

f4lco commented Apr 6, 2021

Thank you so much @chali for the heads up. I compiled your concerns and added the link to the documentation in #215. I wasn't really aware of the Groovy binary incompatibility, thanks for bringing it up 👍

@f4lco f4lco temporarily deployed to Bintray April 6, 2021 08:38 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 08:38 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 08:38 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 08:38 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 08:38 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:41 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:41 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:41 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:41 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:41 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:41 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:48 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:48 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 09:48 Inactive
JCenter has already turned read-only and will be shut down.
Newer artifacts such as Geb 4.0 are not present in jcenter.
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:29 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:29 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:29 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:29 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:29 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:29 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:38 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:38 Inactive
@f4lco f4lco temporarily deployed to Bintray April 6, 2021 11:38 Inactive
@f4lco
Copy link
Collaborator Author

f4lco commented Apr 6, 2021

Finally the build is green. I got some unstable builds resulting from BindException (likely that the port is already in use). I don't know what that is about, let's observe if that's a more frequent occurrence than before. I cannot see direct correlation with the changes in this PR.

@boris-petrov I chose to remove the 'pull_request' trigger from the GH action, it just bloats the executed builds per PR. If both triggers are active, every push to an existing PR results in 6 builds, but we would only require 3. Half of them are identical, they differ only in the trigger method.

@f4lco f4lco merged commit 3d57ebb into master Apr 6, 2021
@f4lco f4lco deleted the gradle7 branch April 6, 2021 12:02
@boris-petrov
Copy link
Member

@f4lco - I also saw the failures with BindException and was wondering what's going on.

Good find about the pull_request trigger. That's very strange naming that GitHub has about those things...

Also, I think you missed one comment of mine that I added a day or two ago. I can't seem to link it - but if you CTRL-F on this page and search for I just realized that this should say 16. 15 is not supported any more so we want to test only 16. Also, please remove 15 from the array on line 13. you should find it. :)

@f4lco
Copy link
Collaborator Author

f4lco commented Apr 6, 2021

@boris-petrov I can't find the comment. Normally I receive all PR comments via mail, but even there the comment is missing.
But gotcha, will do for both branches.

EDIT: maybe #191 could help with the flaky builds. It describes the observed error pretty well.

@boris-petrov
Copy link
Member

@f4lco - I see the Java 16 build failed because Gradle 6 is used for some reason. There are a number of places where the Gradle version is hardcoded. I don't know if that's relevant but probably is...

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.

Update Groovy to version 3
3 participants