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

[Erik Joelsson]

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.
>
> 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 actually looks weird to me too. I can find no usage of EXTRA_CFLAGS 
without the LEGACY_ prefix.
>
> On Windows -LD is a superset of -dll, so it isn't obvious the change 
> is correct.
>
It seems, /LD is a compiler option and /DLL is a linker option. Setting 
/LD to cl.exe makes it set /DLL to link.exe. Since we link using 
link.exe we must use -dll. Looking closer at the change, we are actually 
setting SHARED_LIBRARY_FLAGS=-DL on windows, but then never using that 
variable. Instead hard coding -dll further down. This change corrects 
SHARED_LIBRARY_FLAGS to -dll and then uses that value.
> ---
>
> jdk/make/lib/LibCommon.gmk
>
> + # Disable it here for the jdk binaries until we decide to enable them.
>
> s/binaries/libraries/ ?
>
> 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?
>
In SetupNativeCompilation, we currently disable debug symbols creation 
on Macosx. Back in JDK 8 when we converted the jdk build to build-infra, 
that was how the old build worked and we just mimicked. Until now, we 
haven't gotten around to fixing debug symbols support for macosx. In the 
build-infra forest, since debug symbols are needed for hotspot, we have 
implemented support in SetupNativeCompilation. Magnus intends to bring 
this support into JDK 9 with this change, the motivation being to reduce 
differences between build-infra and JDK 9 which are complicating 
merging. We think actually enabling debug symbols support for the JDK 
libraries should rather be a separate change, that we want to follow up 
with soon after.

/Erik
> Thanks,
> David
>
>
>> /Magnus