On Tue, Nov 3, 2015 at 6:20 PM, Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> 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. > >> >> >> 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. >
Thanks for the detailed explanation. I see the build system is on a good way :) Regarding JDKLIB_LIBS I think we should try to minimize duplication and redundancy. So if we need libjava/libjvm for all libraries anyway it would be good to have it in JDKLIB_LIBS and get rid of all the extra "-ljvm -ljava" options in every call to SetupNativeCompilation. The same holds true for "-lc" on Solaris. But for this to work, we of course need to have LIBS := $(JDKLIB_LIBS) in every call to SetupNativecompilation first. >> 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. I agree - I'm just fixing it the right way now. The downside of this will be that I'll need you as sponsor because it requires the re-generation of the configure script because of the changes in flags.m4 :) > > /Magnus >
