On Tue, 14 Mar 2023 08:30:11 GMT, Alan Bateman <[email protected]> wrote:
>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 953:
>>
>>> 951: // and supported for creating an image through jlink. Else
>>> returns null.
>>> 952: private static ByteOrder
>>> getNativeEndianOfTargetPlatform(String targetPlatform) {
>>> 953: int index = targetPlatform.indexOf("-"); // of the form
>>> <operating system>-<arch>
>>
>> `Platform::parsePlatform` is the utility method to parse `ModuleTarget`.
>> It can be updated to include additional architectures.
>
>> `Platform::parsePlatform` is the utility method to parse `ModuleTarget`. It
>> can be updated to include additional architectures.
>
> Alternatively, don't parse it. If we go with Jim's suggestion of a resource
> file then it is just a simple mapping of the ModuleTarget value, e.g the
> entry for macox-x64 would be:
>
> macos-amd64.endianness = little
Hello Mandy, Alan, Jim,
I've updated this PR to take into account these suggestions. I went ahead with
what Mandy suggested and enhanced the existing (internal)
`jdk.tools.jlink.internal.Platform` `record`to additional parse these
architectures. It was anyway doing it for a select few.
In addition to that, I also moved the architecture to endianness mapping into
this `Platform` class itself. I followed Jim's suggestion to move the mapping
out of the code. The `Platform` code now reads this from a
`endianness.properties` file which is an internal resource of the `jdk.jlink`
module. However, I'm having second thoughts about this part. I'm guessing that
when Jim suggested moving this to a resource, it was probably because the code
resided within the `JLinkTask` class. Now that I've moved this to an existing
`Platform` class which is solely meant for things like these, I feel that we
could probably just hard code these architecture to endianness mapping within
this class itself, instead of reading it from a properties file. I'm looking
for some inputs on what you all think would be the best way to proceed.
Alan, as for the `osname-arch=little/big` vs `arch=little/big` mapping style, I
decided to use the `arch=little/big` and let the Platform class parse the
architecture out of the string in the `ModuleTarget`. I did it for a couple of
reasons:
1. The `Platform` class already has the necessary logic to do that, plus it has
to do that anyway (for us to implement the suggestion that Mandy noted about
using this in `DefaultImageBuilder`)
2. As far as I could see, the OS name shouldn't play any role in the
endianness, so leaving that out felt easier to deal with since we wouldn't have
to think of what OS name, arch combinations would be valid.
If you or others feel that it's better to stick with the
`osname-arch=little/big` approach, instead of this one, please do let me know
and I'll go ahead and do that change.
-------------
PR: https://git.openjdk.org/jdk/pull/11943
