[8.1] Update gradle wrapper to 7.4 (#81963) #83883
Conversation
* Make ForbiddenApisPrecommitPlugin plugin Gradle 8.0 compatible * Fix deprecations on ignoring empty folders for task inputs * Update Gradle wrapper to 7.4 GA
breskeby
added
auto-merge-without-approval
| * the CheckForbiddenApis task which is built with gradle 4 support | ||
| * in mind. | ||
| * */ | ||
| public class CheckForbiddenApisTask extends CheckForbiddenApis { |
There was a problem hiding this comment.
In case it could be helpful, discussions to have a fix in the plugin policeman-tools/forbidden-apis#189
There was a problem hiding this comment.
the solutions discussed there don't sound feasible to me. Ideally there would be a forbidden api version that doesn't require compatibility with old gradle versions. Discussed that before. My ideal plan is to move away from this plugin in the long run as its a burden to maintain more and more tbh
There was a problem hiding this comment.
Hi,
sorry for all this. I agree to update gradle minimum version, IF it would not be 6.8. Many projects that are using the plugin look like still on Gradle 6. Indeed the problem is also the minimum Java version while compiling, but this will change soon.
In the meantime I am about to release forbiddenapis 3.3 soon.
I know that many developers of Elasticsearch want to use forbiddenapis, but because of behavior of Gradle community and people (not willing to help) and with their shitload of changes (partly ununderstandable), this leads to issues that make plugin maintenance harder and harder. Because of this, I will in the future refuse to accept issues / PRs regarding Gradle. I mentioned this on Twitter already: https://twitter.com/thetaph1/status/1498631483887689732
@breskeby As you were working for Gradle before, maybe take your contacts and ask for including the plugin (which is only a few lines of Gradle-specific code) to the distribution and let people only configure the backend forbiddenapis JAR file (like this is done with other checkers like PMD or checkstyle which have plugin stubs in source code of Gradle and seem to be configurable to use a specific checkstyle/PMD backend). I will then only maintain the actual detection code, but I don't need to maintain the plugin in this ever-changing Gradle world which is a pain in the ass.
I may have a solution for the current problem:
- simply skip task execution on my own and remove the @SkipIfEmpty annotation. @reta already tried this out also in a first attempt to fix many other plugins for OpenSearch. From my checks this brings no performance problems, because you just move the check into the code if the FileTree is empty or not. Only downside is that it won't print "NO-SOURCE".
- Do the same like done here in forbiddenapi's
plugin-init.groovyby subclassing and use the subclassed Task class when Gradle 6.8 is detected.
The first fix would be easiest and works also with custom Task setup like done in Elasticsearch (looks like you have your own plugin code to setup, because you have many more sourcesets and also use forbiddenapis to scan JAR files going back to Robert Muir's code.
If you want to have the "NO-SOURCE" message back, you can do the same like done here: subclass.
There was a problem hiding this comment.
@uschindler thanks for reaching out. really appreciated.
Your first suggested solution sounds like a good compromise on keeping version compatibility and fix this problem.
I totally agree that supporting plugins over a wider range of gradle versions is not straight forward and I totally understand your pain. I did that before and it requires also proper test coverage and stepping through quite some loops. On the other hand the "aggressive" way of @gradle moving forward allows quicker innovation IMO. If having the bandwidth to stay up-to-date with gradle versions, this is a huge plus and at elastic we benefit from this a lot imo.
I don't think having the forbidden apis plugin as part of the gradle distribution is something the @gradle team would support, as the general strategy is more of making less plugins part of the distribution instead of more.
Personally, I usually just mimic the gradle version scheme for plugins, meaning that if there is a breaking change in 7.0 that affects my plugin, the major release of my plugin would support that 7.0 version and break older versions. People running with older gradle versions, usually also don't mind running on older plugin versions.
I understand that as an individual contributor doing all this in your spare time is/can be too much to ask and I understand your decision and reasoning, though I have a different perspective on certain things (the gradle community, the cache support etc.)
Declaring no further support for gradle plugin requests from your side would mean for us effectively, creating/forking our own forbidden gradle plugin, that stays up-to-date with latest gradle versions and that would have a different deprecation/support lifecycle. Ideally we make this a public stand alone plugin available via the plugin portal, but to be honest that has not the highest priority for us.
There was a problem hiding this comment.
I think I found an elegant solution: policeman-tools/forbidden-apis#189 (comment)
I can subclass SourceTask which inherits the annotation :-)
There was a problem hiding this comment.
Declaring no further support for gradle plugin requests from your side would mean for us effectively, creating/forking our own forbidden gradle plugin, that stays up-to-date with latest gradle versions and that would have a different deprecation/support lifecycle. Ideally we make this a public stand alone plugin available via the plugin portal, but to be honest that has not the highest priority for us.
I will of course make the plugin in a way that it works. I just won't support any crazy logic to support yet again new cache types invented all the time. Gradle is a cache-desaster (listen to complaints by Lucene people especially Robert Muir and Dawid Weiss). Gradle feels like blowing up at some point because it caches also the brain of developers! :-) Instead of caching everythingand spawning daemons all the time, there should be work on removing slow Groovy support (EA versions of Java users will thank you) and redesigning the DSL.
A standalone plugin that just depends on the forbiddenapis core JAR file would be another options, but for maintenance reasons this was abandoned, but I can reinvestiagte. It would be a simple plgin jar only containing the plugin, but no code. It would just depend on forbiddenapis main JAR file.
There was a problem hiding this comment.
Hi @breskeby: I found a solution for this issue, also done by a lot of other plugin authors - so I am not the first one for this specific issue. Works perfectly, no messages about deprecated features with 7.4.1: policeman-tools/forbidden-apis#193
I also fixed an issue with configuration cache (access of Task#getProject() during task execution caused by stupid Groovy variable/field access scopes) described in policeman-tools/forbidden-apis#179.
There was a problem hiding this comment.
New version with JDK 18 support will come out soon.
There was a problem hiding this comment.
@breskeby: Version 3.3 was release a minute ago. This contains the missing annotation and on top also works with configuration cache. You may update it in Elasticsearch and remove the subclassing introduced here.
Backports the following commits to 8.1: