SapMachine #1894: Add SapMachine tools plugin to jlink #1893
Conversation
|
Hello @parttimenerd, I'm sorry, this pull request doesn't meet the expectations. The pull request description has an invalid format. Please have a look at https://github.com/SAP/SapMachine/wiki/Formal-Requirements-of-Pull-Requests . |
RealCLanger
changed the title
|
retest this please |
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddSapMachineTools.java
Outdated
Show resolved
Hide resolved
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
|
Why is CDSPluginTest used in the test ? |
|
That was a mistake, but @RealCLanger is already modifying the PR, fixing this issue. |
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
|
OK, here is my update. I made some cleanups and updated the testcase such that it would check for the SapMachine tools files in the original images in the beginning, that is whether they are expected to be there and exist or they are expected to be absent and do not exist. I also added the test to tier1 and now I would expect to see test failures on the platforms where async profiler should be included. I'll address that later after seeing the results. |
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
| } | ||
|
|
||
| // async profiler is only available on a subset of platforms | ||
| boolean shouldHaveAsync = Platform.isOSX() || |
There was a problem hiding this comment.
I would probably do
* @requires (os.family=="linux" < os.family == "mac") & !vm.musl
because we do not have s390x in SapMachine, but up to you.
There was a problem hiding this comment.
I want to intentionally run the test on all platforms. And when there are no async binaries (as expected), no errors should happen.
| @@ -287,6 +287,7 @@ Invalid language tag: %s | |||
| include-locales.localedatanotfound=\ | |||
| jdk.localedata module was not specified with --add-modules option | |||
|
|
|||
| # SapMachine 2025-09-01: SapMachine tools plugin | |||
There was a problem hiding this comment.
add-sapmachine-tools.usage=\
-\ --add-sapmachine-tools Add SapMachine specific tools to the image.
+\ --add-sapmachine-tools Add SapMachine specific tools to the image.
to get the right indent in the help output.
|
Hello @parttimenerd, this pull request fulfills all formal requirements. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add
--add-sapmachine-toolsoption to jlinkfixes #1894