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.

Sign up for GitHub

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

Jump to bottom

[Feature]: Introduce java version management #181

Closed
wants to merge 3 commits into from
Closed

[Feature]: Introduce java version management #181

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4fa61c
Introduce a java version manager
marchermans May 18, 2024
fc882a9
Register the java version manager.
marchermans May 18, 2024
5a80e9f
Make it a convention not a hard set.
marchermans May 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions common/src/main/java/net/neoforged/gradle/common/CommonProjectPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public void apply(Project project) {
project.getExtensions().create(Repository.class, "ivyDummyRepository", IvyRepository.class, project);
project.getExtensions().create(MinecraftArtifactCache.class, "minecraftArtifactCache", MinecraftArtifactCacheExtension.class, project);
project.getExtensions().create(DependencyReplacement.class, "dependencyReplacements", ReplacementLogic.class, project);
project.getExtensions().create("minecraftJavaVersionManager", JavaVersionManager.class, project);
AccessTransformers accessTransformers = project.getExtensions().create(AccessTransformers.class, "accessTransformers", AccessTransformersExtension.class, project);

extensionManager.registerExtension("minecraft", Minecraft.class, (p) -> p.getObjects().newInstance(MinecraftExtension.class, p));
Expand Down
29 changes: 29 additions & 0 deletions common/src/main/java/net/neoforged/gradle/common/extensions/JavaVersionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package net.neoforged.gradle.common.extensions;
Copy link
Member

Choose a reason for hiding this comment

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

Just.. set the version? What's the point of this extension?


import org.gradle.api.Project;
import org.gradle.api.plugins.JavaPluginExtension;
import org.gradle.jvm.toolchain.JavaLanguageVersion;
import org.gradle.jvm.toolchain.internal.JavaToolchain;

import javax.inject.Inject;

public class JavaVersionManager {

private final Project project;
private int javaVersion = -1;

@Inject
public JavaVersionManager(Project project) {
this.project = project;
}

public void setJavaVersion(int javaVersion, String context) {
if (this.javaVersion > javaVersion) {
Copy link
Member

Choose a reason for hiding this comment

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

The logic needs to be the other way around. In the case where there's 2 neo deps in the same project, the project must compile with the lowest Java version so that both sourcesets will work. Compiling with the highest will cause the one needing the lower to crash at runtime, and even if target compatibility were set, one can easily run into the issue of using newer Java methods.

Copy link
Contributor

@lukebemish lukebemish May 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The issue is that the toolchain is used for running too, and for running you definitely need the newest. And NG doesn't currently have an easy way to use a separate javalauncher for a given run configuration it seems? Maybe the real solution would be to add that -- then the runs with a given MC version could use the one for that MC version and the root toolchain would use the oldest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarfy, not just running, also decompile and recompile need the newest versions, so this is the correct default behaviour.

project.getLogger().warn("Can not reconfigure java version from {} to {} for {}", this.javaVersion, javaVersion, context);
return;
}

this.javaVersion = javaVersion;
project.getExtensions().getByType(JavaPluginExtension.class).getToolchain().getLanguageVersion().set(JavaLanguageVersion.of(javaVersion));
marchermans marked this conversation as resolved.
Show resolved Hide resolved
}
}
5 changes: 5 additions & 0 deletions .../src/main/java/net/neoforged/gradle/common/runtime/extensions/CommonRuntimeExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package net.neoforged.gradle.common.runtime.extensions;

import com.google.common.collect.Maps;
import net.neoforged.gradle.common.extensions.JavaVersionManager;
import net.neoforged.gradle.common.runtime.definition.CommonRuntimeDefinition;
import net.neoforged.gradle.common.runtime.definition.IDelegatingRuntimeDefinition;
import net.neoforged.gradle.common.runtime.specification.CommonRuntimeSpecification;
Expand Down Expand Up @@ -116,6 +117,10 @@ public final D create(final Action<B> configurator) {
throw new IllegalArgumentException(String.format("Runtime with identifier '%s' already exists", spec.getIdentifier()));

final D runtime = doCreate(spec);

final JavaVersionManager javaVersionManager = project.getExtensions().getByType(JavaVersionManager.class);
javaVersionManager.setJavaVersion(runtime.getVersionJson().getJavaVersion().getMajorVersion(), runtime.getSpecification().getIdentifier());

definitions.put(spec.getIdentifier(), runtime);
return runtime;
}
Expand Down
4 changes: 2 additions & 2 deletions common/src/main/java/net/neoforged/gradle/common/util/VersionJson.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,13 @@ public String getMainClass() {

public static class JavaVersion implements Serializable {
private String component;
private String majorVersion;
private int majorVersion;

public String getComponent() {
return component;
}

public String getMajorVersion() {
public int getMajorVersion() {
return majorVersion;
}
}
Expand Down
Loading