[OpenJDK 2D-Dev] RFR: JDK-8198844 Clean up GensrcX11Wrappers

[Phil Race]

It isn't "unlikely" .. it is just "relatively rare".

-phil.

On 03/05/2018 04:00 PM, Magnus Ihse Bursie wrote:
>
>
> On 2018-03-06 00:53, Phil Race wrote:
>> + $(ECHO) needs to be updated for both 32 and 64 bit platforms. You 
>> have now needs -> need.
> Fixed typo. Thanks! (I really hate how I suck at those English plural 
> s'es :-&)
>> Could we make this easier by trying to generate the 32 bit native 
>> binary as well if running on 64 bits ? Looks like this was not 
>> normally done on Linux but if you have 32 bit compiler + library 
>> support it should work. Or do you consider it easy enough to just 
>> kick off a 32 bit build to get the same ?
> Yes, I consider that easy enough, given how unlikely it is that this 
> needs ever be done.
>
> /Magnus
>
>> -phil.
>>
>> On 03/05/2018 03:30 PM, Magnus Ihse Bursie wrote:
>>>
>>> On 2018-03-05 23:38, Phil Race wrote:
>>>> >I'm not sure what role the "verification" step we had before ever 
>>>> played.
>>>> >For all the years we've been "verifying" this, we've detected no 
>>>> differences.
>>>>
>>>> I think this is useful in the event that you make some changes and
>>>> regenerate the 64 bit sizes but not 32 bit.
>>> That's a good point. I should add a warning message when 
>>> regenerating the checked-in data files that you need to regenerate 
>>> the files on both 32 and 64 bit platforms.
>>>>
>>>> For example this old bug similarly reported a breakage on Solaris ..
>>>> https://bugs.openjdk.java.net/browse/JDK-6804680
>>>>
>>>> ... back in the day when we only had that checked in and the ones 
>>>> used for Linux
>>>> were being generated on the fly. My reading of the history here is 
>>>> sizes.32 and sizes.64
>>>> were added to support cross-compilation, and the verification step 
>>>> meant that  all
>>>> architectures would get checked in some build or other.
>>>
>>> I'd rather say that it was at this time that we stopped run-time 
>>> generation of the X11 size data. The "verification" step was there 
>>> more like a comfort thing for the build team, since we was too 
>>> conservative.
>>>
>>>
>>>> The clean up of removing the solaris specific seems like it could 
>>>> have been
>>>> done a long time ago .. I am not sure why was ever only this one 
>>>> case there.
>>>> I'd have to dig back a very long way.
>>>>
>>>> I do agree we do not need to support 32 + 64 bit concurrently, that 
>>>> went
>>>> away when 64 bit solaris overlay on 32 bit was dropped.
>>>
>>> Thank you.
>>>
>>> Updated webrev with warning message about updating for all platforms:
>>>
>>> http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.03 
>>>
>>>
>>> (Only UpdateX11Wrappers.gmk has changed)
>>>
>>> /Magnus
>>>
>>>>
>>>> -phil.
>>>>
>>>> On 03/02/2018 07:23 AM, Erik Joelsson wrote:
>>>>> Adding 2d-dev in the hopes of getting some input from component 
>>>>> team. Seems like they should be aware of us removing the support 
>>>>> for multiple data models.
>>>>>
>>>>> Looks like you left a debug message at line 40 in 
>>>>> GensrcX11Wrappers.gmk.
>>>>>
>>>>> /Erik
>>>>>
>>>>> On 2018-03-02 03:00, Magnus Ihse Bursie wrote:
>>>>>> On 2018-03-02 00:02, Erik Joelsson wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> In xlibtypes.txt comment, should it be sizes-64.txt?
>>>>>>
>>>>>> Yes, good catch.
>>>>>>
>>>>>>>
>>>>>>> Generating both 32 and 64 seems a bit outdated at this point. 
>>>>>>> Surely this is a remnant of bundling 64 and 32 bit together on 
>>>>>>> Solaris in the past? Perhaps someone in 2d can answer this? 
>>>>>>> Would be nice to be able to clean up that part as well if possible.
>>>>>> Yes, you are right. We should clean up that as well. I was just 
>>>>>> too conservative. I've now actually checked what happens when you 
>>>>>> generate both 32 and 64 bit versions, and the result is that 
>>>>>> instead of code like:
>>>>>>     public static int getSize() { return 96; }
>>>>>> we get code like this:
>>>>>>     public static int getSize() { return ((XlibWrapper.dataModel 
>>>>>> == 32)?(80):(96)); }
>>>>>>
>>>>>> Since we do no longer support multiple data models for the same 
>>>>>> build, this is just unnecessary. In fact, that leads to an even 
>>>>>> better cleanup, since we will always need just a single input file.
>>>>>>
>>>>>> I also wrapped the tool calls in ExecuteWithLog.
>>>>>>
>>>>>> Updated webrev:
>>>>>> http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.02 
>>>>>>
>>>>>>
>>>>>> /Magnus
>>>>>>
>>>>>
>>>>
>>>
>>
>