From 5642e0124391602affb2422d84d3e6958fa7c2b9 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 11 Jun 2021 14:01:18 -0700 Subject: [PATCH] Replace failOnVersionConflict() with custom requireUpperBoundDeps 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. --- alts/build.gradle | 6 +- android-interop-testing/build.gradle | 4 +- android/build.gradle | 2 +- api/build.gradle | 3 +- auth/build.gradle | 6 +- binder/build.gradle | 2 +- build.gradle | 147 ++++++++++++--------------- buildscripts/kokoro/unix.sh | 2 - census/build.gradle | 6 +- core/build.gradle | 6 +- cronet/build.gradle | 2 +- examples/example-gauth/pom.xml | 5 - grpclb/build.gradle | 10 +- interop-testing/build.gradle | 6 +- netty/build.gradle | 7 +- okhttp/build.gradle | 13 +-- protobuf-lite/build.gradle | 4 +- protobuf/build.gradle | 4 +- rls/build.gradle | 4 +- services/build.gradle | 8 +- stub/build.gradle | 4 +- testing/build.gradle | 4 +- xds/build.gradle | 16 +-- 23 files changed, 114 insertions(+), 157 deletions(-) diff --git a/alts/build.gradle b/alts/build.gradle index 7236eee031..8c467f51c1 100644 --- a/alts/build.gradle +++ b/alts/build.gradle @@ -21,10 +21,10 @@ dependencies { project(':grpc-protobuf'), project(':grpc-stub'), libraries.protobuf, - libraries.conscrypt + libraries.conscrypt, + libraries.guava, + libraries.google_auth_oauth2_http def nettyDependency = implementation project(':grpc-netty') - googleOauth2Dependency 'implementation' - guavaDependency 'implementation' compileOnly libraries.javax_annotation shadow configurations.implementation.getDependencies().minus(nettyDependency) diff --git a/android-interop-testing/build.gradle b/android-interop-testing/build.gradle index 0434eb8f29..f1c1d49123 100644 --- a/android-interop-testing/build.gradle +++ b/android-interop-testing/build.gradle @@ -63,12 +63,12 @@ dependencies { project(':grpc-stub'), project(':grpc-testing'), libraries.junit, - libraries.truth + libraries.truth, + libraries.opencensus_contrib_grpc_metrics implementation (libraries.google_auth_oauth2_http) { exclude group: 'org.apache.httpcomponents' } - censusGrpcMetricDependency 'implementation' compileOnly libraries.javax_annotation diff --git a/android/build.gradle b/android/build.gradle index b9942b0745..a50f7b8559 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -31,7 +31,7 @@ repositories { dependencies { api project(':grpc-core') - guavaDependency 'implementation' + implementation libraries.guava testImplementation project('::grpc-okhttp') testImplementation libraries.androidx_test testImplementation libraries.junit diff --git a/api/build.gradle b/api/build.gradle index 1574cdd8b4..a959fed4ea 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -13,7 +13,8 @@ evaluationDependsOn(project(':grpc-context').path) dependencies { api project(':grpc-context'), libraries.jsr305 - guavaDependency 'implementation' + implementation libraries.guava, + libraries.errorprone testImplementation project(':grpc-context').sourceSets.test.output, project(':grpc-testing'), diff --git a/auth/build.gradle b/auth/build.gradle index 1f5e4b4195..233de359b4 100644 --- a/auth/build.gradle +++ b/auth/build.gradle @@ -10,9 +10,9 @@ description = "gRPC: Auth" dependencies { api project(':grpc-api'), libraries.google_auth_credentials - guavaDependency 'implementation' - testImplementation project(':grpc-testing') - googleOauth2Dependency 'testImplementation' + implementation libraries.guava + testImplementation project(':grpc-testing'), + libraries.google_auth_oauth2_http signature "org.codehaus.mojo.signature:java17:1.0@signature" signature "net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature" } diff --git a/binder/build.gradle b/binder/build.gradle index 4881010b95..537c23a009 100644 --- a/binder/build.gradle +++ b/binder/build.gradle @@ -47,9 +47,9 @@ repositories { dependencies { api project(':grpc-core') - guavaDependency 'implementation' implementation libraries.androidx_annotation implementation libraries.androidx_core + implementation libraries.guava testImplementation libraries.androidx_core testImplementation libraries.androidx_test testImplementation libraries.junit diff --git a/build.gradle b/build.gradle index d9f160144d..9d8cbd3ccd 100644 --- a/build.gradle +++ b/build.gradle @@ -205,68 +205,6 @@ subprojects { jetty_alpn_agent: 'org.mortbay.jetty.alpn:jetty-alpn-agent:2.0.10' ] - // A util function to config guava dependency with transitive dependencies - // properly resolved for the failOnVersionConflict strategy. - guavaDependency = { configurationName -> - dependencies."$configurationName"(libraries.guava) { - exclude group: 'com.google.code.findbugs', module: 'jsr305' - exclude group: 'com.google.errorprone', module: 'error_prone_annotations' - exclude group: 'org.codehaus.mojo', module: 'animal-sniffer-annotations' - } - dependencies."$configurationName" libraries.errorprone - dependencies.runtimeOnly libraries.animalsniffer_annotations - dependencies.runtimeOnly libraries.jsr305 - } - - // A util function to config opencensus_api dependency with transitive - // dependencies properly resolved for the failOnVersionConflict strategy. - censusApiDependency = { configurationName -> - dependencies."$configurationName"(libraries.opencensus_api) { - exclude group: 'com.google.code.findbugs', module: 'jsr305' - exclude group: 'com.google.guava', module: 'guava' - // we'll always be more up-to-date - exclude group: 'io.grpc', module: 'grpc-context' - } - dependencies.runtimeOnly project(':grpc-context') - dependencies.runtimeOnly libraries.jsr305 - guavaDependency 'runtimeOnly' - } - - // A util function to config opencensus_contrib_grpc_metrics dependency - // with transitive dependencies properly resolved for the failOnVersionConflict - // strategy. - censusGrpcMetricDependency = { configurationName -> - dependencies."$configurationName"(libraries.opencensus_contrib_grpc_metrics) { - exclude group: 'com.google.code.findbugs', module: 'jsr305' - exclude group: 'com.google.guava', module: 'guava' - // we'll always be more up-to-date - exclude group: 'io.grpc', module: 'grpc-context' - } - dependencies.runtimeOnly project(':grpc-context') - dependencies.runtimeOnly libraries.jsr305 - guavaDependency 'runtimeOnly' - } - - googleOauth2Dependency = { configurationName -> - dependencies."$configurationName"(libraries.google_auth_oauth2_http) { - exclude group: 'com.google.guava', module: 'guava' - exclude group: 'io.grpc', module: 'grpc-context' - exclude group: 'io.opencensus', module: 'opencensus-api' - } - dependencies.runtimeOnly project(':grpc-context') - censusApiDependency 'runtimeOnly' - guavaDependency 'runtimeOnly' - } - - // A util function to config perfmark dependency with transitive - // dependencies properly resolved for the failOnVersionConflict strategy. - perfmarkDependency = { configurationName -> - dependencies."$configurationName"(libraries.perfmark) { - exclude group: 'com.google.errorprone', module: 'error_prone_annotations' - } - dependencies.runtimeOnly libraries.errorprone - } - appendToProperty = { Property property, String value, String separator -> if (property.present) { property.set(property.get() + separator + value) @@ -276,25 +214,6 @@ subprojects { } } - configurations { - // Detect Maven Enforcer's dependencyConvergence failures. We only - // care for artifacts used as libraries by others. - if (isAndroid && !(project.name in ['grpc-android-interop-testing'])) { - releaseRuntimeClasspath { - resolutionStrategy.failOnVersionConflict() - } - } - if (!isAndroid && !(project.name in [ - 'grpc-benchmarks', - 'grpc-interop-testing', - 'grpc-gae-interop-testing-jdk8', - ])) { - runtimeClasspath { - resolutionStrategy.failOnVersionConflict() - } - } - } - // Disable JavaDoc doclint on Java 8. It's annoying. if (JavaVersion.current().isJava8Compatible()) { allprojects { @@ -406,6 +325,19 @@ subprojects { } } + plugins.withId("java-library") { + // Detect Maven Enforcer's dependencyConvergence failures. We only care + // for artifacts used as libraries by others with Maven. + tasks.register('checkUpperBoundDeps') { + doLast { + requireUpperBoundDepsMatch(configurations.runtimeClasspath, project) + } + } + tasks.named('compileJava') { + dependsOn checkUpperBoundDeps + } + } + plugins.withId("me.champeau.gradle.jmh") { dependencies { jmh 'org.openjdk.jmh:jmh-core:1.19', @@ -582,3 +514,56 @@ subprojects { } } } + +class DepAndParents { + DependencyResult dep + List parents +} + +/** + * Make sure that Maven would select the same versions as Gradle selected. + * This is essentially the same as if we used Maven Enforcer's + * requireUpperBoundDeps for our artifacts. + */ +def requireUpperBoundDepsMatch(Configuration conf, Project project) { + // artifact name => version + Map golden = conf.resolvedConfiguration.resolvedArtifacts.collectEntries { + ResolvedArtifact it -> + ModuleVersionIdentifier id = it.moduleVersion.id + [id.group + ":" + id.name, id.version] + } + // Breadth-first search like Maven for dependency resolution + Queue queue = new ArrayDeque<>() + conf.incoming.resolutionResult.root.dependencies.each { + queue.add(new DepAndParents(dep: it, parents: [project.displayName])) + } + Set found = new HashSet<>() + while (!queue.isEmpty()) { + DepAndParents depAndParents = queue.remove() + ResolvedDependencyResult result = (ResolvedDependencyResult) depAndParents.dep + ModuleVersionIdentifier id = result.selected.moduleVersion + String artifact = id.group + ":" + id.name + if (found.contains(artifact)) + continue + found.add(artifact) + String version + if (result.requested instanceof ProjectComponentSelector) { + ProjectComponentSelector selector = (ProjectComponentSelector) result.requested + version = project.findProject(selector.projectPath).version + } else { + version = ((ModuleComponentSelector) result.requested).version + } + String goldenVersion = golden[artifact] + if (goldenVersion != version && "[$goldenVersion]" != version) { + throw new RuntimeException( + "Maven version skew: $artifact ($version != $goldenVersion) " + + "Bad version dependency path: " + depAndParents.parents + + " Run './gradlew $project.path:dependencies --configuration $conf.name' " + + "to diagnose") + } + result.selected.dependencies.each { + queue.add(new DepAndParents( + dep: it, parents: depAndParents.parents + [artifact + ":" + version])) + } + } +} diff --git a/buildscripts/kokoro/unix.sh b/buildscripts/kokoro/unix.sh index 5a720295c6..faa9a6afe1 100755 --- a/buildscripts/kokoro/unix.sh +++ b/buildscripts/kokoro/unix.sh @@ -46,8 +46,6 @@ export LDFLAGS=-L/tmp/protobuf/lib export CXXFLAGS="-I/tmp/protobuf/include" ./gradlew clean $GRADLE_FLAGS -# Ensure dependency convergence -./gradlew :grpc-all:dependencies $GRADLE_FLAGS if [[ -z "${SKIP_TESTS:-}" ]]; then # Ensure all *.proto changes include *.java generated code diff --git a/census/build.gradle b/census/build.gradle index af5445a218..35973a5f01 100644 --- a/census/build.gradle +++ b/census/build.gradle @@ -9,9 +9,9 @@ evaluationDependsOn(project(':grpc-api').path) dependencies { api project(':grpc-api') - guavaDependency 'implementation' - censusApiDependency 'implementation' - censusGrpcMetricDependency 'implementation' + implementation libraries.guava, + libraries.opencensus_api, + libraries.opencensus_contrib_grpc_metrics testImplementation project(':grpc-api').sourceSets.test.output, project(':grpc-context').sourceSets.test.output, diff --git a/core/build.gradle b/core/build.gradle index f4e0ccc3b6..14cfbb7049 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -27,9 +27,9 @@ dependencies { implementation libraries.gson, libraries.android_annotations, libraries.animalsniffer_annotations, - libraries.errorprone - guavaDependency 'implementation' - perfmarkDependency 'implementation' + libraries.errorprone, + libraries.guava, + libraries.perfmark testImplementation project(':grpc-context').sourceSets.test.output, project(':grpc-api').sourceSets.test.output, project(':grpc-testing'), diff --git a/cronet/build.gradle b/cronet/build.gradle index 95aa7e9e23..2d73cc4194 100644 --- a/cronet/build.gradle +++ b/cronet/build.gradle @@ -35,7 +35,7 @@ android { dependencies { api project(':grpc-core'), libraries.cronet_api - guavaDependency 'implementation' + implementation libraries.guava testImplementation project(':grpc-testing') testImplementation libraries.cronet_embedded diff --git a/examples/example-gauth/pom.xml b/examples/example-gauth/pom.xml index 79aec1abcd..b3a1e9f0ff 100644 --- a/examples/example-gauth/pom.xml +++ b/examples/example-gauth/pom.xml @@ -49,11 +49,6 @@ io.grpc grpc-auth - - com.google.protobuf - protobuf-java-util - ${protobuf.version} - org.apache.tomcat annotations-api diff --git a/grpclb/build.gradle b/grpclb/build.gradle index 973770feb2..58ff2f412d 100644 --- a/grpclb/build.gradle +++ b/grpclb/build.gradle @@ -14,13 +14,9 @@ dependencies { implementation project(':grpc-core'), project(':grpc-protobuf'), project(':grpc-stub'), - libraries.protobuf - implementation (libraries.protobuf_util) { - // prefer our own versions instead of protobuf-util's dependency - exclude group: 'com.google.guava', module: 'guava' - exclude group: 'com.google.errorprone', module: 'error_prone_annotations' - } - guavaDependency 'implementation' + libraries.protobuf, + libraries.protobuf_util, + libraries.guava runtimeOnly libraries.errorprone compileOnly libraries.javax_annotation testImplementation libraries.truth, diff --git a/interop-testing/build.gradle b/interop-testing/build.gradle index 01af652932..79aa5356ec 100644 --- a/interop-testing/build.gradle +++ b/interop-testing/build.gradle @@ -28,9 +28,9 @@ dependencies { project(':grpc-testing'), project(path: ':grpc-xds', configuration: 'shadow'), libraries.junit, - libraries.truth - censusGrpcMetricDependency 'implementation' - googleOauth2Dependency 'implementation' + libraries.truth, + libraries.opencensus_contrib_grpc_metrics, + libraries.google_auth_oauth2_http compileOnly libraries.javax_annotation // TODO(sergiitk): replace with com.google.cloud:google-cloud-logging // Used instead of google-cloud-logging because it's failing diff --git a/netty/build.gradle b/netty/build.gradle index 20b35eb36d..00726940cd 100644 --- a/netty/build.gradle +++ b/netty/build.gradle @@ -18,9 +18,10 @@ evaluationDependsOn(project(':grpc-core').path) dependencies { api project(':grpc-core'), libraries.netty - implementation libraries.netty_proxy_handler - guavaDependency 'implementation' - perfmarkDependency 'implementation' + implementation libraries.netty_proxy_handler, + libraries.guava, + libraries.errorprone, + libraries.perfmark // Tests depend on base class defined by core module. testImplementation project(':grpc-core').sourceSets.test.output, diff --git a/okhttp/build.gradle b/okhttp/build.gradle index d28d4e00ca..999f21e7c1 100644 --- a/okhttp/build.gradle +++ b/okhttp/build.gradle @@ -11,14 +11,11 @@ description = "gRPC: OkHttp" evaluationDependsOn(project(':grpc-core').path) dependencies { - api project(':grpc-core') - api (libraries.okhttp) { - // prefer our own versions instead of okhttp's dependency - exclude group: 'com.squareup.okio', module: 'okio' - } - implementation libraries.okio - guavaDependency 'implementation' - perfmarkDependency 'implementation' + api project(':grpc-core'), + libraries.okhttp + implementation libraries.okio, + libraries.guava, + libraries.perfmark // Tests depend on base class defined by core module. testImplementation project(':grpc-core').sourceSets.test.output, project(':grpc-api').sourceSets.test.output, diff --git a/protobuf-lite/build.gradle b/protobuf-lite/build.gradle index 000d8c721e..7b58309c41 100644 --- a/protobuf-lite/build.gradle +++ b/protobuf-lite/build.gradle @@ -12,8 +12,8 @@ description = 'gRPC: Protobuf Lite' dependencies { api project(':grpc-api'), libraries.protobuf_lite - implementation libraries.jsr305 - guavaDependency 'implementation' + implementation libraries.jsr305, + libraries.guava testImplementation project(':grpc-core') diff --git a/protobuf/build.gradle b/protobuf/build.gradle index af7f51c836..bb8546dc70 100644 --- a/protobuf/build.gradle +++ b/protobuf/build.gradle @@ -13,14 +13,12 @@ dependencies { api project(':grpc-api'), libraries.jsr305, libraries.protobuf - guavaDependency 'implementation' + implementation libraries.guava api (libraries.google_api_protos) { // 'com.google.api:api-common' transitively depends on auto-value, which breaks our // annotations. exclude group: 'com.google.api', module: 'api-common' - // Prefer our more up-to-date protobuf over 3.2.0 - exclude group: 'com.google.protobuf', module: 'protobuf-java' } api (project(':grpc-protobuf-lite')) { diff --git a/rls/build.gradle b/rls/build.gradle index 32004a7d43..a2ebf2a62e 100644 --- a/rls/build.gradle +++ b/rls/build.gradle @@ -13,8 +13,8 @@ evaluationDependsOn(project(':grpc-core').path) dependencies { implementation project(':grpc-core'), project(':grpc-protobuf'), - project(':grpc-stub') - guavaDependency 'implementation' + project(':grpc-stub'), + libraries.guava compileOnly libraries.javax_annotation testImplementation libraries.truth, project(':grpc-grpclb'), diff --git a/services/build.gradle b/services/build.gradle index f513251552..2de9418c3c 100644 --- a/services/build.gradle +++ b/services/build.gradle @@ -22,12 +22,8 @@ dependencies { api project(':grpc-protobuf'), project(':grpc-stub'), project(':grpc-core') - implementation (libraries.protobuf_util) { - // prefer our own versions instead of protobuf-util's dependency - exclude group: 'com.google.guava', module: 'guava' - exclude group: 'com.google.errorprone', module: 'error_prone_annotations' - } - guavaDependency 'implementation' + implementation libraries.protobuf_util, + libraries.guava runtimeOnly libraries.errorprone compileOnly libraries.javax_annotation diff --git a/stub/build.gradle b/stub/build.gradle index ce1e742ab3..4076460377 100644 --- a/stub/build.gradle +++ b/stub/build.gradle @@ -8,8 +8,8 @@ plugins { description = "gRPC: Stub" dependencies { - api project(':grpc-api') - guavaDependency 'api' + api project(':grpc-api'), + libraries.guava testImplementation libraries.truth, project(':grpc-testing') signature "org.codehaus.mojo.signature:java17:1.0@signature" diff --git a/testing/build.gradle b/testing/build.gradle index dc619913b6..a0f3181991 100644 --- a/testing/build.gradle +++ b/testing/build.gradle @@ -13,8 +13,8 @@ dependencies { api project(':grpc-core'), project(':grpc-stub'), libraries.junit - - censusApiDependency 'implementation' + implementation libraries.opencensus_api + runtimeOnly project(":grpc-context") // Pull in newer version than census-api testImplementation (libraries.mockito) { // prefer our own versions instead of mockito's dependency diff --git a/xds/build.gradle b/xds/build.gradle index f80c7c5294..ae8d8d208a 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -36,21 +36,11 @@ dependencies { libraries.gson, libraries.re2j, libraries.bouncycastle, - libraries.autovalue_annotation + libraries.autovalue_annotation, + libraries.opencensus_proto, + libraries.protobuf_util def nettyDependency = implementation project(':grpc-netty') - implementation (libraries.opencensus_proto) { - // prefer our own versions instead of opencensus_proto's - exclude group: 'com.google.protobuf', module: 'protobuf-java' - exclude group: 'io.grpc', module: 'grpc-protobuf' - exclude group: 'io.grpc', module: 'grpc-stub' - } - implementation (libraries.protobuf_util) { - // prefer our own versions instead of protobuf-util's dependency - exclude group: 'com.google.guava', module: 'guava' - exclude group: 'com.google.errorprone', module: 'error_prone_annotations' - } - testImplementation project(':grpc-core').sourceSets.test.output annotationProcessor libraries.autovalue