pkl
pkl copied to clipboard
Update to Gradle 8.6
This PR/commit builds on top of #227 to start from a stable Gradle 7.5 build.
- 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.)
- Work around 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.
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?
If you can figure out why the hashes changed
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 pkl project package
.) But I didn't have the nerve to investigate further, updated the hashes and moved on.
Thanks! Will review this after #227 is merged.
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 pkl project package.) But I didn't have the nerve to investigate further, updated the hashes and moved on.
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 build
directory we could also use project.layout.buildDirectory
@StefMa What’s the benefit? It’s quite verbose.
Well, its unlikey to happen, but you can also change the buildDirectory
😬
https://docs.gradle.org/8.6/javadoc/org/gradle/api/Project.html#setBuildDir-java.io.File-
In this case hardcoding build
would fail 😁
Doesn’t sound compelling.
I think the real benefit is automatic dependency tracking for tasks with Property
inputs/outputs.
Perhaps refactoring the existing code could eliminate a ‘dependsOn’ here and there.
There's also the need to parse the strings, and of course string paths can interfere on Windows...
Hm?
This option is meant for debugging, but I put it back. (At some point it didn't work.)
I think we should try to do the "right thing" and use layout.buildDirectory instead.
Can this be done in a separate PR? "build"
is no worse than buildDir
.
Some background:
https://github.com/gradle/gradle/issues/26234
Blindly replacing buildDir
with layout.buildDirectory
doesn't feel like the right solution.
For example, replacing
args("--output-dir", "build/testConfigClasses")
args(fileTree("src/test/resources/codegenPkl"))
with
argumentProviders.add(CommandLineArgumentProvider {
listOf("--output-dir", outputDir.get().asFile.absolutePath) +
fileTree("src/test/resources/codegenPkl").map { it.absolutePath }
})
doesn't improve anything because methods such as fileTree()
are already lazy.
It only adds unnecessary complexity and makes the build less readable.
@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.
args("--output-dir", "build/testConfigClasses")
args(fileTree("src/test/resources/codegenPkl"))
and
argumentProviders.add(CommandLineArgumentProvider {
listOf("--output-dir", outputDir.get().asFile.absolutePath) +
fileTree("src/test/resources/codegenPkl").map { it.absolutePath }
})
Are not identical behavior, though, because the args in the former are computed immediately; the args are deferred as well as fileTree
. layout.buildDirectory
is deferred, so the CommandLineArgumentProvider
is irrelevant anyway. Gradle, however, will output warnings for $buildDir/
and strings have the downside of parsing and path issues cross-platform; truthfully, I wish Gradle kept it less verbose, but here we are.
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'm also not aware of any cross-platform issues that layout.buildDirectory
solves.
I think a separate PR is the way to go. Personally I believe there are more important things to improve (also in the build). For example, I'd love to update the libraries, and I'd LOVE to be able to clone the project on Windows.
Can this be done in a separate PR? "build" is no worse than buildDir.
Fine by me; will submit that patch as a follow-up
@sgammon Would it be possible, and make sense, to use buildless for the Pkl build? Sounds like direct competition for Develocity. And perhaps you could ask your web designer to give pkl-lang.org a facelift. :-)
@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.
@bioball Ready from my side.
@bioball CI is failing due to some missing task dependencies on createTestPackages
. Will send a follow-up PR shortly.