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

Gradle 8.0 #2860

Closed
wants to merge 9 commits into from
Closed

Gradle 8.0 #2860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5eab3ae
Gradle 8.0
Goooler Feb 14, 2023
cd279cf
Shadow 7.1.2 in buildSrc
Goooler Feb 14, 2023
2c5e250
binary-compatibility-validator 0.12.1
Goooler Feb 14, 2023
4791920
Update examples/gradle/dokka-customFormat-example/gradle/wrapper/grad…
Goooler Feb 27, 2023
aa2aa1d
Merge branch 'master' into gradle8
Goooler Mar 6, 2023
432f9f3
Gradle 8.0.2
Goooler Mar 6, 2023
54c1d3e
Shadow 8.1.0
Goooler Mar 6, 2023
cf9d471
binary-compatibility-validator 0.13.0
Goooler Mar 6, 2023
ee5460b
Set apiVersion 1.4
Goooler Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions buildSrc/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ repositories {
}

dependencies {
implementation("com.github.jengelman.gradle.plugins:shadow:2.0.4")
implementation("org.jetbrains.kotlinx:binary-compatibility-validator:0.10.1")
implementation("com.github.johnrengelman:shadow:8.1.0")
implementation("org.jetbrains.kotlinx:binary-compatibility-validator:0.13.0")
implementation("io.github.gradle-nexus:publish-plugin:1.1.0")
}
2 changes: 1 addition & 1 deletion examples/gradle/dokka-customFormat-example/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion examples/gradle/dokka-gradle-example/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion examples/gradle/dokka-kotlinAsJava-example/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion examples/gradle/dokka-library-publishing-example/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion examples/gradle/dokka-multimodule-example/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion examples/gradle/dokka-multiplatform-example/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion ...ples/gradle/dokka-versioning-multimodule-example/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion examples/plugin/hide-internal-api/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion ...ests/gradle/projects/it-multimodule-versioning-0/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
10 changes: 9 additions & 1 deletion runners/gradle-plugin/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import org.gradle.configurationcache.extensions.serviceOf
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
import org.jetbrains.kotlin.gradle.dsl.KotlinVersion
import org.jetbrains.*

plugins {
Expand All @@ -22,6 +23,13 @@ dependencies {
testImplementation("com.android.tools.build:gradle:4.0.1")
}

tasks.withType<KotlinCompile>().configureEach {
compilerOptions {
apiVersion.set(KotlinVersion.fromVersion("1.4"))
languageVersion.set(KotlinVersion.fromVersion("1.4"))
}
}

Comment on lines +26 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

hey @Goooler, thanks for the ping

Gradle 8 bumped the Kotlin language level to 1.8

Also, the kotlin-dsl plugin will automatically configure the language level (as well as other stuff), so there's no need to set it manually.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tasks.withType<KotlinCompile>().configureEach {
compilerOptions {
apiVersion.set(KotlinVersion.fromVersion("1.4"))
languageVersion.set(KotlinVersion.fromVersion("1.4"))
}
}

Copy link
Contributor Author

@Goooler Goooler Mar 6, 2023 •
edited

Choose a reason for hiding this comment

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

Cause there are some outdated APIs used, seems we can't elevate API level to 1.8 for now, that's why I declare the overriding here.

https://github.com/Kotlin/dokka/actions/runs/4344123199/jobs/7587028523
https://github.com/Kotlin/dokka/actions/runs/4344122470/jobs/7587025547

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right, okay.

The deprecations look like minor things to fix

e: warnings found and -Werror specified
w: file:///home/runner/work/dokka/dokka/runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/DokkaPlugin.kt:109:63 'decapitalize(): String' is deprecated. Use replaceFirstChar instead.
w: file:///home/runner/work/dokka/dokka/runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/dokkaDefaultOutputDirectory.kt:11:59 'decapitalize(): String' is deprecated. Use replaceFirstChar instead.
w: file:///home/runner/work/dokka/dokka/runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/kotlin/isMainSourceSet.kt:26:34 'toLowerCase(): String' is deprecated. Use lowercase() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I did, but more things in https://github.com/Kotlin/dokka/actions/runs/4343604835/jobs/7585857751

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here detekt/detekt#1286

Copy link
Contributor

@aSemy aSemy Mar 8, 2023 •
edited

Choose a reason for hiding this comment

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

I've been bitten by using kotlin-dsl in a plugin before, because the plugin was being used in versions of Gradle that were older than the version used to compile the plugin. I'd suggest reverting 2702 and avoiding use of kotlin-dsl in the plugin itself.

The kotlin-dsl plugin just adds some sensible defaults, removing it won't help - those defaults would have to be added back in and it would make the build config more complicated and brittle. So my guess is the problems you're referring to, that detekt/detekt#1286 solved, would be caused by mis-configuration (e.g. adding a runtime dependency on gradleApi()), or it using the Gradle embedded Kotlin. (Both of those problems seem have been resolved in newer Dekect plugin versions.)

In any case, it's not relevant to this PR.

Do I understand correctly that the old version of AGP is holding us from updating? That is, AGP 4.0.1 is not compatible with Gradle 8?

It's unclear. That's what the test failure seemed to suggest, but it doesn't make sense to me. The Android dependency shouldn't be 'aware' of the Gradle version†, so why should there be a missing class just because the Gradle version is different?

I'll do some digging today. It might be that we make DGP variants that are aware of the Gradle version, which on one hand is cool but on the other it will be a pain.

† While it is possible to publish plugins variants that are aware of the Gradle version that say "I'm only compatible with Gradle 7.6", so Gradle won't select them. But that's not the error (it would be a dependency resolution error), and AGP is older than the Gradle version that introduced this feature.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the context on kotlin-dsl! We'll keep an eye on it. So far it's not causing problems and I hope the integration tests cover it well

It's unclear. #2860 (comment), but it doesn't make sense to me.

Yeah, that's what I can't understand as well, the error is quite strange, considering the changes :( Unless AGP was using some Gradle API whch was removed/reworked

Thank you for taking a look!

Copy link
Contributor

Choose a reason for hiding this comment

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

The actual cause of the test failure was hidden in the GitHub logs. I reproduced it locally, and here's the key parts of the stacktrace:

org.gradle.api.ProjectConfigurationException: A problem occurred configuring root project 'android-auto-configuration-test'.

Caused by: org.gradle.internal.event.ListenerNotificationException: Failed to notify project evaluation listener.

Caused by: java.lang.NoClassDefFoundError: org/gradle/api/plugins/MavenPlugin
	at com.android.build.gradle.internal.variant.VariantHelper.setupArchivesConfig(VariantHelper.java:47)
	at com.android.build.gradle.internal.LibraryTaskManager.createBundleTask(LibraryTaskManager.java:396)

Caused by: java.lang.ClassNotFoundException: org.gradle.api.plugins.MavenPlugin

The org.gradle.api.plugins.MavenPlugin class was moved and marked as deprecated in Gradle 7, and now it's removed in Gradle 8 gradle/gradle#22807

So yeah, Android-GP 4.0.1 isn't compatible with Gradle 8 (I also checked with 4.1.3 - same problem).

However, I think the test failure is caused by the test not creating a realistic scenario. It uses org.gradle.testfixtures.ProjectBuilder, which is intrinsically linked to the Gradle version used to build Dokka.

dokka/runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/AndroidAutoConfigurationTest.kt

Lines 13 to 20 in 14c05d7

private val project = ProjectBuilder.builder().build().also { project ->
project.plugins.apply("com.android.library")
project.plugins.apply("org.jetbrains.kotlin.android")
project.plugins.apply("org.jetbrains.dokka")
project.extensions.configure<LibraryExtension> {
compileSdkVersion(28)
}
}

In real life that's not going to happen - Android-GP would have to use Gradle 7 or lower.

As I see it, the workaround is to re-write AndroidAutoConfigurationTest to use Gradle TestKit, which Dokka already has set up, and to specify the Gradle version to be 7 or lower for AndroidAutoConfigurationTest.

dokka/integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/AbstractGradleIntegrationTest.kt

Lines 29 to 49 in 14c05d7

fun createGradleRunner(
vararg arguments: String,
jvmArgs: List<String> = listOf("-Xmx4G", "-XX:MaxMetaspaceSize=2G")
): GradleRunner {
return GradleRunner.create()
.withProjectDir(projectDir)
.forwardOutput()
.withJetBrainsCachedGradleVersion(versions.gradleVersion)
.withTestKitDir(File("build", "gradle-test-kit").absoluteFile)
.withArguments(
listOfNotNull(
"-Pkotlin_version=${versions.kotlinVersion}",
"-Pdokka_it_kotlin_version=${versions.kotlinVersion}",
versions.androidGradlePluginVersion?.let { androidVersion ->
"-Pdokka_it_android_gradle_plugin_version=$androidVersion"
},
* arguments
)
).run { this as DefaultGradleRunner }
.withJvmArguments(jvmArgs)
}

Copy link
Member

Choose a reason for hiding this comment

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

Nice find! It indeed doesn't look like it's Dokka's problem in this case, since we do support AGP 4.X, but AGP 4.X itself is not compatible with Gradle 8 - not Dokka's fault

Refactoring the test to utilize TestKit would be good, preferably in a separate PR. Although I'm not sure if it'll be possible to check the same things as in AndroidAutoConfigurationTest - it requires access to the configured Dokka tasks. Maybe it could be done as verify tasks inside of the test project? We actually already have an integration test for Android - Android0GradleIntegrationTest)

// Gradle will put its own version of the stdlib in the classpath, do not pull our own we will end up with
// warnings like 'Runtime JAR files in the classpath should have the same version'
configurations.api.configure {
Expand Down
2 changes: 1 addition & 1 deletion settings.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pluginManagement {
val kotlin_version: String by settings
plugins {
id("org.jetbrains.kotlin.jvm") version kotlin_version
id("com.github.johnrengelman.shadow") version "7.1.2"
id("com.github.johnrengelman.shadow") version "8.1.0"
id("com.gradle.plugin-publish") version "0.20.0"
}
}
Expand Down