configure: fix libxavs check

Message ID 20171012040257.6560-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Oct. 12, 2017, 4:02 a.m.
libxavs may require pthreads and libm at link time, and without
said ldflags available as global extralibs, the check will fail.

Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63

Signed-off-by: James Almer <jamrial@gmail.com>
---
I tried replacing the require() check with a require_pkg_config()
one, and while it included the pthreads ldflag when libxavs was
compiled with pthreads support, it didn't include the libm one.
Considering the project seems dead, trying to get the installed
pkg-config file fixed is probably futile.

 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Luca Barbato Oct. 12, 2017, 1:11 p.m. | #1
On 12/10/2017 06:02, James Almer wrote:
> libxavs may require pthreads and libm at link time, and without
> said ldflags available as global extralibs, the check will fail.
> 
> Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> I tried replacing the require() check with a require_pkg_config()
> one, and while it included the pthreads ldflag when libxavs was
> compiled with pthreads support, it didn't include the libm one.
> Considering the project seems dead, trying to get the installed
> pkg-config file fixed is probably futile.
> 
>   configure | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 960c1ed032..77d71578cc 100755
> --- a/configure
> +++ b/configure
> @@ -4781,7 +4781,7 @@ enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
>                                  enable libx262; }
>   enabled libx265           && require_pkg_config libx265 x265 x265.h x265_api_get &&
>                                require_cpp_condition x265.h "X265_BUILD >= 57"
> -enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
> +enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
>   enabled libxvid           && require libxvid xvid.h xvid_global -lxvidcore
>   enabled mmal              && { check_lib mmal interface/mmal/mmal.h mmal_port_connect -lmmal_core -lmmal_util -lmmal_vc_client -lbcm_host ||
>                                  { ! enabled cross_compile &&
> 

Sounds fine.
Diego Biurrun Oct. 12, 2017, 5:17 p.m. | #2
On Thu, Oct 12, 2017 at 01:02:57AM -0300, James Almer wrote:
> libxavs may require pthreads and libm at link time, and without
> said ldflags available as global extralibs, the check will fail.
> 
> Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63
> ---
> I tried replacing the require() check with a require_pkg_config()
> one, and while it included the pthreads ldflag when libxavs was
> compiled with pthreads support, it didn't include the libm one.
> Considering the project seems dead, trying to get the installed
> pkg-config file fixed is probably futile.
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/configure
> +++ b/configure
> @@ -4781,7 +4781,7 @@ enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
> -enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
> +enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"

Hmmmmm....

I don't like this. It's clearly a bug in libxavs that you are just working
around here. It should be fixed at the source. Even if libxavs itself may
be unfixable, it's certainly not unforkable.

What problem do you have exactly? libxavs certainly works on my FATE instance
that checks all external libraries.

Diego
James Almer Oct. 12, 2017, 5:36 p.m. | #3
On 10/12/2017 2:17 PM, Diego Biurrun wrote:
> On Thu, Oct 12, 2017 at 01:02:57AM -0300, James Almer wrote:
>> libxavs may require pthreads and libm at link time, and without
>> said ldflags available as global extralibs, the check will fail.
>>
>> Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63
>> ---
>> I tried replacing the require() check with a require_pkg_config()
>> one, and while it included the pthreads ldflag when libxavs was
>> compiled with pthreads support, it didn't include the libm one.
>> Considering the project seems dead, trying to get the installed
>> pkg-config file fixed is probably futile.
>>  configure | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- a/configure
>> +++ b/configure
>> @@ -4781,7 +4781,7 @@ enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
>> -enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
>> +enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
> 
> Hmmmmm....
> 
> I don't like this. It's clearly a bug in libxavs that you are just working
> around here. It should be fixed at the source. Even if libxavs itself may
> be unfixable, it's certainly not unforkable.

Would anyone even bother forking and maintaining this thing?

> 
> What problem do you have exactly? libxavs certainly works on my FATE instance
> that checks all external libraries.

I compiled libxavs with default settings, installed it, then configure
--enable-libxavs failed at the libxavs check.

config.log showed a lot of missing references to pthreads and math
functions when trying to link the test using only -lxavs. As i said, the
project's .pc file includes a pthreads ldflag (-lpthreads in my case)
but not -lm when it's clearly needed (i know some systems don't), so
switching to require_pkg_config() is evidently not enough.

> 
> Diego
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
>
wm4 Oct. 12, 2017, 5:41 p.m. | #4
On Thu, 12 Oct 2017 19:17:32 +0200
Diego Biurrun <diego@biurrun.de> wrote:

> On Thu, Oct 12, 2017 at 01:02:57AM -0300, James Almer wrote:
> > libxavs may require pthreads and libm at link time, and without
> > said ldflags available as global extralibs, the check will fail.
> > 
> > Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63
> > ---
> > I tried replacing the require() check with a require_pkg_config()
> > one, and while it included the pthreads ldflag when libxavs was
> > compiled with pthreads support, it didn't include the libm one.
> > Considering the project seems dead, trying to get the installed
> > pkg-config file fixed is probably futile.
> >  configure | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > --- a/configure
> > +++ b/configure
> > @@ -4781,7 +4781,7 @@ enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
> > -enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
> > +enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"  
> 
> Hmmmmm....
> 
> I don't like this. It's clearly a bug in libxavs that you are just working
> around here. It should be fixed at the source. Even if libxavs itself may
> be unfixable, it's certainly not unforkable.

Forking would imply acquiring ownership, which I'm sure nobody wants to
do. You'd be better off arguing dropping libaxavs - but for such a small
reason?
Diego Biurrun Oct. 12, 2017, 6:25 p.m. | #5
On Thu, Oct 12, 2017 at 02:36:59PM -0300, James Almer wrote:
> On 10/12/2017 2:17 PM, Diego Biurrun wrote:
> > On Thu, Oct 12, 2017 at 01:02:57AM -0300, James Almer wrote:
> >> libxavs may require pthreads and libm at link time, and without
> >> said ldflags available as global extralibs, the check will fail.
> >>
> >> Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63
> >> ---
> >> I tried replacing the require() check with a require_pkg_config()
> >> one, and while it included the pthreads ldflag when libxavs was
> >> compiled with pthreads support, it didn't include the libm one.
> >> Considering the project seems dead, trying to get the installed
> >> pkg-config file fixed is probably futile.
> >>  configure | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> --- a/configure
> >> +++ b/configure
> >> @@ -4781,7 +4781,7 @@ enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
> >> -enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
> >> +enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
> > 
> > Hmmmmm....
> > 
> > I don't like this. It's clearly a bug in libxavs that you are just working
> > around here. It should be fixed at the source. Even if libxavs itself may
> > be unfixable, it's certainly not unforkable.
> 
> Would anyone even bother forking and maintaining this thing?

Maintaining is a big word. Fix the pkg-config file and put on Github; or on
sourceforge, where dead projects belong. ;)

That would benefit a few more people than this local hack.

> > What problem do you have exactly? libxavs certainly works on my FATE instance
> > that checks all external libraries.
> 
> I compiled libxavs with default settings, installed it, then configure
> --enable-libxavs failed at the libxavs check.
> 
> config.log showed a lot of missing references to pthreads and math
> functions when trying to link the test using only -lxavs. As i said, the
> project's .pc file includes a pthreads ldflag (-lpthreads in my case)
> but not -lm when it's clearly needed (i know some systems don't), so
> switching to require_pkg_config() is evidently not enough.

I remember having some trouble to even get it to compile when I set up
that FATE instance. Maybe I also worked around the missing -lm issue
locally in some way.

Diego
Diego Biurrun Oct. 12, 2017, 6:27 p.m. | #6
On Thu, Oct 12, 2017 at 07:41:15PM +0200, wm4 wrote:
> On Thu, 12 Oct 2017 19:17:32 +0200
> Diego Biurrun <diego@biurrun.de> wrote:
> > On Thu, Oct 12, 2017 at 01:02:57AM -0300, James Almer wrote:
> > > libxavs may require pthreads and libm at link time, and without
> > > said ldflags available as global extralibs, the check will fail.
> > > 
> > > Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63
> > > ---
> > > I tried replacing the require() check with a require_pkg_config()
> > > one, and while it included the pthreads ldflag when libxavs was
> > > compiled with pthreads support, it didn't include the libm one.
> > > Considering the project seems dead, trying to get the installed
> > > pkg-config file fixed is probably futile.
> > >  configure | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > --- a/configure
> > > +++ b/configure
> > > @@ -4781,7 +4781,7 @@ enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
> > > -enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
> > > +enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"  
> > 
> > Hmmmmm....
> > 
> > I don't like this. It's clearly a bug in libxavs that you are just working
> > around here. It should be fixed at the source. Even if libxavs itself may
> > be unfixable, it's certainly not unforkable.
> 
> Forking would imply acquiring ownership, which I'm sure nobody wants to
> do.

I wonder how much actual ownership has to be acquired. We could just
recommend the fixed-up version instead of the broken original.

> You'd be better off arguing dropping libaxavs - but for such a small reason?

The reason would be that it's been abandonware for years, it's buggy and
fails to even compile under certain circumstances.

Diego
Martin Storsjö Oct. 12, 2017, 6:35 p.m. | #7
On Thu, 12 Oct 2017, Diego Biurrun wrote:

> On Thu, Oct 12, 2017 at 01:02:57AM -0300, James Almer wrote:
>> libxavs may require pthreads and libm at link time, and without
>> said ldflags available as global extralibs, the check will fail.
>> 
>> Regression since 7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63
>> ---
>> I tried replacing the require() check with a require_pkg_config()
>> one, and while it included the pthreads ldflag when libxavs was
>> compiled with pthreads support, it didn't include the libm one.
>> Considering the project seems dead, trying to get the installed
>> pkg-config file fixed is probably futile.
>>  configure | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> --- a/configure
>> +++ b/configure
>> @@ -4781,7 +4781,7 @@ enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
>> -enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
>> +enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
>
> Hmmmmm....
>
> I don't like this. It's clearly a bug in libxavs that you are just working
> around here. It should be fixed at the source. Even if libxavs itself may
> be unfixable, it's certainly not unforkable.
>
> What problem do you have exactly? libxavs certainly works on my FATE instance
> that checks all external libraries.

I guess it depends on whether you built it as a static or shared library - 
with a shared library you seldom see issues with missing deps in .pc 
files.

// Martin

Patch

diff --git a/configure b/configure
index 960c1ed032..77d71578cc 100755
--- a/configure
+++ b/configure
@@ -4781,7 +4781,7 @@  enabled libx264           && require_pkg_config libx264 x264 "stdint.h x264.h" x
                                enable libx262; }
 enabled libx265           && require_pkg_config libx265 x265 x265.h x265_api_get &&
                              require_cpp_condition x265.h "X265_BUILD >= 57"
-enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode -lxavs
+enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
 enabled libxvid           && require libxvid xvid.h xvid_global -lxvidcore
 enabled mmal              && { check_lib mmal interface/mmal/mmal.h mmal_port_connect -lmmal_core -lmmal_util -lmmal_vc_client -lbcm_host ||
                                { ! enabled cross_compile &&