RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v13]

Magnus Ihse Bursie ihse at openjdk.org
Wed Oct 30 11:08:18 UTC 2024


On Tue, 29 Oct 2024 23:48:22 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix 32/64-bit confusion in comment in VirtualMachineImpl.c
>
> make/scripts/compare.sh line 79:
> 
>> 77: 
>> 78: if [ "$OPENJDK_TARGET_OS" = "windows" ]; then
>> 79:   DIS_DIFF_FILTER="$SED -r \
> 
> This is now being defined for windows-aarch64 too, when it previously wasn't.  Is that intentional?

No, it was not intentional, as in I forgot about the aarch64 version of Windows.

With that said, I think it still might make sense to keep it this way. I don't think anyone has ever tried running the compare script on windows-aarch64; if they had, the lack of a filter at all would have made it basically unusable. This pattern is trying to hide 64-bit hex strings, and it is reasonable to assume it will work for aarch64 as well. If it doesn't, then its better to use this as a starting point for tweaking. Good catch, though!

> make/scripts/compare.sh line 1457:
> 
>> 1455:         THIS_SEC_BIN="$THIS_SEC_DIR/sec-bin.zip"
>> 1456:         if [ "$OPENJDK_TARGET_OS" = "windows" ]; then
>> 1457:             JGSS_WINDOWS_BIN="jgss-windows-x64-bin.zip"
> 
> This is now being defined for windows-aarch64 too, when it previously wasn't.  That seems wrong,
> given the "x64" suffix.

Well... this was broken on windows-aarch64 before, too, since then it would have looked for `jgss-windows-i586-bin.zip`. 

I'm going to leave this as it is. Obviously there is a lot more work needed to get the compare script running on windows-aarch64, and I seriously doubt anyone care about that platform enough to spend that time (Microsoft themselves seems to have all but abandoned the windows-aarch64 port...).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1822382796
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1822386222


More information about the nio-dev mailing list