[PATCH] compat-2.6: Makefile: fixed test expressions for target install

[PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Joerg Albert]

This removes the two errors of [ with target "install"

make[1]: Leaving directory `/home/joerg/src/linux-2.6.30'
[: 9: missing ]
[: 9: missing ]
depmod will prefer updates/ over kernel/ -- OK!


Signed-off-by: Joerg Albert <jal2@gmx.de>
---
  Makefile |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 62eb5db..90279c9 100644
--- a/Makefile
+++ b/Makefile
@@ -80,7 +80,7 @@ install-scripts:
         @install scripts/athload        $(DESTDIR)/usr/sbin/
         @install scripts/b43load        $(DESTDIR)/usr/sbin/
         @install scripts/iwl-load       $(DESTDIR)/usr/sbin/
-       @if [ ! -z $(MADWIFI) && -z "$(DESTDIR)" ]; then \
+       @if [ ! -z "$(MADWIFI)" -a -z "$(DESTDIR)" ]; then \
                 echo ;\
                 echo -n "Note: madwifi detected, we're going to disable it. "  ;\
                 echo "If you would like to enable it later you can run:"  ;\
@@ -89,7 +89,7 @@ install-scripts:
                 echo Running athenable ath5k...;\
                 /usr/sbin/athenable ath5k ;\
         fi
-       @if [ ! -z $(OLD_IWL) && -z "$(DESTDIR)" ]; then \
+       @if [ ! -z "$(OLD_IWL)" -a -z "$(DESTDIR)" ]; then \
                 echo ;\
                 echo -n "Note: iwl4965 detected, we're going to disable it. "  ;\
                 echo "If you would like to enable it later you can run:"  ;\
-- 
1.6.0.4

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Pavel Roskin]

On Thu, 2009-07-30 at 23:25 +0200, Joerg Albert wrote:
> This removes the two errors of [ with target "install"
> 
> make[1]: Leaving directory `/home/joerg/src/linux-2.6.30'
> [: 9: missing ]
> [: 9: missing ]
> depmod will prefer updates/ over kernel/ -- OK!

I believe "-a" in test is not very portable.  I remember getting
complaints about it.  I believe the built-in test command in bash 1.x
doesn't have it.  I'd rather stick with && and || written properly.

"-a" and -o" are currently only used in the clean target and in the
maintenance scripts, so they probably don't get enough testing on
systems with old bash.

-- 
Regards,
Pavel Roskin

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Joerg Albert]

On 07/30/2009 11:42 PM, Pavel Roskin wrote:
> On Thu, 2009-07-30 at 23:25 +0200, Joerg Albert wrote:
>> This removes the two errors of [ with target "install"
>>
>> make[1]: Leaving directory `/home/joerg/src/linux-2.6.30'
>> [: 9: missing ]
>> [: 9: missing ]
>> depmod will prefer updates/ over kernel/ -- OK!
> 
> I believe "-a" in test is not very portable.  I remember getting
> complaints about it.  I believe the built-in test command in bash 1.x
> doesn't have it.  I'd rather stick with && and || written properly.
> 
> "-a" and -o" are currently only used in the clean target and in the
> maintenance scripts, so they probably don't get enough testing on
> systems with old bash.

Bash 3.2.39 seems to have a problem with &&, while [[ ... ]] accept it:

joerg@thinkpad:~$ echo $BASH_VERSION
3.2.39(1)-release
joerg@thinkpad:~$ if [ -z "" && -z "" ]; then echo "both empty"; fi
bash: [: missing `]'
joerg@thinkpad:~$ if [ -z "" -a -z "" ]; then echo "both empty"; fi
both empty
joerg@thinkpad:~$ if [[ -z "" && -z "" ]]; then echo "both empty"; fi
both empty
joerg@thinkpad:~$

If the link for the man page of bash v1 on http://wwwbs.informatik.htw-dresden.de/fbs/bash/old.bash.html is correct,
that version supported -a in test.
Unfortunately [[ ... ]] was introduced after bash v1 (2.02 AFAIR).

Regards,
Joerg.

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Pavel Roskin]

On Fri, 2009-07-31 at 20:50 +0200, Joerg Albert wrote:

> joerg@thinkpad:~$ echo $BASH_VERSION
> 3.2.39(1)-release
> joerg@thinkpad:~$ if [ -z "" && -z "" ]; then echo "both empty"; fi
> bash: [: missing `]'
> joerg@thinkpad:~$ if [ -z "" -a -z "" ]; then echo "both empty"; fi
> both empty
> joerg@thinkpad:~$ if [[ -z "" && -z "" ]]; then echo "both empty"; fi
> both empty
> joerg@thinkpad:~$

No, I didn't mean that.  I meant:

if [ -z "" ] && [ -z "" ]; then echo "both empty"; fi

> If the link for the man page of bash v1 on http://wwwbs.informatik.htw-dresden.de/fbs/bash/old.bash.html is correct,
> that version supported -a in test.
> Unfortunately [[ ... ]] was introduced after bash v1 (2.02 AFAIR).

We should not rely on any bash features as /bin/sh may not be bash at
all.

-- 
Regards,
Pavel Roskin

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Luis R. Rodriguez]

On Fri, Jul 31, 2009 at 11:56 AM, Pavel Roskin<proski@gnu.org> wrote:
> On Fri, 2009-07-31 at 20:50 +0200, Joerg Albert wrote:
>
>> joerg@thinkpad:~$ echo $BASH_VERSION
>> 3.2.39(1)-release
>> joerg@thinkpad:~$ if [ -z "" && -z "" ]; then echo "both empty"; fi
>> bash: [: missing `]'
>> joerg@thinkpad:~$ if [ -z "" -a -z "" ]; then echo "both empty"; fi
>> both empty
>> joerg@thinkpad:~$ if [[ -z "" && -z "" ]]; then echo "both empty"; fi
>> both empty
>> joerg@thinkpad:~$
>
> No, I didn't mean that.  I meant:
>
> if [ -z "" ] && [ -z "" ]; then echo "both empty"; fi
>
>> If the link for the man page of bash v1 on http://wwwbs.informatik.htw-dresden.de/fbs/bash/old.bash.html is correct,
>> that version supported -a in test.
>> Unfortunately [[ ... ]] was introduced after bash v1 (2.02 AFAIR).
>
> We should not rely on any bash features as /bin/sh may not be bash at
> all.

compat-wireless depends on bash, hence /bin/bash at the top.

  Luis

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Joerg Albert]

On 07/31/2009 08:56 PM, Pavel Roskin wrote:

> No, I didn't mean that.  I meant:
> 
> if [ -z "" ] && [ -z "" ]; then echo "both empty"; fi
> ...
> We should not rely on any bash features as /bin/sh may not be bash at
> all.

Agreed. Will repost the patch.

[PATCH v2] compat-2.6: Makefile: fixed test expressions for target install

[Joerg Albert]

This removes the two errors of [ with target "install"

[: 9: missing ]
[: 9: missing ]

and works with sh as well.

Signed-off-by: Joerg Albert <jal2@gmx.de>
---
  Makefile |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 62eb5db..e1ad666 100644
--- a/Makefile
+++ b/Makefile
@@ -80,7 +80,7 @@ install-scripts:
         @install scripts/athload        $(DESTDIR)/usr/sbin/
         @install scripts/b43load        $(DESTDIR)/usr/sbin/
         @install scripts/iwl-load       $(DESTDIR)/usr/sbin/
-       @if [ ! -z $(MADWIFI) && -z "$(DESTDIR)" ]; then \
+       @if [ ! -z "$(MADWIFI)" ] && [ -z "$(DESTDIR)" ]; then \
                 echo ;\
                 echo -n "Note: madwifi detected, we're going to disable it. "  ;\
                 echo "If you would like to enable it later you can run:"  ;\
@@ -89,7 +89,7 @@ install-scripts:
                 echo Running athenable ath5k...;\
                 /usr/sbin/athenable ath5k ;\
         fi
-       @if [ ! -z $(OLD_IWL) && -z "$(DESTDIR)" ]; then \
+       @if [ ! -z "$(OLD_IWL)" ] && [ -z "$(DESTDIR)" ]; then \
                 echo ;\
                 echo -n "Note: iwl4965 detected, we're going to disable it. "  ;\
                 echo "If you would like to enable it later you can run:"  ;\
-- 
1.6.0.4

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Luis R. Rodriguez]

On Fri, Jul 31, 2009 at 4:15 PM, Joerg Albert<jal2@gmx.de> wrote:
> On 07/31/2009 08:56 PM, Pavel Roskin wrote:
>
>> No, I didn't mean that.  I meant:
>>
>> if [ -z "" ] && [ -z "" ]; then echo "both empty"; fi
>> ...
>> We should not rely on any bash features as /bin/sh may not be bash at
>> all.
>
> Agreed. Will repost the patch.

Actually please do keep relying on /bin/bash, all the scripts on
compat-wireless do depend on /bin/bash, I don't expect users of
dash/etc to use compat-wireless.

If we want to make compat-wireless be shell agnostic we'd need to
address all the other scripts. I rather not deal with that now unless
we really think that is also a good idea and someone is up for the
task.

  Luis

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Joerg Albert]

On 08/01/2009 01:22 AM, Luis R. Rodriguez wrote:
> Actually please do keep relying on /bin/bash, all the scripts on
> compat-wireless do depend on /bin/bash, I don't expect users of
> dash/etc to use compat-wireless.
> 
> If we want to make compat-wireless be shell agnostic we'd need to
> address all the other scripts. I rather not deal with that now unless
> we really think that is also a good idea and someone is up for the
> task.

IMHO we could make bug fixes to the Makefile/scripts shell agnostic if it's not much effort and
doesn't break readability, which is neither the case here.

Regards,
Joerg.

Re: [PATCH] compat-2.6: Makefile: fixed test expressions for target install

[Luis R. Rodriguez]

On Sat, Aug 1, 2009 at 3:39 AM, Joerg Albert<jal2@gmx.de> wrote:
> On 08/01/2009 01:22 AM, Luis R. Rodriguez wrote:
>>
>> Actually please do keep relying on /bin/bash, all the scripts on
>> compat-wireless do depend on /bin/bash, I don't expect users of
>> dash/etc to use compat-wireless.
>>
>> If we want to make compat-wireless be shell agnostic we'd need to
>> address all the other scripts. I rather not deal with that now unless
>> we really think that is also a good idea and someone is up for the
>> task.
>
> IMHO we could make bug fixes to the Makefile/scripts shell agnostic if it's
> not much effort and
> doesn't break readability, which is neither the case here.

Applied, thanks.

  Luis