Re: Yet another patch + how to check for stuff in shell scripts
Russ Allbery <rra@debian.org> writes:
> --- a/checks/scripts
> +++ b/checks/scripts
> @@ -697,9 +697,12 @@ while (<SCRIPTS>) {
> $line =~ s/(^|[^\\\'](?:\\\\)*)\"(?:\\.|[^\\\"])+\"/$1""/g;
> for my $re (@bashism_regexs) {
> if ($line =~ m/($re)/) {
> - $found = 1;
> ($match) = m/($re)/;
> - last;
> + unless ($match =~ m/^\s*time\s*$/ &&
> + Dep::implies($deps{depends}, Dep::parse('time'))) {
> + $found = 1;
> + last;
> + }
> }
> }
> }
>
> This is surely wrong. It gives a free pass on any bashisms in a line
> *ending* in "time" if there's a dependency on time.
Oh, no, I see now. You're matching inside the bashism match. Hm, yeah,
that will probably work, although it feels potentially fragile... but I
can't imagine any other bashism that would match that regex.
However, I do think the code would be cleaner if that were pulled out into
a separate check. The Dep::implies just seems to be in an odd location
doing it that way, and accumulating exceptions inside that loop feels like
the wrong direction to go for long-term code maintainability.
--
Russ Allbery (rra@debian.org) <http://www.eyrie.org/~eagle/>
Reply to: