RFR(S): 8141290: AIX: fix build after '8140661: Rename LDFLAGS_SUFFIX to LIBS'

[Volker Simonis]

Hi Magnus,

thanks for checking, but here comes the new (and improved :) version:

http://cr.openjdk.java.net/~simonis/webrevs/2015/8141290_toplevel/
http://cr.openjdk.java.net/~simonis/webrevs/2015/8141290_jdk/

In the toplevel change I've simply removed '-liconv' from JDKLIB_LIBS on AIX.

In the jdk change I've added '-liconv' to LIBS_aix where necessary. I
haven't removed the inserted "LIBS := $(JDKLIB_LIBS)" lines from the
first version of the change but instead cleaned up the '-lc' from the
corresponding LIBS_solaris entries and in one case I've removed
"LIBS_windows := $(JDKLIB_LIBS)" because LIBS now already contains
JDKLIB_LIBS anyway.

Tested on Solaris, AIX and MacOS X.

Could you please sponsor this fix and push the two changes
synchronously to jdk9-dev?

Thank you and best regards,
Volker


On Wed, Nov 4, 2015 at 10:01 AM, Magnus Ihse Bursie
<magnus.ihse.bursie at oracle.com> wrote:
> On 2015-11-03 18:20, Magnus Ihse Bursie wrote:
>>
>> On 2015-11-03 17:20, Volker Simonis wrote:
>>>
>>> Hi,
>>>
>>> unfortunately, change '8140661: Rename LDFLAGS_SUFFIX to LIBS' broke
>>> the AIX build.
>>
>>
>> I'm sorry for that. :-(
>>>
>>>
>>> Can somebody please review this small change which fixes the build?
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2015/8141290/
>>> https://bugs.openjdk.java.net/browse/JDK-8141290
>>
>>
>> The fix looks good to me. However, just to be sure, I've put it to a spin
>> on JPRT, so it does not break any other platforms. I'll let you know the
>> results when I have them.
>
>
> The results are in: no problems.
>
> /Magnus
>
>>
>>>
>>>
>>> Besides this immediate build fix I have some general questions/remarks
>>> regarding change '8140661: Rename LDFLAGS_SUFFIX to LIBS':
>>>
>>> First of all I think it is always good to further cleanup and simplify
>>> the build and the idea to separate linker options from library
>>> dependencies is yet another good step into this direction.
>>
>> Thanks. :)
>>>
>>>
>>> But I have some problems understanding the semantics of the new
>>> JDKLIB_LIBS variable. As far as I understand it, JDKLIB_LIBS holds the
>>> native library dependencies for the native libraries build within the
>>> jdk build (is this true?). As such I'd expect that every call to
>>> SetupNativeCompilation in jdk/make/lib/ should contain a line like
>>> "LIBS := $(JDKLIB_LIBS)". If that would have been true, we wouldn't
>>> have seen any problems on AIX.
>>>
>>> Before change 8140661, LDFLAGS_JDKLIB contained the library dependency
>>> "-liconv" and because every call to SetupNativeCompilation in
>>> jdk/make/lib/ contained a line like "LDFLAGS := $(LDFLAGS_JDKLIB)" the
>>> dependency to libiconv.so was always satisfied. Now I admit, that
>>> having "-liconv" in the LDFLAGS is probably not very elegant and it is
>>> probably also not strictly needed, because potentially not every
>>> native jdk library really depends on libiconv.so. So in general the
>>> cleanup done by 8140661 is good (if it is done right :)
>>>
>>> But until now, not every call to SetupNativeCompilation contains the
>>> line "LIBS := $(JDKLIB_LIBS)". Instead, a lot of the calls still
>>> contain lines like "LIBS_unix := -ljava" although JDKLIB_LIBS already
>>> contains "-ljava -ljvm" by default on all Unix platforms. Also, there
>>> are a lot of lines like "LIBS_solaris := -lc" although JDKLIB_LIBS
>>> already contains "-lc" by default on Solaris.
>>>
>>> So my question: are there plans to further improve the build system by
>>> adding "LIBS := $(JDKLIB_LIBS)" to all the calls to
>>> SetupNativeCompilation in jdk/make/lib/ and remove the then redundant
>>> platform specific settings?
>>
>>
>> Yes, the plan is to continue cleaning up how this is handled. Let me start
>> with some background here:
>>
>> The new build-infra based hotspot build system has a more or less working
>> prototype. The next step is to integrate this properly in the build-infra
>> family. For this to work, we need to re-think some of the assumptions in
>> build-infra, mostly relating to how we handle flags. Right now, we have not
>> really been clear on what flags are needed where, and why. For instance,
>> -shared is needed to compile shared libraries on gcc, and should really
>> belong in SetupNativeCompilation, but is now set as a part of the JDK flags.
>>
>> So the whole area of flags needs an overhaul. My general idea is to break
>> the flags into separate units, somewhat like FREETYPE_LIBS and
>> FREETYPE_CFLAGS, so you only need to add the flags you actually need for
>> every library. (Maybe increase the abstraction so you just need to declare a
>> dependency on freetype to get both FREETYPE_LIBS and FREETYPE_CFLAGS
>> automatically?) But the whole project is of the kind where you have to start
>> clean up the mess and then see where it lands.
>>
>> As for JDKLIB_LIBS, I'm not really sure of the semantics myself. :-) As
>> you say, it is included in most libs, but not all, but the contents are
>> still duplicated in some places. I think it should just go away. Or maybe be
>> split up in a "i need libjava/libjvm" (-ljava -ljvm) and "really, all
>> libraries need this" (-lc on solaris). And make sure that all libraries
>> really get the "really, all libraries need this" part.
>>
>>> I think that would be yet another nice
>>> cleanup and in that case I'd also prefer to remove '-liconv' from
>>> JDKLIB_LIBS on AIX and only add it to the required libraries, in the
>>> same way as this is done for MacOS X.
>>
>>
>> That's something I think you can do already. :-) If it's just a few
>> libraries that need -liconv, I'd suggest using LIBS_aix for those. If it's
>> like almost every library, you should probably wait a while and see where
>> the library handling ends up.
>>
>> /Magnus
>>
>