| Message ID | 20190128010022.18709-1-cus@passwd.hu |
|---|---|
| State | Superseded |
| Headers | show |
2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: > If we enable a component but a dependant library is disabled, then the > enabled > component get silently disabled. Requesting all explicitly enabled > components > allows configure to fail and show the missing dependencies instead of > ignoring > our request. > > For example if libdav1d is not availble ./configure > --enable-decoder=libdav1d > succeeds but the libdav1d decoder will not be enabled. After the patch the > configure line will fail with the following message: > ERROR: libdav1d_decoder requested, but not all dependencies are satisfied: > libdav1d > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > configure | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configure b/configure > index e1412352fa..1f6c6a7311 100755 > --- a/configure > +++ b/configure > @@ -3881,6 +3881,7 @@ for opt do > list=$(filter "$name" $list) > [ "$list" = "" ] && warn "Option $opt did not match anything" > $action $list > + test $action = enable && request $list I strongly suspect that this will break regression tests. What exactly does this fix? If you don't "--enable-libdav1d", you cannot get libdav1d. Also, configure's console output shows what was enabled. I am not happy with this patch, sorry, Carl Eugen
On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: > 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >> If we enable a component but a dependant library is disabled, then the >> enabled >> component get silently disabled. Requesting all explicitly enabled >> components >> allows configure to fail and show the missing dependencies instead of >> ignoring >> our request. >> >> For example if libdav1d is not availble ./configure >> --enable-decoder=libdav1d >> succeeds but the libdav1d decoder will not be enabled. After the patch the >> configure line will fail with the following message: >> ERROR: libdav1d_decoder requested, but not all dependencies are satisfied: >> libdav1d >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> configure | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/configure b/configure >> index e1412352fa..1f6c6a7311 100755 >> --- a/configure >> +++ b/configure >> @@ -3881,6 +3881,7 @@ for opt do >> list=$(filter "$name" $list) >> [ "$list" = "" ] && warn "Option $opt did not match anything" >> $action $list > >> + test $action = enable && request $list > > I strongly suspect that this will break regression tests. You mean fate with different configure options? I didn't find a fate instance explicity enabling a certain component. Even if some previously used configure commands error out, that is good IMHO because the user will know that a certain component won't be included in the build. > > What exactly does this fix? > If you don't "--enable-libdav1d", you cannot get libdav1d. That is the point. If I tell configure that I want libdav1d decoder (as in --enable-decoder=libdav1d), it should either give it to me or error out. It should NOT silently disregard my request. You see the discrepancy: --enable-libdav1d error out if libdav1d is not available --enable-decoder=libdav1d does not error out if the libdav1d decoder is not available Also dependencies are often not trivial. Consider that I want the blackframe filter: ./configure --enable-filter=blackframe This succeeds yet it does not enable the filter. Why? Because the filter is GPL. How should I know why it got silently disabled if configure does not error out? > Also, configure's console output shows what was > enabled. That does not tell you why a component got disabled. It is also unreasonable to ask the user to always verify that everything stayed in which he explicitly enabled. Regards, Marton
On 3 Feb 2019, at 16:24, Marton Balint wrote: > On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: > >> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >>> If we enable a component but a dependant library is disabled, then >>> the >>> enabled >>> component get silently disabled. Requesting all explicitly enabled >>> components >>> allows configure to fail and show the missing dependencies instead >>> of >>> ignoring >>> our request. >>> >>> For example if libdav1d is not availble ./configure >>> --enable-decoder=libdav1d >>> succeeds but the libdav1d decoder will not be enabled. After the >>> patch the >>> configure line will fail with the following message: >>> ERROR: libdav1d_decoder requested, but not all dependencies are >>> satisfied: >>> libdav1d >>> >>> Signed-off-by: Marton Balint <cus@passwd.hu> >>> --- >>> configure | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/configure b/configure >>> index e1412352fa..1f6c6a7311 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -3881,6 +3881,7 @@ for opt do >>> list=$(filter "$name" $list) >>> [ "$list" = "" ] && warn "Option $opt did not match >>> anything" >>> $action $list >> >>> + test $action = enable && request $list >> >> I strongly suspect that this will break regression tests. > > You mean fate with different configure options? I didn't find a fate > instance explicity enabling a certain component. Even if some > previously used configure commands error out, that is good IMHO > because the user will know that a certain component won't be included > in the build. > >> >> What exactly does this fix? >> If you don't "--enable-libdav1d", you cannot get libdav1d. > > That is the point. If I tell configure that I want libdav1d decoder > (as in --enable-decoder=libdav1d), it should either give it to me or > error out. It should NOT silently disregard my request. You see the > discrepancy: > > --enable-libdav1d error out if libdav1d is not available > --enable-decoder=libdav1d does not error out if the libdav1d decoder > is not available > > Also dependencies are often not trivial. Consider that I want the > blackframe filter: > > ./configure --enable-filter=blackframe > > This succeeds yet it does not enable the filter. Why? Because the > filter is GPL. How should I know why it got silently disabled if > configure does not error out? I agree that when explicitly enabling something, it should be enabled or cause an error, this is what all build systems I have worked with do and definitely would be the behavior I would expect from FFmpegs configure script. > >> Also, configure's console output shows what was >> enabled. > > That does not tell you why a component got disabled. It is also > unreasonable to ask the user to always verify that everything stayed > in which he explicitly enabled. > > Regards, > Marton > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: > > > On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: > >> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >>> If we enable a component but a dependant library is disabled, then the >>> enabled >>> component get silently disabled. Requesting all explicitly enabled >>> components >>> allows configure to fail and show the missing dependencies instead of >>> ignoring >>> our request. >>> >>> For example if libdav1d is not availble ./configure >>> --enable-decoder=libdav1d >>> succeeds but the libdav1d decoder will not be enabled. After the patch >>> the >>> configure line will fail with the following message: >>> ERROR: libdav1d_decoder requested, but not all dependencies are >>> satisfied: >>> libdav1d >>> >>> Signed-off-by: Marton Balint <cus@passwd.hu> >>> --- >>> configure | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/configure b/configure >>> index e1412352fa..1f6c6a7311 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -3881,6 +3881,7 @@ for opt do >>> list=$(filter "$name" $list) >>> [ "$list" = "" ] && warn "Option $opt did not match >>> anything" >>> $action $list >> >>> + test $action = enable && request $list >> >> I strongly suspect that this will break regression tests. > > You mean fate with different configure options? No, I believe this would break regression tests with --disable-everything (and an enable for a feature that was added in the meantime and is needed to reproduce the issue). Please print a warning like for "--enable-decoder=foo" to fix the issue you see. Carl Eugen
2019-02-05 0:53 GMT+01:00, Marton Balint <cus@passwd.hu>: > > > On Tue, 5 Feb 2019, Carl Eugen Hoyos wrote: > >> 2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: >>> >>> >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: >>> >>>> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >>>>> If we enable a component but a dependant library is disabled, then the >>>>> enabled >>>>> component get silently disabled. Requesting all explicitly enabled >>>>> components >>>>> allows configure to fail and show the missing dependencies instead of >>>>> ignoring >>>>> our request. >>>>> >>>>> For example if libdav1d is not availble ./configure >>>>> --enable-decoder=libdav1d >>>>> succeeds but the libdav1d decoder will not be enabled. After the patch >>>>> the >>>>> configure line will fail with the following message: >>>>> ERROR: libdav1d_decoder requested, but not all dependencies are >>>>> satisfied: >>>>> libdav1d >>>>> >>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>> --- >>>>> configure | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/configure b/configure >>>>> index e1412352fa..1f6c6a7311 100755 >>>>> --- a/configure >>>>> +++ b/configure >>>>> @@ -3881,6 +3881,7 @@ for opt do >>>>> list=$(filter "$name" $list) >>>>> [ "$list" = "" ] && warn "Option $opt did not match >>>>> anything" >>>>> $action $list >>>> >>>>> + test $action = enable && request $list >>>> >>>> I strongly suspect that this will break regression tests. >>> >>> You mean fate with different configure options? >> >> No, I believe this would break regression tests with >> --disable-everything (and an enable for a feature that >> was added in the meantime and is needed to reproduce >> the issue). > > Could you give a more concrete example? I am not sure I > understand what you mean. $ ./configure --disable-everything --enable-bsf=prores_metadata currently does not fail for current FFmpeg and 936d18fb, this would change for future new features with your patch. Please print a warning for --enable-decoder=libdav1d if libdav1d was not enabled. Carl Eugen
On Tue, Feb 5, 2019 at 1:01 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-02-05 0:53 GMT+01:00, Marton Balint <cus@passwd.hu>: > > > > > > On Tue, 5 Feb 2019, Carl Eugen Hoyos wrote: > > > >> 2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: > >>> > >>> > >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: > >>> > >>>> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: > >>>>> If we enable a component but a dependant library is disabled, then the > >>>>> enabled > >>>>> component get silently disabled. Requesting all explicitly enabled > >>>>> components > >>>>> allows configure to fail and show the missing dependencies instead of > >>>>> ignoring > >>>>> our request. > >>>>> > >>>>> For example if libdav1d is not availble ./configure > >>>>> --enable-decoder=libdav1d > >>>>> succeeds but the libdav1d decoder will not be enabled. After the patch > >>>>> the > >>>>> configure line will fail with the following message: > >>>>> ERROR: libdav1d_decoder requested, but not all dependencies are > >>>>> satisfied: > >>>>> libdav1d > >>>>> > >>>>> Signed-off-by: Marton Balint <cus@passwd.hu> > >>>>> --- > >>>>> configure | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git a/configure b/configure > >>>>> index e1412352fa..1f6c6a7311 100755 > >>>>> --- a/configure > >>>>> +++ b/configure > >>>>> @@ -3881,6 +3881,7 @@ for opt do > >>>>> list=$(filter "$name" $list) > >>>>> [ "$list" = "" ] && warn "Option $opt did not match > >>>>> anything" > >>>>> $action $list > >>>> > >>>>> + test $action = enable && request $list > >>>> > >>>> I strongly suspect that this will break regression tests. > >>> > >>> You mean fate with different configure options? > >> > >> No, I believe this would break regression tests with > >> --disable-everything (and an enable for a feature that > >> was added in the meantime and is needed to reproduce > >> the issue). > > > > Could you give a more concrete example? I am not sure I > > understand what you mean. > > $ ./configure --disable-everything --enable-bsf=prores_metadata > currently does not fail for current FFmpeg and 936d18fb, this > would change for future new features with your patch. > What sort of testing would involve trying to enable a certain component, and then *succeeding* when the component does not exist? If anything, you would make it fail sooner, before any actual testing that would presumably actually fail later (otherwise, what would be the point exactly?) Please elaborate more then one random command line that without explanation does not really make sense. If I request a certain component, I would expect one of two things: I get an error, or I get a build that includes that component. Anything else would be unexpected, and therefor bad. We have this wanted behavior with other options already, just not this one, so its not even consistent at all. - Hendrik
2019-02-05 1:13 GMT+01:00, Hendrik Leppkes <h.leppkes@gmail.com>: > On Tue, Feb 5, 2019 at 1:01 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> 2019-02-05 0:53 GMT+01:00, Marton Balint <cus@passwd.hu>: >> > >> > >> > On Tue, 5 Feb 2019, Carl Eugen Hoyos wrote: >> > >> >> 2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: >> >>> >> >>> >> >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: >> >>> >> >>>> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >> >>>>> If we enable a component but a dependant library is disabled, then >> >>>>> the >> >>>>> enabled >> >>>>> component get silently disabled. Requesting all explicitly enabled >> >>>>> components >> >>>>> allows configure to fail and show the missing dependencies instead >> >>>>> of >> >>>>> ignoring >> >>>>> our request. >> >>>>> >> >>>>> For example if libdav1d is not availble ./configure >> >>>>> --enable-decoder=libdav1d >> >>>>> succeeds but the libdav1d decoder will not be enabled. After the >> >>>>> patch >> >>>>> the >> >>>>> configure line will fail with the following message: >> >>>>> ERROR: libdav1d_decoder requested, but not all dependencies are >> >>>>> satisfied: >> >>>>> libdav1d >> >>>>> >> >>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >> >>>>> --- >> >>>>> configure | 1 + >> >>>>> 1 file changed, 1 insertion(+) >> >>>>> >> >>>>> diff --git a/configure b/configure >> >>>>> index e1412352fa..1f6c6a7311 100755 >> >>>>> --- a/configure >> >>>>> +++ b/configure >> >>>>> @@ -3881,6 +3881,7 @@ for opt do >> >>>>> list=$(filter "$name" $list) >> >>>>> [ "$list" = "" ] && warn "Option $opt did not match >> >>>>> anything" >> >>>>> $action $list >> >>>> >> >>>>> + test $action = enable && request $list >> >>>> >> >>>> I strongly suspect that this will break regression tests. >> >>> >> >>> You mean fate with different configure options? >> >> >> >> No, I believe this would break regression tests with >> >> --disable-everything (and an enable for a feature that >> >> was added in the meantime and is needed to reproduce >> >> the issue). >> > >> > Could you give a more concrete example? I am not sure I >> > understand what you mean. >> >> $ ./configure --disable-everything --enable-bsf=prores_metadata >> currently does not fail for current FFmpeg and 936d18fb, this >> would change for future new features with your patch. >> > > What sort of testing would involve trying to enable a certain > component, and then *succeeding* when the component does not exist? Several regression tests have needed that in the past, I find it strange that you seem to imply this will not happen in the future. Or is that not what you were saying? Carl Eugen
On Tue, Feb 5, 2019 at 2:43 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-02-05 1:13 GMT+01:00, Hendrik Leppkes <h.leppkes@gmail.com>: > > On Tue, Feb 5, 2019 at 1:01 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > >> 2019-02-05 0:53 GMT+01:00, Marton Balint <cus@passwd.hu>: > >> > > >> > > >> > On Tue, 5 Feb 2019, Carl Eugen Hoyos wrote: > >> > > >> >> 2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: > >> >>> > >> >>> > >> >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: > >> >>> > >> >>>> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: > >> >>>>> If we enable a component but a dependant library is disabled, then > >> >>>>> the > >> >>>>> enabled > >> >>>>> component get silently disabled. Requesting all explicitly enabled > >> >>>>> components > >> >>>>> allows configure to fail and show the missing dependencies instead > >> >>>>> of > >> >>>>> ignoring > >> >>>>> our request. > >> >>>>> > >> >>>>> For example if libdav1d is not availble ./configure > >> >>>>> --enable-decoder=libdav1d > >> >>>>> succeeds but the libdav1d decoder will not be enabled. After the > >> >>>>> patch > >> >>>>> the > >> >>>>> configure line will fail with the following message: > >> >>>>> ERROR: libdav1d_decoder requested, but not all dependencies are > >> >>>>> satisfied: > >> >>>>> libdav1d > >> >>>>> > >> >>>>> Signed-off-by: Marton Balint <cus@passwd.hu> > >> >>>>> --- > >> >>>>> configure | 1 + > >> >>>>> 1 file changed, 1 insertion(+) > >> >>>>> > >> >>>>> diff --git a/configure b/configure > >> >>>>> index e1412352fa..1f6c6a7311 100755 > >> >>>>> --- a/configure > >> >>>>> +++ b/configure > >> >>>>> @@ -3881,6 +3881,7 @@ for opt do > >> >>>>> list=$(filter "$name" $list) > >> >>>>> [ "$list" = "" ] && warn "Option $opt did not match > >> >>>>> anything" > >> >>>>> $action $list > >> >>>> > >> >>>>> + test $action = enable && request $list > >> >>>> > >> >>>> I strongly suspect that this will break regression tests. > >> >>> > >> >>> You mean fate with different configure options? > >> >> > >> >> No, I believe this would break regression tests with > >> >> --disable-everything (and an enable for a feature that > >> >> was added in the meantime and is needed to reproduce > >> >> the issue). > >> > > >> > Could you give a more concrete example? I am not sure I > >> > understand what you mean. > >> > >> $ ./configure --disable-everything --enable-bsf=prores_metadata > >> currently does not fail for current FFmpeg and 936d18fb, this > >> would change for future new features with your patch. > >> > > > > What sort of testing would involve trying to enable a certain > > component, and then *succeeding* when the component does not exist? > > Several regression tests have needed that in the past, I find it > strange that you seem to imply this will not happen in the > future. > Or is that not what you were saying? > I'm trying to understand what type of test you are running that would need this. Please explain isntead of saying "something needs this". It just confuses me that any test would be meaningful if you try to enable a component and end up with a build without it even present. - Hendrik
2019.02.05. 0:45 keltezéssel, Carl Eugen Hoyos írta: > 2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: >> >> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: >> >>> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >>>> If we enable a component but a dependant library is disabled, then the >>>> enabled >>>> component get silently disabled. Requesting all explicitly enabled >>>> components >>>> allows configure to fail and show the missing dependencies instead of >>>> ignoring >>>> our request. >>>> >>>> For example if libdav1d is not availble ./configure >>>> --enable-decoder=libdav1d >>>> succeeds but the libdav1d decoder will not be enabled. After the patch >>>> the >>>> configure line will fail with the following message: >>>> ERROR: libdav1d_decoder requested, but not all dependencies are >>>> satisfied: >>>> libdav1d >>>> >>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>> --- >>>> configure | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/configure b/configure >>>> index e1412352fa..1f6c6a7311 100755 >>>> --- a/configure >>>> +++ b/configure >>>> @@ -3881,6 +3881,7 @@ for opt do >>>> list=$(filter "$name" $list) >>>> [ "$list" = "" ] && warn "Option $opt did not match >>>> anything" >>>> $action $list >>>> + test $action = enable && request $list >>> I strongly suspect that this will break regression tests. >> You mean fate with different configure options? > No, I believe this would break regression tests with > --disable-everything (and an enable for a feature that > was added in the meantime and is needed to reproduce > the issue). > Please print a warning like for "--enable-decoder=foo" > to fix the issue you see. what about to have a new switch of configure script that controls the behaviour at missing dependencies? Something like --stop_on_missing_deps its defult value would be "no" as current state, but Marton Balint new patch functionality may work at "yes" state? bb > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
2019-02-05 10:13 GMT+01:00, Hendrik Leppkes <h.leppkes@gmail.com>: > On Tue, Feb 5, 2019 at 2:43 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> 2019-02-05 1:13 GMT+01:00, Hendrik Leppkes <h.leppkes@gmail.com>: >> > On Tue, Feb 5, 2019 at 1:01 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> >> > wrote: >> >> >> >> 2019-02-05 0:53 GMT+01:00, Marton Balint <cus@passwd.hu>: >> >> > >> >> > >> >> > On Tue, 5 Feb 2019, Carl Eugen Hoyos wrote: >> >> > >> >> >> 2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: >> >> >>> >> >> >>> >> >> >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: >> >> >>> >> >> >>>> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >> >> >>>>> If we enable a component but a dependant library is disabled, >> >> >>>>> then >> >> >>>>> the >> >> >>>>> enabled >> >> >>>>> component get silently disabled. Requesting all explicitly >> >> >>>>> enabled >> >> >>>>> components >> >> >>>>> allows configure to fail and show the missing dependencies >> >> >>>>> instead >> >> >>>>> of >> >> >>>>> ignoring >> >> >>>>> our request. >> >> >>>>> >> >> >>>>> For example if libdav1d is not availble ./configure >> >> >>>>> --enable-decoder=libdav1d >> >> >>>>> succeeds but the libdav1d decoder will not be enabled. After the >> >> >>>>> patch >> >> >>>>> the >> >> >>>>> configure line will fail with the following message: >> >> >>>>> ERROR: libdav1d_decoder requested, but not all dependencies are >> >> >>>>> satisfied: >> >> >>>>> libdav1d >> >> >>>>> >> >> >>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >> >> >>>>> --- >> >> >>>>> configure | 1 + >> >> >>>>> 1 file changed, 1 insertion(+) >> >> >>>>> >> >> >>>>> diff --git a/configure b/configure >> >> >>>>> index e1412352fa..1f6c6a7311 100755 >> >> >>>>> --- a/configure >> >> >>>>> +++ b/configure >> >> >>>>> @@ -3881,6 +3881,7 @@ for opt do >> >> >>>>> list=$(filter "$name" $list) >> >> >>>>> [ "$list" = "" ] && warn "Option $opt did not match >> >> >>>>> anything" >> >> >>>>> $action $list >> >> >>>> >> >> >>>>> + test $action = enable && request $list >> >> >>>> >> >> >>>> I strongly suspect that this will break regression tests. >> >> >>> >> >> >>> You mean fate with different configure options? >> >> >> >> >> >> No, I believe this would break regression tests with >> >> >> --disable-everything (and an enable for a feature that >> >> >> was added in the meantime and is needed to reproduce >> >> >> the issue). >> >> > >> >> > Could you give a more concrete example? I am not sure I >> >> > understand what you mean. >> >> >> >> $ ./configure --disable-everything --enable-bsf=prores_metadata >> >> currently does not fail for current FFmpeg and 936d18fb, this >> >> would change for future new features with your patch. >> >> >> > >> > What sort of testing would involve trying to enable a certain >> > component, and then *succeeding* when the component does not exist? >> >> Several regression tests have needed that in the past, I find it >> strange that you seem to imply this will not happen in the >> future. >> Or is that not what you were saying? >> > > I'm trying to understand what type of test you are running that would > need this. Please explain isntead of saying "something needs this". > It just confuses me that any test would be meaningful if you try to > enable a component and end up with a build without it even present. Bugs exist that need --enable-feature=foo to be reproducible with a later version of FFmpeg while they were reproducible with older FFmpeg before foo existed. Carl Eugen
2019-02-05 11:20 GMT+01:00, Bodecs Bela <bodecsb@vivanet.hu>: > > 2019.02.05. 0:45 keltezéssel, Carl Eugen Hoyos írta: >> 2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: >>> >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: >>> >>>> 2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: >>>>> If we enable a component but a dependant library is disabled, then the >>>>> enabled >>>>> component get silently disabled. Requesting all explicitly enabled >>>>> components >>>>> allows configure to fail and show the missing dependencies instead of >>>>> ignoring >>>>> our request. >>>>> >>>>> For example if libdav1d is not availble ./configure >>>>> --enable-decoder=libdav1d >>>>> succeeds but the libdav1d decoder will not be enabled. After the patch >>>>> the >>>>> configure line will fail with the following message: >>>>> ERROR: libdav1d_decoder requested, but not all dependencies are >>>>> satisfied: >>>>> libdav1d >>>>> >>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>> --- >>>>> configure | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/configure b/configure >>>>> index e1412352fa..1f6c6a7311 100755 >>>>> --- a/configure >>>>> +++ b/configure >>>>> @@ -3881,6 +3881,7 @@ for opt do >>>>> list=$(filter "$name" $list) >>>>> [ "$list" = "" ] && warn "Option $opt did not match >>>>> anything" >>>>> $action $list >>>>> + test $action = enable && request $list >>>> I strongly suspect that this will break regression tests. >>> You mean fate with different configure options? >> No, I believe this would break regression tests with >> --disable-everything (and an enable for a feature that >> was added in the meantime and is needed to reproduce >> the issue). >> Please print a warning like for "--enable-decoder=foo" >> to fix the issue you see. > > > what about to have a new switch of configure script that controls the > behaviour at missing dependencies? Something like --stop_on_missing_deps > > its defult value would be "no" as current state, but Marton Balint new > patch functionality may work at "yes" state? How would this be better than printing a warning if the feature could not be enabled as it is already done in some situations? Carl Eugen
On Tue, Feb 5, 2019 at 6:29 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > How would this be better than printing a warning if the feature > could not be enabled as it is already done in some situations? In most systems I've worked with, if I say "enable something" and it cannot be enabled I want the ./configure to exit out immediately with a non-zero error code. Especially when doing scripted builds, the last thing I want to do is have the script run to completion and then have to pipe stderr to a file and grep the output for warnings that something didn't get included. That's just my opinion though. :-) Devin
On Tue, Feb 05, 2019 at 11:20:03AM +0100, Bodecs Bela wrote: > > 2019.02.05. 0:45 keltezéssel, Carl Eugen Hoyos írta: > >2019-02-03 16:24 GMT+01:00, Marton Balint <cus@passwd.hu>: > >> > >>On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote: > >> > >>>2019-01-28 2:00 GMT+01:00, Marton Balint <cus@passwd.hu>: > >>>>If we enable a component but a dependant library is disabled, then the > >>>>enabled > >>>>component get silently disabled. Requesting all explicitly enabled > >>>>components > >>>>allows configure to fail and show the missing dependencies instead of > >>>>ignoring > >>>>our request. > >>>> > >>>>For example if libdav1d is not availble ./configure > >>>>--enable-decoder=libdav1d > >>>>succeeds but the libdav1d decoder will not be enabled. After the patch > >>>>the > >>>>configure line will fail with the following message: > >>>>ERROR: libdav1d_decoder requested, but not all dependencies are > >>>>satisfied: > >>>>libdav1d > >>>> > >>>>Signed-off-by: Marton Balint <cus@passwd.hu> > >>>>--- > >>>> configure | 1 + > >>>> 1 file changed, 1 insertion(+) > >>>> > >>>>diff --git a/configure b/configure > >>>>index e1412352fa..1f6c6a7311 100755 > >>>>--- a/configure > >>>>+++ b/configure > >>>>@@ -3881,6 +3881,7 @@ for opt do > >>>> list=$(filter "$name" $list) > >>>> [ "$list" = "" ] && warn "Option $opt did not match > >>>>anything" > >>>> $action $list > >>>>+ test $action = enable && request $list > >>>I strongly suspect that this will break regression tests. > >>You mean fate with different configure options? > >No, I believe this would break regression tests with > >--disable-everything (and an enable for a feature that > >was added in the meantime and is needed to reproduce > >the issue). > >Please print a warning like for "--enable-decoder=foo" > >to fix the issue you see. > > > what about to have a new switch of configure script that controls the > behaviour at missing dependencies? Something like --stop_on_missing_deps > > its defult value would be "no" as current state, but Marton Balint new patch > functionality may work at "yes" state? I think this goes in the right direction but it is too limited. The problem if i understand it correctly is that there are 2 semantically different "commands" the user can give configure. 1. feature F should be enabled or disabled, the user wants that, the user wants to know via easy detectable failure if that fails 2. The user doesnt actually care about F but wants it to be enabled/disabled if possible. One use case is to disable all and then enable one. The user doesnt actually want ALL to be disabled its instead part of the "language" used to specify what the user wants enabled a "soft" enable/disable would solve this, where a hard en/disable causes failure if not achieved a soft one can be discarded with a warning if it cannot be achieved [...]
diff --git a/configure b/configure index e1412352fa..1f6c6a7311 100755 --- a/configure +++ b/configure @@ -3881,6 +3881,7 @@ for opt do list=$(filter "$name" $list) [ "$list" = "" ] && warn "Option $opt did not match anything" $action $list + test $action = enable && request $list ;; --enable-yasm|--disable-yasm) warn "The ${opt} option is only provided for compatibility and will be\n"\
If we enable a component but a dependant library is disabled, then the enabled component get silently disabled. Requesting all explicitly enabled components allows configure to fail and show the missing dependencies instead of ignoring our request. For example if libdav1d is not availble ./configure --enable-decoder=libdav1d succeeds but the libdav1d decoder will not be enabled. After the patch the configure line will fail with the following message: ERROR: libdav1d_decoder requested, but not all dependencies are satisfied: libdav1d Signed-off-by: Marton Balint <cus@passwd.hu> --- configure | 1 + 1 file changed, 1 insertion(+)