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

[Magnus Ihse Bursie]

On 2015-11-04 12:04, Volker Simonis wrote:
> 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/
Looks good.
> 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?

I will. Thanks!

/Magnus

>
> 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
>>>