Update to Gradle 8.6 #245
Update to Gradle 8.6 #245
Conversation
translatenix
force-pushed
the
gradle-8
branch
from
06c16e6
to
3a23ee3
Compare
|
If you can figure out why the hashes changed, I'm up against the same issue in #204. I wonder if hashes broke in the same way for both of us? |
Test package zip files are created with Gradle's Zip task, so perhaps something changed there. (I think it would be better to use the same code as |
translatenix
force-pushed
the
gradle-8
branch
2 times, most recently
from
fce4a00
to
8490c58
Compare
|
Thanks! Will review this after #227 is merged.
Gradle must have changed something about how their zip balls are produced. But, as long as these hashes are reproducible, it doesn't really matter that much. |
|
@translatenix can you rebase again? Thanks! |
|
@bioball done |
|
Instead of using a hardcoded string for the |
|
@StefMa What’s the benefit? It’s quite verbose. |
|
Well, its unlikey to happen, but you can also change the In this case hardcoding |
|
Doesn’t sound compelling. |
|
There's also the need to parse the strings, and of course string paths can interfere on Windows... |
There was a problem hiding this comment.
I think we should try to do the "right thing" and use layout.buildDirectory instead.
I had started providing suggestions line by line, but it was taking too long, so, here's a diff for you to patch in as a whole:
diff --git a/build.gradle.kts b/build.gradle.kts
index a0bb1050e..28056dcdb 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -41,7 +41,7 @@ idea {
}
val clean by tasks.registering(Delete::class) {
- delete("build")
+ delete(layout.buildDirectory)
}
val printVersion by tasks.registering {
diff --git a/buildSrc/src/main/kotlin/pklFatJar.gradle.kts b/buildSrc/src/main/kotlin/pklFatJar.gradle.kts
index 844cee2d2..675e2dbf8 100644
--- a/buildSrc/src/main/kotlin/pklFatJar.gradle.kts
+++ b/buildSrc/src/main/kotlin/pklFatJar.gradle.kts
@@ -105,7 +105,7 @@ tasks.check {
}
val validateFatJar by tasks.registering {
- val outputFile = file("build/validateFatJar/result.txt")
+ val outputFile = layout.buildDirectory.file("validateFatJar/result.txt")
inputs.files(tasks.shadowJar)
inputs.property("nonRelocations", nonRelocations)
outputs.file(outputFile)
@@ -125,9 +125,9 @@ val validateFatJar by tasks.registering {
}
}
if (unshadowedFiles.isEmpty()) {
- outputFile.writeText("SUCCESS")
+ outputFile.get().asFile.writeText("SUCCESS")
} else {
- outputFile.writeText("FAILURE")
+ outputFile.get().asFile.writeText("FAILURE")
throw GradleException("Found unshadowed files:\n" + unshadowedFiles.joinToString("\n"))
}
}
@@ -138,7 +138,7 @@ tasks.check {
val resolveSourcesJars by tasks.registering(ResolveSourcesJars::class) {
configuration.set(configurations.runtimeClasspath)
- outputDir.set(project.file("build/resolveSourcesJars"))
+ outputDir.set(layout.buildDirectory.dir("resolveSourcesJars"))
}
val fatSourcesJar by tasks.registering(MergeSourcesJars::class) {
diff --git a/buildSrc/src/main/kotlin/pklHtmlValidator.gradle.kts b/buildSrc/src/main/kotlin/pklHtmlValidator.gradle.kts
index 9eef0a6d6..65a8ecb11 100644
--- a/buildSrc/src/main/kotlin/pklHtmlValidator.gradle.kts
+++ b/buildSrc/src/main/kotlin/pklHtmlValidator.gradle.kts
@@ -33,7 +33,7 @@ dependencies {
}
val validateHtml by tasks.registering(JavaExec::class) {
- val resultFile = file("build/validateHtml/result.txt")
+ val resultFile = layout.buildDirectory.file("validateHtml/result.txt")
inputs.files(htmlValidator.sources)
outputs.file(resultFile)
@@ -50,7 +50,7 @@ val validateHtml by tasks.registering(JavaExec::class) {
// write a basic result file s.t. gradle can consider task up-to-date
// writing a result file in case validation fails is not easily possible with JavaExec, but also not strictly necessary
doFirst { project.delete(resultFile) }
- doLast { resultFile.writeText("Success.") }
+ doLast { resultFile.get().asFile.writeText("Success.") }
}
tasks.check {
diff --git a/buildSrc/src/main/kotlin/pklJavaLibrary.gradle.kts b/buildSrc/src/main/kotlin/pklJavaLibrary.gradle.kts
index 93679dbc6..a79d78347 100644
--- a/buildSrc/src/main/kotlin/pklJavaLibrary.gradle.kts
+++ b/buildSrc/src/main/kotlin/pklJavaLibrary.gradle.kts
@@ -55,7 +55,7 @@ val workAroundKotlinGradlePluginBug by tasks.registering {
// A problem was found with the configuration of task ':pkl-executor:compileJava' (type 'JavaCompile').
// > Directory '[...]/pkl/pkl-executor/build/classes/kotlin/main'
// specified for property 'compileKotlinOutputClasses' does not exist.
- file("build/classes/kotlin/main").mkdirs()
+ layout.buildDirectory.dir("classes/kotlin/main").get().asFile.mkdirs()
}
}
diff --git a/buildSrc/src/main/kotlin/pklPublishLibrary.gradle.kts b/buildSrc/src/main/kotlin/pklPublishLibrary.gradle.kts
index 212227bc8..e08830525 100644
--- a/buildSrc/src/main/kotlin/pklPublishLibrary.gradle.kts
+++ b/buildSrc/src/main/kotlin/pklPublishLibrary.gradle.kts
@@ -54,14 +54,14 @@ val validatePom by tasks.registering {
return@registering
}
val generatePomFileForLibraryPublication by tasks.existing(GenerateMavenPom::class)
- val outputFile = file("build/validatePom") // dummy output to satisfy up-to-date check
+ val outputFile = layout.buildDirectory.file("validatePom") // dummy output to satisfy up-to-date check
dependsOn(generatePomFileForLibraryPublication)
inputs.file(generatePomFileForLibraryPublication.get().destination)
outputs.file(outputFile)
doLast {
- outputFile.delete()
+ outputFile.get().asFile.delete()
val pomFile = generatePomFileForLibraryPublication.get().destination
assert(pomFile.exists())
@@ -95,7 +95,7 @@ val validatePom by tasks.registering {
}
}
- outputFile.writeText("OK")
+ outputFile.get().asFile.writeText("OK")
}
}
diff --git a/pkl-cli/pkl-cli.gradle.kts b/pkl-cli/pkl-cli.gradle.kts
index a98d977ea..d779e46d1 100644
--- a/pkl-cli/pkl-cli.gradle.kts
+++ b/pkl-cli/pkl-cli.gradle.kts
@@ -56,11 +56,13 @@ dependencies {
testImplementation(projects.pklCommonsTest)
- stagedMacAmd64Executable(files("build/executable/pkl-macos-amd64"))
- stagedMacAarch64Executable(files("build/executable/pkl-macos-aarch64"))
- stagedLinuxAmd64Executable(files("build/executable/pkl-linux-amd64"))
- stagedLinuxAarch64Executable(files("build/executable/pkl-linux-aarch64"))
- stagedAlpineLinuxAmd64Executable(files("build/executable/pkl-alpine-linux-amd64"))
+ val buildDir = layout.buildDirectory
+ stagedMacAmd64Executable(files(buildDir.dir("executable/pkl-macos-amd64")))
+ stagedMacAmd64Executable(files(buildDir.dir("executable/pkl-macos-amd64")))
+ stagedMacAarch64Executable(files(buildDir.dir("executable/pkl-macos-aarch64")))
+ stagedLinuxAmd64Executable(files(buildDir.dir("executable/pkl-linux-amd64")))
+ stagedLinuxAarch64Executable(files(buildDir.dir("executable/pkl-linux-aarch64")))
+ stagedAlpineLinuxAmd64Executable(files(buildDir.dir("executable/pkl-alpine-linux-amd64")))
}
tasks.jar {
@@ -90,7 +92,7 @@ tasks.shadowJar {
val javaExecutable by tasks.registering(ExecutableJar::class) {
inJar.set(tasks.shadowJar.flatMap { it.archiveFile })
- outJar.set(file("build/executable/jpkl"))
+ outJar.set(layout.buildDirectory.file("executable/jpkl"))
// uncomment for debugging
//jvmArgs.addAll("-ea", "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005")
@@ -117,22 +119,22 @@ tasks.check {
// To catch this and similar problems, test that Java executable starts successfully.
val testStartJavaExecutable by tasks.registering(Exec::class) {
dependsOn(javaExecutable)
-val outputFile = file("build/testStartJavaExecutable") // dummy output to satisfy up-to-date check
+ val outputFile = layout.buildDirectory.file("testStartJavaExecutable") // dummy output to satisfy up-to-date check
outputs.file(outputFile)
executable = javaExecutable.get().outputs.files.singleFile.toString()
args("--version")
- doFirst { outputFile.delete() }
+ doFirst { outputFile.get().asFile.delete() }
- doLast { outputFile.writeText("OK") }
+ doLast { outputFile.get().asFile.writeText("OK") }
}
tasks.check {
dependsOn(testStartJavaExecutable)
}
-fun Exec.configureExecutable(isEnabled: Boolean, outputFile: File, extraArgs: List<String> = listOf()) {
+fun Exec.configureExecutable(isEnabled: Boolean, outputFile: Provider<RegularFile>, extraArgs: List<String> = listOf()) {
enabled = isEnabled
dependsOn(":installGraalVm")
@@ -140,7 +142,7 @@ fun Exec.configureExecutable(isEnabled: Boolean, outputFile: File, extraArgs: Li
inputs.files(configurations.runtimeClasspath)
outputs.file(outputFile)
- workingDir = outputFile.parentFile
+ workingDir(outputFile.map { it.asFile.parentFile })
executable = "${buildInfo.graalVm.baseDir}/bin/native-image"
// JARs to exclude from the class path for the native-image build.
@@ -161,7 +163,7 @@ fun Exec.configureExecutable(isEnabled: Boolean, outputFile: File, extraArgs: Li
,"-H:IncludeResourceBundles=org.pkl.core.errorMessages"
,"--macro:truffle"
,"-H:Class=org.pkl.cli.Main"
- ,"-H:Name=${outputFile.name}"
+ ,"-H:Name=${outputFile.get().asFile.name}"
//,"--native-image-info"
//,"-Dpolyglot.image-build-time.PreinitializeContexts=pkl"
// the actual limit (currently) used by native-image is this number + 1400 (idea is to compensate for Truffle's own nodes)
@@ -209,7 +211,7 @@ fun Exec.configureExecutable(isEnabled: Boolean, outputFile: File, extraArgs: Li
* Builds the pkl CLI for macOS/amd64.
*/
val macExecutableAmd64: TaskProvider<Exec> by tasks.registering(Exec::class) {
- configureExecutable(buildInfo.os.isMacOsX && buildInfo.graalVm.isGraal22, file("build/executable/pkl-macos-amd64"))
+ configureExecutable(buildInfo.os.isMacOsX && buildInfo.graalVm.isGraal22, layout.buildDirectory.file("executable/pkl-macos-amd64"))
}
/**
@@ -221,7 +223,7 @@ val macExecutableAmd64: TaskProvider<Exec> by tasks.registering(Exec::class) {
val macExecutableAarch64: TaskProvider<Exec> by tasks.registering(Exec::class) {
configureExecutable(
buildInfo.os.isMacOsX && !buildInfo.graalVm.isGraal22,
- file("build/executable/pkl-macos-aarch64"),
+ layout.buildDirectory.file("executable/pkl-macos-aarch64"),
listOf(
"--initialize-at-run-time=org.msgpack.core.buffer.DirectBufferAccess",
"-H:+AllowDeprecatedBuilderClassesOnImageClasspath"
@@ -233,7 +235,7 @@ val macExecutableAarch64: TaskProvider<Exec> by tasks.registering(Exec::class) {
* Builds the pkl CLI for linux/amd64.
*/
val linuxExecutableAmd64: TaskProvider<Exec> by tasks.registering(Exec::class) {
- configureExecutable(buildInfo.os.isLinux && buildInfo.arch == "amd64", file("build/executable/pkl-linux-amd64"))
+ configureExecutable(buildInfo.os.isLinux && buildInfo.arch == "amd64", layout.buildDirectory.file("executable/pkl-linux-amd64"))
}
/**
@@ -243,7 +245,7 @@ val linuxExecutableAmd64: TaskProvider<Exec> by tasks.registering(Exec::class) {
* ARM instances.
*/
val linuxExecutableAarch64: TaskProvider<Exec> by tasks.registering(Exec::class) {
- configureExecutable(buildInfo.os.isLinux && buildInfo.arch == "aarch64", file("build/executable/pkl-linux-aarch64"))
+ configureExecutable(buildInfo.os.isLinux && buildInfo.arch == "aarch64", layout.buildDirectory.file("executable/pkl-linux-aarch64"))
}
/**
@@ -255,7 +257,7 @@ val linuxExecutableAarch64: TaskProvider<Exec> by tasks.registering(Exec::class)
val alpineExecutableAmd64: TaskProvider<Exec> by tasks.registering(Exec::class) {
configureExecutable(
buildInfo.os.isLinux && buildInfo.arch == "amd64" && buildInfo.hasMuslToolchain,
- file("build/executable/pkl-alpine-linux-amd64"),
+ layout.buildDirectory.file("executable/pkl-alpine-linux-amd64"),
listOf(
"--static",
"--libc=musl",
diff --git a/pkl-commons-test/pkl-commons-test.gradle.kts b/pkl-commons-test/pkl-commons-test.gradle.kts
index 25e47634b..b3873905d 100644
--- a/pkl-commons-test/pkl-commons-test.gradle.kts
+++ b/pkl-commons-test/pkl-commons-test.gradle.kts
@@ -31,11 +31,11 @@ tasks.test {
for (packageDir in file("src/main/files/packages").listFiles()!!) {
if (!packageDir.isDirectory) continue
- val destinationDir = file("build/test-packages/${packageDir.name}")
+ val destinationDir = layout.buildDirectory.dir("test-packages/${packageDir.name}")
val metadataJson = file("$packageDir/${packageDir.name}.json")
val packageContents = packageDir.resolve("package")
val zipFileName = "${packageDir.name}.zip"
- val archiveFile = destinationDir.resolve(zipFileName)
+ val archiveFile = destinationDir.map { it.file(zipFileName) }
val zipTask = tasks.register("zip-${packageDir.name}", Zip::class) {
destinationDirectory.set(destinationDir)
@@ -53,10 +53,10 @@ for (packageDir in file("src/main/files/packages").listFiles()!!) {
val shasumFile = file("$destinationDir/${packageDir.name}.json.sha256")
outputs.file(shasumFile)
filter { line ->
- line.replaceFirst("\$computedChecksum", archiveFile.computeChecksum())
+ line.replaceFirst("\$computedChecksum", archiveFile.get().asFile.computeChecksum())
}
doLast {
- val outputFile = destinationDir.resolve("${packageDir.name}.json")
+ val outputFile = destinationDir.get().asFile.resolve("${packageDir.name}.json")
shasumFile.writeText(outputFile.computeChecksum())
}
}
@@ -66,7 +66,7 @@ for (packageDir in file("src/main/files/packages").listFiles()!!) {
}
}
-val keystoreDir = file("build/keystore")
+val keystoreDir = layout.buildDirectory.dir("keystore")
val keystoreName = "localhost.p12"
val certsFileName = "localhost.pem"
@@ -82,7 +82,7 @@ val generateKeys by tasks.registering(JavaExec::class) {
"-storepass", "password",
"-dname", "CN=localhost"
)
- workingDir = keystoreDir
+ workingDir(keystoreDir)
doFirst {
workingDir.mkdirs()
outputFile.delete()
@@ -103,7 +103,7 @@ val exportCerts by tasks.registering(JavaExec::class) {
"-rfc",
"-file", certsFileName
)
- workingDir = keystoreDir
+ workingDir(keystoreDir)
doFirst {
workingDir.mkdirs()
outputFile.delete()
diff --git a/pkl-config-java/pkl-config-java.gradle.kts b/pkl-config-java/pkl-config-java.gradle.kts
index 749d26415..d24d6cd61 100644
--- a/pkl-config-java/pkl-config-java.gradle.kts
+++ b/pkl-config-java/pkl-config-java.gradle.kts
@@ -10,14 +10,18 @@ val pklCodegenJava: Configuration by configurations.creating
val firstPartySourcesJars by configurations.existing
val generateTestConfigClasses by tasks.registering(JavaExec::class) {
- outputs.dir("build/testConfigClasses")
+ val outputDir = layout.buildDirectory.dir("testConfigClasses")
+ outputs.dir(outputDir)
inputs.dir("src/test/resources/codegenPkl")
classpath = pklCodegenJava
mainClass.set("org.pkl.codegen.java.Main")
- args("--output-dir", "build/testConfigClasses")
- args("--generate-javadoc")
- args(fileTree("src/test/resources/codegenPkl"))
+ argumentProviders.add(CommandLineArgumentProvider {
+ listOf(
+ "--output-dir", outputDir.get().asFile.absolutePath,
+ "--generate-javadoc"
+ ) + fileTree("src/test/resources/codegenPkl").map { it.absolutePath }
+ })
}
tasks.processTestResources {
@@ -56,8 +60,8 @@ val testFromJar by tasks.registering(Test::class) {
//}
sourceSets.getByName("test") {
- java.srcDir("build/testConfigClasses/java")
- resources.srcDir("build/testConfigClasses/resources")
+ java.srcDir(layout.buildDirectory.dir("testConfigClasses/java"))
+ resources.srcDir(layout.buildDirectory.dir("testConfigClasses/resources"))
}
dependencies {
diff --git a/pkl-config-kotlin/pkl-config-kotlin.gradle.kts b/pkl-config-kotlin/pkl-config-kotlin.gradle.kts
index bbef36f0a..21b52ee3a 100644
--- a/pkl-config-kotlin/pkl-config-kotlin.gradle.kts
+++ b/pkl-config-kotlin/pkl-config-kotlin.gradle.kts
@@ -28,18 +28,21 @@ dependencies {
}
val generateTestConfigClasses by tasks.registering(JavaExec::class) {
- outputs.dir("build/testConfigClasses")
+ val outputDir = layout.buildDirectory.dir("testConfigClasses")
+ outputs.dir(outputDir)
inputs.dir("src/test/resources/codegenPkl")
classpath = pklCodegenKotlin
mainClass.set("org.pkl.codegen.kotlin.Main")
- args("--output-dir", "build/testConfigClasses")
- args(fileTree("src/test/resources/codegenPkl"))
+ argumentProviders.add(CommandLineArgumentProvider {
+ listOf("--output-dir", outputDir.get().asFile.absolutePath) +
+ fileTree("src/test/resources/codegenPkl").map { it.absolutePath }
+ })
}
sourceSets.getByName("test") {
- java.srcDir("build/testConfigClasses/kotlin")
- resources.srcDir("build/testConfigClasses/resources")
+ java.srcDir(layout.buildDirectory.dir("testConfigClasses/kotlin"))
+ resources.srcDir(layout.buildDirectory.dir("testConfigClasses/resources"))
}
tasks.processTestResources {
diff --git a/pkl-core/pkl-core.gradle.kts b/pkl-core/pkl-core.gradle.kts
index 532768c31..f358e428b 100644
--- a/pkl-core/pkl-core.gradle.kts
+++ b/pkl-core/pkl-core.gradle.kts
@@ -282,7 +282,7 @@ tasks.testNative {
tasks.clean {
delete("generated/")
- delete("build/test-packages")
+ delete(layout.buildDirectory.dir("test-packages"))
}
spotless {
diff --git a/stdlib/stdlib.gradle.kts b/stdlib/stdlib.gradle.kts
index 48cd0b156..4f0159ce5 100644
--- a/stdlib/stdlib.gradle.kts
+++ b/stdlib/stdlib.gradle.kts
@@ -10,7 +10,7 @@ plugins {
// create and publish a self-contained stdlib archive
// purpose is to provide non-jvm tools/projects with a versioned stdlib
val stdlibZip by tasks.registering(Zip::class) {
- destinationDirectory.set(file("build/libs"))
+ destinationDirectory.set(layout.buildDirectory.dir("libs"))
archiveBaseName.set("pkl-stdlib")
archiveVersion.set(project.version as String)
into("org/pkl/stdlib") {| @@ -28,7 +28,7 @@ abstract class AbstractTest { | |||
| .withProjectDir(testProjectDir.toFile()) | |||
| .withArguments("--stacktrace", "--no-build-cache", taskName) | |||
| .withPluginClasspath() | |||
| .withDebug(true) | |||
| //.withDebug(true) | |||
This option is meant for debugging, but I put it back. (At some point it didn't work.)
Can this be done in a separate PR? Some background: Blindly replacing with doesn't improve anything because methods such as |
|
@translatenix From the issue, this: testResults.from("${layout.buildDirectory.get().asFile}/test-results/testIntegration/binary")Can actually be: testResults.from(layout.buildDirectory.dir("test-results/testIntegration/binary")) |
|
@sgammon Yeah I wouldn't write such code. But my point still stands--just take a look at the suggested diff. |
|
@translatenix I was just trying to make a case that it can be slightly less verbose than expressed in that issue. and Are not identical behavior, though, because the args in the former are computed immediately; the args are deferred as well as |
|
In any case, I'm just another PR filer / user, my only intent is to offer a less verbose route in case it is less of a change / less painful to implement. 👍 |
|
There's nothing to compute here unless you're trying to hypothetically save a microsecond. I think a separate PR is the way to go. |
Fine by me; will submit that patch as a follow-up |
| } | ||
| } | ||
| private val packagesDir: Path = | ||
| FileTestUtils.rootProjectDir.resolve("pkl-commons-test/build/test-packages") |
There was a problem hiding this comment.
Yeah, this is much simpler. Thanks!
|
@translatenix Ha! Our web designer is a theme from Webflow but that is very kind of you. Buildless is open and can be self-hosted. We'd be happy to give Pkl a free instance if the team would like to try it. It accelerates local builds and CI builds, especially in GHA and Circle. The best part of Buildless is that you can open the cache for read-only traffic publicly. So contributors can gain the benefits, too. Develocity is a whole different beast: the reporting, test execution/avoidance, etc., are not things we cover. We are a rosetta stone of build caching protocols, so you can use Buildless with Bazel and SCCache, too. The two are complementary because we can replicate writes to Develocity and we can front it with our CDN layer, which is Cloudflare Enterprise. In any case, thank you for asking. I am a big fan of Pkl and here to help however I can. |
|
Sounds good. I'm not part of the team, just trying to help where I can. |
|
Well I will still give you an instance if you want one, just fill in the form and we can get you in the next beta wave 😎. I am using it on my fork and it works great in the Pkl build. But I'm a build caching nerd and I don't expect everyone to be 🤣 |
|
I think you should demo buildless to the Pkl team! |
|
@bioball 👀 if you'd have us we'd be glad! But anyway, it's not right to talk about buildless on your PR 😄 thanks for the Gradle upgrade because it will let me rebase a lot away. |
| doFirst { | ||
| expand(mapOf("computedChecksum" to archiveFile.computeChecksum())) | ||
| filter { line -> | ||
| line.replaceFirst("\$computedChecksum", archiveFile.computeChecksum()) |
There was a problem hiding this comment.
Curious why you changed this? The original approach is not deprecated.
There was a problem hiding this comment.
The original approach is wrong.
expand is a config option, i.e., it's supposed to be called at configuration time, not in doFirst.
More recent tasks will fail if a config option is set at execution time, but apparently Copy has yet to be retrofitted.
Besides, expand is hiding a monster:
The expand() method treats the source files as Groovy templates, which evaluate and expand expressions of the form ${expression}. You can pass in property names and values that are then expanded in the source files. expand() allows for more than basic token substitution as the embedded expressions are full-blown Groovy expressions.
pkl-gradle/pkl-gradle.gradle.kts
Outdated
| tasks.compileTestKotlin { | ||
| // Work around a conflict between Pkl's and Gradle's | ||
| // Kotlin dependencies on the test compile class path. | ||
| // | ||
| // My preferred solution would be to clean up the test | ||
| // compile class path to no longer contain a Gradle distribution. | ||
| // However, my Gradle knowledge proved insufficient to accomplish this. | ||
| // | ||
| // Another potential workaround is to port plugin tests to Java. | ||
| // (If the plugin was written in Kotlin, its compilation would | ||
| // currently fail with the same error.) |
There was a problem hiding this comment.
| tasks.compileTestKotlin { | |
| // Work around a conflict between Pkl's and Gradle's | |
| // Kotlin dependencies on the test compile class path. | |
| // | |
| // My preferred solution would be to clean up the test | |
| // compile class path to no longer contain a Gradle distribution. | |
| // However, my Gradle knowledge proved insufficient to accomplish this. | |
| // | |
| // Another potential workaround is to port plugin tests to Java. | |
| // (If the plugin was written in Kotlin, its compilation would | |
| // currently fail with the same error.) | |
| // TODO: remove me when Kotlin is upgraded to 1.9 | |
| tasks.compileTestKotlin { | |
| // Work around a conflict between Pkl's and Gradle's | |
| // Kotlin dependencies on the test compile class path. | |
| // | |
| // Another potential workaround is to port plugin tests to Java. | |
| // (If the plugin was written in Kotlin, its compilation would | |
| // currently fail with the same error.) |
I don't think we can eschew Gradle's distribution from the test compile classpath, because our tests necessarily include Gradle's SDKs.
But, we can get around this issue if/when we upgrade to Kotlin 1.9 too. Let's add a TODO comment about that.
There was a problem hiding this comment.
I don't think we can eschew Gradle's distribution from the test compile classpath, because our tests necessarily include Gradle's SDKs.
We can, if we can figure out how to implement this in Gradle.
These tests just run builds with GradleRunner.
GradleRunner is similar to Pkl's Executor.
It loads Gradle in a separate class loader.
Unless withDebug() is used, it even runs Gradle in a separate JVM.
I believe Gradle JARs and their dependencies end up on the test compile class path because
the Gradle plugin plugin declares them as implementation (or even api) dependencies,
which the testCompileClassPath configuration inherits from.
This setup doesn't make much sense when black-box testing a Gradle plugin with GradleRunner,
but it is unlikely to cause hard errors unless plugin tests are written in Kotlin.
But, we can get around this issue if/when we upgrade to Kotlin 1.9 too.
This cannot be relied upon. The next time Gradle or Pkl updates their Kotlin version, the version skew will be back.
There was a problem hiding this comment.
Okay, I played around with this a little bit more, and this works:
sourceSets {
test {
// Exclude Gradle's kotlin stdlib from test compile classpath (conflicts with project's kotlin)
compileClasspath = compileClasspath.filter { !it.path.matches(Regex(".*gradle-\\d+\\.\\d+/lib/kotlin-stdlib.*")) }
}
}
I couldn't figure out how to exclude this by matching on group or artifact id. Seems like Gradle injects these deps out-of-band of the normal dependency structure. This seems related: gradle/gradle#19973
Might be interesting to try out https://github.com/gradle-plugins/toolbox at some point
There was a problem hiding this comment.
Well done. I replaced my workaround with an improved version of your solution that removes all Gradle distribution JARs. Ready to merge from my side.
| fun toHex(hash: ByteArray): String { | ||
| val hexDigitTable = charArrayOf('0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f') | ||
| val builder = StringBuilder(hash.size * 2) | ||
| for (b in hash) { | ||
| builder.append(hexDigitTable[b.toInt() shr 4 and 0xF]) | ||
| builder.append(hexDigitTable[b.toInt() and 0xF]) | ||
| } | ||
| return builder.toString() | ||
| } |
There was a problem hiding this comment.
While we are here changing things:
| fun toHex(hash: ByteArray): String { | |
| val hexDigitTable = charArrayOf('0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f') | |
| val builder = StringBuilder(hash.size * 2) | |
| for (b in hash) { | |
| builder.append(hexDigitTable[b.toInt() shr 4 and 0xF]) | |
| builder.append(hexDigitTable[b.toInt() and 0xF]) | |
| } | |
| return builder.toString() | |
| } | |
| fun toHex(hash: ByteArray): String { | |
| val hexDigitTable = charArrayOf('0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f') | |
| return buildString(hash.size * 2) { | |
| for (b in hash) { | |
| append(hexDigitTable[b.toInt() shr 4 and 0xF]) | |
| append(hexDigitTable[b.toInt() and 0xF]) | |
| } | |
| } | |
| } |
|
@bioball Ready from my side. |
- Fix and clean up the pkl-commons-test build script. - Change tests to read test packages/certs directly from the file system instead of packaging and reading them from the class path, which is both slower and more complicated. - Update expected checksums of some test packages. (Not sure why they changed.) - Fix a conflict between Pkl's and Gradle's Kotlin libraries in the pkl-gradle project. - Fix build deprecation warnings. - Ensure Gradle distribution integrity with `distributionSha256Sum`. - Manually verify integrity of Gradle wrapper added by this commit. Result: `gw clean build buildNative` succeeds with Gradle 8.6. Hunting down the remaining build deprecation warnings shown by `--warning-mode all` is worthwhile but not urgent. Most (possibly all) of them are caused by Gradle plugins, which I decided not to update in this commit.
|
@bioball CI is failing due to some missing task dependencies on |
This PR/commit builds on top of #227 to start from a stable Gradle 7.5 build.
the file system instead of packaging and reading them
from the class path, which is both slower and more complicated.
(Not sure why they changed.)
Kotlin libraries in the pkl-gradle project.
distributionSha256Sum.Result:
gw clean build buildNativesucceeds with Gradle 8.6.Hunting down the remaining build deprecation warnings shown
by
--warning-mode allis worthwhile but not urgent.Most (possibly all) of them are caused by Gradle plugins,
which I decided not to update in this commit.