Bug#464026: [checks/scripts] Sync bashism tests with checkbashisms
Adam D. Barratt [EMAIL PROTECTED] writes: As discussed recently, please find attached a patch (against lintian and devscripts SVN) to checks/scripts that adds the bashism tests that devscripts's checkbashisms performs but lintian does not. Thank you! You beat me to doing the merger. One question, though: + 'echo\s+-[e]', # echo -e + 'exec\s+-[acl]', # exec -c/-l/-a name + '\blet\s', # let ... Shouldn't these be prefixed with (^|\s+) instead of \b or nothing to not catch things like echo echo -e (as odd as that might be)? We do run the risk of missing cases where people put the statement on the same line as an if statement or the like, but I think it makes sense to err on the side of conservatism. -- Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#464026: [checks/scripts] Sync bashism tests with checkbashisms
Package: lintian
Version: 1.2.42
Tags: patch
X-Debbugs-Cc: [EMAIL PROTECTED]
Hi,
As discussed recently, please find attached a patch (against lintian and
devscripts SVN) to checks/scripts that adds the bashism tests that
devscripts's checkbashisms performs but lintian does not.
The only test I've not included is that for echo -n. That test is
optional in checkbashisms as Policy requires that it be supported.
Regards,
Adam
--- checks/scripts.orig 2008-02-04 18:48:25.0 +
+++ checks/scripts 2008-02-04 19:05:57.0 +
@@ -530,6 +530,9 @@
'\$\{\w+\:\d+(?::\d+)?\}', # ${foo:3[:1]}
'\$\{\w+(/.+?){1,2}\}', # ${parm/?/pat[/str]}
'[EMAIL PROTECTED]',# bash arrays, ${name[0|*|@]}
+ '[EMAIL PROTECTED]', # ${!prefix[*|@]}
+ '\$\{!\w+\}',# ${!name}
+ '(\$\(|\`)\s*\\s*\S.+(\)|\`)',# $(\ foo)' should be '$(cat foo)'
);
my @bashism_regexs = (
'function \w+\(\s*\)', # function is useless
@@ -544,6 +547,14 @@
'(?:^|\s+)trap\s+[\']?.*[\']?\s+.*[1-9]', # trap with signal numbers
'\', # cshism
'\[\[(?!:)', # alternative test command
+ '(?:^|\s+)select\s+\w+', # 'select' is not POSIX
+ '\$\(\([A-Za-z]',# cnt=$((cnt + 1)) does not work in dash
+ 'echo\s+-[e]', # echo -e
+ 'exec\s+-[acl]', # exec -c/-l/-a name
+ '\blet\s', # let ...
+ '\$RANDOM\b',# $RANDOM
+ '(?!\$)\(\(', # '((' should be '$(('
+ '(\[|test)\s+-a',# test with unary -a (should be -e)
);
# since this test is ugly, I have to do it by itself
Bug#464026: [checks/scripts] Sync bashism tests with checkbashisms
Adam D. Barratt [EMAIL PROTECTED] writes: The first two certainly should be prefixed by something to avoid them matching strings that happen to end in echo -e. I've updated checkbashisms to do so for all three. As far as I can see that won't stop echo echo -e matching, however. I was proposing using: '(?:^|\s+)echo\s+-e',# echo -e '(?:^|\s+)exec\s+-[acl]',# exec -c/-l/-a name '(?:^|\s+)let\s',# let ... -- Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#464026: [checks/scripts] Sync bashism tests with checkbashisms
On Mon, 2008-02-04 at 14:50 -0800, Russ Allbery wrote: Adam D. Barratt [EMAIL PROTECTED] writes: As discussed recently, please find attached a patch (against lintian and devscripts SVN) to checks/scripts that adds the bashism tests that devscripts's checkbashisms performs but lintian does not. Thank you! You beat me to doing the merger. One question, though: + 'echo\s+-[e]', # echo -e + 'exec\s+-[acl]', # exec -c/-l/-a name + '\blet\s', # let ... Shouldn't these be prefixed with (^|\s+) instead of \b or nothing to not catch things like echo echo -e (as odd as that might be)? The first two certainly should be prefixed by something to avoid them matching strings that happen to end in echo -e. I've updated checkbashisms to do so for all three. As far as I can see that won't stop echo echo -e matching, however. Adam -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#464026: [checks/scripts] Sync bashism tests with checkbashisms
On Mon, 2008-02-04 at 16:06 -0800, Russ Allbery wrote: Adam D. Barratt [EMAIL PROTECTED] writes: The first two certainly should be prefixed by something to avoid them matching strings that happen to end in echo -e. I've updated checkbashisms to do so for all three. As far as I can see that won't stop echo echo -e matching, however. I was proposing using: '(?:^|\s+)echo\s+-e',# echo -e '(?:^|\s+)exec\s+-[acl]',# exec -c/-l/-a name '(?:^|\s+)let\s',# let ... That's what I've changed checkbashsims to use. Unless I'm missing something (which I'm more than happy to have pointed out :) echo echo -e matches \s+echo\s+-e as the expression isn't (in that case) anchored to the start of the line. Specifically: [EMAIL PROTECTED]:~/debian/packages/devscripts/svn/trunk$ cat echo.sh #!/bin/sh echo echo -e [EMAIL PROTECTED]:~/debian/packages/devscripts/svn/trunk$ scripts/checkbashisms.pl echo.sh possible bashism in echo.sh line 3 (echo -e): echo echo -e Adam -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#464026: [checks/scripts] Sync bashism tests with checkbashisms
Adam D. Barratt [EMAIL PROTECTED] writes: That's what I've changed checkbashsims to use. Unless I'm missing something (which I'm more than happy to have pointed out :) echo echo -e matches \s+echo\s+-e as the expression isn't (in that case) anchored to the start of the line. Oh, d'oh, you're entirely correct. Sorry about that. I misread that regex. Hm, that's more aggressive than I expected, but since I've not gotten any bug reports, I guess I'll leave it and see if anyone complains. -- Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
