Bug#691389: [checkbashisms] multiple bug fixes and new checks

[Raphael Geissert]

Hi,

On Thursday 25 October 2012 16:44:03 Benjamin Drung wrote:
> Am Mittwoch, den 24.10.2012, 21:15 -0500 schrieb Raphael Geissert:
> > Attached is an mbox with a bunch of bug fixes and new checks, #687450
> > included.
> 
> Thanks. I have applied your patches 1 up to 14. I grabbed the working
> test cases from your git repository and added them to devscripts. After
> applying a patch, I added the newly passing test cases. Feel free to add
> more test cases to devscripts or to improve/simplify the shunit2 tests.

Thanks.

For now I'm going to continue using my test suite, it: has more test cases, 
allows TODO items, and allows two versions of checkbashisms to be compared.
I like the output-comparison approach of the shunit-based suite, but it is 
annoying to use while developing.

> Patch 15 refers to commit 5dc48224, but the commit hashes do not survive
> git format-patch & git am. To which commit do you refer?

Patch 15 refers to patch 14.
(the commit hash _is_ included in the git format-patch output.)

> Please do not sign off your own patches. Your patches will be signed off
> by the person that applies your patches.

Perhaps we differ on what the sign off means, and that shouldn't cause any 
issue for another person to sign off them as well.

> You can send your patches directly to the mailing list if have more than
> one instead of opening a new bug report.
> 
> Can you add a commit with your changelog entries?

Since doing that oftentimes leads to merge conflicts it is easier if the 
committer adds it after merging, and on whatever branch those changes are 
merged.

> > On IRC I mentioned a "regression" when checking autoconf, but the issue
> > is now visible thanks to some bug fixes. In one configure script the
> > bug fix actually revealed a bashism.
> 
> Have you created a test case for this regression?

Not per se, I've only copied autoconf(1) to my test suite. 

It all comes down to fixing the parsing of things like
printf '#!/bin/sh\nbar="moo"; foo="foo \"bar\" ... \nmoo"\n'

And this (which is what actually triggers the 'error' in the autoconf case):
checkbashisms <<'EOF'
case foo in
*\'*) arg=`$as_echo "$1" | sed "s/'/'\\\\\\\\''/g"` ;; #'
esac
EOF

(this actually made me notice that the error messages aren't displaying 
'(stdin)', so attache patch fixes that.)

Cheers,
-- 
Raphael Geissert - Debian Developer
www.debian.org - get.debian.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-checkbashisms-display-the-fancy-filename-in-error-me.patch
Type: text/x-patch
Size: 1402 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/devscripts-devel/attachments/20121025/c51140f1/attachment-0001.bin>