[FFmpeg-devel] [PATCH 1/3] configure: improve pkg-config support

[Reinhard Tartler]

On Sun, Mar 06, 2011 at 14:57:52 (CET), Mans Rullgard wrote:

> This adds helper functions for checking packages with pkg-config
> and managing the associated flags.
>
> Note that pkg-config use is still discouraged due to widespread
> poor practices resulting in broken flags in many situations.  A
> few badly designed packages require flags only obtainable using
> pkg-config, and these functions are intended for those cases.
>
> Signed-off-by: Mans Rullgard <mans at mansr.com>
> ---
>  configure |   36 +++++++++++++++++++++++++++++++++++-
>  1 files changed, 35 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 7cb0410..b58c291 100755
> --- a/configure
> +++ b/configure
> @@ -356,6 +356,16 @@ set_weak(){
>      done
>  }
>  
> +set_safe(){
> +    var=$1
> +    shift
> +    eval $(echo "$var" | sed 's/[^A-Za-z0-9_]/_/g')='$*'
> +}
> +
> +get_safe(){
> +    eval echo \$$(echo "$1" | sed 's/[^A-Za-z0-9_]/_/g')
> +}
> +

I wonder what was the motivation for these two functions. can you please
give an example for what cases (packages) they are necessary?

>  pushvar(){
>      for var in $*; do
>          eval level=\${${var}_level:=0}
> @@ -744,6 +754,20 @@ check_lib2(){
>      check_func_headers "$headers" $func "$@" && add_extralibs "$@"
>  }
>  
> +check_pkg_config(){
> +    log check_pkg_config "$@"
> +    pkg="$1"
> +    headers="$2"
> +    func="$3"
> +    shift 3
> +    $pkg_config --exists $pkg || return
> +    pkg_cflags=$($pkg_config --cflags $pkg)
> +    pkg_libs=$($pkg_config --libs $pkg)
> +    check_func_headers "$headers" $func $pkg_cflags $pkg_libs "$@" &&
> +        set_safe ${pkg}_cflags $pkg_cflags   &&
> +        set_safe ${pkg}_libs   $pkg_libs
> +}
> +

I understand your deep distrust in pkg-config, but additionally checking
for missing headers here just checks for 'correctness' of pkg-config
files. I think we can and should rely that existing .pc files are not
broken and if we encouter broken ones, report them to upstreams and
distros. This additional check feels like overdoing that to me.

Or do I miss some important point here?

>  check_exec(){
>      check_ld "$@" && { enabled cross_compile || $TMPE >> $logfile 2>&1; }
>  }
> @@ -825,6 +849,13 @@ require2(){
>      check_lib2 "$headers" $func "$@" || die "ERROR: $name not found"
>  }
>  
> +require_pkg_config(){
> +    pkg="$1"
> +    check_pkg_config "$@" || die "ERROR: $pkg not found"
> +    add_cflags    $(get_safe ${pkg}_cflags)
> +    add_extralibs $(get_safe ${pkg}_libs)
> +}
> +
>  check_host_cc(){
>      log check_host_cc "$@"
>      cat > $TMPC
> @@ -1147,6 +1178,7 @@ CMDLINE_SET="
>      logfile
>      malloc_prefix
>      nm
> +    pkg_config
>      samples
>      source_path
>      strip
> @@ -1575,6 +1607,7 @@ ranlib="ranlib"
>  strip_default="strip"
>  yasmexe="yasm"
>  nogas=":"
> +pkg_config_default=pkg-config
>  
>  nm_opts='-g'
>  
> @@ -1783,10 +1816,11 @@ cc_default="${cross_prefix}${cc_default}"
>  nm_default="${cross_prefix}${nm_default}"
>  ranlib="${cross_prefix}${ranlib}"
>  strip_default="${cross_prefix}${strip_default}"
> +pkg_config_default="${cross_prefix}${pkg_config_default}"
>  
>  sysinclude_default="${sysroot}/usr/include"
>  
> -set_default cc nm strip sysinclude
> +set_default cc nm strip sysinclude pkg_config
>  enabled cross_compile || host_cc_default=$cc
>  set_default host_cc

rest look OK to me.

-- 
Gruesse/greetings,
Reinhard Tartler, KeyID 945348A4