Skip to content
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 support for RISC-V #13670

Merged
merged 18 commits into from
Dec 11, 2023
Merged

Add support for RISC-V #13670

merged 18 commits into from
Dec 11, 2023

Conversation

luhenry
Copy link
Contributor

@luhenry luhenry commented Oct 27, 2023

Motivation:

RISC-V is gaining traction across the industry and many projects rely on Netty. To run these projects, Netty needs to support RISC-V.

Modifications:

Mostly build scripts and CI files. CentOS doesn't have packages with recent-enough versions for autoconf, automake, libtool, and the GNU compiler toolchain.

Result:

Fixes #13667

linux-riscv64 packages are created in a similar manner to linux-aarch64 packages.

Motivation:

RISC-V is gaining traction across the industry and many projects rely on
Netty. To run these projects, Netty needs to support RISC-V.

Modifications:

Mostly build scripts and CI files. CentOS doesn't have packages with
recent-enough versions for autoconf, automake, libtool, and the GNU
compiler toolchain.

Result:

linux-riscv64 packages are created in a similar manner to linux-aarch64
packages.
@luhenry
Copy link
Contributor Author

luhenry commented Oct 27, 2023

This PR still depends on brotli4j 1.13.0 to be released with support for RISC-V [1] [2]

@luhenry
Copy link
Contributor Author

luhenry commented Oct 27, 2023

I've verified that it builds with:

  1. building and installing locally https://github.com/hyperxpro/Brotli4j/tree/e37831bdc7ff12244561df4d9e984293b71a93e5
  2. running docker compose -f docker/docker-compose.centos-7.yaml run cross-compile-riscv64-build

@luhenry luhenry marked this pull request as ready for review November 3, 2023 08:00
It's very difficult to have a recent enough stack on CentOS necessary to compile for RISC-V. Some missing dependencies are recent complete GNU toolchain, python 3.7, cmake 3.20, and more. Ubuntu 20.04 on the other hand doesn't have such issues.
@luhenry luhenry marked this pull request as draft November 8, 2023 00:14
@normanmaurer
Copy link
Member

@luhenry there are still some test failures

Copy link

@Kichura Kichura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The netty tests did not pass, You might wanna correct the code in order to pass the native loading test. (There are also 2 skipped tests aswell)

@luhenry
Copy link
Contributor Author

luhenry commented Nov 13, 2023

@Kichura @normanmaurer looking into the test failure today or tomorrow.

In terms of CI, are you ok with the run taking nearly 2 hours? The newer version of jdk 17.0.9 should be a lot faster given the backport of RISC-V (removing the need for Zero VM), but that’s not done yet in the Ubuntu repositories.

@Kichura
Copy link

Kichura commented Nov 13, 2023

@Kichura @normanmaurer looking into the test failure today or tomorrow.

In terms of CI, are you ok with the run taking nearly 2 hours? The newer version of jdk 17.0.9 should be a lot faster given the backport of RISC-V (removing the need for Zero VM), but that’s not done yet in the Ubuntu repositories.

Shouldn't be a problem, also for jdk - usually check-latest: true is a choice when aiming for newer jdks in terms of setup-jdk action involvement although unsure if that's comfortable for use in netty's workflow system.

Copy link

@Kichura Kichura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NativeLoadingEpoll test has failed.

@Kichura
Copy link

Kichura commented Nov 15, 2023

Consider this extra comment a bit off-topic but is a native ppc64le PR planned aswell or is it out of scope?

@luhenry
Copy link
Contributor Author

luhenry commented Nov 15, 2023

Consider this extra comment a bit off-topic but is a native ppc64le PR planned aswell or is it out of scope?

It's currently out of scope. I can explore it, see how much work it would be, and report back, but I want to prioritize riscv64 in this PR :)

@luhenry luhenry marked this pull request as ready for review November 22, 2023 22:40
@luhenry
Copy link
Contributor Author

luhenry commented Nov 22, 2023

@Kichura @normanmaurer it's now passing tests on CI.

Copy link

@Kichura Kichura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since tests have passed, approving this.

@normanmaurer normanmaurer added this to the 4.1.102.Final milestone Nov 27, 2023
@normanmaurer
Copy link
Member

@chrisvest any concerns ?

Copy link
Contributor

@chrisvest chrisvest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this will start making riscv64 release artifacts, we should also include those in all/pom.xml and bom/pom.xml.

docker/Dockerfile.cross_compile_riscv64 Show resolved Hide resolved
@luhenry
Copy link
Contributor Author

luhenry commented Dec 11, 2023

@chrisvest that's fixed now. Sorry I only did it today, I thought I did it 2 weeks ago and was OOO last week.

@chrisvest
Copy link
Contributor

No worries. Approved.

@chrisvest chrisvest merged commit ade1223 into netty:4.1 Dec 11, 2023
15 checks passed
@luhenry
Copy link
Contributor Author

luhenry commented Dec 11, 2023

Thank you!

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.

Add support for RISC-V
4 participants