Use of == in shell scripts

Use of == in shell scripts

[Andreas Oberritter]

Dear all,

I noticed a common mistake in various OE recipes, classes and contrib
scripts: Instead of =, == is used as equality operator, in at least 91
places in 64 files (without recipes/obsolete: 83 in 60 files). This
isn't a problem with bash, but it isn't the standard syntax and at least
dash does not support it.

I used the following command line to find the occurences:

git grep 'if\s*\[.*==.*\]'

This command fixes all occurences:

for i in `git grep -l 'if\s*\[.*==.*\]'`; do
	sed -i $i -e 's,\(if\s*\[.*\)==\(.*\]\),\1=\2,';
done

One of the matches is a false positive:
recipes/uclibc/uclibc-0.9.29/uClibc-0.9.29-nonposix_bashisms.patch

How do I submit such a patch? 60 patches may be too much for such a
relatively simple change, but, on the other hand, everything in a single
patch may be unhandy as well.

How about splitting it into 5 patches?

- classes (7 files)
- contrib (4 files)
- recipes/*/*.bb (24 files)
- recipes/*/*.inc (14 files)
- other files in recipes except recipes/obsolete (11 files)

Any opinions?

Regards,
Andreas

Re: Use of == in shell scripts

[Andreas Oberritter]

On 10/26/2010 05:31 PM, Andreas Oberritter wrote:
> How about splitting it into 5 patches?
> 
> - classes (7 files)
> - contrib (4 files)
> - recipes/*/*.bb (24 files)
> - recipes/*/*.inc (14 files)
> - other files in recipes except recipes/obsolete (11 files)

Or how about those 37 patches?

http://git.opendreambox.org/?p=obi/openembedded.git;a=shortlog;h=refs/heads/equality-operator

Rationale:
- bbclasses: 1 patch per file
- contrib and recipes: 1 patch per directory

Regards,
Andreas

Re: Use of == in shell scripts

[Michael Smith]

Andreas Oberritter wrote:
> On 10/26/2010 05:31 PM, Andreas Oberritter wrote:
>> How about splitting it into 5 patches?
>>
>> - classes (7 files)
>> - contrib (4 files)
>> - recipes/*/*.bb (24 files)
>> - recipes/*/*.inc (14 files)
>> - other files in recipes except recipes/obsolete (11 files)
> 
> Or how about those 37 patches?
> 
> http://git.opendreambox.org/?p=obi/openembedded.git;a=shortlog;h=refs/heads/equality-operator
> 
> Rationale:
> - bbclasses: 1 patch per file
> - contrib and recipes: 1 patch per directory

For the series:

Acked-By: Michael Smith <msmith@cbnco.com>

Except this one:

http://git.opendreambox.org/?p=obi/openembedded.git;a=commitdiff;h=ad7b2c9ab0c305034c39d2efc5a52789965bb5c2

where some lines had == twice, looks like your sed may not have caught it.



BTW, some of the files have constructs like this:

if [ "x$HAS_MADWIFI" = "x1" ]

I think this "x" business is for old broken shells that can't handle an 
empty quoted string as the first argument. Does anyone know the standard 
well enough to say? I figure [ "" = "1" ] should be legal everywhere. 
This is just a pet peeve/bike shed on my part.

Mike

Re: Use of == in shell scripts

[Andreas Oberritter]

On 10/26/2010 06:29 PM, Michael Smith wrote:
> Andreas Oberritter wrote:
>> On 10/26/2010 05:31 PM, Andreas Oberritter wrote:
>>> How about splitting it into 5 patches?
>>>
>>> - classes (7 files)
>>> - contrib (4 files)
>>> - recipes/*/*.bb (24 files)
>>> - recipes/*/*.inc (14 files)
>>> - other files in recipes except recipes/obsolete (11 files)
>>
>> Or how about those 37 patches?
>>
>> http://git.opendreambox.org/?p=obi/openembedded.git;a=shortlog;h=refs/heads/equality-operator
>>
>>
>> Rationale:
>> - bbclasses: 1 patch per file
>> - contrib and recipes: 1 patch per directory
> 
> For the series:
> 
> Acked-By: Michael Smith <msmith@cbnco.com>
> 
> Except this one:
> 
> http://git.opendreambox.org/?p=obi/openembedded.git;a=commitdiff;h=ad7b2c9ab0c305034c39d2efc5a52789965bb5c2
> 
> 
> where some lines had == twice, looks like your sed may not have caught it.

Thanks for spotting! I've updated the patch series and added your ack.

> BTW, some of the files have constructs like this:
> 
> if [ "x$HAS_MADWIFI" = "x1" ]
> 
> I think this "x" business is for old broken shells that can't handle an
> empty quoted string as the first argument. Does anyone know the standard
> well enough to say? I figure [ "" = "1" ] should be legal everywhere.
> This is just a pet peeve/bike shed on my part.

Autotools use constructs like that everywhere, so it's likely increasing
portability while decreasing readability.

A random forum post suggests that some versions of '[' have problems, if
the first operand equals '!' or '(' [1].

It's not easy to search for [ or test or x on google. ;-)

Regards,
Andreas

[1]
http://www.techtalkz.com/unix/78138-testing-equality-bash-scripts.html#post329122

Re: Use of == in shell scripts

[Mike Detwiler]

On Tue, Oct 26, 2010 at 7:36 PM, Andreas Oberritter
<obi@opendreambox.org> wrote:
> On 10/26/2010 06:29 PM, Michael Smith wrote:
>> Andreas Oberritter wrote:
>>> On 10/26/2010 05:31 PM, Andreas Oberritter wrote:
>>>> How about splitting it into 5 patches?
>>>>
>>>> - classes (7 files)
>>>> - contrib (4 files)
>>>> - recipes/*/*.bb (24 files)
>>>> - recipes/*/*.inc (14 files)
>>>> - other files in recipes except recipes/obsolete (11 files)
>>>
>>> Or how about those 37 patches?
>>>
>>> http://git.opendreambox.org/?p=obi/openembedded.git;a=shortlog;h=refs/heads/equality-operator
>>>
>>>
>>> Rationale:
>>> - bbclasses: 1 patch per file
>>> - contrib and recipes: 1 patch per directory
>>
>> For the series:
>>
>> Acked-By: Michael Smith <msmith@cbnco.com>
>>
>> Except this one:
>>
>> http://git.opendreambox.org/?p=obi/openembedded.git;a=commitdiff;h=ad7b2c9ab0c305034c39d2efc5a52789965bb5c2
>>
>>
>> where some lines had == twice, looks like your sed may not have caught it.
>
> Thanks for spotting! I've updated the patch series and added your ack.
>
>> BTW, some of the files have constructs like this:
>>
>> if [ "x$HAS_MADWIFI" = "x1" ]
>>
>> I think this "x" business is for old broken shells that can't handle an
>> empty quoted string as the first argument. Does anyone know the standard
>> well enough to say? I figure [ "" = "1" ] should be legal everywhere.
>> This is just a pet peeve/bike shed on my part.
>
> Autotools use constructs like that everywhere, so it's likely increasing
> portability while decreasing readability.
>
> A random forum post suggests that some versions of '[' have problems, if
> the first operand equals '!' or '(' [1].

And the less random Autoconf manual has this to say about the shell
builtin, test, and it's more common name '[':

"Similarly, Posix says that both ‘test "string1" = "string2"’ and
‘test "string1" != "string2"’ work for any pairs of strings, but in
practice this is not true for troublesome strings that look like
operators or parentheses, or that begin with ‘-’.

It is best to protect such strings with a leading ‘X’, e.g., ‘test
"Xstring" != X’ rather than ‘test -n "string"’ or ‘test !
"string"’."[2]

See the discussion of 'test(string)' at the link below.

[2] http://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins

>
> It's not easy to search for [ or test or x on google. ;-)
>
> Regards,
> Andreas
>
> [1]
> http://www.techtalkz.com/unix/78138-testing-equality-bash-scripts.html#post329122

Re: Use of == in shell scripts

[Andreas Oberritter]

Hello Michael,

On 10/27/2010 01:36 AM, Andreas Oberritter wrote:
>>> http://git.opendreambox.org/?p=obi/openembedded.git;a=shortlog;h=refs/heads/equality-operator

can you please pull and push these patches? I've rebased the tree on
today's master. My write access still isn't working.

The patch to recipes/linux/linux-openmoko.inc was dropped, because it
became obsolete in the meantime.

Regards,
Andreas

Re: Use of == in shell scripts

[Michael Smith]

On Sat, 13 Nov 2010, Andreas Oberritter wrote:

> On 10/27/2010 01:36 AM, Andreas Oberritter wrote:
> >>> http://git.opendreambox.org/?p=obi/openembedded.git;a=shortlog;h=refs/heads/equality-operator
> 
> can you please pull and push these patches? I've rebased the tree on
> today's master. My write access still isn't working.

Pushed, thanks. I bumped PRs for a couple of recipes where the fix was in 
a file that gets packaged.

Mike

Re: Use of == in shell scripts

[Maupin, Chase]

> -----Original Message-----
> From: openembedded-devel-bounces@lists.openembedded.org
> [mailto:openembedded-devel-bounces@lists.openembedded.org] On Behalf Of
> Andreas Oberritter
> Sent: Tuesday, October 26, 2010 10:31 AM
> To: openembedded-devel@lists.openembedded.org
> Subject: [oe] Use of == in shell scripts
> 
> Dear all,
> 
> I noticed a common mistake in various OE recipes, classes and contrib
> scripts: Instead of =, == is used as equality operator, in at least 91
> places in 64 files (without recipes/obsolete: 83 in 60 files). This
> isn't a problem with bash, but it isn't the standard syntax and at least
> dash does not support it.

Isn't bash the supported shell?  If I remember correctly in bash "==" will also work with numbers whereas "=" is only for strings.

> 
> I used the following command line to find the occurences:
> 
> git grep 'if\s*\[.*==.*\]'
> 
> This command fixes all occurences:
> 
> for i in `git grep -l 'if\s*\[.*==.*\]'`; do
> 	sed -i $i -e 's,\(if\s*\[.*\)==\(.*\]\),\1=\2,';
> done
> 
> One of the matches is a false positive:
> recipes/uclibc/uclibc-0.9.29/uClibc-0.9.29-nonposix_bashisms.patch
> 
> How do I submit such a patch? 60 patches may be too much for such a
> relatively simple change, but, on the other hand, everything in a single
> patch may be unhandy as well.
> 
> How about splitting it into 5 patches?
> 
> - classes (7 files)
> - contrib (4 files)
> - recipes/*/*.bb (24 files)
> - recipes/*/*.inc (14 files)
> - other files in recipes except recipes/obsolete (11 files)
> 
> Any opinions?
> 
> Regards,
> Andreas
> 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel

Re: Use of == in shell scripts

[Chris Larson]

On Tue, Oct 26, 2010 at 8:51 AM, Maupin, Chase <chase.maupin@ti.com> wrote:

> > -----Original Message-----
> > From: openembedded-devel-bounces@lists.openembedded.org
> > [mailto:openembedded-devel-bounces@lists.openembedded.org] On Behalf Of
> > Andreas Oberritter
> > Sent: Tuesday, October 26, 2010 10:31 AM
> > To: openembedded-devel@lists.openembedded.org
> > Subject: [oe] Use of == in shell scripts
> >
> > Dear all,
> >
> > I noticed a common mistake in various OE recipes, classes and contrib
> > scripts: Instead of =, == is used as equality operator, in at least 91
> > places in 64 files (without recipes/obsolete: 83 in 60 files). This
> > isn't a problem with bash, but it isn't the standard syntax and at least
> > dash does not support it.
>
> Isn't bash the supported shell?  If I remember correctly in bash "==" will
> also work with numbers whereas "=" is only for strings.


For now, it's required, but not by our choice.  We were forced to it due to
portability issues with scripts in the buildsystems of things we build.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Re: Use of == in shell scripts

[Andreas Oberritter]

On 10/26/2010 05:51 PM, Maupin, Chase wrote:
> Isn't bash the supported shell?

Yes, at least on the build system. Some of the affected files are
executed on the target.

I understand that bash is a requirement for now, but if possible, that
should be changed. I don't think that root access to a machine should be
necessary to build OE on Ubuntu (or maybe even on BSD flavors). Does
Darwin come with bash?

Btw., I discovered the first occurence of this problem with bash on
Debian lenny, where wpa-supplicant unconditionally enabled madwifi
support, because == was used. Maybe under some circumstances the
bash-builtins of '[' and 'test' are not used.

> If I remember correctly in bash "==" will also work with numbers whereas "=" is only for strings.

Both bash and dash on my system handle "[ "0" = 0 ]" or [ "0" = 1 ]
correctly. Another operator "-eq" exists, which is only for numbers.

Regards,
Andreas

Re: Use of == in shell scripts

[Michael Smith]

Andreas Oberritter wrote:
> On 10/26/2010 05:51 PM, Maupin, Chase wrote:

> I understand that bash is a requirement for now, but if possible, that
> should be changed. I don't think that root access to a machine should be
> necessary to build OE on Ubuntu (or maybe even on BSD flavors).

Well, maybe Ubuntu should be changed to use bash :)

>> If I remember correctly in bash "==" will also work with numbers whereas "=" is only for strings.
> 
> Both bash and dash on my system handle "[ "0" = 0 ]" or [ "0" = 1 ]
> correctly. Another operator "-eq" exists, which is only for numbers.

 From what I can see, it looks like = and == are equivalent in bash, so 
we should use = so we're not tied to bash.

-eq is for numeric comparison. For example, [ 01 -eq 1 ] but not
[ "01" = "1" ].

Mike