Integrate OpenJCEPlus into Semeru OpenJDK #238
Conversation
|
It is a draft PR, please help to review, if the makefiles which we updated are correct. And in the “closed/custom/Main-pre.gmk”, I put the native codes building “openjceplus-libs” under the “java.base-copy”, same as what the zOS team did when they integrated the OpenJCEPlus building with Semeru. But, since we add a new module “openjceplus”, so I am not sure if under the “java.base-copy” is still good, or do we need another, like “openjceplus-copy” to tigger the OpenJCEPlus native codes building. Please help to review and advise. |
|
While working with Java 17 might provide a more stable development environment, changes for the head stream, and jdk21 will be needed before this can be merged here. |
|
Please address the copyright check failures. |
taoliult
force-pushed
the
OpenJCEPlus
branch
3 times, most recently
from
d8d25eb to
28b7e50
Compare
|
@keithc-ca |
taoliult
force-pushed
the
OpenJCEPlus
branch
4 times, most recently
from
d582785 to
7c24fe7
Compare
|
@keithc-ca For the code reviews, some of them updated the codes, some of them replied the questions. Please help to review and advise. |
|
@keithc-ca Please help to review and advise. Thanks. |
|
@keithc-ca |
taoliult
force-pushed
the
OpenJCEPlus
branch
5 times, most recently
from
1e0ff01 to
09f5e80
Compare
taoliult
force-pushed
the
OpenJCEPlus
branch
from
3f9b69d to
5fcb629
Compare
taoliult
force-pushed
the
OpenJCEPlus
branch
3 times, most recently
from
97cc6d5 to
ebd0ba7
Compare
|
Updated the codes in OpenJ9 PR eclipse-openj9/openj9#18544. Move the "/[IF OPENJCEPLUS_SUPPORT]/" exports to this extensions module-info.java. Run the builds on the OpenJCEPlus not supported platforms, s390x_linux and aarch64_linux, links below. The builds look good, no openjceplus module generated and no exports to unknown module error. https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/19593/ @pshipton Please help to review and advise. |
|
lgtm. @keithc-ca Depends on eclipse-openj9/openj9#18544 being merged first. |
closed/get_j9_source.sh
Outdated
| # Create OpenJCEPlus Java module folder | ||
| mkdir -p ./src/main/openjceplus/share/classes/ | ||
| cp -r ./src/main/java/* ./src/main/openjceplus/share/classes/ |
There was a problem hiding this comment.
This isn't going to be pleasant for anyone actively working on OpenJCEPlus and the integration with openjdk. One should be able to modify any source files within OpenJCEPlus without manually managing the copies this creates. This makes it a more serious issue from my perspective.
| /*[IF OPENJCEPLUS_SUPPORT]*/ | ||
| openjceplus, | ||
| /*[ENDIF] OPENJCEPLUS_SUPPORT */ |
There was a problem hiding this comment.
It should be a priority to eliminate the use of internal classes by OpenJCEPlus.
There was a problem hiding this comment.
@keithc-ca
Yes, we are working on the elimination of the use of the internal classes by OpenJCEPlus. But for the current phase, we still have to use those internal classes.
@jasonkatonica FYI.
There was a problem hiding this comment.
The team faced a decision of maintaining 100k lines of code that was a duplicate of the sun internal classes or use some of the sun internal classes. A decision was made to use the sun internal classes. We are however in the long run planning on trying to find alternates and more and more external functions that could be used as an equivalent wherever possible.
This particular package is something we certainly plan on removing as there are many alternatives to the jdk.internal.logger package. It will however not be done in this phase of development.
|
@keithc-ca Codes updated according to code review, please help to review and advice. @jasonkatonica FYI. |
taoliult
changed the title
closed/custom/Main.gmk
Outdated
| com_ibm_crypto_plus_provider_icc_NativeInterface.h) | ||
| $(MAKE) -C $(OPENJCEPLUS_TOPDIR)/src/main/native -f $(JGSKIT_MAKE) cleanAll | ||
|
|
||
| clean : openjceplus-clean |
There was a problem hiding this comment.
The target clean-openjceplus already exists; it should depend upon openjceplus-clean:
clean-openjceplus : openjceplus-clean
(In turn, clean depends on clean-openjceplus.)
There was a problem hiding this comment.
@keithc-ca
I searched the OpenJDK codes but I did not find the target clean-openjceplus you mentioned. Could you tell us where this target clean-openjceplus defined?
There was a problem hiding this comment.
|
Codes updated according to the code review, the x86 linux build looks good when building in my fyre VM. Committed the codes to run the builds on all the supported platforms. Once the builds are good, then I will update the comment for code review. |
|
Build looks good in my x86 linux fyre VM. Commit the changes to run the builds on all the supported platforms. |
|
After fixing the use of MixedPath, please squash and fix the commit message. Please only use ASCII quotes, (e.g. not “.so”) and put the "Signed-off-by:" line after a blank line at the end (not in the middle). |
Integrate the building of OpenJCEPlus with Semeru OpenJDK as a module "openjceplus". The java codes will be built with Semeru OpenJDK as a jmod, and the native codes will be built as a ".so" or ".dll" library. Signed-off-by: Tao Liu <[email protected]>
|
@keithc-ca |
There was a problem hiding this comment.
This should be used as a model for pull requests to newer versions, e.g. ibmruntimes/openj9-openjdk-jdk#710. Changes for newer versions should be merged first.
I added @pshipton as a reviewer so this and related PRs can proceed during my absence.
|
@keithc-ca Thanks so much for the review. I am working on the JDK next PR ibmruntimes/openj9-openjdk-jdk#710. Will update all the new changes from here to the JDK next PR. |
|
Jenkins builds look good for all the supported platforms. x86-64_linux: ppc64le_linux: x86-64_windows: Also run the build on OpenJCEPlus not supported platform s390x_linux, the build looks good: @jasonkatonica FYI. |
Depends on eclipse-openj9/openj9#18544 being merged first.
Integrate the building of OpenJCEPlus with Semeru OpenJDK as a module "openjceplus". The java codes will be built with Semeru OpenJDK as a jmod, and the native codes will be built as a ".so" or ".dll" library.
The "bash get_source.sh" will download the OpenJCEPlus repo and also the ICC binaries which needed when building the OpenJCEPlus native codes.
Example:
bash get_source.sh -openjceplus-repo=[OpenJCEPlus repo] -openjceplus-branch=[OpenJCEPlus branch] -gskit-bin=[ICC binary tar file] -gskit-sdk-bin=[ICC SDK binary tar file] -gskit-credential=[Credential for downloading ICC]
bash configure --with-boot-jdk=[Boot JDK] --enable-openjceplus
Signed-off-by: Tao Liu [email protected]