Re: OS checks with configuration time DLOPEN_NEEDS_LIBDL

Le 22/09/15 18:51, Norbert Thiebaud a écrit :
> On Tue, Sep 22, 2015 at 6:31 AM, Richard PALO <richard@netbsd.org> wrote:
> > Since GUIBASE was replaced with the equivalent OS checks, there seems
> > to be a bit of abandon in keeping multiple OSes up to snuff.
> >
> > in https://gerrit.libreoffice.org/#/c/18591/  I took our base in pkgsrc and
> > given the biggest difference in unix systems is the need for -ldl or not for dlopen,
> >
> > I added a configuration check for DLOPEN_NEEDS_LIBDL and updated a number of the existing
> > OS checks to check simultaneously for, among others, %BSD and SOLARIS in addition to LINUX.
> >
> > It would be nice to get some review on this approach.
>
> Jenkins got us a first review. the patch break on windows because it seems that
> AC_SEARCH_LIBS([dlopen],
> trigger a search of gcc...
>
> is there any chance that is relevant for windows ? if not the 'if
> test' that out of configure ofr cygwin case (and prolly for macosx
> case as at bes it will work , but it can pick up accidentally some
> crap if the mac has some extra stuff installed)
>
> that being said couldn't this be done without putting all that logic
> in the 'client' makefile.
>
> and just have $(if $(DLOPEN_NEEDS_LIBDL), -ldl) logic in gbuild and
> define a gb_LIBDL defined to -ldl or empty (most likely in
> RepositoryExternal.mk or in platform/* not sure on top of my head
> which is the cleanest.
>
> and then have for example
>
> $(eval $(call gb_Executable_add_libs,gengal,\
>     $(gb_LIBDL) \
>     -lpthread \
>
> (and yes that could be extended to the few place that do the same kind
> of exercise with lpthread... and there is maybe an even better way but
> one step at the time...)
>
>
> also you may want to split the part of the patch that touch
> Repositoty.mk as that seems somewhat orthogonal with the -ldl stuff
>
> Norbert

Well, personally I'm of the opinion that configure is the typical place for this type
of check, in order to leave more pertinent things in platform...

I added an `if test $_OS != "WINNT"` guard, I believe that should be ok.
As far as -lpthread, I guess I agree...  I can add that to this now as well,
or it could be a follow up patch.

gerrit updated, if it's possible to launch a jenkins run.

Also, I wonder if it is worthwhile to already fix all (or most of) the cases :
> richard@omnis:/home/richard/src/libreoffice$ git grep -w '\-ldl\>' -- '*.mk' |grep -v DLOPEN_NEED
> bridges/Library_cpp_uno.mk:   -ldl \
> chart2/Library_chartcore.mk:  -ldl \
> chart2/Library_chartopengl.mk:        -ldl \
> clew/Library_clew.mk: -ldl \
> connectivity/Library_postgresql-sdbc-impl.mk: $(if $(filter-out MACOSX,$(OS)),-ldl) \
> extensions/Executable_pluginapp.bin.mk:       -ldl \
> extensions/Executable_pluginapp.bin.mk:       -ldl \
> extensions/Library_scn.mk:    -ldl \
> external/clucene/Library_clucene.mk:  -ldl \
> external/libxslt/ExternalProject_xslt.mk:                     $(if $(SYSBASE),$(if $(filter 
> SOLARIS LINUX,$(OS)),-L$(SYSBASE)/lib -L$(SYSBASE)/usr/lib -lpthread -ldl))" \
> external/mysqlcppconn/Library_mysqlcppconn.mk:        $(if $(filter-out MACOSX,$(OS)),-ldl) \
> external/neon/Library_neon.mk:        -ldl \
> external/redland/ExternalProject_raptor.mk:                   $(if $(SYSBASE),$(if $(filter LINUX 
> SOLARIS,$(OS)),-L$(SYSBASE)/lib -L$(SYSBASE)/usr/lib -lpthread -ldl))" \
> external/redland/ExternalProject_rasqal.mk:                   $(if $(SYSBASE),$(if $(filter LINUX 
> SOLARIS,$(OS)),-L$(SYSBASE)/lib -L$(SYSBASE)/usr/lib -lpthread -ldl))" \
> external/redland/ExternalProject_redland.mk:                  $(if $(SYSBASE),$(if $(filter LINUX 
> SOLARIS,$(OS)),-L$(SYSBASE)/lib -L$(SYSBASE)/usr/lib -lpthread -ldl))" \
> libreofficekit/CppunitTest_libreofficekit_tiledrendering.mk:    -ldl \
> odk/Executable_unoapploader.mk:    $(if $(filter-out FREEBSD NETBSD DRAGONFLY,$(OS)),-ldl) \
> pyuno/Library_pyuno_wrapper.mk:       -ldl \
> sal/Library_sal.mk:           -ldl \
> setup_native/Library_getuid.mk:       -ldl \
> smoketest/Executable_libtest.mk:      -ldl \
> ucb/Library_ucpdav1.mk:       -ldl \
> vcl/Executable_xid_fullscreen_on_all_monitors.mk:    -ldl \
> vcl/Library_vclplug_gen.mk:   -ldl \
> vcl/Library_vclplug_gtk.mk:   -ldl \
> vcl/Library_vclplug_gtk3.mk:  -ldl \
> vcl/Library_vclplug_kde4.mk:  -ldl \
> vcl/Library_vclplug_tde.mk:   -ldl \
> vcl/StaticLibrary_glxtest.mk: -ldl \
> xmlsecurity/Library_xsec_xmlsec.mk:   -ldl \

-- 
Richard PALO