Patchwork [U-Boot,2/3] mkconfig: create CONFIG_ defines without relying on GNU extensions

login
register
mail settings
Submitter Jeroen Hofstee
Date July 19, 2011, 8:41 p.m.
Message ID <1311108110-37409-3-git-send-email-jeroen@myspectrum.nl>
Download mbox | patch
Permalink /patch/105536/
State Superseded
Headers show

Comments

Jeroen Hofstee - July 19, 2011, 8:41 p.m.
Parsing of boards.cfg relies on sed GNU extensions and fails if sed
doesn't support these. On FreeBSD this leads to the error:

sed: 1: "/=/ {s/=/\t/;q } ; { s/ ...": extra characters at the end
of q command

Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
Cc: Marek Vasut <marek.vasut@gmail.com>
---
 mkconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Mike Frysinger - July 19, 2011, 9:03 p.m.
On Tue, Jul 19, 2011 at 16:41, Jeroen Hofstee wrote:
> Parsing of boards.cfg relies on sed GNU extensions and fails if sed
> doesn't support these. On FreeBSD this leads to the error:
>
> sed: 1: "/=/ {s/=/\t/;q } ; { s/ ...": extra characters at the end
> of q command
>
> -       i="`echo ${i} | sed '/=/ {s/=/\t/;q } ; { s/$/\t1/ }'`"
> +       i="`echo ${i} | sed -e '/=/!s/$/=1/' -e 's/=/   /'`"

maybe i havent read enough sed scripts, but i dont think ive seen "!"
used before.  how about this more straightforward replacement:
sed -e '/=/{s/=/\t/;q}' -e 's/$/\t1/'
-mike
Jeroen Hofstee - July 19, 2011, 9:28 p.m.
Hi Mike,
> maybe i havent read enough sed scripts, but i dont think ive seen "!"
I took this for irony, so I triple checked:

jeroen@green-ubuntu:~$ echo config | sed -e '/=/!s/$/=1/'
config=1
jeroen@green-ubuntu:~$ echo config=2 | sed -e '/=/!s/$/=1/'
config=2
> used before.  how about this more straightforward replacement:
> sed -e '/=/{s/=/\t/;q}' -e 's/$/\t1/'
> -mike
Won't work on FreeBSD since it can't quite early [afaik] (perhaps with a 
label, but gets rather ugly..)

[jeroen@blue ~]$ echo configflag | sed -e '/=/{s/=/\t/;q}' -e 's/$/\t1/'
sed: 1: "/=/{s/=/\t/;q}
": extra characters at the end of q command

Regards,
Jeroen
Mike Frysinger - July 19, 2011, 9:38 p.m.
On Tue, Jul 19, 2011 at 17:28, Jeroen Hofstee wrote:
>> maybe i havent read enough sed scripts, but i dont think ive seen "!"
>
> I took this for irony, so I triple checked:

i didnt mean "it isnt in POSIX" (because it is), i meant it as "no one
uses it, so it wont be obvious to people reading this code later as to
what's going on"

>> used before.  how about this more straightforward replacement:
>> sed -e '/=/{s/=/\t/;q}' -e 's/$/\t1/'
>
> Won't work on FreeBSD since it can't quite early [afaik] (perhaps with a
> label, but gets rather ugly..)
>
> [jeroen@blue ~]$ echo configflag | sed -e '/=/{s/=/\t/;q}' -e 's/$/\t1/'
> sed: 1: "/=/{s/=/\t/;q}
> ": extra characters at the end of q command

i'm not sure you've diagnosed the problem correctly.  the fact that
the output says "end of q command" indicates that FreeBSD does support
the "q" command (as required by POSIX).

perhaps the problem is that the ";" extension to separating commands
(which is not in POSIX afaics) does not work the same in FreeBSD's sed
as GNU's sed.  i imagine if you stick a ";" after the "q" command
it'll work ...

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
-mike
Jeroen Hofstee - July 19, 2011, 10:05 p.m.
Hi Mike,
>> Won't work on FreeBSD since it can't quite early [afaik] (perhaps with a
>> label, but gets rather ugly..)
>>
>> [jeroen@blue ~]$ echo configflag | sed -e '/=/{s/=/\t/;q}' -e 's/$/\t1/'
>> sed: 1: "/=/{s/=/\t/;q}
>> ": extra characters at the end of q command
> i'm not sure you've diagnosed the problem correctly.  the fact that
> the output says "end of q command" indicates that FreeBSD does support
> the "q" command (as required by POSIX).
>
I didn't apparently..
> perhaps the problem is that the ";" extension to separating commands
> (which is not in POSIX afaics) does not work the same in FreeBSD's sed
> as GNU's sed.  i imagine if you stick a ";" after the "q" command
> it'll work ...
>
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
> -mike
Yes, the ; does work as expected, thanks!

[jeroen@blue ~]$ echo config | sed -e '/=/{s/=/ /;q;}' -e 's/$/ 1/'
config    1
[jeroen@blue ~]$ echo config=2 | sed -e '/=/{s/=/       /;q;}' -e 's/$/ 1/'
config    2

Shall change the patch accordingly tomorrow.

Regards,
Jeroen
Marek Vasut - July 19, 2011, 10:57 p.m.
On Tuesday, July 19, 2011 10:41:49 PM Jeroen Hofstee wrote:
> Parsing of boards.cfg relies on sed GNU extensions and fails if sed
> doesn't support these. On FreeBSD this leads to the error:
> 
> sed: 1: "/=/ {s/=/\t/;q } ; { s/ ...": extra characters at the end
> of q command

Does it still work on linux as well? Did you test? If so, I'm all for it being 
merged, but just from a brief look, I see it'll be missing the TAB. Am I right?

> 
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> Cc: Marek Vasut <marek.vasut@gmail.com>
> ---
>  mkconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mkconfig b/mkconfig
> index 6ff533f..b9cfc94 100755
> --- a/mkconfig
> +++ b/mkconfig
> @@ -148,7 +148,7 @@ fi
>  echo "/* Automatically generated - do not edit */" >>config.h
> 
>  for i in ${TARGETS} ; do
> -	i="`echo ${i} | sed '/=/ {s/=/\t/;q } ; { s/$/\t1/ }'`"
> +	i="`echo ${i} | sed -e '/=/!s/$/=1/' -e 's/=/	/'`"
>  	echo "#define CONFIG_${i}" >>config.h ;
>  done
Mike Frysinger - July 19, 2011, 11:03 p.m.
On Tue, Jul 19, 2011 at 18:57, Marek Vasut wrote:
> On Tuesday, July 19, 2011 10:41:49 PM Jeroen Hofstee wrote:
>> Parsing of boards.cfg relies on sed GNU extensions and fails if sed
>> doesn't support these. On FreeBSD this leads to the error:
>>
>> sed: 1: "/=/ {s/=/\t/;q } ; { s/ ...": extra characters at the end
>> of q command
>
> Does it still work on linux as well? Did you test? If so, I'm all for it being
> merged, but just from a brief look, I see it'll be missing the TAB. Am I right?

he inlined the tab.  it isnt actually a space there.

not that it matters as this is used to create "#define CONFIG_FOO 1"
and preprocessors dont care if it's a tab or space before that "1".
-mike
Jeroen Hofstee - July 19, 2011, 11:09 p.m.
Parsing of boards.cfg relies on sed GNU extensions and fails if sed
>> doesn't support these. On FreeBSD this leads to the error:
>>
>> sed: 1: "/=/ {s/=/\t/;q } ; { s/ ...": extra characters at the end
>> of q command
> Does it still work on linux as well? Did you test? If so, I'm all for it being
> merged, but just from a brief look, I see it'll be missing the TAB. Am I right?
>
yes I did build all arm boards on ubuntu (and i386?) on ubuntu. (it 
didn't build more
nor less boards). As pointed out by Mike there might be a a patch more 
closely to
the original, but I will need to test that (especially if GNU support 
it), and will do that
tomorrow (today actually CEST). So preferably take that one.

The BSD version doesn't understand \t, so it is a literal tab character. 
Can't help it.

Regards,
Jeroen

Patch

diff --git a/mkconfig b/mkconfig
index 6ff533f..b9cfc94 100755
--- a/mkconfig
+++ b/mkconfig
@@ -148,7 +148,7 @@  fi
 echo "/* Automatically generated - do not edit */" >>config.h
 
 for i in ${TARGETS} ; do
-	i="`echo ${i} | sed '/=/ {s/=/\t/;q } ; { s/$/\t1/ }'`"
+	i="`echo ${i} | sed -e '/=/!s/$/=1/' -e 's/=/	/'`"
 	echo "#define CONFIG_${i}" >>config.h ;
 done