Re: Yet another patch + how to check for stuff in shell scripts
Raphael Geissert <atomo64+debian@gmail.com> writes:
> Besides that patch, I'd like to know the opinion of others about how
> shell scripts should be checked. At the moment checks/scripts now makes
> use of $LEADIN as a separator/command identifier, but other checks also
> checking for stuff in shell script don't.
>
> This caused me troubles when I prefixed the invoke-rc.d calls in
> testset/scripts/debian/postinst with 'time'. Doing that caused most, if
> not all, of checks/init.d results to be completely incorrect because it
> failed to detect the invoke-rc.d calls.
Given that running invoke-rc.d under time in a Debian maintainer script
doesn't seem like sane behavior, I don't think I have a problem with
that specific case. However, in general, there are a lot of shell script
checks in Lintian that try to figure out what's being run, some of which
dealing with conditionals, and they're not done in a consistent way.
I would really like to see this solved by taking a higher-level look at
shell parsing and writing a module that has some deeper understanding of
conditionals or at least a structure where we can apply that. Just
applying $LEADIN is going to be wrong in a lot of cases. I think we need
a documented module with a clearer picture of what it can and can't do
which understands some of the common conditional constructs, such as
checking whether a command exists, and tracks things like if/fi nesting.
> Although that problem could not be avoided just by using $LEADIN
> (because 'time' is not in the list of separators, so someone please add
> it) similar problems can.
I don't think it makes sense to add time to $LEADIN in general, since time
changes the nature of the command similar to adding env in front of
something; it forces the command to be external rather than a built-in,
which means that many of the things we check for become okay, such as echo
with flags or the full syntax of kill.
--- 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.
I think to fix this properly you'd need to pull time out of the bashism
regexes and check for it independently.
--
Russ Allbery (rra@debian.org) <http://www.eyrie.org/~eagle/>
Reply to: