| Message ID | 20190115203038.10574-1-tboegi@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCH/RFC,v1,1/1] test-lint: sed -E (or -a, -l) are not portable | expand |
On Tue, Jan 15, 2019 at 3:31 PM <tboegi@web.de> wrote: > From `man sed` (on a Mac OS X box): > The -E, -a and -i options are non-standard FreeBSD extensions and may not be available > on other operating systems. > [...] > To be on the safe side, don't allow it. > > Signed-off-by: Torsten Bögershausen <tboegi@web.de> > --- > diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl > @@ -35,7 +35,7 @@ sub err { > - /\bsed\s+-i/ and err 'sed -i is not portable'; > + /\bsed\s+-[Eail]/ and err 'Not portable option with sed. Only -e -f -n are portable'; > /\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)'; > /^\s*declare\s+/ and err 'arrays/declare not portable'; > /^\s*[^#]\s*which\s/ and err 'which is not portable (use type)'; Please update the new message to be more consistent with existing surrounding error messages. For instance: err 'sed -i/-a/-l/-E not portable (use only -e/-f/-n)' or something. Thanks.
On Tue, Jan 15 2019, tboegi@web.de wrote: > From: Torsten Bögershausen <tboegi@web.de> > > From `man sed` (on a Mac OS X box): > The -E, -a and -i options are non-standard FreeBSD extensions and may not be available > on other operating systems. > > From `man sed` on a Linux box: > REGULAR EXPRESSIONS > POSIX.2 BREs should be supported, but they aren't completely because of > performance problems. The \n sequence in a regular expression matches > the newline character, and similarly for \a, \t, and other sequences. > The -E option switches to using extended regular expressions instead; > the -E option has been supported for years by GNU sed, and is now > included in POSIX. > > Well, there are still a lot of systems out there, which don't support it. > > Beside that, see IEEE Std 1003.1TM-2017 > http://pubs.opengroup.org/onlinepubs/9699919799/ > does not mention -E either. > > To be on the safe side, don't allow it. > > Reported-by: SZEDER Gábor <szeder.dev@gmail.com> > Signed-off-by: Torsten Bögershausen <tboegi@web.de> > --- > > I am somewhat unsure if we should disable all options except -e -f -n > instead ? > /\bsed\s+-[^efn]/ and err 'Not portable option with sed. Only -n -e -f are portable'; > > That would cause a false positive in t9001 here: > "--cc-cmd=./cccmd-sed --suppress-cc=self" > > which could either be fixed by an anchor: > /^\s*sed\s+-[^efn]/ > > Or by allowing '--' like this: > /\bsed\s+-[^-efn]/ > > Any thoughts, please ? > > t/check-non-portable-shell.pl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl > index b45bdac688..96b6afdeb8 100755 > --- a/t/check-non-portable-shell.pl > +++ b/t/check-non-portable-shell.pl > @@ -35,7 +35,7 @@ sub err { > chomp; > } > > - /\bsed\s+-i/ and err 'sed -i is not portable'; > + /\bsed\s+-[Eail]/ and err 'Not portable option with sed. Only -e -f -n are portable'; > /\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)'; > /^\s*declare\s+/ and err 'arrays/declare not portable'; > /^\s*[^#]\s*which\s/ and err 'which is not portable (use type)'; I'd just go for your /\bsed\s+-[^-efn]/ suggestion. Just a note if we do go for the whitelist: According to GNU sed's manpage -E is also known as -r, so /\bsed\s+-[Erail]/ would be better.
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl index b45bdac688..96b6afdeb8 100755 --- a/t/check-non-portable-shell.pl +++ b/t/check-non-portable-shell.pl @@ -35,7 +35,7 @@ sub err { chomp; } - /\bsed\s+-i/ and err 'sed -i is not portable'; + /\bsed\s+-[Eail]/ and err 'Not portable option with sed. Only -e -f -n are portable'; /\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)'; /^\s*declare\s+/ and err 'arrays/declare not portable'; /^\s*[^#]\s*which\s/ and err 'which is not portable (use type)';