RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

Fri May 8 08:20:40 UTC 2020

On 2020-05-07 23:27, Mikael Vidstedt wrote:
>
>> On May 7, 2020, at 7:52 AM, Kumar Srinivasan <kusrinivasan at vmware.com> wrote:
>>
>> Hi Mikael,
>>
>> I may have created solinux when the macosx port was merged and in an effort to reduce the CPP conditionals.
>> solinux = solaris + linux  ie. Vanilla unix code vs Darwin code IIRC has Objective-C, MacOSX specific thread initialization etc.
> Thank you for the background - it all makes sense now! :)
If the file is truly only used on linux, it should be moved to 
java.base/linux/native/libjli instead. (And be renamed.) If it's still 
used on other unix platforms (read: AIX) it should probably be renamed 
java_md_unix.h instead, and kept in place.

If this change should be done in a follow-up fix or as part of JEP 381, 
I leave to you to decide.

(edit: *actually looking at the files* ... )

Hm. The include block looks like this:
#if defined(_AIX)
#include "java_md_aix.h"
#endif

#ifdef MACOSX
#include "java_md_macosx.h"
#else  /* !MACOSX */
#include "java_md_solinux.h"
#endif /* MACOSX */
#endif /* JAVA_MD_H */

It would probably make sense to make this a three-pronged include with 
separate files for aix, macosx and linux, but that will probably require 
stuff to migrate from java_md_solinux.h to java_md_aix.h, or at the very 
least, proper testing on AIX. Recommend filing follow-up issue to sort 
this out.

/Magnus
>
>> I looked over the launcher related code looks ok to me.
>>
>> I did notice the \n endings for the messages that Brent pointed out.
> Thank you for the review! The line ending should be fixed in webrev.01.
>
> Cheers,
> Mikael
>
>>> On May 6, 2020, at 9:43 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com <mailto:mikael.vidstedt at oracle.com>> wrote:
>>>
>>>
>>> I have always wondered what “solinux” is supposed to mean - though not enough to actually ask anybody :)
>>>
>>> I’ll file a follow-up enhancement to cover renaming the files.
>>>
>>> Thank you for the review!
>>>
>>> Cheers,
>>> Mikael
>>>
>>>> On May 4, 2020, at 7:59 AM, Roger Riggs <Roger.Riggs at Oracle.com <mailto:Roger.Riggs at Oracle.com>> wrote:
>>>>
>>>> Hi Michael,
>>>>
>>>> Looks good.
>>>>
>>>> Maybe just a future cleanup to rename files, since the "...so..." is refering to solaris.
>>>>
>>>> src/java.base/unix/native/libjli/java_md_solinux.h
>>>> src/java.base/unix/native/libjli/java_md_solinux.h
>>>>
>>>> Regards, Roger
>>>>
>>>>
>>>> On 5/4/20 4:49 AM, Alan Bateman wrote:
>>>>> On 04/05/2020 06:12, Mikael Vidstedt wrote:
>>>>>> Please review this change which implements part of JEP 381:
>>>>>>
>>>>>> JBS: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8244224&data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&sdata=W5VvD1owIGoUcbcRkcwixXGPkFLjHUFof2v6cxMqchk%3D&reserved=0 <https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8244224&data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&sdata=W5VvD1owIGoUcbcRkcwixXGPkFLjHUFof2v6cxMqchk%3D&reserved=0>
>>>>>> webrev: https://nam04.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~mikael%2Fwebrevs%2F8244224%2Fwebrev.00%2Fcorelibs%2Fopen%2Fwebrev%2F&data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&sdata=tQZIjuF5cIPs%2FNqTRtY2WtmwAgwa0iv205wQ9vk0vOQ%3D&reserved=0 <https://nam04.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~mikael%2Fwebrevs%2F8244224%2Fwebrev.00%2Fcorelibs%2Fopen%2Fwebrev%2F&data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&sdata=tQZIjuF5cIPs%2FNqTRtY2WtmwAgwa0iv205wQ9vk0vOQ%3D&reserved=0>
>>>>>> JEP: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8241787&data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&sdata=rDvBE%2BJ2F1IIFC9fbA92rs0KZNgYPg0JZM2ynqp7mcs%3D&reserved=0 <https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8241787&data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&sdata=rDvBE%2BJ2F1IIFC9fbA92rs0KZNgYPg0JZM2ynqp7mcs%3D&reserved=0>
>>>>>>
>>>>>>
>>>>>> Note: When reviewing this, please be aware that this exercise was *extremely* mind-numbing, so I appreciate your help reviewing all the individual changes carefully. You may want to get that coffee cup filled up (or whatever keeps you awake)!
>>>>>>
>>>>> I took a pass over the changes. I agree its a bit tedious. I'm sure there will be a few follow up issues as there are opportunities for cleanup in several areas. Just a few comments/questions from a first pass.
>>>>>
>>>>> ExtendedSocketOption.SO_FLOW_SLA is the Solaris specific socket option that was terminally deprecated in 14. The patch removes the implementation but leave the API (SO_FLOW_SA and jdk.net.SocketFlow). Do you want a someone to take a follow-on issue to remove the API?
>>>>>
>>>>> ResolverConfigurationImpl.localDomain0 can be removed.
>>>>>
>>>>> The comment on mcast_join_leave in PlainDatagramSocketImpl.c has a residual reference to Solaris.
>>>>>
>>>>> JISAutoDetect - might be simpler to just initialize EUCJPName to "EUC_JP".
>>>>>
>>>>> Socket.setTrafficClass(int) swallows exceptions to workaround strange behaviour on Solaris. Tracked as JDK-8221487 so okay to leave it to that issue if you want. There is also cruft in the old plain SocketImpl that to work around eagerness to report "connection reset errors - I think we should just leave that because the old socket impl is not used by default and will be removed at some point.
>>>>>
>>>>> -Alan.