GNOME Bugzilla – Bug 747954
osx: fix libcheck and "make check" builds with clang
Last modified: 2016-04-11 17:11:18 UTC
clang has -pthread option but it's only valid for the compiler. It should not be passed to the linker. Currently libcheckinternal.la contains "inherited_linker_flags='-pthread'" which make fail anything linking with with libgstcheck-1.0 when using -Werror. This is the case for "make check" builds: "error: argument unused during compilation: '-pthread'" (libcheck itself does not fail to build because it does not set -Werror so the error is just a warning in that case) $ gcc --version Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1 Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn) Target: x86_64-apple-darwin14.1.0 Thread model: posix
Created attachment 301685 [details] [review] libcheck: fix build on osx with clang
See my comment in bug #747730
Some infos: * From https://autotools.io/libtool/lafiles.html: "inherited_linker_flags: This variable lists selected link editor flags that are needed to ensure the correct ABI is used with the library, for instance. In most cases, all you'll be seeing in here is -pthread, as it's probably the only such flag used on modern systems." * In libtool's git log: commit 2f15fc7d18de0492067f4c2c782d1c8bb624194e Author: Christoph Egger <Christoph_Egger@gmx.de> Date: Wed Feb 23 03:30:30 2005 +0000 * config/ltmain.m4sh (func_mode_link): Add -pthread like flags to inherited_linker_flags. --- a/config/ltmain.m4sh +++ b/config/ltmain.m4sh @@ -2686,6 +2686,10 @@ func_mode_link () compiler_flags="$compiler_flags $arg" compile_command="$compile_command $arg" finalize_command="$finalize_command $arg" + case "$new_inherited_linker_flags " in + *" $arg "*) ;; + * ) new_inherited_linker_flags="$new_inherited_linker_flags $arg" ;; + esac continue ;; Also: http://git.savannah.gnu.org/cgit/libtool.git/tree/build-aux/ltmain.in#n5161 http://git.savannah.gnu.org/cgit/libtool.git/tree/build-aux/ltmain.in#n5691
So it's fixed in newer libtool?
(In reply to Sebastian Dröge (slomo) from comment #4) > So it's fixed in newer libtool? Nop, sorry I should have commented these infos. (In reply to Julien Isorce from comment #3) > Some infos: > > * From https://autotools.io/libtool/lafiles.html: > > "inherited_linker_flags: > This variable lists selected link editor flags that are needed to ensure the > correct ABI is used with the library, for instance. In most cases, all > you'll be seeing in here is -pthread, as it's probably the only such flag > used on modern systems." It is confusing because I do not know any linker that support -pthread. But at the time this sentence has been written maybe "modern" became "old" and even "obsolete". > > * In libtool's git log: > > commit 2f15fc7d18de0492067f4c2c782d1c8bb624194e > Author: Christoph Egger <Christoph_Egger@gmx.de> > Date: Wed Feb 23 03:30:30 2005 +0000 > > * config/ltmain.m4sh (func_mode_link): Add -pthread like flags to > inherited_linker_flags. > > --- a/config/ltmain.m4sh > +++ b/config/ltmain.m4sh > @@ -2686,6 +2686,10 @@ func_mode_link () > compiler_flags="$compiler_flags $arg" > compile_command="$compile_command $arg" > finalize_command="$finalize_command $arg" > + case "$new_inherited_linker_flags " in > + *" $arg "*) ;; > + * ) new_inherited_linker_flags="$new_inherited_linker_flags > $arg" ;; > + esac > continue > ;; Here it was to mention a feature that has been add in 2005. Maybe it was for older linkers. > > Also: > http://git.savannah.gnu.org/cgit/libtool.git/tree/build-aux/ltmain.in#n5161 > http://git.savannah.gnu.org/cgit/libtool.git/tree/build-aux/ltmain.in#n5691 Here it was to mention that this is still present 10 years after.
Well, it's a clear bug in libtool/autotools somewhere and should be fixed there. And if it's fixed there by just not doing these things when it detects clang as a compiler
I totally agree let's try to fix libtool
I had not time to look into libtool. Even if a fix is made in the future I think it will take time to get it merged and spread everywhere. So I suggest the following alternative to turn the error into a warning to prevent build failure which is annoying especially with gst-uninstalled automatic build. Add -Wno-error=unused-command-line-argument to OBJCFLAGS (or CFLAGS). Maybe we can add this in configure.ac .
(In reply to Julien Isorce from comment #8) > I had not time to look into libtool. Even if a fix is made in the future I > think it will take time to get it merged and spread everywhere. So I suggest > the following alternative to turn the error into a warning to prevent build > failure which is annoying especially with gst-uninstalled automatic build. > > Add -Wno-error=unused-command-line-argument to OBJCFLAGS (or CFLAGS). Maybe > we can add this in configure.ac . You mean s/CFLAGS/LDFLAGS/ ? I applied that that in cerbero for osx/ios builds.
Is bug #747730 a duplicate of this?
*** Bug 747730 has been marked as a duplicate of this bug. ***
I had another try and my conclusion is that this is not a libtool/autotools issue. It looks like to be a clang issue because clang behaves differently between linux and darwin. Indeed on linux the -pthread option is never forwarded to the linker. clang does not filter -pthread on darwin at link time. Then looking at clang bug tracker it seems it has been reported in 2010: https://llvm.org/bugs/show_bug.cgi?id=7798 I have added steps to reproduce the problem (add -v to clang to actually see ld invocation) Also I have added a link to the driver file that needs to be patched. But in order to support all clang versions on darwin I suggest we add everywhere -Qunused-arguments to LDFLAGS when compiling gstreamer on darwin. (The difference with -Wno-error=unused-command-line-argument is that it turns it back to a warning whereas -Qunused-arguments make it invisible)
Created attachment 306963 [details] [review] configure.ac: add Qunused-arguments to LDFLAGS on darwin
ping ?
I met compile error on GStreamer 1.7.0 (GIT) "error: argument unused during compilation: '-pthread'" Now compile error is cleared with this patch. Thanks.
(In reply to Steve Jonghee Yun from comment #15) > I met compile error on GStreamer 1.7.0 (GIT) > "error: argument unused during compilation: '-pthread'" > > Now compile error is cleared with this patch. > > Thanks. Great! Anyone against attached patch ? Explanations in comment #12
Comment on attachment 306963 [details] [review] configure.ac: add Qunused-arguments to LDFLAGS on darwin Let's get it in then but add a comment there why it is added... also maybe this should instead be inside AX_PTHREAD() directly instead of having to duplicate it in every configure.ac?
(In reply to Sebastian Dröge (slomo) from comment #17) > Comment on attachment 306963 [details] [review] [review] > configure.ac: add Qunused-arguments to LDFLAGS on darwin > > Let's get it in then but add a comment there why it is added... also maybe > this should instead be inside AX_PTHREAD() directly instead of having to > duplicate it in every configure.ac? Sorry for the delay I sent email to Daniel Richard G. who maintains ax_pthread.m4. First of all you are right in worst scenario we should have a similar change locally in our http://cgit.freedesktop.org/gstreamer/common/tree/m4/ax_pthread.m4 But Daniel told me that he will have a look next month. It seems he has other pending changes to do in ax_pthread.m4. So I suggest we wait a bit more for now. Though if someone has an idea where to properly put LDFLAGS="$LDFLAGS -Qunused-arguments" (or something equivalent) in http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_pthread.m4 I can try a such patch. So that we will save some time to Daniel I suppose. (He will still have to accept that patch, no guaranties so far)
Best to ask Daniel I guess, not sure where to best add that. Question is also when exactly to add it. Best might be to check if the compiler gives an error with -Werror and -pthread, and if so use -Qunused-arguments (if compilation succeeds with that). Something along the lines of the "if compiler needs -Werror to reject unknown flags" check.
Created attachment 320853 [details] [review] Diff with future ax_pthreads.m4 Daniel made a new version ax_pthreads.m4. Still a draft but should be released soon enough. See http://savannah.gnu.org/patch/?8186#comment21 (ax_pthread-draft3.m4). Once it is ready I can push it to gstreamer/common repo.
Created attachment 320854 [details] [review] tests: add PTHREAD_CFLAGS for make check to pass on OS X grep -rin --include \*.am PTHREAD_CFLAGS * libs/gst/check/libcheck/Makefile.am:56:libcheckinternal_la_CFLAGS += $(PTHREAD_CFLAGS) -D_GNU_SOURCE -DHAVE_PTHREAD tests/check/Makefile.am:198:AM_CFLAGS = $(PTHREAD_CFLAGS) $(GST_OBJ_CFLAGS) -UG_DISABLE_ASSERT -UG_DISABLE_CAST_CHECKS \ tests/examples/manual/Makefile.am:136:testrtpool_CFLAGS = $(GST_OBJ_CFLAGS) $(PTHREAD_CFLAGS) tests/examples/streams/Makefile.am:12:rtpool_test_CFLAGS = $(GST_OBJ_CFLAGS) $(PTHREAD_CFLAGS)
Feel free to merge this already before it is accepted there if it works on the platforms we care about.
(In reply to Sebastian Dröge (slomo) from comment #22) > Feel free to merge this already before it is accepted there if it works on > the platforms we care about. I was waiting that it lands, and actually there were regression on FreeBSD with draft3. It is now fixed. The new ax_pthreads.m4 is now merged in the autoconf repo: http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=history;f=m4/ax_pthread.m4 and the issue is closed: https://savannah.gnu.org/patch/?8186. So I think it is safe to use it now. Since 1.8 RC has been released few days ago, is it still possible to update https://cgit.freedesktop.org/gstreamer/common/tree/m4/ax_pthread.m4 with that new version ? If yes, I just need to push to "common" and it will propagate new sha to other repo ? Also can I push attached patch ?
After 1.8.0 now :)
Official ax_pthread.m4 has been updated on March 20th (http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=history;f=m4/ax_pthread.m4) and today I tested that new version which also contains the fix for osx (like previous version). It works fine on my setup. Can I push it to "common" ?
Yes please
commit ac2f647695e7bd4b433ea108ee1d0e23901797d4 Author: Julien Isorce <j.isorce@samsung.com> Date: Mon Apr 11 17:51:26 2016 +0100 m4: update ax_pthread.m4 to latest release to fix '-pthread' arguments unused errors on osx. On osx, make check in gstreamer directory currently outputs: "error: argument unused during compilation: '-pthread'". This new ax_pthread.m4 adds -Qunused-arguments when needed. Source: http://www.gnu.org/software/autoconf-archive/ax_pthread.html History: http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=history;f=m4/ax_pthread.m4 https://bugzilla.gnome.org/show_bug.cgi?id=747954
Comment on attachment 320854 [details] [review] tests: add PTHREAD_CFLAGS for make check to pass on OS X commit 7d0abcc0326d2ad949fb83a3c3f0023b43ce3715 Author: Julien Isorce <j.isorce@samsung.com> Date: Thu Feb 11 09:33:28 2016 +0100 tests: add PTHREAD_CFLAGS for make check to pass on OS X Currently "make check" fails with: "error: argument unused during compilation: '-pthread'" PTHREAD_CFLAGS now contains -Qunused-arguments to fix that. Explanation here: http://savannah.gnu.org/patch/?8186#comment21 https://bugzilla.gnome.org/show_bug.cgi?id=747954