compat-2.6: Makefile: fixed test expressions for target install

PATCH Applied Joerg Albert 3 participants 10 messages Jul 30, 2009 – Aug 5, 2009 Kbuild

lore

Share & tools

lore

Joerg Albert

author

2009-07-30 21:25 UTC #0 lore raw

PATCH 1/1 compat-2.6: Makefile: fixed test expressions for target install

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 <[email protected]>

---
 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Makefile +2 -2

index 62eb5db..90279c9 100644
@@ -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:"  ;\

Signature · 1 line

1.6.0.4

Pavel Roskin

2009-07-30 21:42 UTC #1 lore raw

Quoted 6 lines · 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!

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.

Signature · 2 lines

Regards,
Pavel Roskin

Joerg Albert

author

2009-07-31 18:50 UTC #2 lore raw

Quoted 8 lines · Pavel Roskin

Quoted 6 lines · 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!

Quoted 7 lines

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.

Pavel Roskin

2009-07-31 18:56 UTC #3 lore raw

Quoted 9 lines · Joerg Albert

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

Quoted 3 lines

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.

Signature · 2 lines

Regards,
Pavel Roskin

Luis R. Rodriguez

2009-07-31 19:04 UTC #4 lore raw

Quoted 14 lines · Pavel Roskin

Quoted 9 lines · Joerg Albert

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:

Quoted 8 lines

if [ -z "" ] && [ -z "" ]; then echo "both empty"; fi
Quoted 3 lines
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

Joerg Albert

author

2009-07-31 23:15 UTC #5 lore raw

In reply to Pavel Roskin

Quoted 6 lines · Pavel Roskin

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.

Luis R. Rodriguez

2009-07-31 23:22 UTC #6 lore raw

Quoted 10 lines · Joerg Albert

Quoted 6 lines · Pavel Roskin
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

Joerg Albert

author

2009-08-01 10:39 UTC #7 lore raw

Quoted 8 lines · Luis R. Rodriguez

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.

Luis R. Rodriguez

Applied

2009-08-05 18:32 UTC #8 lore raw

Quoted 6 lines · Joerg Albert

Quoted 5 lines · Luis R. Rodriguez
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.

Quoted 8 lines

Quoted 4 lines
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

Joerg Albert

author

2009-07-31 23:21 UTC #9 lore raw

PATCH v2 1/1 compat-2.6: Makefile: fixed test expressions for target install

In reply to Pavel Roskin

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

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

and works with sh as well.

Signed-off-by Joerg Albert <[email protected]>

---
 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Makefile +2 -2

index 62eb5db..e1ad666 100644
@@ -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:"  ;\

Signature · 1 line

1.6.0.4