Linkage Monitor failed to catch missing transitive dependencies of google-cloud-spanner-jdbc

[suztomo]

GoogleCloudPlatform/java-docs-samples#4278 (comment)

Linkage Monitor couldn't catch the missing classes because Linkage Monitor checks one big dependency tree that is generated by the list of BOM artifacts. If it would check the artifact one by one, it could detect the missing classes due to the excluded dependencies.

[elharo]

I'm not yet convinced it should have caught anything. If the exclusion was at fault, then google-cloud-spanner would not have compiled. The problem is that the sample code was missing a dependency, and linkage monitor never checked that. When google-cloud-spanner removed a transitive dependency it didn't need, the sample broke. This is why projects should declare all their dependencies and not rely on transitive dependencies.

[suztomo]

Google-cloud-spanner is fine. I'm suspecting google-cloud-spanner-jdbc's dependency on it with the exclusions.

[suztomo]

Giving more background. Running LinkageCheckerMain with -a com.google.cloud:google-cloud-spanner-jdbc:1.17.3 arguments gives the following message among other linkage errors:

...
Class com.google.spanner.admin.instance.v1.InstanceAdminGrpc is not found;
  referenced by 1 class file
    com.google.cloud.spanner.spi.v1.GapicSpannerRpc (com.google.cloud:google-cloud-spanner:2.0.2)
  Cause:
    The valid symbol is in com.google.api.grpc:grpc-google-cloud-spanner-admin-instance-v1:jar:2.0.2 at 
com.google.cloud:google-cloud-spanner-jdbc:jar:1.17.3 / com.google.cloud:google-cloud-spanner:2.0.2 (compile) /
 com.google.api.grpc:grpc-google-cloud-spanner-admin-instance-v1:2.0.2 (compile) but it was not selected because 
com.google.cloud:google-cloud-spanner-jdbc:1.17.3 excludes com.google.api.grpc:grpc-google-cloud-spanner-admin-instance-v1.
...

This is the very problem we were looking for in GoogleCloudPlatform/java-docs-samples#4278 (comment).

On the other hand, LinkageCheckerMain with -a com.google.cloud:google-cloud-spanner-jdbc:1.17.3,com.google.cloud:google-cloud-spanner:2.0.2 arguments does not give the error above because the missing dependency is supplied by google-cloud-spanner.

Google-cloud-bom's Linkage Monitor could have found this if we ran Linkage Monitor differently.

[suztomo]

To file a bug to maven-flatten-plugin.

[suztomo]

Periodic check for the published artifacts?

[suztomo]

To file a bug to maven-flatten-plugin.

Filed mojohaus/flatten-maven-plugin#185

[suztomo]

the flatten plugin works as install phase, "verify" phase might be able to catch this at the library's repository's pull request.

[saturnism]

This is an interesting issue.

If you look at java-spanner-jdbc project, https://github.com/googleapis/java-spanner-jdbc/blob/v1.17.2/pom.xml, and run mvn dependency:tree -Dscope=runtime or mvn dependency:list -Dscope=runtime, the grpc-google-cloud-spanner-admin-database-v1 was not resolved in compile scope either.

however, when java-spanner-jdbc is pulled in as a dependency, the compile scope did resolve.

a side question is why are these deps (e.g., grpc-google-cloud-spanner-admin-database-v1) explicitly declared for test scope, when compile scope is already included within the test scope? if not needed, a quick work around is to remove them from the test scope while flatten is updated.

[suztomo]

however, when java-spanner-jdbc is pulled in as a dependency, the compile scope did resolve.

That's an interesting observation. I think the fact that Maven ignores non-direct test-scope dependencies ("test: ... This scope is not transitive.") explain the difference.

a side question is why are these deps (e.g., grpc-google-cloud-spanner-admin-database-v1) explicitly declared for test scope

If a project uses a class of an artifact, then it's better to declare to the artifact explicitly. "strict deps" in Bazel's term https://github.com/GoogleCloudPlatform/cloud-opensource-java/pull/1767/files#diff-957ff069d393343b026a87ea30d4000884c8cf0ba1622ec861eb6598e034d91eR9

a quick work around

An workaround has been in place : googleapis/java-spanner-jdbc#278

[elharo]

Anything linkage monitor needs to do or should we close this?

[suztomo]

Nothing on Linkage Monitor. We may come up with a solution to validate artifacts one by one in future.