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

[Magnus Ihse Bursie]

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
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20180306/6af03f2d/attachment-0001.html>