neoforged / ModDevGradle

A Gradle plugin for developing Minecraft mods using NeoForge
https://projects.neoforged.net/neoforged/moddevgradle
GNU Lesser General Public License v2.1
30 stars 5 forks source link

Eclipse run configurations don't honor the Java toolchain version #98

Closed ljfa-ag closed 2 months ago

ljfa-ag commented 2 months ago

I'm not sure if this only affects Eclipse or if this is a more general problem. When importing the Gradle project into Eclipse, the run configs that are being generated are set to the workspace's default JVM, which in my case is 17, rather than the project's JVM 21. The result is that I can't directly run the configurations unless I change the run config's JVM: Bildschirmfoto vom 2024-07-12 09-51-22 Notice how it is set to "Alternate JRE". Neither ForgeGradle nor NeoGradle run configs were using this setting, they would use the "Project execution environment" setting instead, so they worked regardless of Eclipse's defaults. As you can see, the project JVM is still set to the correct version as specified in build.gradle.

I tried it with MDK-1.21-ModDevGradle, only modified to change the ModDevGradle version to 1.0.1.

shartte commented 2 months ago

Yes, setting it doesn't work. I tried: https://github.com/neoforged/ModDevGradle/blob/main/src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java#L951

Can you check what the name of your VM container for 21 is?

p.s.: I suppose we can remove the setting and hope for the best, but we did have the situation where eclipse didn't set an appropriate project-level JRE either.

ljfa-ag commented 2 months ago

Do you mean the value of the org.eclipse.jdt.launching.JRE_CONTAINER attribute in the .launch file? It's org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/java-21-openjdk/.

Omitting this attribute and letting it use the project standard makes more sense to me since the project itsn't going to work anyway in case of a wrong project JRE.

shartte commented 2 months ago

Hm that is probably true, although I think we tried making it use a 21 JDK on the project-level too if we could. Can you go to your Eclipse settings and check on this screen what it says for "JavaSE-21"?

image

ljfa-ag commented 2 months ago

Sure: grafik This is on Arch Linux with Eclipse 2024-06. I have OpenJDK 8, 17, 21 and latest (22) installed.

Shadows-of-Fire commented 2 months ago

@shartte note the comment from ELC:

        /**
         * Defines the JRE to use when executing the application. The definition must take the following three-part format, each part separated by a forward slash:
         * <ol>
         * <li>The header, which is "org.eclipse.jdt.launching.JRE_CONTAINER"</li>
         * <li>The VM type, which is one of "org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType" or "org.eclipse.jdt.launching.EEVMType"</li>
         * <li>The VM name, which is a user-specified value for a JVM in Eclipse, such as "jdk-17.0.5.8-hotspot"</li>
         * </ol>
         * For example, a full declaration may look like
         * "org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/jdk-17.0.5.8-hotspot"
         * <p>
         * E-Attribute type: String
         * <br>
         * Required: False
         * <br>
         * Default Value: The default JVM of the Eclipse project specified by {@link #ATTR_PROJECT_NAME}.
         */

Just setting it to JavaSE-21 would be invalid and cause Eclipse to fallback to the default. You will need to set it to the following magic string: org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-21/

As a test, @ljfa-ag could you set the value of org.eclipse.jdt.launching.JRE_CONTAINER in the run config file to the aforementioned value and see if eclipse properly picks up J21? It works for me in my local testing.

Shadows-of-Fire commented 2 months ago

Though based on the broader context it may be better to just omit it and let it use the project default. We didn't supply this attribute in FG nor NG, and it is kinda brittle.

If the project default isn't set appropriately then a bunch of other things will be on fire (because Eclipse will fail to build the project)

ljfa-ag commented 2 months ago

As a test, @ljfa-ag could you set the value of org.eclipse.jdt.launching.JRE_CONTAINER in the run config file to the aforementioned value and see if eclipse properly picks up J21? It works for me in my local testing.

Yup, that works for me as well.

shartte commented 2 months ago

@Shadows-of-Fire Thanks, that gives me some idea for what to look 😓 And yes, if I can't figure something out, I'll just yeet it.

shartte commented 2 months ago

Okay, I investigated some more, and it seems Eclipse does choose the project JDK correctly based on Gradle. I am not sure what caused me to set the JRE on the run configuration 🤔 Probably some misconfiguration during my initial testing led it to not pick the right project JDK. Anyway, I'll yeet the jreContainer from the launch configs.

shartte commented 2 months ago

@ljfa-ag Thanks for your patience, this should now be fixed in 1.0.7 1.0.8

ljfa-ag commented 2 months ago

No worries, thank you guys for the quick fix :)