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

build: Set the target JVM to 11 #8403

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions buildSrc/src/main/kotlin/ort-kotlin-conventions.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ detekt {
}

val javaVersion = JavaVersion.current()
val maxKotlinJvmTarget = runCatching { JvmTarget.fromTarget(javaVersion.majorVersion) }
val jvmSourceCompatibility = runCatching { JvmTarget.fromTarget(javaVersion.majorVersion) }
.getOrDefault(enumValues<JvmTarget>().max())
val jvmTargetCompatibility = JvmTarget.JVM_11
Copy link
Member Author

Choose a reason for hiding this comment

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

@sschuberth Does it make sense to keep the logic to determine the max version and use different versions for source and target or should we just hard code both to Java 11?

Copy link
Member

@sschuberth sschuberth Mar 12, 2024

Choose a reason for hiding this comment

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

Good question. As IIRC the JVM source compatibility is only also set to avoid the warning mentioned in https://youtrack.jetbrains.com/issue/KT-48745, we probably should just hard-code it. Or yet better, do as advised in this comment and use Gradle's toolchains mechanism with version 11 hard-coded there.

If you want to, I can take over the task to refactor this PR accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, please go ahead.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sschuberth Any chance we can get this change into tomorrow's release?

Copy link
Member

Choose a reason for hiding this comment

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

That's my goal, yes, and I'm on it. Just learned a few unexpected things about Gradle toolchains that require me to think about it a bit more.

Copy link
Member

Choose a reason for hiding this comment

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

See #8411, which I propose to use instead of this PR.


val mergeDetektReportsTaskName = "mergeDetektReports"
val mergeDetektReports = if (rootProject.tasks.findByName(mergeDetektReportsTaskName) != null) {
Expand All @@ -124,7 +125,7 @@ val mergeDetektReports = if (rootProject.tasks.findByName(mergeDetektReportsTask
}

tasks.withType<Detekt>().configureEach detekt@{
jvmTarget = maxKotlinJvmTarget.target
jvmTarget = jvmTargetCompatibility.target

dependsOn(":detekt-rules:assemble")

Expand Down Expand Up @@ -154,8 +155,8 @@ tasks.withType<Detekt>().configureEach detekt@{

tasks.withType<JavaCompile>().configureEach {
// Align this with Kotlin to avoid errors, see https://youtrack.jetbrains.com/issue/KT-48745.
sourceCompatibility = maxKotlinJvmTarget.target
targetCompatibility = maxKotlinJvmTarget.target
sourceCompatibility = jvmSourceCompatibility.target
targetCompatibility = jvmTargetCompatibility.target
}

tasks.withType<KotlinCompile>().configureEach {
Expand All @@ -172,7 +173,7 @@ tasks.withType<KotlinCompile>().configureEach {
compilerOptions {
allWarningsAsErrors = true
freeCompilerArgs.addAll(customCompilerArgs)
jvmTarget = maxKotlinJvmTarget
jvmTarget = jvmTargetCompatibility
}
}

Expand Down
Loading