Replace failOnVersionConflict() with custom requireUpperBoundDeps - #8238
Merged
Conversation
voidzcy
reviewed
Jun 5, 2021
voidzcy
reviewed
Jun 5, 2021
voidzcy
approved these changes
Jun 5, 2021
voidzcy
approved these changes
Jun 5, 2021
voidzcy
left a comment
voidzcy
left a comment
Contributor
There was a problem hiding this comment.
The buildscript change looks cool.
Member
Author
|
I'm going to wait until #8243 is in to resolve the build failure. I'll end up adding a direct dependency on error-prone from grpc-netty; right now it is getting error-prone transitively from Guava. |
failOnVersionConflict has never been good for us. It is equivalent to Maven dependencyConvergence which we discourage our users to use because it is too tempermental and _creates_ version skew issues over time. However, we had no real alternative for determining if our deps would be misinterpeted by Maven. failOnVersionConflict has been a constant drain and makes it really hard to do seemingly-trivial upgrades. As evidenced by protobuf/build.gradle in this change, it also caused _us_ to introduce a version downgrade. This introduces our own custom requireUpperBoundDeps implementation so that we can get back to simple dependency upgrades _and_ increase our confidence in a consistent dependency tree.
The changes didn't introduce a bug. It is just we previously excluded and re-defined errorprone with guava such that it would be closer to the root and ended up beating protobuf-java-util's dep.
…ying on Guava's transitive dep
ejona86
force-pushed
the
requireUpperBoundDeps
branch
from
June 8, 2021 22:27
1052e18 to
1443ffc
Compare
Member
Author
|
Looks like #8243 did indeed solve the build failure. |
dapengzhang0
approved these changes
Jun 10, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
failOnVersionConflict has never been good for us. It is equivalent to
Maven dependencyConvergence which we discourage our users to use because
it is too tempermental and creates version skew issues over time.
However, we had no real alternative for determining if our deps would be
misinterpeted by Maven.
failOnVersionConflict has been a constant drain and makes it really hard
to do seemingly-trivial upgrades. As evidenced by protobuf/build.gradle,
in this change, it also caused us to introduce a version downgrade.
This introduces our own custom requireUpperBoundDeps implementation so
that we can get back to simple dependency upgrades and increase our
confidence in a consistent dependency tree.
The main
build.gradleis where the real changes are, but the rest wouldhave been easy to introduce an accidental bug/change.
CC @elharo