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.

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

Add test case that enables dependency verification. #375

Merged
merged 3 commits into from
Feb 8, 2020

Conversation

anuraaga
Copy link
Collaborator

@anuraaga anuraaga commented Feb 7, 2020

I believe I have enabled dependency verification by including the XML file and seeing a log message about it. Things still work fine. @vlsi can you check if this is the case you're concerned about?

For #366

@anuraaga anuraaga requested a review from ben-manes February 7, 2020 07:47
@anuraaga
Copy link
Collaborator Author

anuraaga commented Feb 7, 2020

I noticed the test maven repo doesn't have a guice 2.0 POM file in it so thought switching to 3.0 might bring up the potentially broken behavior, but even with 3.0, which does have a POM in the repo, still seems to work ok

@ben-manes
Copy link
Owner

What about in an actual project? Can you get anything to break with this feature?

@anuraaga
Copy link
Collaborator Author

anuraaga commented Feb 7, 2020

Works fine I think - https://github.com/anuraaga/gradle-scratch

Actually, I could test out the verification itself since when I added the versions plugin, it complained about the versions plugin's checksum not being in the XML - expected behavior when checking checksums. Updated the XML and then proceeds fine.

Dependency verification is an incubating feature.

> Task :dependencyUpdates

------------------------------------------------------------
: Project Dependency Updates (report to plain text file)
------------------------------------------------------------

The following dependencies are using the latest milestone version:
 - com.github.ben-manes.versions:com.github.ben-manes.versions.gradle.plugin:0.27.0

The following dependencies have later milestone versions:
 - com.google.inject:guice [3.0 -> 4.2.2]

Gradle release-candidate updates:
 - Gradle: [6.2-rc-1: UP-TO-DATE]

Generated report file C:\tools\msys64\home\Anuraag\git\gradle-scratch\build\dependencyUpdates\report.txt

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.2-rc-1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 5s
1 actionable task: 1 executed

@vlsi Does it make sense that the versions plugin works fine even with verification enabled? If so, glad we won't have any issues with 6.2 :)

@ben-manes
Copy link
Owner

Maybe it’s because we only resolve metadata and not any artifacts, so the checksum isn’t triggered?

@vlsi
Copy link

vlsi commented Feb 7, 2020

@melix, can you please double-check the dependency verification case here?

It looks like the verification does not stop resolution of detached configurations which seems odd to me.

Just in case, I've checked with Caffeine sources as well, and dependencyUpdates task works even after --write-verification-metadata sha256 (Gradle 6.1.1)

@melix
Copy link

melix commented Feb 7, 2020

It's hard to figure out what you mean without an example. Dependency verification should be tested with 6.2 only (not 6.1). Verification of detached configurations should always happen, unless it's disabled in the verification configuration file.

@anuraaga
Copy link
Collaborator Author

anuraaga commented Feb 7, 2020

@melix @vlsi I realized that this probably works since we resolve new versions using a dynamic version number

https://github.com/ben-manes/gradle-versions-plugin/blob/master/src/main/groovy/com/github/benmanes/gradle/versions/updates/Resolver.groovy#L151

I presume verification isn't applied to dynamic versions since it's not clear what the checksum should actually refer to.

Does this sound right? And if so I guess it's why we don't have to worry about the effect of verification on this plugin?

@melix
Copy link

melix commented Feb 7, 2020

Verification happens once the artifact or metadata is accessed. So it works for dynamic versions too (once resolved to a version number). It doesn't, however, apply to changing versions (like snapshots) because by definition their checksum would always change.

@anuraaga
Copy link
Collaborator Author

anuraaga commented Feb 7, 2020

I see. Well, the line of code that resolves a bunch of dependencies with something like version latest is here

https://github.com/ben-manes/gradle-versions-plugin/blob/master/src/main/groovy/com/github/benmanes/gradle/versions/updates/Resolver.groovy#L84

These newer versions that get resolved to don't appear in our XML and I guess we are expecting a verification failure. That being said, we don't use the resolved artifacts at all, we only do this to get resolved version numbers from Gradle, not to actually use any dependency. So perhaps we bail out of a flow before verification triggers.

I guess knowing which exact methods have a chance of producing a verification failure would help clear our confusion.

Copy link
Owner

@ben-manes ben-manes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anuraaga you are welcome to merge (and even release) when you're satisfied. Thank you for this!

@anuraaga
Copy link
Collaborator Author

anuraaga commented Feb 8, 2020

Thanks - guess we can be confident that Gradle 6.2 will still work with versions plugin :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants