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

[Phil Race]

+ $(ECHO) needs to be updated for both 32 and 64 bit platforms. You have 
now needs -> need. 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 ? -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
>>>>
>>>
>>
>