RFR: JDK-8142907 Integration of minor fixes from the build-infra project

[Erik Joelsson]

Looks good to me.

/Erik

On 2015-12-11 23:24, Magnus Ihse Bursie wrote:
> On 2015-12-11 22:43, Magnus Ihse Bursie wrote:
>> Hi David,
>>
>> Resurrecting the second part of the build-infra integrations. :) 
>> Fortunately, no major code changes in the meantime has affected this 
>> patch. (I did need to update the new 
>> hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.)
>
> It seems I forgot to include the link to the new webrev:
>
> http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05 
>
>
> /Magnus
>
>>
>>
>> On 2015-11-16 05:33, David Holmes wrote:
>>> Hi Magnus,
>>>
>>> I had a flick through most of the files. Overall seems okay but I 
>>> have a few queries below.
>>
>> Replies inline.
>>
>>>
>>> On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote:
>>>> The build-infra project has collected a number of minor fixes and
>>>> changes during the new hotspot build development. It's a mix of 
>>>> code
>>>> cleanup and new capabilities.
>>>>
>>>> Not all of these new features are immediately beneficial to the 
>>>> JDK, but
>>>> they will be needed for the upcoming new Hotspot build, and it will 
>>>> not
>>>> hurt to have them in mainline. (In fact, it will tremendously help
>>>> merging between mainline and build-infra.)
>>>>
>>>> The fix addresses these issues:
>>>>
>>>> In general:
>>>> * Break out hotspot configuration into hotspot.m4
>>>> * Long link lines uses @-files
>>>> * Consistently use -Wl instead of -Xlinker
>>>> * Improve clang on linux compilation
>>>> * Set shared library name explicitely on solaris
>>>> * Set correct shared library flag on Windows (-dll)
>>>> * Consistency fixes for build toolchain
>>>> * Bring compare script up to date
>>>> * General code/whitespace cleanup
>>>> * Additional functionality in MakeBase
>>>>
>>>> In NativeCompilation.gmk:
>>>> * More efficient vardeps for per-file CFLAGS
>>>> * Fewer shell executions (means better performance on Windows)
>>>> * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES
>>>> * Debug symbols on macosx (disabled for existing code to keep current
>>>> behavior)
>>>>
>>>> Enabling debug info on macosx on existing jdk should be treated in a
>>>> follow-up bug.
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8142907
>>>> WebRev:
>>>> http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 
>>>>
>>>>
>>>>
>>>> (It turned out that WebRev could not at the same time include files 
>>>> from
>>>> multiple repos and track the history of a "hg cp":ied file. So I 
>>>> created
>>>> an alternative revision here:
>>>> http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ 
>>>>
>>>>
>>>>
>>>> It does not include the jdk files, but hotspot.m4 might be easier to
>>>> understand)
>>>
>>> flags.m4:
>>>
>>>   60   AC_SUBST(LEGACY_EXTRA_CFLAGS)
>>>   61   AC_SUBST(LEGACY_EXTRA_CXXFLAGS)
>>>   62   AC_SUBST(LEGACY_EXTRA_LDFLAGS)
>>>   63
>>>   64   AC_SUBST(EXTRA_CFLAGS)
>>>   65   AC_SUBST(EXTRA_CXXFLAGS)
>>>   66   AC_SUBST(EXTRA_LDFLAGS)
>>>
>>> IIRC we added the legacy flags purely to pass the cross-compilation 
>>> args through to hotspot. Not sure why we need both legacy and 
>>> non-legancy variants now ??
>>
>> This is part of an ongoing effort to split CFLAGS_JDK into more 
>> reasonable parts. The current model is that we throw all possible 
>> flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et 
>> al). While it was questionable even before, it did work when we only 
>> built the JDK native libraries. Now that we need to build libjvm.so 
>> as well, which has (for historical reason) a set of flags that are 
>> both partially the same and partially different, this is not so good.
>>
>> We would like to see a change to a situation where the different 
>> "parts" that we build CFLAGS_JDK from would be handled separately, 
>> and then combined as needed for JDK libraries, and for Hotspot. So, 
>> currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and 
>> then drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to 
>> add them to the Hotspot flags. Of course, we could have used 
>> LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new 
>> stuff.
>>
>> With that being said, I now realize that maybe this change will not 
>> be needed anyway, at least not right now. Since doing a proper flag 
>> cleanup is a major task, we decided to copy the CFLAGS_JDK behavior 
>> for Hotspot flags as well in the current implementation of the new 
>> hotspot build, and saving the cleanup for another day (so it will not 
>> block the new build system). So I'll revert this part of the change 
>> for now.
>>
>>> On Windows -LD is a superset of -dll, so it isn't obvious the change 
>>> is correct.
>> I hope Erik's reply to that was satisfactory.
>>>
>>> ---
>>>
>>> jdk/make/lib/LibCommon.gmk
>>>
>>> + # Disable it here for the jdk binaries until we decide to enable 
>>> them.
>>>
>>> s/binaries/libraries/ ?
>> Sure, I'll fix.
>>
>>> Actually both this fragment and the one in 
>>> jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the 
>>> relation with hotspot here, and the role of SetupNativeCompilation?
>> I hope Erik's reply to that is acceptable as well. As for the other 
>> changes in the jdk repo, most of them is to either standardize on 
>> -Wl, or to fix some places were we didn't do a proper $$ (instead of 
>> $) in an macro that was supposed to be eval'd. (This was incorrect 
>> before as well, but it broke the build when the LDFLAGS started 
>> having commas in them.)
>>
>> /Magnus
>>
>>>
>>> Thanks,
>>> David
>>>
>>>
>>>> /Magnus
>>
>