-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
Built-in gradle dependency verification #759
base: master
Are you sure you want to change the base?
Conversation
./gradlew -q calculateChecksums | grep -Ev "^(Skipping|Verifying)" | grep -Ev "files-2.1:|caches:transforms-3:|:build-tools:core-lambda-stubs.jar:|:platforms:android.jar:|-linux.jar:" > $WITNESS | ||
# to clean up the file, remove ./gradle/verification-metadata.xml, | ||
# run the command below and manually (re-)add checksums for missing operating systems windows, osx or linux for aapt2 | ||
# checksums can be computed after downloading the respective jars of https://maven.google.com/web/index.html?q=aapt2#com.android.tools.build:aapt2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a pity we can't do that automatically. Would be nice to have that file reflect all changes and not grow bigger over time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, that would be better, but only adding new entries avoids removing manually added checksums as the ones for windows and osx for aapt
. I think it will make sense anyway to look at the diff after running this command, so perhaps it is viable to remove old versions manually then. After all, this should be only updated after library upgrades.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only adding new entries avoids removing manually added checksums as the ones for windows and osx for aapt.
but it won't help after we upgrade the gradle plugin as this will pull in new dependencies again that we would need to add manually, right?
perhaps it is viable to remove old versions manually then
maybe easier to re-add windows/mac versions if we even want to support those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only adding new entries avoids removing manually added checksums as the ones for windows and osx for aapt.
but it won't help after we upgrade the gradle plugin as this will pull in new dependencies again that we would need to add manually, right?
That's correct.
perhaps it is viable to remove old versions manually then
maybe easier to re-add windows/mac versions if we even want to support those?
We could certainly change the bash script to always remove the old verification data (as was done with the witness plugin before), but I would have to check if the order of the dependencies is deterministic to ensure sensible diffs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to gradle/gradle#11664 the entries are sorted, so diffs should be fine
As discussed in #693, this PR switches from the gradle witness plugin to the built-in gradle dependency verification. As before with the gradle witness approach, only the checksums of the dependencies are verified, not the GPG signatures. However, the metadata files (
.pom
) are now verified, too, and locally built dependencies are(should be)already automatically excluded. The only platform-dependent dependency isaapt2
, I had to add the checksums for the windows and osx version manually.Let's see if the verification works properly in the CIAs the verification worked in the CI, this should be ready to be merged 😄