On 04/29/2014 02:20 PM, Lluís Vilanova wrote: > Markus Armbruster writes: > >> Lluís Vilanova <vilan...@ac.upc.edu> writes: >>> Use an explicit input file on the command-line instead of reading from >>> standard input > >> Please limit commit message line length to 70 characters. > >> Worth mentioning that this commit improves error messages! > >> <stdin>:123: Borked > >> becomes > >> qapi-schema.json:123: Borked > >> which enables Emacs to jump to the error.
>>> @diff -q $(SRC_PATH)/$*.out $*.test.out >>> - @diff -q $(SRC_PATH)/$*.err $*.test.err >>> + @# Sanitize error messages (make them independent of build directory) >>> + @sed 's|$(SRC_PATH)/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - >>> @diff -q $(SRC_PATH)/$*.exit $*.test.exit >>> > >> Breaks when $(SRC_PATH) interpreted as regexp matches anything but a >> prefix of the source file part. In particular, it breaks when SRC_PATH >> is ".." (which is common) and the error message contains "/". > >> Here's a safer version: > >> @perl -p -e 's|^\Q$(SRC_PATH)\E/||' $*.test.err | diff -q >> $(SRC_PATH)/$*.err - I guess there's other ways to avoid the problems without using perl (such as sanitizing all metacharacters before passing a munged $(SRC_PATH) to sed), but it gets hairy. > > Not a perl fan, so I'll assume your line as correct. Also, can perl be assumed > as present? Makefile already uses perl to generate documentation from .pod files; although this appears to be the first use of perl in the testsuite. > >> This one reports missing input file name argument as "IndexError: list >> index out of range". Again, fits right in. > >> [...] > > AFAIR, Eric said that it'd be better to leave it as simple as possible, since > no > one else uses this script. > > Same applies to the argument parsing of the other scripts (since no one is > sufficiently annoyed to rewrite it with argparse or similar). Careful - I think that keeping _this patch_ simple is okay, but I would also _welcome_ further patches in this (or other) series that further improves the error message quality and consistency when using the various .py generators from the command line. I think the point we are making here is that your code has very ugly inconsistent results when detecting errors across the various scripts, but that 1) it's no uglier than pre-patch, and 2) in the normal case, 'make' and 'make check' will not trigger these ugly error paths. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
