Skip to content

Commit

Permalink
Moves to only support LTS JDKs (#3618)
Browse files Browse the repository at this point in the history
This fixes errorprone configuration blocking use of JDK 17. At this
point, we no longer use a non-LTS JDK 15 to compile.

This also introduces CI to ensure JDK 11 still works. This is needed
because some libraries such as Zulu aggressively age out JDKs.

As a reminder, we currently target 1.6 for the core jar on release, so
that zipkin-reporter can still work in old clients. We may need to
vendor those classes in zipkin-reporter-java to unlock things in the
future.

To reduce scope this also removes CI from the no-longer active v3
(skywalking) branch.

Signed-off-by: Adrian Cole <[email protected]>
  • Loading branch information
codefromthecrypt authored Dec 6, 2023
1 parent 68c4949 commit 7df908c
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 93 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/readme_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
uses: actions/setup-java@v4
with:
distribution: 'zulu' # zulu as it supports a wide version range
java-version: '15' # highest value allowed by maven-enforcer-plugin
java-version: '17' # Most recent LTS (TODO: change to 21 including docker images as a separate step)
- name: Cache NPM Packages
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -65,7 +65,7 @@ jobs:
uses: actions/setup-java@v4
with:
distribution: 'zulu' # zulu as it supports a wide version range
java-version: '15' # most recent LTS we support
java-version: '17' # Most recent LTS (TODO: change to 21 including docker images as a separate step)
# We can't cache Docker without using buildx because GH actions restricts /var/lib/docker
# That's ok because DOCKER_PARENT_IMAGE is always ghcr.io and local anyway.
- name: Cache NPM Packages
Expand Down
73 changes: 0 additions & 73 deletions .github/workflows/test-v3.yml

This file was deleted.

17 changes: 13 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,16 @@ on:

jobs:
test:
name: test (JDK ${{ matrix.java_version }})
runs-on: ubuntu-22.04 # newest available distribution, aka jellyfish
if: "!contains(github.event.head_commit.message, 'maven-release-plugin')"
strategy:
fail-fast: false # don't fail fast as sometimes failures are operating system specific
matrix: # use latest available versions and be consistent on all workflows!
include:
- java_version: 11 # Last that can compile zipkin core to 1.6 for zipkin-reporter
maven_args: -Prelease -Dgpg.skip
- java_version: 17 # Most recent LTS (TODO: change to 21 including docker images as a separate step)
steps:
- name: Checkout Repository
uses: actions/checkout@v4
Expand All @@ -31,20 +39,21 @@ jobs:
uses: actions/setup-java@v4
with:
distribution: 'zulu' # zulu as it supports a wide version range
java-version: '15' # highest value allowed by maven-enforcer-plugin
java-version: ${{ matrix.java_version }}
- name: Cache local Maven repository
uses: actions/cache@v3
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
key: ${{ runner.os }}-jdk-${{ matrix.java_version }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: ${{ runner.os }}-maven-
- name: Cache NPM Packages
uses: actions/cache@v3
with:
path: ~/.npm
key: ${{ runner.os }}-npm-packages-${{ hashFiles('zipkin-lens/package-lock.json') }}
- name: Test without Docker
run: build-bin/maven_go_offline && build-bin/test -Ddocker.skip=true
run: build-bin/maven_go_offline && build-bin/test -Ddocker.skip=true ${{ matrix.maven_args }}

test_docker:
runs-on: ubuntu-22.04 # newest available distribution, aka jellyfish
if: "!contains(github.event.head_commit.message, 'maven-release-plugin')"
Expand All @@ -65,7 +74,7 @@ jobs:
uses: actions/setup-java@v4
with:
distribution: 'zulu' # zulu as it supports a wide version range
java-version: '15' # highest value allowed by maven-enforcer-plugin
java-version: '17' # Most recent LTS (TODO: change to 21 including docker images as a separate step)
- name: Cache local Maven repository
uses: actions/cache@v3
with:
Expand Down
19 changes: 14 additions & 5 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,7 @@
<requireJavaVersion>
<!-- Change this to control LTS JDK versions allowed to build
the project. Keep in sync with .github/workflows -->
<!-- TODO: JDK 15 isn't LTS, so we want at least JDK 17.
However, `./mvnw clean package -pl :zipkin` fails -->
<version>[11,16)</version>
<version>11,17</version>
</requireJavaVersion>
</rules>
</configuration>
Expand Down Expand Up @@ -662,9 +660,9 @@
</profile>

<profile>
<id>error-prone-9+</id>
<id>error-prone-11+</id>
<activation>
<jdk>[9,)</jdk>
<jdk>11,17</jdk>
</activation>
<build>
<plugins>
Expand All @@ -691,6 +689,17 @@
<compilerArgs>
<arg>-XDcompilePolicy=simple</arg>
<arg>-Xplugin:ErrorProne ${errorprone.args}</arg>
<!-- below needed for JDK16+ per https://errorprone.info/docs/installation -->
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
</compilerArgs>
<annotationProcessorPaths>
<processorPath>
Expand Down
10 changes: 4 additions & 6 deletions zipkin/RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,19 @@ inconvenience to core library maintainers.

#### Why is the compiler plugin set to source level 1.7?
JDK 11 was the last JDK to allow writing Java 1.6 bytecode. We set the default
compile target to 1.7 in order to allow passing users to use any recent JDK.
compile target to 1.7 in order to allow passing users to the LTS JDK 17.

We enforce 1.6 only on the `release` Maven profile. This also requires a JDK no
later than 11. We will have a decision to make when JDK 11 goes out of support
in September 2023. Choices could include switching to a more flexibly supported
JDK such as Zulu for the release process.
later than 11. JDK 11 went out of Oracle support in September 2023, but not for
Zulu. Hence, we use Zulu for both testing and releasing Zipkin using JDK 11.

The disparity between 1.7 and 1.6 can cause some problems in maintenance. For
example, the IDE may suggest switching to diamond syntax, which will break
compilation in 1.6. This is why the `release` profile is used in at least one
CI matrix. The poor experience is limited to a rarely modified and smaller part
of the project (this library).


Note, an alternative is to use [Retrolambda](https://github.com/luontola/retrolambda) to backport 1.7 features to 1.6
Note, an alternative is to use [Retrolambda](https://github.com/luontola/retrolambda) to backport 1.8 features to 1.6
bytecode. However, this [stopped working in JDK 15](https://github.com/luontola/retrolambda/issues/161), so we can't use this tactic
until that's resolved or another tool surfaces to do the same.

Expand Down
7 changes: 4 additions & 3 deletions zipkin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@

<main.basedir>${project.basedir}/..</main.basedir>

<!-- CI should run the "release" profile during tests to ensure no 1.7 features are in use -->
<!-- CI should run the "release" profile during tests to ensure no 1.7 features are in use.
TODO: for JDK 21, bump to 1.8: https://bugs.openjdk.org/browse/JDK-8173605 -->
<main.java.version>1.7</main.java.version>
<main.signature.artifact>java17</main.signature.artifact>
</properties>
Expand Down Expand Up @@ -142,10 +143,10 @@
</goals>
<configuration>
<rules>
<!-- JDK 12+ can no longer generate Java 1.6 bytecode, so we require JDK 11.
<!-- Only 1.8 and 11 are LTS JDKs that can compile 1.6 bytecode.
https://www.oracle.com/java/technologies/javase/12-relnote-issues.html -->
<requireJavaVersion>
<version>[1.8,12)</version>
<version>1.8,11</version>
</requireJavaVersion>
</rules>
</configuration>
Expand Down

0 comments on commit 7df908c

Please sign in to comment.