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

Fri Dec 11 22:24:09 UTC 2015

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
>