On 2016-02-11 08:35, Jesper Wilhelmsson wrote:
Den 11/2/16 kl. 03:07, skrev David Holmes:
Jesper,
Magnus is rewriting all of the hotspot build system. Are these
cleanups really
worthwhile at this stage?
The cleanups are worthwhile to me since I work on mostly Mac and port
all my changes over to the linux makefiles in bulk, and without these
cleanups my patches won't apply cleanly. The reason I want to push
them now even though it is a while left until the GTest stuff is done,
is that every time anyone makes a change in the makefiles (which
happens more often than I had expected) I get merge conflicts everywhere.
If you want to make whitespace change, and/or structural changes that do
not affect the behavior (and you can swear honest-to-god that it does
not do any such thing), I have no objection. It is perhaps not the best
spent time to clean up the old makefiles, but if you've already done
it, what the heck.
However, I'm wary of *actual* changes.
First, I'm afraid that any real change may have unintended consequences.
I saw your and Kim's discussion about -Wconversion. I didn't follow it
entirely, but I know that we have previously actively *disabled*
-Wconversion since it lead to problems. Just haphazardly enabling it
again sounds like a bad idea.
Second, any real changes pushed to the old hotspot make system must be
re-implemented by me in the new hotspot build (until the point comes
where it is merged into mainline). If they are hidden in the midst of a
sea of whitespace changes (and even worse, if they are done
unintentionally), that's a hopelessly time-consuming and in essence
unneccessary work for me.
So, I will not reject your patch, but I do require that you at least
separate whitespace (and other structural but non-functional) changes
into a separate fix. If you have any *real* changes, these must be
tested thoroughly on all relevant platforms and compilers.
/Magnus
/Jesper
David
On 11/02/2016 8:10 AM, Jesper Wilhelmsson wrote:
Sending again to include the build-dev list.
/Jesper
Den 10/2/16 kl. 21:31, skrev Jesper Wilhelmsson:
Hi,
Please review this cleanup of the Hotspot makefiles.
Since I have been spending some time in the makefiles lately there
were a few
random cleanups that I couldn't stop myself from doing. Most of these
are made
to make the linux and bsd makefiles more alike. This has helped a lot
when
porting the framework to the different platforms.
There are a couple of preparing alignment changes that I included
in this
cleanup to make the Google test patch easier to review later.
There are also a couple of "real" changes:
* In make/bsd/makefiles/buildtree.make we set up OS_VENDOR with the
motivation
that we don't include defs.make. Three lines below we include
defs.make.
* In make/bsd/makefiles/buildtree.make the 'install' target depends on
'install_jsigs'. There is no rule called 'install_jsigs', it is called
'install_jsig'.
Another difference that I find interesting but that I have not changed
in this
patch (I can do that if requested) is that in the bsd version of
fastdebug.make
VERSION is set to "fastdebug" but in the linux version it is set to
"optimized".
Given the name of the makefile fastdebug seems more correct, but
whichever is
the correct value, shouldn't they be the same on linux and bsd?
https://bugs.openjdk.java.net/browse/JDK-8149594
http://cr.openjdk.java.net/~jwilhelm/8149594/webrev.00/
Thanks,
/Jesper
