Tool to run build libraries using LTS BOM #1982
Closed
Conversation
This file contains 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
suztomo
commented
Mar 16, 2021
Comment on lines
196
to
199
| if ("guava".equals(artifactId)) { | ||
| // Guava JRE does not work with Yoshi's shared config. | ||
| // https://github.com/GoogleCloudPlatform/cloud-opensource-java/issues/1974#issuecomment-799862063 | ||
| continue; |
There was a problem hiding this comment.
This is an unexpected blocker when building the project with Guava's JRE flavor.
suztomo
commented
Mar 16, 2021
Comment on lines
115
to
123
| // Modify build file to use the BOM | ||
| if ("Maven".equals(modification)) { | ||
| modifyPomFiles(testRoot, bom); | ||
| } else if ("Gradle".equals(modification)) { | ||
| modifyGradleFiles(testRoot, bom); | ||
| } else { | ||
| System.out.println("Invalid value for modification. 'Maven' or 'Gradle'"); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Based on the modification parameter "Maven" or "Gradle", it updates the build configuration files.
suztomo
commented
Mar 16, 2021
| - name: java-spanner | ||
| url: https://github.com/googleapis/java-spanner.git | ||
| tag: v5.1.0 | ||
| modification: Maven |
There was a problem hiding this comment.
"Maven" updates pom.xml
https://github.com/GoogleCloudPlatform/cloud-opensource-java/pull/1982/files#r595271705
suztomo
commented
Mar 16, 2021
| - name: gRPC | ||
| url: https://github.com/grpc/grpc-java.git | ||
| tag: v1.36.0 | ||
| modification: Gradle |
There was a problem hiding this comment.
"Gradle" updates build.gradle files.
suztomo
commented
Mar 16, 2021
|
|
||
|
|
||
| List<String> replacedLines = lines.stream().map(line -> line.replaceAll("^dependencies \\{", | ||
| "dependencies {\n testRuntime enforcedPlatform('"+bomCoordinates+"')")).collect(Collectors.toList()); |
There was a problem hiding this comment.
Gradle can configure runtime of test. How about Maven?
There was a problem hiding this comment.
Command line option to override "1.7" part of the compiler plugin?
There was a problem hiding this comment.
Can I configure surefire plugin to add special class path?
elharo
reviewed
Mar 16, 2021
dependencies/pom.xml
Outdated
| <dependency> | ||
| <groupId>xom</groupId> | ||
| <artifactId>xom</artifactId> | ||
| <version>1.3.5</version> |
| throws IOException, ArtifactDescriptorException, InterruptedException, ParsingException { | ||
| String inputFileName = arguments[0]; | ||
|
|
||
| Yaml yaml = new Yaml(); |
There was a problem hiding this comment.
| if (buildStatusCode != 0) { | ||
| System.out | ||
| .println("Failed to build " + url + ". Exiting. Check the logs in " + projectDirectory); | ||
| break; |
There was a problem hiding this comment.
we might want to build the others anyway
|
With |
suztomo
commented
Mar 29, 2021
| tag: v0.18.0 | ||
| modification: Gradle | ||
| commands: | | ||
| perl -i -p -e 's/mavenCentral\(\)/mavenCentral\(\)\nmavenLocal\(\)/;' build.gradle |
There was a problem hiding this comment.
Let's do this in the "modification" step.
|
I'll make this as GitHub Actions. |
|
Now I need the LTS BOM snapshot in the master branch. |
Merged
|
This grpc-java failure (https://github.com/GoogleCloudPlatform/cloud-opensource-java/pull/1982/checks?check_run_id=2242270199) may be true positive: ExperimentWith the following experiment, I think this is the behavior change incompatibility we want to catch through this testing tool (no compilation error). Checkout grpc-java at v1.36.1: Modify the googleauthVersion for Run the test. It fails: |
|
With jre/lib/ext approach, Maven stopped working. Probably due to Guava. |
suztomo
commented
Apr 1, 2021
Comment on lines
56
to
60
| if ("guava".equals(artifact.getArtifactId())) { | ||
| // Placing Guava in the ext directory stops Maven | ||
| continue; | ||
| } |
There was a problem hiding this comment.
To avoid Maven-guava problem, I excluded Guava from being copied to jre/lib/ext. But now GitHub Actions get lots errors (link):
Error: com.google.api.client.http.BasicAuthenticationTest.testConstructor Time elapsed: 0 s <<< ERROR!
java.lang.NoClassDefFoundError: com/google/common/base/Preconditions
at com.google.api.client.http.BasicAuthenticationTest.testConstructor(BasicAuthenticationTest.java:35)
[INFO] Running com.google.api.client.http.HttpResponseExceptionTest
Error: Tests run: 10, Failures: 0, Errors: 10, Skipped: 0, Time elapsed: 0 s <<< FAILURE! - in com.google.api.client.http.HttpResponseExceptionTest
Error: com.google.api.client.http.HttpResponseExceptionTest.testConstructor_messageButNoStatusCode Time elapsed: 0 s <<< ERROR!
java.lang.NoClassDefFoundError: Could not initialize class com.google.api.client.testing.http.HttpTesting
at com.google.api.client.http.HttpResponseExceptionTest.testConstructor_messageButNoStatusCode(HttpResponseExceptionTest.java:141)
Maybe the classes in jre/lib/ext cannot reference to the classes not in jre/lib/ext?
ClassLoader
With simple debug print statements, it shows that BasicAuthentication's class loader is ExtClassLoader:
[INFO] Running com.google.api.client.http.BasicAuthenticationTest
BasicAuthenticationTest's class loader: sun.misc.Launcher$AppClassLoader@5674cd4d
BasicAuthentication's class loader: sun.misc.Launcher$ExtClassLoader@50675690
My removal of Guava from JRE's ext class loader caused the following issues:
- BasicAuthentication tried to load Guava's Preconditions class.
- The class loader (extension class loader) cannot find it. It tries to ask its parent class loader
- The parent class loader (bootstrap loader) cannot find it.
- Therefore, the class was not found, without searching in the application class loader.
There was a problem hiding this comment.
Maven debug output: https://gist.github.com/suztomo/b596e02a09bd35b2426ae0f3014e083e
When I remove google-http-client.jar from jre/lib/ext, then BasicAuthenticationTest succeeds.
Analysis
BasicAuthenticationTest calls BasicAuthentication class. Because BasicAuthentication was in jre/lib/ext's google-http-client.jar, JVM loaded the class from the JAR file, not from the Maven project. Because the class is from ext directory, it cannot reference the Guavas' Precondition class because Guava is not in jre/lib/ext. Guava is the Maven project's dependency.
|
Digging bigtable-hbase-beam's errors: Missing TableRow
With this debugging session below, it seems that original revision does not touch BigQueryCoderProviderRegistrar class. This implies that The class path for surefire plugin does not include google-api-services-bigquery (gist). This must be new dependency. Mockito canont mock CloudBigtableIO$AbstractSource |
java-bigquery
The following linkage error is also in the dashboard: java-monitoringcom.google.api.MetricDescriptor is from
|
|
Some of the test in Beam throw credential error due to the runtime environment not having default credential. To investigate precondition further. |
|
google-cloud-storage failed with obscure message. How can I print the error message or stack trace of the forked process? I ran it locally and got NoClassDefFoundError:
The missing google-cloud-storage/pom.xml has This seems because the test tool inserts the test tool regardless of the classifier. |
|
java-pubsub suffers from dependency conflict even if this tool uses Gradle dependency resolution
The class is in grpc-google-cloud-pubsub-v1: 1.94.0:
but not in Why did this choose 1.90.3, which is lower than 1.94.0? Google-cloud-pubsub is not declaring the "proto-google-cloud-pubsub" dependency?
It's a test dependency This implies we better to control grpc-google-cloud-pubsub-v1's version (probably through a BOM; maybe outside the LTS scope) |
Closed
suztomo
commented
Apr 9, 2021
boms/cloud-lts-bom/pom.xml
Outdated
Comment on lines
260
to
267
| <!-- TODO: review this benefit and cost --> | ||
| <dependency> | ||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>google-cloud-pubsub-bom</artifactId> | ||
| <version>${google.cloud.pubsub.version}</version> | ||
| <type>pom</type> | ||
| <scope>import</scope> | ||
| </dependency> |
There was a problem hiding this comment.
If we import pubsub-bom, java-pubsub test succeeds; otherwise it fails with dependency conflict. The lts-bom gets old grpc-google-cloud-pubsub-v1:1.90.3.
There was a problem hiding this comment.
But this has side-effect to Beam:
> Task :sdks:java:io:google-cloud-platform:test
org.apache.beam.sdk.io.gcp.pubsub.PubsubGrpcClientTest > publishOneMessage FAILED
io.grpc.StatusRuntimeException at PubsubGrpcClientTest.java:200
Caused by: java.lang.NoSuchFieldError at PubsubGrpcClientTest.java:200
org.apache.beam.sdk.io.gcp.pubsub.PubsubGrpcClientTest > pullOneMessage FAILED
io.grpc.StatusRuntimeException at PubsubGrpcClientTest.java:145
Caused by: java.lang.NoSuchFieldError at PubsubGrpcClientTest.java:145
java.lang.NoSuchFieldError: ATTR_LOAD_BALANCING_CONFIG |
-- | --
| at io.grpc.internal.AutoConfiguredLoadBalancerFactory$AutoConfiguredLoadBalancer.tryHandleResolvedAddresses(AutoConfiguredLoadBalancerFactory.java:114) |
| at io.grpc.internal.ManagedChannelImpl$NameResolverListener$1NamesResolved.run(ManagedChannelImpl.java:1601) |
| at io.grpc.SynchronizationContext.drain(SynchronizationContext.java:95) |
| at io.grpc.SynchronizationContext.execute(SynchronizationContext.java:127) |
| at io.grpc.internal.ManagedChannelImpl$ChannelStreamProvider.getTransport(ManagedChannelImpl.java:493) |
| at io.grpc.internal.ManagedChannelImpl$ChannelStreamProvider.newStream(ManagedChannelImpl.java:523) |
| at io.grpc.internal.ClientCallImpl.startInternal(ClientCallImpl.java:245) |
| at io.grpc.internal.ClientCallImpl.start(ClientCallImpl.java:204) |
| at io.grpc.auth.ClientAuthInterceptor$1.checkedStart(ClientAuthInterceptor.java:87) |
| at io.grpc.ClientInterceptors$CheckedForwardingClientCall.start(ClientInterceptors.java:231) |
| at io.grpc.stub.ClientCalls.startCall(ClientCalls.java:332) |
| at io.grpc.stub.ClientCalls.asyncUnaryRequestCall(ClientCalls.java:306) |
| at io.grpc.stub.ClientCalls.futureUnaryCall(ClientCalls.java:218) |
| at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:146) |
| at com.google.pubsub.v1.PublisherGrpc$PublisherBlockingStub.publish(PublisherGrpc.java:892) |
| at org.apache.beam.sdk.io.gcp.pubsub.PubsubGrpcClient.publish(PubsubGrpcClient.java:206) |
| at org.apache.beam.sdk.io.gcp.pubsub.PubsubGrpcClientTest.publishOneMessage(PubsubGrpcClientTest.java:200)
- grpc-core has
AutoConfiguredLoadBalancerFactory$AutoConfiguredLoadBalancer - grpc-api has
io.grpc.LoadBalancer.
As per Gradle's report, testRuntimeClasspath had grpc-core:1.32.2 and grpc-api:1.36.0. The LoadBalancer class in the new grpc-api does not have ATTR_LOAD_BALANCING_CONFIG field.
Both grpc-core and grpc-api are in the LTS BOM. Why do they have difference?
It seems that the direct dependencies are controlled by the libraries bom if I use "compile" configuration. Moving it to "compileOnly" configuration lets the tool to run the tests with the LTS BOM.
However, making it compileOnly fails the build due to gradle-dependency-analyze plugin unable to find the proper version.
* What went wrong:
Could not determine the dependencies of task ':sdks:java:io:google-cloud-platform:analyzeTestClassesDependencies'.
> Could not resolve all dependencies for configuration ':sdks:java:io:google-cloud-platform:compile'.
> Could not find com.google.cloud:google-cloud-bigquerystorage:.
Required by:
project :sdks:java:io:google-cloud-platform
> Could not find com.google.cloud:google-cloud-core-grpc:.
Required by:
project :sdks:java:io:google-cloud-platform
> Could not find com.google.cloud:google-cloud-spanner:.
Required by:
project :sdks:java:io:google-cloud-platform
> Could not find io.grpc:grpc-netty:.
Required by:
project :sdks:java:io:google-cloud-platform
> Could not find com.google.api.grpc:grpc-google-cloud-pubsub-v1:.
Required by:
project :sdks:java:io:google-cloud-platform
> Could not find com.google.api.grpc:proto-google-cloud-bigquerystorage-v1beta1:.
Required by:
project :sdks:java:io:google-cloud-platform
> Could not find com.google.api.grpc:proto-google-cloud-spanner-admin-database-v1:.
Required by:
project :sdks:java:io:google-cloud-platform
There was a problem hiding this comment.
Beam uses force for each direct dependencies. The force function overrides the ones specified by the gcp-lts-bom.
Even if I apply the change:
suztomo-macbookpro44% git diff sdks/java/io/google-cloud-platform/build.gradle
diff --git a/sdks/java/io/google-cloud-platform/build.gradle b/sdks/java/io/google-cloud-platform/build.gradle
index 7ac6878..57bd6d3 100644
--- a/sdks/java/io/google-cloud-platform/build.gradle
+++ b/sdks/java/io/google-cloud-platform/build.gradle
@@ -29,6 +29,8 @@ description = "Apache Beam :: SDKs :: Java :: IO :: Google Cloud Platform"
ext.summary = "IO library to read and write Google Cloud Platform systems from Beam."
dependencies {
+ compile enforcedPlatform("com.google.cloud:gcp-lts-bom:0.1.0-SNAPSHOT")
+ testRuntime enforcedPlatform("com.google.cloud:gcp-lts-bom:0.1.0-SNAPSHOT")
compile enforcedPlatform(library.java.google_cloud_platform_libraries_bom)
compile project(path: ":sdks:java:core", configuration: "shadow")
compile project(":sdks:java:expansion-service")
suztomo-macbookpro44% ./gradlew -PdisableSpotlessCheck=true :sdks:java:io:google-cloud-platform:dependencies |pbcopy
The testRuntimeClasspath uses Guava 25.1-jre
testRuntimeClasspath - Runtime classpath of source set 'test'.
+--- org.checkerframework:checker-qual:3.7.0
+--- com.google.cloud:gcp-lts-bom:0.1.0-SNAPSHOT
| +--- com.google.guava:guava:30.1-jre -> 25.1-jre (c)
| +--- com.google.protobuf:protobuf-java:3.15.6 -> 3.12.0 (c)
| +--- io.grpc:grpc-alts:1.36.1 (c)
...
https://gist.github.com/suztomo/c965014ad2a06bf5780894d94c741ebb
suztomo
commented
Apr 12, 2021
Comment on lines
371
to
467
| buildGradleContent = | ||
| buildGradleContent.replaceAll( | ||
| "config.getName\\(\\) != \"errorprone\"", | ||
| "![\"errorprone\", \"testRuntimeClasspath\"].contains(config.getName())"); |
There was a problem hiding this comment.
This causes an error where JUnit 4.12 does not have Assert.assertThrows, which is available after 4.13.
java.lang.NoSuchMethodError: org.junit.Assert.assertThrows(Ljava/lang/Class;Lorg/junit/function/ThrowingRunnable;)Ljava/lang/Throwable;
at org.apache.beam.sdk.values.PCollectionViewsTest.assertNonEmptyRangesAndPositions(PCollectionViewsTest.java:257)
at org.apache.beam.sdk.values.PCollectionViewsTest.testNestedOverlaps(PCollectionViewsTest.java:204)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
There was a problem hiding this comment.
Now :sdks:java:extensions:google-cloud-platform-core:test somehow uses Guava's Android flavor. The dependencies task shows no Android flavor.
java.lang.NoSuchMethodError: com.google.common.cache.CacheBuilder.expireAfterWrite(Ljava/time/Duration;)Lcom/google/common/cache/CacheBuilder;
at com.google.cloud.hadoop.gcsio.GoogleCloudStorageImpl.<init>(GoogleCloudStorageImpl.java:141)
at org.apache.beam.sdk.extensions.gcp.util.GcsUtil.<init>(GcsUtil.java:201)
at org.apache.beam.sdk.extensions.gcp.util.GcsUtil.<init>(GcsUtil.java:88)
at org.apache.beam.sdk.extensions.gcp.util.GcsUtil$GcsUtilFactory.create(GcsUtil.java:110)
at org.apache.beam.sdk.extensions.gcp.util.GcsUtil$GcsUtilFactory.create(GcsUtil.java:93)
at org.apache.beam.sdk.options.ProxyInvocationHandler.returnDefaultHelper(ProxyInvocationHandler.java:605)
at org.apache.beam.sdk.options.ProxyInvocationHandler.getDefault(ProxyInvocationHandler.java:546)
at org.apache.beam.sdk.options.ProxyInvocationHandler.invoke(ProxyInvocationHandler.java:171)
at com.sun.proxy.$Proxy41.getGcsUtil(Unknown Source)
at org.apache.beam.sdk.extensions.gcp.util.GcsUtilTest.testNonExistentObjectReturnsEmptyResult(GcsUtilTest.java:317)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
|
java-bigtable-hbase fails: |
|
grpc-okhttp fails. It fails even with the master branch without modifying build files. Why? Troubleshooting at grpc/grpc-java#8080 |
|
Your concern is valid and I agree that the test-artifact approach is more robust. That's why this modify-build-file approach is a temporary solution until the test-artifact approach is ready. If you prefer, we can keep this PR unmerged. Every time we update the LTS BOM, we can click "Update branch" button to run the tests. |
I think it's better to have the check run on the PRs that touches the LTS BOM. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
It was false positive. The version conflict between gax and gax-grpc. This gax-grpc was not upgraded to the version of the LTS BOM, because the project has its testlib classifier dependency (the tool has special handling for testlib classifier link) |
|
gRPC 1.36.2 works fine. |
|
Beam 2.30 passed. |
|
google-api-client-appengine passed with appengine-api-1.0-sdk 1.9.89 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is the implementation of the idea #1974 .
Each test case has commands on how to execute tests
lts.test.repositorysystem property. It then runs the LtsCompatibilityTestRunner for the case.In future, the repositories of the libraries in the LTS BOM will reference it. (PR for an example installation)
This is for the upcoming release. For the long-term, we want to a better approach that distribute responsibilities to library owners.