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

Gradle Execution Customizer with Runtime #5158

Merged
merged 9 commits into from
Jan 15, 2023

Conversation

lkishalmi
Copy link
Contributor

This one is about evolving the Java Runtime selection further, beyond the recent patches, that made it possible in NB16u1.

The old implementation was mimicking what Maven/Ant does at the Build/Compile customizer. Those days the runtime platform pretty much determined the compile platform as well. The introduction of JVM Toolchains made that different.

Nowadays it is enough to have the JRE installed to start a Gradle build, Gradle can download and use different JVM-s if needed.

I also made the Java Runtime selection available on root project only, sub-projects would take the root project setting.

That's why the GradleJavaRuntimeManager and JavaRuntime got introduced as an API. Feedbacks on that are welcome, I think it's good enough, but could be improved to be more future proof.

I've implemented this as a series of commits, so reviewing might be easier that way. JavaDoc and API doc would be provided later and in this PR.

Eventually I would like to deprecate all the methods in JavaRunUtils, but they are used in the BootClasspath support at the moment. The bootclasspath support is not JVM Toolchain aware now, so that needs to be addressed, though I thought that would be a separate PR.

image

Changed to

image

While the Build/Compile customizer got removed.

@lkishalmi lkishalmi added the Gradle [ci] enable "build tools" tests label Dec 29, 2022
@lkishalmi lkishalmi added this to the NB17 milestone Dec 29, 2022
@lkishalmi lkishalmi marked this pull request as ready for review January 7, 2023 17:30
*
* @author Laszlo Kishalmi
*/
public interface JavaRuntimeManager {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If intending to merge before NB17 branching, javadocs + apichanges should be added for this + other API changes. Maybe some overview javadoc that explains purpose / difference between JavaPlatform and JavaRuntime.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

@mbien
Copy link
Member

mbien commented Jan 14, 2023

So this moves the item in build -> compile to build -> gradle execution. This seems to have the side effect that something which was configurable on a per-sup-project basis is now only configurable in the root project?

UI wise i would consider swapping the Java Runtime panel with Trust Level panel.

@lkishalmi
Copy link
Contributor Author

@mbien Yes, we are going to loose the per sub-project level of the Java Runtime Setting. I think that's a happy loss.
I'm switching the panels...

@lkishalmi
Copy link
Contributor Author

image

@mbien
Copy link
Member

mbien commented Jan 15, 2023

looks good to me. Layout is also working. The setting is also successfully changing the runtime JDK of the project.

The "release" version (javac --release) can't be changed via the gradle UI?

I don't see "Compile" options in the UI btw which is on the last screenshot you posted.

The options window likely needs a scrollpane for the sub options since some don't fit the window (e.g Formatting options) - but this is also the case for maven project and out of the scope of this PR.

@lkishalmi
Copy link
Contributor Author

I've posted the screenshot with only the Gradle Project recompiled, so the Gradle Java Projects were on master, so that provided the Compile Customizer.

The Gradle UI is kind of read-only. Yes, the runtime setting can affect the java release/platform used in projects. Though that is a bad practice. That means that the Gradle build files does not set that by any other means. It's possible to set the Java release version per source set. Since the introduction of Java Toolchains, the actual platform in use can be specified even on task level.

It's getting close to the freeze. Any approver?

Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you are asked so nicely I approve.

But I ran only rudimentary tests today. Code looks good to me.

final String displayName;
final File javaHome;

private JavaRuntime(@NonNull String id, String displayName, File javaHome) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like it could use Path?

@lkishalmi lkishalmi merged commit 051f55d into apache:master Jan 15, 2023
pepness pushed a commit to pepness/incubator-netbeans that referenced this pull request Jan 17, 2023
* Moved Gradle Java Runtime Selection into Gradle Projects

* GradleJavaPlatformProviderImpl uses the JavaRuntimeManager now.

* Moved GradleJavaProjectProblemProvider to Gradle Project

* Added change aware bridge between JavaRuntime and JavaPlatform

* Removed unloadable project warning from customizer, other fixes

* Polishing the code according suggestions.

* Adjusted Bundle.properties for the customizers.

* Added API documentation.

* Mage JavaRuntime on top of Gradle Execution Customizer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gradle [ci] enable "build tools" tests Need Squashing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants