diff mbox

[FFmpeg-devel] configure: warn about disabled explicitly enabled components

Message ID 20190205211459.24691-1-cus@passwd.hu
State Accepted
Commit 0c3333faf6b7bfd45ce666ae697759d9239ba587
Headers show

Commit Message

Marton Balint Feb. 5, 2019, 9:14 p.m. UTC 
If we enable a component but a dependant library is disabled, then the enabled
component gets silently disabled. Warning about disabled explicitly enabled components
allows configure to show the missing dependencies and if --fatal-warnings is
used it can also fail if the user wants it so.

For example if libdav1d is not availble ./configure --enable-decoder=libdav1d
succeeds but the libdav1d decoder is not be enabled. After the patch configure
will warn about this:

WARNING: Disabled libdav1d_decoder because not all dependencies are satisfied: libdav1d

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 configure | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

Comments

Marton Balint Feb. 8, 2019, 10:20 p.m. UTC | #1 
On Tue, 5 Feb 2019, Marton Balint wrote:

> If we enable a component but a dependant library is disabled, then the enabled
> component gets silently disabled. Warning about disabled explicitly enabled components
> allows configure to show the missing dependencies and if --fatal-warnings is
> used it can also fail if the user wants it so.
>
> For example if libdav1d is not availble ./configure --enable-decoder=libdav1d
> succeeds but the libdav1d decoder is not be enabled. After the patch configure
> will warn about this:
>
> WARNING: Disabled libdav1d_decoder because not all dependencies are satisfied: libdav1d
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> configure | 32 ++++++++++++++++++++++++++++----
> 1 file changed, 28 insertions(+), 4 deletions(-)
>

Ping.

Thanks,
Marton

> diff --git a/configure b/configure
> index e1412352fa..c1a203daa3 100755
> --- a/configure
> +++ b/configure
> @@ -648,6 +648,12 @@ request(){
>     done
> }
> 
> +warn_if_gets_disabled(){
> +    for var in $*; do
> +        WARN_IF_GETS_DISABLED_LIST="$WARN_IF_GETS_DISABLED_LIST $var"
> +    done
> +}
> +
> enable(){
>     set_all yes $*
> }
> @@ -656,6 +662,14 @@ disable(){
>     set_all no $*
> }
> 
> +disable_with_reason(){
> +    disable $1
> +    eval "${1}_disable_reason=\"$2\""
> +    if requested $1; then
> +        die "ERROR: $1 requested, but $2"
> +    fi
> +}
> +
> enable_weak(){
>     set_weak yes $*
> }
> @@ -784,10 +798,10 @@ check_deps(){
>
>             [ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
>             [ -n "$dep_ifn" ] && { enabled_any $dep_ifn && enable_weak $cfg; }
> -            enabled_all  $dep_all || { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but not all dependencies are satisfied: $dep_all"; }
> -            enabled_any  $dep_any || { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but not any dependency is satisfied: $dep_any"; }
> -            disabled_all $dep_con || { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but some conflicting dependencies are unsatisfied: $dep_con"; }
> -            disabled_any $dep_sel && { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but some selected dependency is unsatisfied: $dep_sel"; }
> +            enabled_all  $dep_all || { disable_with_reason $cfg "not all dependencies are satisfied: $dep_all"; }
> +            enabled_any  $dep_any || { disable_with_reason $cfg "not any dependency is satisfied: $dep_any"; }
> +            disabled_all $dep_con || { disable_with_reason $cfg "some conflicting dependencies are unsatisfied: $dep_con"; }
> +            disabled_any $dep_sel && { disable_with_reason $cfg "some selected dependency is unsatisfied: $dep_sel"; }
>
>             enabled $cfg && enable_deep_weak $dep_sel $dep_sgs
> 
> @@ -3880,6 +3894,7 @@ for opt do
>             name=$(echo "${optval}" | sed "s/,/_${thing}|/g")_${thing}
>             list=$(filter "$name" $list)
>             [ "$list" = "" ] && warn "Option $opt did not match anything"
> +            test $action = enable && warn_if_gets_disabled $list
>             $action $list
>         ;;
>         --enable-yasm|--disable-yasm)
> @@ -7103,6 +7118,15 @@ echo "License: $license"
> 
> fi # test "$quiet" != "yes"
> 
> +if test -n "$WARN_IF_GETS_DISABLED_LIST"; then
> +    for cfg in $WARN_IF_GETS_DISABLED_LIST; do
> +        if disabled $cfg; then
> +            varname=${cfg}_disable_reason
> +            eval "warn \"Disabled $cfg because \$$varname\""
> +        fi
> +    done
> +fi
> +
> if test -n "$WARNINGS"; then
>     printf "\n%s%s$WARNINGS%s" "$warn_color" "$bold_color" "$reset_color"
>     enabled fatal_warnings && exit 1
> -- 
> 2.16.4
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Carl Eugen Hoyos Feb. 8, 2019, 11:46 p.m. UTC | #2 
2019-02-05 22:14 GMT+01:00, Marton Balint <cus@passwd.hu>:
> If we enable a component but a dependant library is disabled, then the
> enabled
> component gets silently disabled. Warning about disabled explicitly enabled
> components
> allows configure to show the missing dependencies and if --fatal-warnings is
> used it can also fail if the user wants it so.
>
> For example if libdav1d is not availble ./configure
> --enable-decoder=libdav1d
> succeeds but the libdav1d decoder is not be enabled. After the patch
> configure
> will warn about this:
>
> WARNING: Disabled libdav1d_decoder because not all dependencies are
> satisfied: libdav1d

The patch produces many warnings for:
$ ./configure --disable-everything --enable-decoder=mp*
Is this intended?

Carl Eugen
Marton Balint Feb. 9, 2019, 12:14 a.m. UTC | #3 
On Sat, 9 Feb 2019, Carl Eugen Hoyos wrote:

> 2019-02-05 22:14 GMT+01:00, Marton Balint <cus@passwd.hu>:
>> If we enable a component but a dependant library is disabled, then the
>> enabled
>> component gets silently disabled. Warning about disabled explicitly enabled
>> components
>> allows configure to show the missing dependencies and if --fatal-warnings is
>> used it can also fail if the user wants it so.
>>
>> For example if libdav1d is not availble ./configure
>> --enable-decoder=libdav1d
>> succeeds but the libdav1d decoder is not be enabled. After the patch
>> configure
>> will warn about this:
>>
>> WARNING: Disabled libdav1d_decoder because not all dependencies are
>> satisfied: libdav1d
>
> The patch produces many warnings for:
> $ ./configure --disable-everything --enable-decoder=mp*
> Is this intended?

It reports only disabled decoders which start with mp, so it works as it 
should the way I see it.

Regards,
Marton
Marton Balint Feb. 10, 2019, 10:10 p.m. UTC | #4 
On Sat, 9 Feb 2019, Marton Balint wrote:

>
>
> On Sat, 9 Feb 2019, Carl Eugen Hoyos wrote:
>
>> 2019-02-05 22:14 GMT+01:00, Marton Balint <cus@passwd.hu>:
>>> If we enable a component but a dependant library is disabled, then the
>>> enabled
>>> component gets silently disabled. Warning about disabled explicitly 
> enabled
>>> components
>>> allows configure to show the missing dependencies and if --fatal-warnings 
> is
>>> used it can also fail if the user wants it so.
>>>
>>> For example if libdav1d is not availble ./configure
>>> --enable-decoder=libdav1d
>>> succeeds but the libdav1d decoder is not be enabled. After the patch
>>> configure
>>> will warn about this:
>>>
>>> WARNING: Disabled libdav1d_decoder because not all dependencies are
>>> satisfied: libdav1d
>>
>> The patch produces many warnings for:
>> $ ./configure --disable-everything --enable-decoder=mp*
>> Is this intended?
>
> It reports only disabled decoders which start with mp, so it works as it 
> should the way I see it.
>

Will push this soon.

Regards,
Marton
Marton Balint Feb. 11, 2019, 9:03 p.m. UTC | #5 
On Sun, 10 Feb 2019, Marton Balint wrote:

>
>
> On Sat, 9 Feb 2019, Marton Balint wrote:
>
>>
>>
>> On Sat, 9 Feb 2019, Carl Eugen Hoyos wrote:
>>
>>> 2019-02-05 22:14 GMT+01:00, Marton Balint <cus@passwd.hu>:
>>>> If we enable a component but a dependant library is disabled, then the
>>>> enabled
>>>> component gets silently disabled. Warning about disabled explicitly 
>> enabled
>>>> components
>>>> allows configure to show the missing dependencies and if --fatal-warnings 
>> is
>>>> used it can also fail if the user wants it so.
>>>>
>>>> For example if libdav1d is not availble ./configure
>>>> --enable-decoder=libdav1d
>>>> succeeds but the libdav1d decoder is not be enabled. After the patch
>>>> configure
>>>> will warn about this:
>>>>
>>>> WARNING: Disabled libdav1d_decoder because not all dependencies are
>>>> satisfied: libdav1d
>>>
>>> The patch produces many warnings for:
>>> $ ./configure --disable-everything --enable-decoder=mp*
>>> Is this intended?
>>
>> It reports only disabled decoders which start with mp, so it works as it 
>> should the way I see it.
>>
>
> Will push this soon.

Applied.

Regards,
Marton
diff mbox

Patch

diff --git a/configure b/configure
index e1412352fa..c1a203daa3 100755
--- a/configure
+++ b/configure
@@ -648,6 +648,12 @@  request(){
     done
 }
 
+warn_if_gets_disabled(){
+    for var in $*; do
+        WARN_IF_GETS_DISABLED_LIST="$WARN_IF_GETS_DISABLED_LIST $var"
+    done
+}
+
 enable(){
     set_all yes $*
 }
@@ -656,6 +662,14 @@  disable(){
     set_all no $*
 }
 
+disable_with_reason(){
+    disable $1
+    eval "${1}_disable_reason=\"$2\""
+    if requested $1; then
+        die "ERROR: $1 requested, but $2"
+    fi
+}
+
 enable_weak(){
     set_weak yes $*
 }
@@ -784,10 +798,10 @@  check_deps(){
 
             [ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
             [ -n "$dep_ifn" ] && { enabled_any $dep_ifn && enable_weak $cfg; }
-            enabled_all  $dep_all || { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but not all dependencies are satisfied: $dep_all"; }
-            enabled_any  $dep_any || { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but not any dependency is satisfied: $dep_any"; }
-            disabled_all $dep_con || { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but some conflicting dependencies are unsatisfied: $dep_con"; }
-            disabled_any $dep_sel && { disable $cfg && requested $cfg && die "ERROR: $cfg requested, but some selected dependency is unsatisfied: $dep_sel"; }
+            enabled_all  $dep_all || { disable_with_reason $cfg "not all dependencies are satisfied: $dep_all"; }
+            enabled_any  $dep_any || { disable_with_reason $cfg "not any dependency is satisfied: $dep_any"; }
+            disabled_all $dep_con || { disable_with_reason $cfg "some conflicting dependencies are unsatisfied: $dep_con"; }
+            disabled_any $dep_sel && { disable_with_reason $cfg "some selected dependency is unsatisfied: $dep_sel"; }
 
             enabled $cfg && enable_deep_weak $dep_sel $dep_sgs
 
@@ -3880,6 +3894,7 @@  for opt do
             name=$(echo "${optval}" | sed "s/,/_${thing}|/g")_${thing}
             list=$(filter "$name" $list)
             [ "$list" = "" ] && warn "Option $opt did not match anything"
+            test $action = enable && warn_if_gets_disabled $list
             $action $list
         ;;
         --enable-yasm|--disable-yasm)
@@ -7103,6 +7118,15 @@  echo "License: $license"
 
 fi # test "$quiet" != "yes"
 
+if test -n "$WARN_IF_GETS_DISABLED_LIST"; then
+    for cfg in $WARN_IF_GETS_DISABLED_LIST; do
+        if disabled $cfg; then
+            varname=${cfg}_disable_reason
+            eval "warn \"Disabled $cfg because \$$varname\""
+        fi
+    done
+fi
+
 if test -n "$WARNINGS"; then
     printf "\n%s%s$WARNINGS%s" "$warn_color" "$bold_color" "$reset_color"
     enabled fatal_warnings && exit 1