Update Gradle wrapper to 8.8 - supports Java 22 #13453
Conversation
|
While not strictly necessary to be able to run tests with JDK 22, upgrading the gradle wrapper will simplify usage. Since one can just set JAVA_HOME, rather than both JAVA_HOME and RUNTIME_JAVA_HOME. |
|
@uschindler I was motivated to do this upgrade since stumbling into issues trying to verify the 9.11 RC1 build with e.g. failure I was seeing locally |
|
I'm planning on backporting this to the 9.x branch, unless there are concerns. |
|
Thanks for the review @dweiss. ❤️ |
|
You're very welcome. Upgrading gradle is risky as it may entail changes in their APIs that only surface once a task is actually used (dynamic code) but there's really no other way to do it than by trying. |
Yes, the smoker runs the whole testsuite including gradle with all the alternativive JDKs. This was the reason why I did not test Java 22 in my release checks. I ran the tests separately. +1, all fine here. But I have not tested it, I trust that all gradle paths were executed. |
|
The problem with gradle updates is always: theres is always a small piece of build logic not tested that breaks. Its hard to test the whole thing unless you also run regenerate or do a release. |
|
I ran regenerate - it found another small issue with InstallationLocation, now fixed. |
Yeah, I think the biggest problem is to find one task which executes everything in the whole Gradle Build... We would need some Gradle Coverage Report! |
I don't think we can find one particular task to do that, but we can try to include common stuff - see |
This commit updates the Gradle wrapper to 8.8, which has support for Java 22.
|
I'm getting errors since upgrading to Gradle 8.8 that I can't figure out how to fix. When running with EDIT: It looks like this is related to my system having the |
|
Hi, |
|
i still see some problems during tests regarding vector module. I have to check what's going on. Possibly some test system properties issue. It looks like the vector module is not always readable due to some strange behaviour with module names. Let's find that issue. Working on it: And this one in codecs:test , not sure why this happens: All a bit strange. |
|
Not sure what's the problem with both projects; in the test configuration all looks fine. Maybe we have not seen those messages before as Gradle did not print them. New version seems to pass logger event to console: // Enable the vector incubator module on supported Java versions:
if (rootProject.vectorIncubatorJavaVersions.contains(rootProject.runtimeJavaVersion)) {
jvmArgs '--add-modules', 'jdk.incubator.vector'
}
jvmArgs '--enable-native-access=' + (project.path == ':lucene:core' ? 'ALL-UNNAMED' : 'org.apache.lucene.core')I will check this out, it could be that the classfification module has different dependency to lucene core. It uses "moduleApi" instead of "moduleImplementation". Maybe that's the issue. @dweiss ? |
|
If the "moduleApi" dependency is wanted then it makes sense to me. But then we should only add the jvmArg if the module is linked at all. Not sure how to test this. So basically I am at moment only usure why it complain that vector incubator is not enabled for some Gradle projects. |
|
This seems to be a message out of context. When running expressions tests it has the vector incubator enabled. The message comes from somewhere about the not readable module comes from somewhere else....
Mega strange.... |
|
When checking jenkisn builds, the message... ...appears on random gradle projects (different ones on each run). So it has nothing to do with |
|
I think I know the problem..... It looks like Gradle test runner loads those classes for some checks outside of tests. This is why the message appear randomly in the output. In the past we had that issue when Gradle was configured to "detect" tests automatically from bytecode instead of filename. Looks like this is a similar issue here. Will dig into changes of Gradle Test runner and disable loading any test classes outside of test runner. |
|
Most error messages regarding the modules are also seen on Gradle 8.6, so its not a new issue. Sorry for false alarm. I will still investigate this. It seems to have indeed something to do with moduleApi vs moduleImplementation |
|
FWIW moving my |
|
the api vs implementation difference is about how configurations are inherited and then exported. See [1]. I'm not sure what's happening with regard to the warning - let me know if you'd like me to look into it so we're not duplicating the effort. |
|
What's the command/ jdk that produces these warnings for you, Uwe? |
Java 21 with "Gradlew test". You see it also on Policeman Jenkins on every Lucene main build. I can reproduce it also when running tests locally for a specific project |
|
These messages are sys-outed from each forked test runner and echoed as 'system.err' by the main gradle process. You can see them in the debug log if you run gradle with -debug: and when you launch the full runner executable (again - provided in debug logs), you'll see the same thing: Perhaps gradle finally improved the runner to echo jvm messages emitted before the tests commence (which should help with security log audits...)? I don't think there's anything we can do about these messages - they're really emitted by the jvm that runs tests. |
|
And looking at JDK sources, I don't think there is a way to dodge those warnings. |
|
I also see these warnings, which you've mentioned: Looking whether I can figure out why they're there. |
I don't care about those warnings they are expected. See the warnings I posted above. They make no sense, because it says that incubator is enabled but later it says not readable. But this appears in random m modules. I think this comes not from running our tests, but Gradle loading some class in it's own JVM and triggering the message on clinit. The other one with the warning about the So please only look at those 2 messages: |
|
It appears random because there are multiple runners and messages are proxied back to the console interleaved with everything else. I managed to debug this by running with TestCodecLoadingDeadlock forks a subprocess, which inherits IO - this bypasses System.out/err, which I presume is redirected for tests, and logs directly to the worker's system error descriptor, which in turn shows up on the console. The other message is from :lucene:codecs:test. The way tests are executed for that project is that it runs without modules - see modules.gradle: Yet, the defaults-tests.gradle appends enable-native-access and the JVM complains about it (rightfully). So it seems like everything is actually explainable. It is really annoying that forked test JVMs cram their syserrs directly to gradle console - makes debugging very difficult. But at least they show up, which is an improvement (?). I think a follow-up issue should be to correct the test to not inherit IO but redirect somewhere else and to either enable modular mode for the codecs module or add proper exclusion to enable-native-access. |
|
I filed #13471 to quiet down those two warnings. |
|
Thanks. The other one which is special is distribution.tests project. This one starts a blank test runner without modules and initializes module system inside. I think we should handle those modules like Lucene core, see default tests. |
|
Feel free to push to that PR, Uwe. I think I'm done for the day. |
This commit updates the Gradle wrapper to 8.8, which has support for Java 22.