Re: [lmi] git: committing content and mode changes together

[Vadim Zeitlin]

On Fri, 4 Nov 2016 22:42:49 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2016-11-04 21:28, Vadim Zeitlin wrote:
...
GC> >  AFAICS the only way to customize the hooks with this approach is to avoid
GC> > symlinking them for (2) and (3) or override hooksPath (if only to its
GC> > default value) with (1). Neither is particularly bad and I'll probably 
just
GC> > have .git/hooks/pre-commit calling hooks/pre-commit and then doing its own
GC> > thing (or vice versa), but it doesn't seem ideal neither. Unfortunately I
GC> > don't see any simple way to make it better, the only thing I could
GC> > propose would be to look for .git/hooks/$hook_name.local and execute it if
GC> > it's present, but I'm not even sure myself if it's worth the extra
GC> > complexity and even less sure that I'm going to be able to convince you
GC> > that it is.
GC> 
GC> I saw a related idea (with $hook.local scripts) here:
GC>   
http://stackoverflow.com/questions/3462955/putting-git-hooks-into-repository/3464399#3464399
GC> but it looked way too complicated.

 It's a bit more complicated than what I was thinking of, but this is
indeed the same idea. Anyhow, as I said, I agree that it's not worth it for
our use.

GC> The pre-commit hook you shared with me included:
GC>   # Hook verifying that "FIXME-VZ" markers are never committed.
GC> and, if you like, I'd be happy to add that to 'test_coding_rules.cpp', e.g.:
GC> 
GC>      // Former addresses of the Free Software Foundation.
GC>      taboo(f, "Cambridge");
GC>      taboo(f, "Temple");
GC> +    // Vadim's personal commit-me-not marker.
GC> +    taboo(f, "FIXME-VZ");

 This would seem to be, to steal a word from you, inelegant and I think it
would be better for me to just do whatever I need here.

GC> >  If you'd like to check for the file mode here, you have several
GC> > possibilities to do it and there must be a simpler way to do it locally,
GC> > but I'm not sure what it is.
GC> 
GC> That's already done in the 'check_concinnity' target:
GC> 
GC>     @$(LS) --classify ./* \
GC>       | $(SED) -e'/\*$$/!d' -e'/^\.\//!d' -e'/.sh\*$$/d' -e'/.sed\*$$/d' \
GC>       | $(SED) -e's/^/Improperly executable: /'

 Ah, sorry, I've missed this. But, to be pedantically precise, this isn't
quite the right check as you could have executable file in the working copy
but be committing a non-executable file (which is not too bad as it would
just result in false positives) or also somehow end up with an executable
file in the index but non-executable one in the working copy (which would
be worse as it would commit the file with the wrong mode). Granted, I don't
see how could this happen accidentally, but if anything can possibly go
wrong, it will, won't it?

GC> As always, whenever we look into this target, we perceive ugliness, but it's
GC> a black hole that wants to suck all our time, so we must content ourselves
GC> with incremental progress. Forcing 'make check_concinnity' before each 
commit
GC> (via hooks/pre-commit) has a very favorable cost-benefit ratio.

 I do agree with this. For some new projects I'm setting up recently I'm
even experimenting with automatically running clang-tidy to detect problems
in the code being checked in and, at least for small projects, it really
works wonderfully well (but I'm not sure how well does it scale).

 Regards,
VZ