Re: [libvirt] consistency: push "update" hook vs. "make syntax-check"
On Wed, Jul 15, 2009 at 11:51:41PM +0200, Jim Meyering wrote:
Currently the server side hook that runs "git diff
--check"
to prevent pushing a change that adds trailing blanks is
more strict than our "make syntax-check" hook, since the former
rejects any change that adds blank lines at the end of a file,
while "make syntax-check" doesn't complain about that.
Yes, I have been bitten by this yesterday, having this
raised at push time when everything has been commited is
really inconvenient, especially when pushing someone else
patches.
The two should be consistent.
One way is to make "make syntax-check" more strict.
If we were to do that, we'd have to choose between
cleaning existing files and exempting them from the new test.
Cleaning is easy and doesn't impact tests at all, so I prefer it.
I tend to think extra trailing lines at the end of a file
are not a problem, after all we don't complain for those between
2 functions, and that's semantically equivalent. But we need
to allow any patches passing "make syntax-check", since fixing this
server side is near impossible, I agree that adding the rule at
syntax-check time is the best way to solve the difference.
Here's what would be involved:
- remove 121 trailing newlines from 109 files by running this command:
git ls-files -z | xargs -0 perl -pi -0777 -e 's/\n\n+$/\n/'
Add a rule to cfg.mk so that "make syntax-check" warns about
any new violations. It might run something like this:
git ls-files -z \
| xargs -0 perl -ln -0777 -e '/\n(\n+)$/ and print "$ARGV: ".length
$1'
I would rather fix the current set of files to comply
as this would make the base content consistent with what is
allowed. One simple example is copying a file to create a new
one, this should not lead to problems.
I'm leaning towards the simplicity of the former, in spite of its
cost.
I'll bet someone can come up with a simple *and* efficient script
(probably using sed) to list files with one or more trailing blank line.
Even if not optimal, we already load the full code base in the
cache at "make syntax-check" time, so this will not make a big
difference IMHO, and going for the simpler sounds fine to me.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/