JAVA-2799: Create GraalVM substitutions for Guava's UnsignedBytes

[absurdfarce]

No description provided.

[absurdfarce]

Weirdly this was required... without this change Javadoc generation started failing the build:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  28.949 s
[INFO] Finished at: 2020-05-19T13:18:42-05:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.1.1:jar (attach-javadocs) on project java-driver-core: MavenReportException: Error while generating Javadoc: 
[ERROR] Exit code: 1 - javadoc: error - class file for com.datastax.oss.driver.shaded.guava.errorprone.annotations.CheckReturnValue not found
[ERROR] 
[ERROR] Command line was: /work/local/graalvm-ce-java8-19.3.1/jre/../bin/javadoc @options @packages
[ERROR] 
[ERROR] Refer to the generated Javadoc files in '/work/git/java-driver/core/target/apidocs' dir.
[ERROR] 
[ERROR] -> [Help 1]

I'm not sure where the reference to that class is coming from; it isn't introduced by the new dependency added in this change and isn't in the shaded Guava JAR.

[adutra]

I think Graal generates the substituted class inside driver-core own classes, i.e. in driver-core/target/classes which is why many tools (javadoc, bundle) are picking it up.

I think you can actually exclude everything that is shaded here, just in case we generate some more substitutions in the future:

<excludePackageNames>com.datastax.*.driver.internal*,com.datastax.*.driver.shaded*</excludePackageNames>

[absurdfarce]

This change also winds up splitting the com.datastax.oss.driver.shaded.guava.common.primitives across two JARs (this one and the shaded Guava JAR) which appeared to make the Felix bundler plugin pretty unhappy as well:

[INFO] --- maven-bundle-plugin:3.5.1:bundle (default-bundle) @ java-driver-core ---                                                                                               
[WARNING] Bundle com.datastax.oss:java-driver-core:bundle:4.7.0-SNAPSHOT : Split package, multiple jars provide the same package:com/datastax/oss/driver/shaded/guava/common/primi
tives                                                                                                                                                                             
Use Import/Export Package directive -split-package:=(merge-first|merge-last|error|first) to get rid of this warning                                                               
Package found in   [Jar:., Jar:java-driver-shaded-guava]                                                                                                                          
Class path         [Jar:., Jar:native-protocol, Jar:netty-handler, Jar:netty-common, Jar:netty-buffer, Jar:netty-transport, Jar:netty-resolver, Jar:netty-codec, Jar:java-driver-s
haded-guava, Jar:config, Jar:jnr-posix, Jar:jnr-ffi, Jar:jffi, Jar:jffi, Jar:asm-commons, Jar:asm-analysis, Jar:asm-tree, Jar:asm-util, Jar:jnr-a64asm, Jar:jnr-x86asm, Jar:jnr-co
nstants, Jar:javatuples, Jar:snappy-java, Jar:lz4-java, Jar:slf4j-api, Jar:metrics-core, Jar:HdrHistogram, Jar:esri-geometry-api, Jar:json, Jar:jackson-core-asl, Jar:gremlin-core
, Jar:gremlin-shaded, Jar:commons-configuration, Jar:commons-lang, Jar:commons-collections, Jar:commons-lang3, Jar:snakeyaml, Jar:hppc, Jar:jcabi-manifests, Jar:jcabi-log, Jar:ja
vapoet, Jar:exp4j, Jar:jcl-over-slf4j, Jar:tinkergraph-gremlin, Jar:gremlin-driver, Jar:groovy, Jar:groovy-json, Jar:jackson-core, Jar:jackson-databind, Jar:jackson-annotations, 
Jar:reactive-streams, Jar:jcip-annotations, Jar:spotbugs-annotations, Jar:jsr305, Jar:graal-sdk, Jar:svm, Jar:objectfile, Jar:pointsto, Jar:truffle-nfi, Jar:truffle-api, Jar:comp
iler, Jar:asm]  

I get the general idea but I don't understand enough yet about Felix and the bundler plugin to clearly understand how to resolve and/or test something like this.

[adutra]

Split packages are an absolute showstopper in OSGi I'm afraid, so we'll need to find a solution for this. I will investigate tomorrow.

[adutra]

Maybe if you change the <Export-Package> directive in the bundle plugin configuration for driver-core to this:

<Export-Package>com.datastax.oss.driver.*.core.*, com.datastax.dse.driver.*.core.*, !com.datastax.oss.driver.shaded.*</Export-Package>

For driver-core-shaded, I think we are safe.

(I didn't test this yet, this is just a guess from the top of my head)

[adutra]

@absurdfarce as I said here I think we should try to fix this in shaded-guava itself. This would certainly make our javadocs and OSGi problems go away.

[tomekl007]

Is it safe to hardcode the version of the graal here?
The quarkus takes it from the env runtime variable.
What will happen if someone will try to compile it in earlier versions?

[absurdfarce]

This lib is only required to gain access to the annotations needed to implement the substitution. So long as the Graal native build respects those annotations there shouldn't be an issue. I've tested this with a toolchain going back to 19.2.1 and it worked without issue so we should be fine.

Note that Quarkus only supports 19.3.1 so even with 19.2.1 we'd be talking about users writing their own apps for Graal and not using Quarkus.

[absurdfarce]

Worth noting: we're only "fixing" the shaded Guava artifact with these changes. If a customer also includes a (perhaps different) version of Guava in their classpath they might wind up seeing the warnings in question. I don't think that's necessarily wrong; seems like we should be on the hook to fix our part of this and anything in user-land is something the user should address.

I'm also not 100% sure we can (or should) fix this for the base Guava impl either. Perhaps we can mention it in the Graal-specific docs being discussed as part of JAVA-2777 and let users take action with user code as needed.

[absurdfarce]

As discussed in standup: we'll keep the substitutions in java-driver, which means we need to resolve the OSGi issues.

[adutra]

Still haven't had tome to look. Also, we need a proper ticket for capturing the Guava substitutions, JAVA-2782 is for the Quarkus extension and has been migrated.

[adutra]

As discussed offline, it's not that simple. I was hoping to eliminate the split package completely, but that's not possible. I'm not very confident on the idea of allowing this split package to exist, even if we include a strategy for merging split package occurrences. I suggest that we go back to the idea of doing these substitutions inside the guava artifact itself. Just for the record, the substitutions only add new classes, they do not modify existing ones, and from that perspective, they are pretty safe.
\cc @absurdfarce @olim7t

[adutra]

Created JAVA-2799 to isolate the changes required to Guava.

[absurdfarce]

After mucking around with the OSGi stuff myself for a while yesterday and today I'm inclined to agree with you @adutra . OSGi just doesn't like this split package concept... well, at least the Maven tools certainly don't

Anyways, if we take this path...what do we do to address the versioning question? Do we just add some extra qualifying names to the existing artifact (something like "25.1-jre-graal-subs-1")?

[absurdfarce]

Version of the Guava shaded JAR PR now up at datastax/java-driver-shaded-guava#4. Hopefully this should give us something concrete to look at and compare to. Presumably if we move forward with that change this PR would simply be closed.

[absurdfarce]

We're addressing this in the shaded Guava artifact so I'm closing this out