| Message ID | 20171014155926.5276-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 10/14/2017 12:59 PM, James Almer wrote: > It's used by every library, and by making it global we simplify a lot > of checks. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > configure | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/configure b/configure > index 22026ef832..a0bfb269e8 100755 > --- a/configure > +++ b/configure > @@ -3332,23 +3332,17 @@ cws2fws_extralibs="zlib_extralibs" > > # libraries, in linking order > avcodec_deps="avutil" > -avcodec_suggest="libm" > avcodec_select="null_bsf" > avdevice_deps="avformat avcodec avutil" > -avdevice_suggest="libm" > avfilter_deps="avutil" > -avfilter_suggest="libm" > avformat_deps="avcodec avutil" > -avformat_suggest="libm network" > +avformat_suggest="network" > avresample_deps="avutil" > -avresample_suggest="libm" > -avutil_suggest="clock_gettime libm libdrm libmfx opencl user32 vaapi videotoolbox corefoundation corevideo coremedia wincrypt" > +avutil_suggest="clock_gettime libdrm libmfx opencl user32 vaapi videotoolbox corefoundation corevideo coremedia wincrypt" > postproc_deps="avutil gpl" > -postproc_suggest="libm" > swresample_deps="avutil" > -swresample_suggest="libm libsoxr" > +swresample_suggest="libsoxr" > swscale_deps="avutil" > -swscale_suggest="libm" > > avcodec_extralibs="pthreads_extralibs iconv_extralibs" > avfilter_extralibs="pthreads_extralibs" > @@ -5944,7 +5938,9 @@ enabled lzma && check_lib lzma lzma.h lzma_version_number -llzma > # On some systems dynamic loading requires no extra linker flags > check_lib libdl dlfcn.h "dlopen dlsym" || check_lib libdl dlfcn.h "dlopen dlsym" -ldl > > -check_lib libm math.h sin -lm > +# Add -lm to global extralibs if required. Every library uses it, and it simplifies > +# several of the external library checks below. > +check_lib libm math.h sin -lm && add_extralibs $libm_extralibs > > atan2f_args=2 > copysign_args=2 > @@ -6098,7 +6094,7 @@ enabled libx264 && { use_pkg_config libx264 x264 "stdint.h x264.h" x26 > enable libx262; } > enabled libx265 && require_pkg_config libx265 x265 x265.h x265_api_get && > require_cpp_condition x265.h "X265_BUILD >= 68" > -enabled libxavs && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs" > +enabled libxavs && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs" > enabled libxvid && require libxvid xvid.h xvid_global -lxvidcore > enabled libzimg && require_pkg_config libzimg "zimg >= 2.3.0" zimg.h zimg_get_api_version > enabled libzmq && require_pkg_config libzmq libzmq zmq.h zmq_ctx_new Ping for this and the other two patches in the thread.
On Mon, Oct 16, 2017 at 7:31 PM, James Almer <jamrial@gmail.com> wrote: > On 10/14/2017 12:59 PM, James Almer wrote: >> It's used by every library, and by making it global we simplify a lot >> of checks. >> LGTM. I dislike the fact that we have to fix issues caused by 3rd party libraries' pkg-config files but at this point the simplification for things we also utilize kind of makes sense. Jan
On 10/16/2017 2:36 PM, Jan Ekstrom wrote: > On Mon, Oct 16, 2017 at 7:31 PM, James Almer <jamrial@gmail.com> wrote: >> On 10/14/2017 12:59 PM, James Almer wrote: >>> It's used by every library, and by making it global we simplify a lot >>> of checks. >>> > > LGTM. > > I dislike the fact that we have to fix issues caused by 3rd party > libraries' pkg-config files but at this point the simplification for > things we also utilize kind of makes sense. Some failures are indeed pkg-config files not listing libm and pthreads as required, but with others we're doing custom checks, the require() ones, where we're the ones that should state what libraries to link to in static builds. This is the case with libilbc, libmysofa and apparently libmp3lame. The remaining issue after this patchset is applied is linking to the C++ standard library with some external deps. According to Jörn Heusipp, the libopenmpt maintainer, it's not something that can be safely included in a pkg-config file, and by hardcoding a -lstdc++ ldflag in configure checks we're apparently breaking detection on systems where the compiler uses libc++, like it seems to be the case with Clang on MacOS. We were hardcoding -lstdc++ to the check of some libraries even before the commit that made extralibs module-specific rather than global, and after it i had to do it with a couple more, but if that's not the correct approach then i don't what should be done. Perhaps it's something the user will have to manually deal with using --extra-libs.
On Mon, Oct 16, 2017 at 11:02 PM, James Almer <jamrial@gmail.com> wrote: > On 10/16/2017 2:36 PM, Jan Ekstrom wrote: >> On Mon, Oct 16, 2017 at 7:31 PM, James Almer <jamrial@gmail.com> wrote: >>> On 10/14/2017 12:59 PM, James Almer wrote: >>>> It's used by every library, and by making it global we simplify a lot >>>> of checks. >>>> >> >> LGTM. >> >> I dislike the fact that we have to fix issues caused by 3rd party >> libraries' pkg-config files but at this point the simplification for >> things we also utilize kind of makes sense. > > Some failures are indeed pkg-config files not listing libm and pthreads > as required, but with others we're doing custom checks, the require() > ones, where we're the ones that should state what libraries to link to > in static builds. This is the case with libilbc, libmysofa and > apparently libmp3lame. > > The remaining issue after this patchset is applied is linking to the C++ > standard library with some external deps. According to Jörn Heusipp, the > libopenmpt maintainer, it's not something that can be safely included in > a pkg-config file, and by hardcoding a -lstdc++ ldflag in configure > checks we're apparently breaking detection on systems where the compiler > uses libc++, like it seems to be the case with Clang on MacOS. > Perhaps such libraries shouldn't hardcode -lstdc++ in there, but dynamically put whichever C++ library they built against in there instead? Its not like you can actually use a static library build with another C++ library, that *may* randomly work, but its not something anyone should be doing. - Hendrik
On Tue, Oct 17, 2017 at 12:38 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > Perhaps such libraries shouldn't hardcode -lstdc++ in there, but > dynamically put whichever C++ library they built against in there > instead? > Its not like you can actually use a static library build with another > C++ library, that *may* randomly work, but its not something anyone > should be doing. > > - Hendrik Yup, The project should know which C++ runtime it is built against and put that into the .pc file's Libs.private. If the project doesn't have an idea on how to automate this, a solution like the one utilized by zimg is valid as well: * Check for an environment variable during configuration and set the default to `-lstdc++` (https://github.com/sekrit-twc/zimg/blob/ae673e02c8e934915e05daeb120c7cb1b500a0a4/configure.ac#L55..L56) * Set that value in your pkg-config template (https://github.com/sekrit-twc/zimg/blob/ae673e02c8e934915e05daeb120c7cb1b500a0a4/zimg.pc.in#L13) This way in case the value is incorrect the user can set the environment variable, and then get what is needed. Jan
2017-10-16 23:02 GMT+02:00 James Almer <jamrial@gmail.com>: > and by hardcoding a -lstdc++ ldflag in configure checks we're > apparently breaking detection on systems where the compiler > uses libc++, like it seems to be the case with Clang on MacOS. Did you test this? I ask because I was never able to reproduce (it was claimed here before). Carl Eugen
On 10/16/2017 10:04 PM, Carl Eugen Hoyos wrote: > 2017-10-16 23:02 GMT+02:00 James Almer <jamrial@gmail.com>: >> and by hardcoding a -lstdc++ ldflag in configure checks we're >> apparently breaking detection on systems where the compiler >> uses libc++, like it seems to be the case with Clang on MacOS. > > Did you test this? > I ask because I was never able to reproduce (it > was claimed here before). > > Carl Eugen No, i can't since i lack such a system. Otherwise I'd have already removed all the old hardcoded checks for -lstdc++ and never added new ones.
On 10/16/2017 11:38 PM, Hendrik Leppkes wrote: > On Mon, Oct 16, 2017 at 11:02 PM, James Almer <jamrial@gmail.com> wrote: >> The remaining issue after this patchset is applied is linking to the C++ >> standard library with some external deps. According to Jörn Heusipp, the >> libopenmpt maintainer, it's not something that can be safely included in >> a pkg-config file, and by hardcoding a -lstdc++ ldflag in configure >> checks we're apparently breaking detection on systems where the compiler >> uses libc++, like it seems to be the case with Clang on MacOS. > Perhaps such libraries shouldn't hardcode -lstdc++ in there, but > dynamically put whichever C++ library they built against in there > instead? The problem here is, that we (libopenmpt) do not actually know what the name of the standard library is. It is chosen basically by the toolchain and there is no standard way to ask it what it uses. There is also no standard autoconf check which determines it (but the more I think about the issue, the more I think there actually needs to be one). > Its not like you can actually use a static library build with another > C++ library, that *may* randomly work, but its not something anyone > should be doing. Correct, mismatching C++ standard libraries will not work. Regards, Jörn (libopenmpt maintainer)
On 10/17/2017 12:13 AM, Jan Ekstrom wrote: > On Tue, Oct 17, 2017 at 12:38 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: >> Perhaps such libraries shouldn't hardcode -lstdc++ in there, but >> dynamically put whichever C++ library they built against in there >> instead? >> Its not like you can actually use a static library build with another >> C++ library, that *may* randomly work, but its not something anyone >> should be doing. >> >> - Hendrik > > Yup, > > The project should know which C++ runtime it is built against and put > that into the .pc file's Libs.private. If the project doesn't have an > idea on how to automate this, a solution like the one utilized by zimg > is valid as well: > > * Check for an environment variable during configuration and set the > default to `-lstdc++` > (https://github.com/sekrit-twc/zimg/blob/ae673e02c8e934915e05daeb120c7cb1b500a0a4/configure.ac#L55..L56) > * Set that value in your pkg-config template > (https://github.com/sekrit-twc/zimg/blob/ae673e02c8e934915e05daeb120c7cb1b500a0a4/zimg.pc.in#L13) As already explained in another mail, I actually have no idea how to reliably determine the name of the C++ standard library. (see https://lists.freedesktop.org/archives/pkg-config/2016-August/001055.html on the pkg-config mailing list for a similar discussion). Do you happen to know of any other libraries that do exactly that (use of STL_LIBS variable)? If this is somewhat common practice I could add support for that variable to libopenmpt. If it is not, I am considering adding a configure option to libopenmpt, something like "--pkgconfig-static-cxxstdlib=-lstdc++" (and also add proper documentation about this issue with static linking). However I do not like defaulting it to some guess (libstdc++), as a wrong value here is worse than no value (additional libs can be added easily by libraries or programs that use libopenmpt, removing/ignoring wrong libs from the .pc file is harder and will cause even more trouble). For reference: https://bugs.openmpt.org/view.php?id=1045 (libopenmpt issue) Regards, Jörn (libopenmpt maintainer)
On 10/17/2017 03:04 AM, Carl Eugen Hoyos wrote: > 2017-10-16 23:02 GMT+02:00 James Almer <jamrial@gmail.com>: >> and by hardcoding a -lstdc++ ldflag in configure checks we're >> apparently breaking detection on systems where the compiler >> uses libc++, like it seems to be the case with Clang on MacOS. > > Did you test this? > I ask because I was never able to reproduce (it > was claimed here before). My FreeBSD (10.3, rather minimal install) system does not have any libstdc++. "-lstdc++" thus simply fails to link and clearly is the wrong thing to do. I cannot test on macOS. Regards, Jörn (libopenmpt maintainer)
diff --git a/configure b/configure index 22026ef832..a0bfb269e8 100755 --- a/configure +++ b/configure @@ -3332,23 +3332,17 @@ cws2fws_extralibs="zlib_extralibs" # libraries, in linking order avcodec_deps="avutil" -avcodec_suggest="libm" avcodec_select="null_bsf" avdevice_deps="avformat avcodec avutil" -avdevice_suggest="libm" avfilter_deps="avutil" -avfilter_suggest="libm" avformat_deps="avcodec avutil" -avformat_suggest="libm network" +avformat_suggest="network" avresample_deps="avutil" -avresample_suggest="libm" -avutil_suggest="clock_gettime libm libdrm libmfx opencl user32 vaapi videotoolbox corefoundation corevideo coremedia wincrypt" +avutil_suggest="clock_gettime libdrm libmfx opencl user32 vaapi videotoolbox corefoundation corevideo coremedia wincrypt" postproc_deps="avutil gpl" -postproc_suggest="libm" swresample_deps="avutil" -swresample_suggest="libm libsoxr" +swresample_suggest="libsoxr" swscale_deps="avutil" -swscale_suggest="libm" avcodec_extralibs="pthreads_extralibs iconv_extralibs" avfilter_extralibs="pthreads_extralibs" @@ -5944,7 +5938,9 @@ enabled lzma && check_lib lzma lzma.h lzma_version_number -llzma # On some systems dynamic loading requires no extra linker flags check_lib libdl dlfcn.h "dlopen dlsym" || check_lib libdl dlfcn.h "dlopen dlsym" -ldl -check_lib libm math.h sin -lm +# Add -lm to global extralibs if required. Every library uses it, and it simplifies +# several of the external library checks below. +check_lib libm math.h sin -lm && add_extralibs $libm_extralibs atan2f_args=2 copysign_args=2 @@ -6098,7 +6094,7 @@ enabled libx264 && { use_pkg_config libx264 x264 "stdint.h x264.h" x26 enable libx262; } enabled libx265 && require_pkg_config libx265 x265 x265.h x265_api_get && require_cpp_condition x265.h "X265_BUILD >= 68" -enabled libxavs && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs" +enabled libxavs && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs" enabled libxvid && require libxvid xvid.h xvid_global -lxvidcore enabled libzimg && require_pkg_config libzimg "zimg >= 2.3.0" zimg.h zimg_get_api_version enabled libzmq && require_pkg_config libzmq libzmq zmq.h zmq_ctx_new
It's used by every library, and by making it global we simplify a lot of checks. Signed-off-by: James Almer <jamrial@gmail.com> --- configure | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)