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

Built-in gradle dependency verification #759

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Built-in gradle dependency verification #759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ apply plugin: 'com.android.application'
apply plugin: 'kotlin-android'
apply plugin: 'kotlin-android-extensions'
apply plugin: 'kotlin-kapt'
apply plugin: 'witness'
apply from: 'witness.gradle'

android {

Expand Down
Binary file removed app/libs/gradle-witness.jar
View file
Open in desktop
Binary file not shown.
219 changes: 0 additions & 219 deletions app/witness.gradle
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ buildscript {
dependencies {
classpath 'com.android.tools.build:gradle:4.1.3'
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion"
classpath files('app/libs/gradle-witness.jar')
}

}
Expand Down
2,594 changes: 2,594 additions & 0 deletions gradle/verification-metadata.xml
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions update-dependency-pinning.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#!/bin/bash

WITNESS=app/witness.gradle
# this will just append new checksums and not remove unnecessary ones

echo "" > $WITNESS
./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
Copy link
Owner

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.

Copy link
Collaborator Author

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.

Copy link
Owner

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?

Copy link
Collaborator Author

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.

Copy link

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


./gradlew --write-verification-metadata sha256 build
grote marked this conversation as resolved.
Show resolved Hide resolved