+ $(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
