LUCENE-9897 Change dependency checking mechanism to use gradle checksum verification #108
Conversation
Remove custom checksum generation logic from our gradle build. We can now use gradle to both generate checksums and verify our dependencies to check that they match that checksum. Additionally, gradle generates checksums for all artifacts (not just JARs). These checksums are stored in verification-metadata.xml
|
Thanks, this looks suspiciously simple!... :) I'll be glad to experiment with it a bit. I'm not a big fan of the monolithic checksum file - the expanded version (per-jar checksum) seems easier. Checksums should only be generated for a subset of configurations - I don't think it's realistic to assume we can get checksums of everything (detached configurations, etc.). |
| def jarInfoTasks = subprojects.collectMany { it.tasks.matching { it.name == "collectJarInfos" } } | ||
|
|
||
| // Update dependency checksums. | ||
| task updateLicenses() { |
There was a problem hiding this comment.
Same here. I'd leave this task and use:
./gradlew --write-verification-metadata sha256 updateLicenses
I hate to remember these option switches... the task could verify if they're in place in doFirst and maybe with a hint on how to issue the full command properly if they're missing. Or, alternatively, it could be a GradleBuild task that would recursively invoke the same build with the right options...
There was a problem hiding this comment.
The mistake here was that we were only generating checksums for the dependencies and plugins that help or updateLicenses tasks use. The ./gradlew --write-verification-metadata sha256 check command records all dependencies used by the check task. Fixing this makes ./gradlew check work.
There was a problem hiding this comment.
Thanks. But does it mean that if we run some other task that pulls dependencies outside of check's scope the build will fail then?
There was a problem hiding this comment.
I don't think it will. If we run ./gradlew assemble it checks whether dependencies used in assemble are defined in the metadata. If we run ./gradlew check it checks if dependencies of check are defined in the metadata. As far as I know, our builds mainly use these tasks.
Our build will fail if you run ./gradlew <somenewtask> with the <somenewtask> dependencies not added to the metadata file. I'll try this case out and get back here..
There was a problem hiding this comment.
Alright! I tested a sample detachedConfiguration
tasks.register("checkDetachedDependencies") {
doLast {
def detachedConf = configurations.detachedConfiguration(dependencies.create("org.aspectj:aspectjrt:1.8.6"))
println(detachedConf.files)
}
}
This task fails if you execute it separately with ./gradlew checkDetachedDependencies giving the error that it needs the checksum of aspectj. However, ./gradlew check passes because check does not depend on checkDetachedDependencies
There was a problem hiding this comment.
But it's the opposite of what we want: we'd like those tasks that have detached dependencies to pass instead of fail. It's really hard to predict where such configurations may occur and it's just not possible to list all possible tasks to "--write-verification-metadata"...
The only way I see it as a functional substitute is if it can be passed an explicit set of configurations from which to generate the metadata and still work with any other configurations. Otherwise it's going to be very difficult to work with - I don't see how we can activate all possible configurations when writing checksums.
There was a problem hiding this comment.
My understanding was that check was a superset of almost every other configuration. In case, we wanted to add other detached configurations we could disableDependencyVerification() for them.
However, this seems like too much work for something so simple :)
I'll close this PR and wait silently until gradle adds an option for verifying just some select configurations. Or maybe work on adding it :p
Thanks for reviewing anyways!
There was a problem hiding this comment.
Check is just a convention aggregation task, nothing else. We have tons of other stuff that isn't connected with check in the execution graph - I bet some of these dependencies/ configurations are in in plugins and it'd be difficult to even hook into them to disable automatic dependency verification.
The work on this attempt isn't lost though (thank you!). Let's keep an eye on what happens with gradle's built-in checkums and retry the attempt when it's more flexible.
💯
I actually thought having a single file would be better for editing and understanding dependencies from a single place. |
|
It's fine. I kind of prefer filesystem (file name)-based correspondence of checksums to files but I can live with a monolithic file too. |
Update the dependencies.txt instructions
|
Some closing notes: We found the current gradle checksum verification functionality unsuitable for our use case. When could this possibly work? If gradle were to check dependencies only when the task that was used to generate the metadata file is executed, it would allow developers to safely create more configurations (with other dependencies) without the restriction of adding them to the global metadata file. |
Remove custom checksum generation logic from our gradle build.
We can now use gradle to both generate checksums and verify our dependencies to
check that they match that checksum.
Additionally, gradle generates checksums for all artifacts (not just JARs).
These checksums are stored in verification-metadata.xml
Description
I've followed the JIRA recommended method of solving the issue by using gradle based metadata verification.
This solution uses SHA256 over the previously used SHA1 used in the repo. We also generate checksums of all transitive artifacts now (pom files etc). It is possible to disable this feature if needed.
Solution
We add a basic file first and then use
./gradlew --write-verification-metadata sha256 helpto regenerate the checksum file. I've also updated the instructions with ways to regenerate the checksum file once we add a new dependency.The build will now automatically fail if you add a dependency (that is imported by gradle) and is not added to the metadata file.
The error log prints a helpful error stating that the verification-metdata.xml file has to be regenerated.
I could not find a way to run this command automatically everytime we run
./gradlew assemble(this would've removed the need for a developer to manually run a command and update checksum metadata).Tests
Run
./gradlew assemble. Modified the SHA256 checksums and runnning./gradlew assemblefails.Verified that using SHA1 produced the same checksums as the one in the previous checksum files (that were in the repo and this PR removes)
However,
./gradlew checkfails with an error for thespotlessInternalRegisterDependenciestask:These dependencies should've ideally been added automatically by gradle.
I was hoping that someone with better understanding of the Lucene gradle system could comment here?
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.