Skip to content

Conversation

@gnodet
Copy link
Contributor

@gnodet gnodet commented May 25, 2023

Currently, the need to support JDK 8 as a target kinda implies that maven itself needs to run with JDK 8. Toolchains are not easy to use and require a few manual steps.
This commit aims at reducing the burden of using toolchains so that maven get use JDK 11 as a requirement for running, but making it easy for users to target a different JDK.

The code currently adds automatic discovery of JDK on various platforms (though no WSL which looks quite complicated to support). The idea is also to see if the configuration of the toolchain plugin in the pom can also be simplified somehow.

This borrows code from IntelliJ Idea (which is ASL2).

@kwin
Copy link
Member

kwin commented May 25, 2023

implies that maven itself needs to run with JDK 8

Why not using cross compilation with newest javac still supporting Java 8 bytecode?

@gnodet gnodet force-pushed the discover-toolchains branch from 19acf20 to b50e2d7 Compare May 25, 2023 18:29
@gnodet
Copy link
Contributor Author

gnodet commented May 25, 2023

implies that maven itself needs to run with JDK 8

Why not using cross compilation with newest javac still supporting Java 8 bytecode?

What do you mean exactly ? I may have missed something...

@kwin
Copy link
Member

kwin commented May 25, 2023

implies that maven itself needs to run with JDK 8

Why not using cross compilation with newest javac still supporting Java 8 bytecode?

What do you mean exactly ? I may have missed something...

Usually using maven.compiler.release set to 8 is enough. No need to run the build with JDK 8 or toolchains (except for running ITs)

@gnodet
Copy link
Contributor Author

gnodet commented May 25, 2023

implies that maven itself needs to run with JDK 8

Why not using cross compilation with newest javac still supporting Java 8 bytecode?

What do you mean exactly ? I may have missed something...

Usually using maven.compiler.release set to 8 is enough. No need to run the build with JDK 8 or toolchains.

Yes, if they are people are happy building with this option, they definitely don't need to use toolchains. My understanding is that when running tests, people quite like the tests to be run in the target environment.
For those that want to use the target JDK, I think simplifying the usage of toolchains would be a win.

@olamy
Copy link
Member

olamy commented May 25, 2023

very interesting/nice feature!
sdkman can be used with osx as well. I haven’t reviewed the code enough but not sure if the current draft takes care of this? IntelliJ does ;)

@gnodet
Copy link
Contributor Author

gnodet commented May 25, 2023

very interesting/nice feature!
sdkman can be used with osx as well. I haven’t reviewed the code enough but not sure if the current draft takes care of this? IntelliJ does ;)

yes, that's taken care of !

@gnodet
Copy link
Contributor Author

gnodet commented May 26, 2023

The PR apache/maven-toolchains-plugin#14 provides a new mojo that displays the list of discovered toolchains.

➜  maven git:(discover-toolchains) ✗ ~/work/git/maven/apache-maven/target/apache-maven-4.0.0-alpha-6-SNAPSHOT/bin/mvn org.apache.maven.plugins:maven-toolchains-plugin:4.0.0-SNAPSHOT:display-discovered-toolchains -N
[INFO] Scanning for projects...
[INFO] 
[INFO] ------------------------------------------------< org.apache.maven:maven >------------------------------------------------
[INFO] Building Apache Maven 4.0.0-alpha-6-SNAPSHOT
[INFO]   from pom.xml
[INFO] ---------------------------------------------------------[ pom ]----------------------------------------------------------
[INFO] 
[INFO] --- toolchains:4.0.0-SNAPSHOT:display-discovered-toolchains (default-cli) @ maven ---
<?xml version="1.0" encoding="UTF-8"?>
<toolchains xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns="http://maven.apache.org/POM/4.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>1.8.0_372</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>1.8.0_372-b07</runtime.version>
      <vendor>Azul Systems, Inc.</vendor>
    </provides>
    <configuration>
      <jdkHome>/Library/Java/JavaVirtualMachines/zulu-8.jdk/Contents/Home/jre</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>1.8.0_362</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>1.8.0_362-b09</runtime.version>
      <vendor>BellSoft</vendor>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/8.0.362-librca/jre</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>17.0.6</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>17.0.6+10</runtime.version>
      <vendor>Eclipse Adoptium</vendor>
      <vendor.version>Temurin-17.0.6+10</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/17.0.6-tem</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>20</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>20+36</runtime.version>
      <vendor>Eclipse Adoptium</vendor>
      <vendor.version>Temurin-20+36</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/20-tem</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>11.0.17</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>11.0.17+8-jvmci-22.3-b08</runtime.version>
      <vendor>GraalVM Community</vendor>
      <vendor.version>GraalVM CE 22.3.0</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/22.3.r11-grl</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>11.0.18</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>11.0.18+10-jvmci-22.3-b13</runtime.version>
      <vendor>GraalVM Community</vendor>
      <vendor.version>GraalVM CE 22.3.1</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Library/Java/JavaVirtualMachines/graalvm-ce-java11-22.3.1/Contents/Home</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>17.0.5</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>17.0.5+8-jvmci-22.3-b08</runtime.version>
      <vendor>GraalVM Community</vendor>
      <vendor.version>GraalVM CE 22.3.0</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/22.3.r17-grl</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>17.0.5</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>17.0.5+8-jvmci-22.3-b08</runtime.version>
      <vendor>GraalVM Community</vendor>
      <vendor.version>GraalVM CE 22.3.0</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/22.3.r17-grl</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>17.0.6</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>17.0.6+10-jvmci-22.3-b13</runtime.version>
      <vendor>GraalVM Community</vendor>
      <vendor.version>GraalVM CE 22.3.1</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Library/Java/JavaVirtualMachines/graalvm-ce-java17-22.3.1/Contents/Home</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>17.0.6</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>17.0.6+10-jvmci-22.3-b13</runtime.version>
      <vendor>GraalVM Community</vendor>
      <vendor.version>GraalVM CE 22.3.1</vendor.version>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/22.3.1.r17-grl</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>20-ea</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>20-ea+24-1795</runtime.version>
      <vendor>Oracle Corporation</vendor>
    </provides>
    <configuration>
      <jdkHome>/Library/Java/JavaVirtualMachines/jdk-20.jdk/Contents/Home</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>21-ea</version>
      <runtime.name>OpenJDK Runtime Environment</runtime.name>
      <runtime.version>21-ea+22-1890</runtime.version>
      <vendor>Oracle Corporation</vendor>
    </provides>
    <configuration>
      <jdkHome>/Users/gnodet/.sdkman/candidates/java/21.ea.22-open</jdkHome>
    </configuration>
  </toolchain>
</toolchains>

[INFO] Copying org.apache.maven:maven:pom:4.0.0-alpha-6-SNAPSHOT to project local repository
[INFO] Copying org.apache.maven:maven:pom:consumer:4.0.0-alpha-6-SNAPSHOT to project local repository
[INFO] --------------------------------------------------------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] --------------------------------------------------------------------------------------------------------------------------
[INFO] Total time:  1.051 s
[INFO] Finished at: 2023-05-26T21:07:20+02:00
[INFO] --------------------------------------------------------------------------------------------------------------------------
➜  maven git:(discover-toolchains) ✗ 

@gnodet
Copy link
Contributor Author

gnodet commented May 26, 2023

I'm planning to add better support for the current JDK toolchain. The selection of the toolchain by the mojo, in the case of the JDK toolchain, should take into account the current toolchain with several modes:

  • ignore : always use a toolchain
  • if-same : if the selection selects the current toolchain, then do not use any toolchain to avoid forking tools
  • if-match : first match the current toolchain against the set of requirements, and use it (i.e. do not use any toolchain) if they match. This should also avoid discovering all JDK toolchains if the current one matches

Also, the selection may be made slightly easier for the JDK toolchains, as the set of properties is kinda determined by the discovery mechanism, so a dedicated mojo may be needed for the JDK toolchain.

The discovery could be stored to disk, at least the mapping between the jdk home and the properties, so that we don't have to launch all the JVMs to get the information.

I also wonder if the jdk profile activation should use the toolchain.

@gnodet
Copy link
Contributor Author

gnodet commented May 30, 2023

I actually wonder if it would make more sense to move that code into the maven-toolchains-plugin. This plugin is actually required to select the toolchain, as there's no other official way afaik (one can always rewrite a different plugin).
So the discovery could happen at the time the toolchain is selected, which would avoid the need to perform the discovery if not needed.

@gnodet
Copy link
Contributor Author

gnodet commented May 30, 2023

Given the way toolchains are defined and used in the code, there's no real reason to split the discovery process from the selection process, so I'm closing this PR in favour of apache/maven-toolchains-plugin#14 which adds 3 mojos to discover and display / generate an xml / select the toolchain.

@gnodet gnodet closed this May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants