upper-bound-check to work with google-cloud-java's split repository in more generic way

[suztomo]

Regarding google-cloud-java's split repositories, as of now the upper-bound-check (https://github.com/googleapis/google-cloud-java/pull/6606/files) does not work for individual libraries, because the libraries-bom only references google-cloud-bom's version. It cannot update individual library's versions referenced by google-cloud-bom.

@chingor13 says:

it seems like we might be able to generalize it to check for arbitrary artifacts with any bom so like the pubsub repo could check itself in google-cloud-bom

Note that, contrary to upper-bound-check, Linkage Monitor should work for the individual repositories.

[suztomo]

This check, if implemented, could have detected the typo in google-cloud-errorreporting-bom https://github.com/googleapis/java-errorreporting/pull/23

[suztomo]

No, it did not. I tested cloud-opensource-java/boms/upper-bounds-check for the latest google-cloud-bom that has com.google.cloud:google-cloud-errorreporting-bom:0.118.0-beta, which contains nonexistent artifact com.google.cloud:grpc-google-cloud-error-reporting-v1beta1. It just ran without causing an error.

Elliotte's finding was on testMaximumLinkageErrors, which builds a class path by downloading the all artifacts in the BOM.

Why upper-bounds-check cannot detect the typo?

When there's a typo in groupID or artifactID, such entries in managed dependencies in BOMs are just unused in the upper-bounds-check.

[suztomo]

For implementation, use properties to specify BOM versions:

suztomo@suxtomo24:~/java-cloud-bom$ git diff
diff --git a/pom.xml b/pom.xml
index 5c163ff..a487a8f 100644
--- a/pom.xml
+++ b/pom.xml
@@ -10,6 +10,9 @@
   <description>
     BOM for google-cloud-java
   </description>
+  <properties>
+    <google.cloud.errorreporting.java.version>0.118.0-beta</google.cloud.errorreporting.java.version>
+  </properties>
   <developers>
     <developer>
       <id>garrettjonesgoogle</id>
@@ -353,7 +356,7 @@
       <dependency>
         <groupId>com.google.cloud</groupId>
         <artifactId>google-cloud-errorreporting-bom</artifactId>
-        <version>0.118.0-beta</version>
+        <version>${google.cloud.errorreporting.java.version}</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>

[suztomo]

As of now, the upper-bounds-check checks a dependency tree of com.google.cloud:google-cloud-core and com.google.cloud:google-cloud-storage only. The TODO comment says:

TODO is there a way to run this test directly on the BOM instead of on a pom.xml that imports the BOM?

Enforcer rule's RequireUpperBoundDeps class has following code to get dependencyNode. If this returns a node having the same list of dependencyManagement/dependencies, then upper-bounds-check becomes more generic.

    private DependencyNode getNode( EnforcerRuleHelper helper )
        throws EnforcerRuleException
    {
        try
        {
            MavenProject project = (MavenProject) helper.evaluate( "${project}" );
            DependencyTreeBuilder dependencyTreeBuilder =
                (DependencyTreeBuilder) helper.getComponent( DependencyTreeBuilder.class );
            ArtifactRepository repository = (ArtifactRepository) helper.evaluate( "${localRepository}" );
            ArtifactFactory factory = (ArtifactFactory) helper.getComponent( ArtifactFactory.class );
            ArtifactMetadataSource metadataSource =
                (ArtifactMetadataSource) helper.getComponent( ArtifactMetadataSource.class );
            ArtifactCollector collector = (ArtifactCollector) helper.getComponent( ArtifactCollector.class );
            ArtifactFilter filter = null; // we need to evaluate all scopes
            DependencyNode node =
                dependencyTreeBuilder.buildDependencyTree( project, repository, factory, metadataSource, filter,
                                                           collector );
            return node;
        }

Furthermore, once such enforcer rule exists, BOM's pom.xml can install the enforcer rule. We don't need to setup additional project such as our boms/upper-bounds-check to run the rule.

[suztomo]

@elharo Example failure for google-cloud-bom version 0.101.0-alpha:

suztomo@suxtomo24:~/cloud-opensource-java/boms/upper-bounds-check$ mvn validate -Dgoogle.cloud.java.version=0.101.0-alpha
...
[INFO] --- maven-enforcer-plugin:3.0.0-M2:enforce (enforce) @ upper-bounds-check ---
[WARNING] Rule 0: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for com.google.errorprone:error_prone_annotations:2.3.2 paths to dependency are:
+-com.google.cloud.tools.opensource:upper-bounds-check:2.0.0-SNAPSHOT
  +-com.google.cloud:google-cloud-core:1.83.0
    +-com.google.guava:guava:28.1-android (managed) <-- com.google.guava:guava:28.0-android
      +-com.google.errorprone:error_prone_annotations:2.3.2
and
+-com.google.cloud.tools.opensource:upper-bounds-check:2.0.0-SNAPSHOT
  +-com.google.cloud:google-cloud-core:1.83.0
    +-com.google.protobuf:protobuf-java-util:3.11.0 (managed) <-- com.google.protobuf:protobuf-java-util:3.7.1
      +-com.google.errorprone:error_prone_annotations:2.3.3

How about including error_prone_annotations:2.3.2 (old) in BOM?

The enforcer rule still throws error:

[WARNING] Rule 0: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for com.google.errorprone:error_prone_annotations:2.3.2 paths to dependency are:
+-com.google.cloud.tools.opensource:upper-bounds-check:2.0.0-SNAPSHOT
  +-com.google.cloud:google-cloud-core:1.83.0
    +-com.google.guava:guava:28.1-android (managed) <-- com.google.guava:guava:28.0-android
      +-com.google.errorprone:error_prone_annotations:2.3.2
and
+-com.google.cloud.tools.opensource:upper-bounds-check:2.0.0-SNAPSHOT
  +-com.google.cloud:google-cloud-core:1.83.0
    +-com.google.protobuf:protobuf-java-util:3.11.0 (managed) <-- com.google.protobuf:protobuf-java-util:3.7.1
      +-com.google.errorprone:error_prone_annotations:2.3.2 (managed) <-- com.google.errorprone:error_prone_annotations:2.3.3