cryostatio / cryostat-core

Core library providing a convenience wrapper and headless stubs for managing JFR with JDK Mission Control API
https://cryostat.io/
Other
8 stars 21 forks source link

[CI] `libcryostat` should not only target JDK 11, but should be built with JDK 11 #425

Closed andrewazores closed 2 months ago

andrewazores commented 2 months ago

See #416

Currently, the cryostat-core and cryostat-core-parent define Java and Maven/compiler versions at 17:

  <java.version>17</java.version>
  <maven.compiler.target>${java.version}</maven.compiler.target>
  <maven.compiler.source>${java.version}</maven.compiler.source>

(https://github.com/cryostatio/cryostat-core/blob/f6da0a36fb63e5bef6fffabc16d2f7fc892ae192/pom.xml#L68 , https://github.com/cryostatio/cryostat-core/blob/f6da0a36fb63e5bef6fffabc16d2f7fc892ae192/cryostat-core/pom.xml#L55)

whereas libcryostat is intentionally requiring only Java 11:

(https://github.com/cryostatio/cryostat-core/blob/f6da0a36fb63e5bef6fffabc16d2f7fc892ae192/libcryostat/pom.xml#L49)

However, the two modules and the parent project are all built together in the end using JDK 17:

https://github.com/cryostatio/cryostat-core/blob/f6da0a36fb63e5bef6fffabc16d2f7fc892ae192/.github/workflows/push-ci.yml#L18

This results in classes with the correct class file version, ex.:

$ javap -verbose Agent | less
Warning: File ./Agent.class does not contain class Agent
Classfile /home/work/workspace/quarkus-test/target/dependency/io/cryostat/agent/Agent.class
  Last modified Jul. 18, 2024; size 18175 bytes
  SHA-256 checksum 0914ef3cc5892ec85175af25a48b4ef4578b3ad0b5dd17abae8f4f89133f7772
  Compiled from "Agent.java"
public class io.cryostat.agent.Agent extends java.lang.Object implements java.util.concurrent.Callable<java.lang.Integer>, java.util.function.Consumer<io.cryostat.agent.AgentArgs>
  minor version: 0
  major version: 55

but it is not actually guaranteed that JDK 17 is able to generate bytecode that a JDK 11 JVM will understand. To guarantee this, cryostat-core should be built with JDK 17, but libcryostat must be built with JDK 11, then both of those must be published as-is rather than rebuilt using JDK 17 from cryostat-core-parent.

andrewazores commented 2 months ago

It seems like toolchains are probably the right answer here.

https://github.com/actions/setup-java?tab=readme-ov-file#install-multiple-jdks

https://maven.apache.org/guides/mini/guide-using-toolchains.html

https://stackoverflow.com/a/26082749