-
Notifications
You must be signed in to change notification settings - Fork 867
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
Gradle init shall honor the configured Java Runtime #8223
Conversation
code looks good - thanks for filling the catch blocks. However I am not sure if this would work. when testing on JDK 24 it picked 8.10 and gave up on the upgrade step (which would have been 8.12.1 and in future a JDK 24 compatible version). So this indicates that it still won't work in future? edit: also first run I got this exception but only once - is likely a separate issue:
edit2: neil explained to me that I likely misunderstood the PR here. This is about having the option to downgrade the gradle RT as workaround (not about what we talked on slack of upgrading to latest in one go). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good to me, and a good fix for NB 25.
In addition (as discussed on Slack), as a future step it would be good to remove the whole This, while a bad idea without caching and configuration, seems to work and leave the right wrapper in place - public Set<FileObject> execute() {
GradleConnector gconn = GradleConnector.newConnector();
try {
gconn.useGradleVersion(GradleDistributionManager.get().availableDistributions(true).get(0).getVersion());
} catch (Exception ex) {
Exceptions.printStackTrace(ex);
}
JavaRuntimeManager.JavaRuntime defaultRuntime = GradleExperimentalSettings.getDefault().getDefaultJavaRuntime(); |
@mbien I think the tests needs to be updated to provide a |
nothing against it in principle. Just so you know that we don't do such thing anywhere so far. For sake of simplicity the JDK is defined by the matrix and everything uses the same JDK within that instance of the job. Also how would tests run locally? Would it need two JDK paths? #7871 is also not blocked by this, since gradle supports JDK 21 (and 23) and it can only test on 17 atm. Long ago we had something like that for the build itself to influence the JDK used for the daemon f61c2c9#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R114 which is gone now. |
Well, the issue is that running the IDE with Java 24, new Gradle project were initialized using unsupported JDK version, that results not updating the wrapper to the latest version.
That could result some unexpected behavior. Now the Java Runtime configured at Tools > Options > Java > Gradle will be used for the
init
andwrapper
operations as well.