apache / accumulo-access

Apache Accumulo Access Control Library
https://accumulo.apache.org
Apache License 2.0
4 stars 11 forks source link

Change JDK distribution, add build for JDK 21 #23

Closed dlmarion closed 1 year ago

dlmarion commented 1 year ago

@ctubbsii - Should we change the JDK distribution from adopt to temurin? There is a note at setup-java that says:

NOTE: AdoptOpenJDK got moved to Eclipse Temurin and won't be updated anymore. It is highly recommended to migrate workflows from adopt and adopt-openj9, to temurin and semeru respectively, to keep receiving software and security updates. See more details in the Good-bye AdoptOpenJDK post.

dlmarion commented 1 year ago

Adding the distribution and JDK21 are fine, but the compiler properties should be set in the POM, along with maven.compiler.release to ensure Java 11 compliance... until the project is ready to require something newer.

Why wouldn't we want to check compliance for other JDK versions? This project is intended to be used outside of Accumulo by other projects, which may be using a different version of the JDK.

ctubbsii commented 1 year ago

Adding the distribution and JDK21 are fine, but the compiler properties should be set in the POM, along with maven.compiler.release to ensure Java 11 compliance... until the project is ready to require something newer.

Why wouldn't we want to check compliance for other JDK versions? This project is intended to be used outside of Accumulo by other projects, which may be using a different version of the JDK.

I'm not sure what you're trying to test by adding this. Do you anticipate Java will at some point in the future be changed so it can no longer read or execute Java 11 byte code? The whole point of the maven.compiler.release is to ensure that the byte code that is produced will properly and strictly comply with the requirements of a particular Java byte code version (and link only to the JVM features available at the time), so that future JDK versions can execute it compliant with that older byte code version without surprises. I don't really understand what you're testing if you're trying to build it with different compiler compliance levels.

Accumulo itself could run in a newer JDK, too, but we consider it sufficient to test using compiler compliance of JDK 11 as well. I just don't think we need this, because it's not testing anything of use to us. What we should test is that we are still able to build and produce JDK 11 compliant byte code using the newer JDKs, but to do that, you need to omit these command-line parameters.

dlmarion commented 1 year ago

I'm not sure what you're trying to test by adding this.

I was under the impression that compiling with a newer JDK might produce different or more optimized byte code. I did some reading about this last night, and there are only a few cases where this is the case. So, generally, my impression was incorrect. I'll remove the compiler flags from the flags from the command line args.