diff mbox

[FFmpeg-devel] configure: Avoid use of nonstandard features of sed

Message ID dc6bb522-2342-c21f-f9be-081f51b154b3@jkqxz.net
State Accepted
Headers show

Commit Message

Mark Thompson Nov. 22, 2018, 10:47 p.m. UTC
Standard sed does not support EREs.
---
The Darwin version script part was only tested by running it on the version scripts and looking at the output, not actually building on that platform - it would be helpful if someone could check that it actually works.

 configure | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Carl Eugen Hoyos Nov. 23, 2018, 12:48 a.m. UTC | #1
2018-11-22 23:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
> Standard sed does not support EREs.

Please mention ticket #7310 in the commit message.

Thank you, Carl Eugen
Carl Eugen Hoyos Dec. 2, 2018, 12:49 p.m. UTC | #2
2018-11-23 1:48 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2018-11-22 23:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>> Standard sed does not support EREs.
>
> Please mention ticket #7310 in the commit message.

Ping.

Carl Eugen
Mark Thompson Dec. 2, 2018, 7:46 p.m. UTC | #3
On 02/12/2018 12:49, Carl Eugen Hoyos wrote:
> 2018-11-23 1:48 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>> 2018-11-22 23:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>> Standard sed does not support EREs.
>>
>> Please mention ticket #7310 in the commit message.
> 
> Ping.

I was waiting for:

> The Darwin version script part was only tested by running it on the version scripts and looking at the output, not actually building on that platform - it would be helpful if someone could check that it actually works.

Can someone check this?

Thanks,

- Mark
Carl Eugen Hoyos Dec. 2, 2018, 9:12 p.m. UTC | #4
2018-12-02 20:46 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
> On 02/12/2018 12:49, Carl Eugen Hoyos wrote:
>> 2018-11-23 1:48 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>> 2018-11-22 23:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>>> Standard sed does not support EREs.
>>>
>>> Please mention ticket #7310 in the commit message.
>>
>> Ping.
>
> I was waiting for:

Sorry, thank you!

>> The Darwin version script part was only tested by running it on the
>> version scripts and looking at the output, not actually building on that
>> platform - it would be helpful if someone could check that it actually
>> works.
>
> Can someone check this?

Why is this needed?
What I mean is: Is there a Darwin platform with a too old sed where
your change would help?

Carl Eugen
Mark Thompson Dec. 2, 2018, 9:35 p.m. UTC | #5
On 02/12/2018 21:12, Carl Eugen Hoyos wrote:
> 2018-12-02 20:46 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>> On 02/12/2018 12:49, Carl Eugen Hoyos wrote:
>>> 2018-11-23 1:48 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>>> 2018-11-22 23:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>>>> Standard sed does not support EREs.
>>>>
>>>> Please mention ticket #7310 in the commit message.
>>>
>>> Ping.
>>
>> I was waiting for:
> 
> Sorry, thank you!
> 
>>> The Darwin version script part was only tested by running it on the
>>> version scripts and looking at the output, not actually building on that
>>> platform - it would be helpful if someone could check that it actually
>>> works.
>>
>> Can someone check this?
> 
> Why is this needed?
> What I mean is: Is there a Darwin platform with a too old sed where
> your change would help?

I have no idea - I know nothing about tools on Apple platforms.  If all versions of sed you might be able to run on Darwin have a nonstandard extension supporting EREs via a -E option then it is indeed unnecessary, but given the nature of the patch I thought it would be sensible to eliminate all of the potentially-troublesome nonstandard use.  On the other hand it is a change and could break things which currently work, so ensuring that it has been tested on the actual target platform seemed like a good idea.

Thanks,

- Mark
Carl Eugen Hoyos Dec. 2, 2018, 9:58 p.m. UTC | #6
2018-12-02 22:35 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
> On 02/12/2018 21:12, Carl Eugen Hoyos wrote:
>> 2018-12-02 20:46 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>> On 02/12/2018 12:49, Carl Eugen Hoyos wrote:
>>>> 2018-11-23 1:48 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>>>> 2018-11-22 23:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>>>>> Standard sed does not support EREs.
>>>>>
>>>>> Please mention ticket #7310 in the commit message.
>>>>
>>>> Ping.
>>>
>>> I was waiting for:
>>
>> Sorry, thank you!
>>
>>>> The Darwin version script part was only tested by running it on the
>>>> version scripts and looking at the output, not actually building on that
>>>> platform - it would be helpful if someone could check that it actually
>>>> works.
>>>
>>> Can someone check this?
>>
>> Why is this needed?
>> What I mean is: Is there a Darwin platform with a too old sed where
>> your change would help?
>
> I have no idea - I know nothing about tools on Apple platforms.  If all
> versions of sed you might be able to run on Darwin have a nonstandard
> extension supporting EREs via a -E option then it is indeed unnecessary, but
> given the nature of the patch I thought it would be sensible to eliminate
> all of the potentially-troublesome nonstandard use.  On the other hand it is
> a change and could break things which currently work, so ensuring that it
> has been tested on the actual target platform seemed like a good idea.

In the meantime, you could commit the tested part of the patch.

Thank you, Carl Eugen
Mark Thompson Dec. 2, 2018, 11:37 p.m. UTC | #7
On 02/12/2018 21:58, Carl Eugen Hoyos wrote:
> 2018-12-02 22:35 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>> On 02/12/2018 21:12, Carl Eugen Hoyos wrote:
>>> 2018-12-02 20:46 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>>> On 02/12/2018 12:49, Carl Eugen Hoyos wrote:
>>>>> 2018-11-23 1:48 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>>>>> 2018-11-22 23:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>>>>>> Standard sed does not support EREs.
>>>>>>
>>>>>> Please mention ticket #7310 in the commit message.
>>>>>
>>>>> Ping.
>>>>
>>>> I was waiting for:
>>>
>>> Sorry, thank you!
>>>
>>>>> The Darwin version script part was only tested by running it on the
>>>>> version scripts and looking at the output, not actually building on that
>>>>> platform - it would be helpful if someone could check that it actually
>>>>> works.
>>>>
>>>> Can someone check this?
>>>
>>> Why is this needed?
>>> What I mean is: Is there a Darwin platform with a too old sed where
>>> your change would help?
>>
>> I have no idea - I know nothing about tools on Apple platforms.  If all
>> versions of sed you might be able to run on Darwin have a nonstandard
>> extension supporting EREs via a -E option then it is indeed unnecessary, but
>> given the nature of the patch I thought it would be sensible to eliminate
>> all of the potentially-troublesome nonstandard use.  On the other hand it is
>> a change and could break things which currently work, so ensuring that it
>> has been tested on the actual target platform seemed like a good idea.
> 
> In the meantime, you could commit the tested part of the patch.

Sure; applied.

Thanks,

- Mark
diff mbox

Patch

diff --git a/configure b/configure
index b4f944cfb0..370c6e9993 100755
--- a/configure
+++ b/configure
@@ -3723,8 +3723,7 @@  find_things_extern(){
 
 find_filters_extern(){
     file=$source_path/$1
-    #sed -n "s/^extern AVFilter ff_\([avfsinkrc]\{2,5\}\)_\(\w\+\);/\2_filter/p" $file
-    sed -E -n "s/^extern AVFilter ff_([avfsinkrc]{2,5})_([a-zA-Z0-9_]+);/\2_filter/p" $file
+    sed -n 's/^extern AVFilter ff_[avfsinkrc]\{2,5\}_\([[:alnum:]_]\{1,\}\);/\1_filter/p' $file
 }
 
 FILTER_LIST=$(find_filters_extern libavfilter/allfilters.c)
@@ -5190,7 +5189,7 @@  case $target_os in
             is_in -isysroot $ld $LDFLAGS          || check_ldflags  -isysroot $sysroot
         fi
         version_script='-exported_symbols_list'
-        VERSION_SCRIPT_POSTPROCESS_CMD='tr " " "\n" | sed -n /global:/,/local:/p | grep ";" | tr ";" "\n" | sed -E "s/(.+)/_\1/g" | sed -E "s/(.+[^*])$$$$/\1*/"'
+        VERSION_SCRIPT_POSTPROCESS_CMD='tr " " "\n" | sed -n /global:/,/local:/p | grep ";" | tr ";" "\n" | sed "s/\(.\{1,\}\)/_\1/g" | sed "s/\(.\{1,\}[^*]\)$$$$/\1*/"'
         ;;
     msys*)
         die "Native MSYS builds are discouraged, please use the MINGW environment."