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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list