-
-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add native-linux-riscv64 package #111
Add native-linux-riscv64 package #111
Conversation
This patch adds support for linux-riscv64. It requires updating uraimo/run-on-arch-action to v2.4.0 for support for riscv64. It also uses JDK 21 for riscv64 as it's the first LTS version with support for riscv64 (backports to JDK 17 done and JDK 11 in progress).
Actually the problem is, if I compile this with JDK 21 then Maven will be pulling this automatically to downstream JDK(s) like 8 or 11 which will eventually break them badly. |
I'm not sure I understand the issue here. Do you mean that if we build with JDK 21, then it will mark the package as requiring JDK 21 which will break packages depending on brotli4j? |
Yup, that's right. Also, Netty 4.1 will never go beyond Java 6 and Netty 5 is also built with Java 11. |
I'm working on adding CI for JDK 11 and 17 on CI as well. Once it's working there, I'll add it to that PR. Practically speaking nothing stops us from building on JDK 11 and 17 as well. They are supported on linux-riscv64, even if more slowly (until backport are complete) as it uses the full interpreter mode in the JDK (no architecture specific depencencies). |
Right but even running JDK 11 code on JDK 8 will throw a direct error. Is there any hopes for JDK 8? |
We can easily specify to build the packages to be runnable on JDK 8 with setting |
I'd prefer that. :) |
@luhenry That's not necessarily true, see an example here. Given how |
@slandelle seems like we can use |
+1 for running tests JDK 8 distros. |
Ah cool. Actually, JEP 247 (or maven) was probably buggy last time I tried. My sample now works. |
@slandelle These incompatibility issues have actually prevented me from adding more variety of architectures. 🥲 |
I've added the use of |
This JDK has a dependency on glibc 2.34 which is not available in Ubuntu 20.04
I've added JDK 11 and 17 using the same workaround as s390x and aarch64 platforms ( On the issue of no building on JDK 8, I don't expect JDK 8 to support RISC-V is the near-term. Where that change is IMO ok is that:
Additionally we know it supports it because there is no RISC-V specific compilation flags for the Java code, so the same code is generated for RISC-V or any other platforms. Given Brotli4j, Netty, and any other project depending on Brotli4j will only run on JDK 11+ on RISC-V, checking that it runs on JDK 11 on RISC-V in Brotli4j is acceptable. |
I'd need a special test case to ensure the following: When we run this on JDK 8, it should run fine, except the native library file is not being loaded. We need some conditional checks for JDK 8. And JDK 11+, we need to make sure the library loads fine. Basically, I want to test this:
|
Let's compile this on JDK 11, and make sure tests pass normally. Then on same Docker runtime, install JDK 8 and run the special case without recompiling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not familiar with this codebase. But, to me, the changes look good.
Would you do that for RISC-V only, or all platforms? If for all platforms, that would help verify that the code generated by newer JDK is indeed compatible with JDK 8, however given you're only distributing the code generated by JDK 8, you're already pretty much guaranteed of that. And for RISC-V, the issue is there is no JDK 8 on RISC-V and I don't expect there will be one any time soon. The oldest supported version of the JDK on RISC-V is JDK 11. I'm currently working on testing with JDK 8 on all platforms (expect RISC-V) when built with JDK 11 and 17. |
Except on RISC-V where JDK 8 is not supported. But given there is no RISC-V specific code, it would be verified that compiling with JDK 11/17 and running on JDK 8 still functions.
Yup, that's right. I want to test for all platforms (preferably x64 for now) on JDK 8 compiled with JDK 11 just to be double sure (since I have some trust issues with Maven builds). |
@hyperxpro ace44cb adds just that for all JDK 11 and 17 configurations (except RISC-V again given we don't have JDK 8 on RISC-V). |
Actually, I mean, running RISC-V build (JDK 11) on x64 (JDK 8) without recompilation. |
Let me know if you need helping hands for this. |
@hyperxpro that's done with running RISC-V build (with JDK 11) on aarch64 (with JDK 8) without recompilation. I picked |
It should now be ready for final review. It tests on JDK 8 the RISC-V package build with JDK 11. @hyperxpro @slandelle let me know if there is anything you'd like me to change. Thanks! |
export MAVEN_OPTS="-Djdk.lang.Process.launchMechanism=vfork" | ||
chmod +x ./mvnw | ||
# Build native-linux-aarch64 only | ||
./mvnw -B --show-version -ntp clean package -pl :native-linux-aarch64 -DskipTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we do this, it will compile native-linux-aarch64
and run its tests not native-linux-riscv64
.
I guess this is not easily achievable, let me think of something. I will do a direct push.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will take it from here. :)
* Add native-linux-riscv64 package (#111) Motivation: This patch adds support for linux-riscv64. There was no code change other than adding support for linux-riscv64. Modification: It requires updating uraimo/run-on-arch-action to v2.4.0 for support for riscv64. It also uses JDK 21 for riscv64 as it's the first LTS version with support for riscv64 (backports to JDK 17 done and JDK 11 in progress). Result: Support for riscv64 architecture. --------- Co-authored-by: Ludovic Henry <git@ludovic.dev>
Motivation:
This patch adds support for linux-riscv64.
There was no code change other than adding support for linux-riscv64.
Modification:
It requires updating uraimo/run-on-arch-action to v2.4.0 for support for riscv64.
It also uses JDK 21 for riscv64 as it's the first LTS version with support for riscv64 (backports to JDK 17 done and JDK 11 in progress).
Result: