RFR: 8149594 - Clean up Hotspot makefiles

[Magnus Ihse Bursie]

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