On Sat, 18 Mar 2023 13:28:52 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>>> `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.
> 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.that out felt easier to deal with since
> we wouldn't have to think of what OS name, arch combinations would be valid.
My preference would be something like target.properties so we have one place
with the properties for target platform when cross linking.
${ModuleTarget}.${property} could work as keys. At some point we will need to
extend or replace the ModuleTarget as it too basic to describe the target
platform, that's why I was hoping to avoid parsing it here.
jlink cannot be reliably used to cross link to target a different JDK release.
Reasons include changes to the run-time image, jimage changes, and plugins that
are deeply tied to java.base or other modules. I mention this because JDK 11
was the last release to build solaris-sparcv9, so I think you can this and
several others from the properties file. Probably just stick the the builds
that are possible in the main line.
-------------
PR: https://git.openjdk.org/jdk/pull/11943
