[BUILD] pin Maven compiler target version to Java 8 and remove explict compiler release version #6766

bowenliang123 · 2024-10-21T15:17:18Z

🔍 Description

Issue References 🔗

This pull request fixes #

Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes 🔖

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

codecov-commenter · 2024-10-21T17:26:19Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (c391d16) to head (c374494).

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #6766   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         687     687           
  Lines       42442   42442           
  Branches     5793    5793           
======================================
  Misses      42442   42442

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

pan3793 · 2024-10-22T02:22:43Z

since we set parent pom to org.apache:apache:33, we don't need to define those properties again, just follow apache/maven-apache-parent@ded34a8 to override maven.compiler.target then everything is fine.

bowenliang123 · 2024-10-22T03:01:53Z

since we set parent pom to org.apache:apache:33, we don't need to define those properties again, just follow apache/maven-apache-parent@ded34a8 to override maven.compiler.target then everything is fine.

SGTM. Applied a similar patch to our pom. And it could be removed when apache parent pom 34 released in the future.

bowenliang123 · 2024-10-22T03:18:04Z

Builds failed for missing class/package sun.misc.Signal with error logs:
https://github.com/apache/kyuubi/actions/runs/11452487137/job/31863464278?pr=6766#step:9:197

Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:26: not found: object sun
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:36: not found: type Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:54: not found: type Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:54: not found: type SignalHandler
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:56: not found: type SignalHandler
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:56: not found: value Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:58: not found: type Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:59: not found: value Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:65: not found: value Signal
Error: [ERROR] 9 errors found

cc @pan3793 @wForget

pan3793 · 2024-10-22T03:45:14Z

cc @LuciferYang, could you please give some advice if you have time? thanks in advance.

LuciferYang · 2024-10-22T11:02:03Z

pom.xml

-        <maven.compiler.source>${java.version}</maven.compiler.source>
-        <maven.compiler.target>${java.version}</maven.compiler.target>
+        <!-- Pinning compiler target to Java 8-->
+        <maven.compiler.target>8</maven.compiler.target>


IIRC，In principle, maven.compiler.source and maven.compiler.target should be set as a pair.

LuciferYang · 2024-10-22T11:17:53Z

Builds failed for missing class/package sun.misc.Signal with error logs: https://github.com/apache/kyuubi/actions/runs/11452487137/job/31863464278?pr=6766#step:9:197

Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:26: not found: object sun
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:36: not found: type Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:54: not found: type Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:54: not found: type SignalHandler
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:56: not found: type SignalHandler
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:56: not found: value Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:58: not found: type Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:59: not found: value Signal
Error: ] /home/runner/work/kyuubi/kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/util/SignalRegister.scala:65: not found: value Signal
Error: [ERROR] 9 errors found

cc @pan3793 @wForget

~~After version 4.7.1, the scala-maven-plugin will, by default, add the -release compilation option for newer~~
~~Scala versions, such as 2.13.9+ and possibly some newer versions of 2.12.x (I can't recall the specifics).~~

-release can lead to compilation errors when crossing Java versions, such as using Java APIs that are not opened(SignalHandler in Java 8 belongs to the internal API, If the target version is 9+ using SignalHandler is also visible during cross compilation). ~~If it's not possible to avoid using them,we can try downgrading the~~
~~scala-maven-plugin to version 4.7.1 to avoid the forced addition of the -release compilation option~~

Ignoring the above comments, it seems that it's not the plugin that adds -release, but rather the apache-33.pom that activates the jdk9+ profile, which then uses maven.compiler.release.

 <profile>
      <id>jdk9+</id>
      <activation>
        <!-- this does not support toolchains, https://issues.apache.org/jira/browse/MNG-6943 -->
        <jdk>[9,)</jdk>
      </activation>
      <properties>
        <!-- https://maven.apache.org/plugins/maven-compiler-plugin/examples/set-compiler-release.html, affects m-compiler-p and m-javadoc-p -->
        <maven.compiler.release>${maven.compiler.target}</maven.compiler.release>
      </properties>
    </profile>

So, without modifying the code, when building with Java 17/21, the -release option might only be able to use a higher version, such as 11. I haven't thought of any better solutions at the moment. @pan3793

github-actions bot added the kind:build label Oct 21, 2024

bowenliang123 mentioned this pull request Oct 21, 2024

[CI] Specify JDK 11 for spark engine on kubernetes IT #6763

Closed

4 tasks

bowenliang123 changed the title ~~pin Maven compiler release version to Java 8~~ [BUILD] pin Maven compiler release version to Java 8 Oct 21, 2024

bowenliang123 changed the title ~~[BUILD] pin Maven compiler release version to Java 8~~ [BUILD] pin Maven compiler target version to Java 8 and remove explict compiler release version Oct 22, 2024

bowenliang123 added this to the v1.10.0 milestone Oct 22, 2024

bowenliang123 self-assigned this Oct 22, 2024

bowenliang123 removed this from the v1.10.0 milestone Oct 22, 2024

bowenliang123 removed their assignment Oct 22, 2024

bowenliang123 force-pushed the compiler-release-8 branch 5 times, most recently from 72c92e5 to 0114f56 Compare October 22, 2024 05:50

LuciferYang reviewed Oct 22, 2024

View reviewed changes

bowenliang123 force-pushed the compiler-release-8 branch from 0114f56 to ffe3c85 Compare November 19, 2024 07:36

update

df76065

bowenliang123 force-pushed the compiler-release-8 branch from ffe3c85 to df76065 Compare November 19, 2024 09:29

bowenliang123 added 2 commits November 19, 2024 18:07

add-opens and add-exports

da3645b

update

c374494

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUILD] pin Maven compiler target version to Java 8 and remove explict compiler release version #6766

[BUILD] pin Maven compiler target version to Java 8 and remove explict compiler release version #6766

bowenliang123 commented Oct 21, 2024

codecov-commenter commented Oct 21, 2024 •

edited

Loading

pan3793 commented Oct 22, 2024 •

edited

Loading

bowenliang123 commented Oct 22, 2024 •

edited

Loading

bowenliang123 commented Oct 22, 2024 •

edited

Loading

pan3793 commented Oct 22, 2024

LuciferYang Oct 22, 2024

LuciferYang commented Oct 22, 2024 •

edited

Loading

[BUILD] pin Maven compiler target version to Java 8 and remove explict compiler release version #6766

Are you sure you want to change the base?

[BUILD] pin Maven compiler target version to Java 8 and remove explict compiler release version #6766

Conversation

bowenliang123 commented Oct 21, 2024

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Types of changes 🔖

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

codecov-commenter commented Oct 21, 2024 • edited Loading

Codecov Report

pan3793 commented Oct 22, 2024 • edited Loading

bowenliang123 commented Oct 22, 2024 • edited Loading

bowenliang123 commented Oct 22, 2024 • edited Loading

pan3793 commented Oct 22, 2024

LuciferYang Oct 22, 2024

Choose a reason for hiding this comment

LuciferYang commented Oct 22, 2024 • edited Loading

codecov-commenter commented Oct 21, 2024 •

edited

Loading

pan3793 commented Oct 22, 2024 •

edited

Loading

bowenliang123 commented Oct 22, 2024 •

edited

Loading

bowenliang123 commented Oct 22, 2024 •

edited

Loading

LuciferYang commented Oct 22, 2024 •

edited

Loading