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

[Greg Chicares]

On 2016-11-04 21:28, Vadim Zeitlin wrote:
> On Fri, 4 Nov 2016 20:00:03 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-11-04 15:30, Vadim Zeitlin wrote:
> ...
> GC> >  Yes, having either a pre-commit or update (server-side) hook for 
> checking
> GC> > for the wrong permissions is definitely useful. I could send you the 
> update
> GC> > hook I use for this purpose
> GC> 
> GC> You shared it in personal email to me on 2016-03-15T23:39, thanks.
> 
>  Although I do admit that I completely forgot about having sent that
> pre-commit hook to you, it's not the one I had in mind here. I rather meant
> the update hook on the server preventing pushing executable text files to
> it accidentally.

Okay, I confused update with pre-commit hooks. But I want a mechanism that
also works with our proprietary repository, which has no server per se
(although each user has a "blessed" bare repo that functions much the same
way). And I really do want a pre-commit hook, because it's too easy to omit
running 'make check_concinnity' before every commit.

> GC> > if you'd like to configure it for Savannah repository.
> GC> 
> GC> Do you have a strong preference for the configuration method?
> 
>  Sorry if I'm missing anything, but isn't asking Savannah admins to
> manually put the hook into place the only configuration method that works
> for Savannah? AFAICS for server-side hooks, there is nothing else that can
> be done anyhow.

Indeed. Again, I confused update with pre-commit hooks. What I want is a
pre-commit hook.

>  As for the local (e.g. pre-commit) hooks, I don't have use any smart
> system for them because I often customize them locally anyhow

Okay, I don't want to prevent you from doing that.

> GC> Options:
> GC> 
> GC> (1) Specify 'core.hooksPath' in 'git config'. The problem is that this
> GC> requires git-2.9, which neither Kim nor I yet use.
> ...
> GC> (2) Symlink a hooks directory:
> GC> 
> GC>   cd $(git rev-parse --show-toplevel)
> GC>   mkdir hooks
> GC>   ln --symbolic $(git rev-parse --show-toplevel)/hooks $(git rev-parse 
> --show-toplevel)/.git/hooks
> ...
> GC> (3) Symlink only 'pre-commit' for the time being.
> ...
> GC> I think (2) is the clear winner.
> 
>  Yes, it looks like the best choice for now.

I will make it so.

> GC> And if you personally want to customize hooks in a different way, all
> GC> of these methods still allow that.
> 
>  AFAICS the only way to customize the hooks with this approach is to avoid
> symlinking them for (2) and (3) or override hooksPath (if only to its
> default value) with (1). Neither is particularly bad and I'll probably just
> have .git/hooks/pre-commit calling hooks/pre-commit and then doing its own
> thing (or vice versa), but it doesn't seem ideal neither. Unfortunately I
> don't see any _simple_ way to make it better, the only thing I could
> propose would be to look for .git/hooks/$hook_name.local and execute it if
> it's present, but I'm not even sure myself if it's worth the extra
> complexity and even less sure that I'm going to be able to convince you
> that it is.

I saw a related idea (with $hook.local scripts) here:
  
http://stackoverflow.com/questions/3462955/putting-git-hooks-into-repository/3464399#3464399
but it looked way too complicated.

The pre-commit hook you shared with me included:
  # Hook verifying that "FIXME-VZ" markers are never committed.
and, if you like, I'd be happy to add that to 'test_coding_rules.cpp', e.g.:

     // Former addresses of the Free Software Foundation.
     taboo(f, "Cambridge");
     taboo(f, "Temple");
+    // Vadim's personal commit-me-not marker.
+    taboo(f, "FIXME-VZ");

If you desire further customization, couldn't you maintain a locally-modified
'lmi/hooks/pre-commit' as easily as you maintain your 
'lmi/.git/hooks/pre-commit'
today? or simply not symlink the hooks/ directory? I don't want to make things
(much) more difficult for you.

> GC> Here's my current 'pre-commit' script, just BTW:

[...it really just calls 'make check_concinnity'...]

>  If you'd like to check for the file mode here, you have several
> possibilities to do it and there must be a simpler way to do it locally,
> but I'm not sure what it is.

That's already done in the 'check_concinnity' target:

        @$(LS) --classify ./* \
          | $(SED) -e'/\*$$/!d' -e'/^\.\//!d' -e'/.sh\*$$/d' -e'/.sed\*$$/d' \
          | $(SED) -e's/^/Improperly executable: /'

I had to look that up: 'ls --classify' appends '*' to indicate executability,
so filtering out everything else and removing '.sh' and '.sed' scripts yields
a list of questionably-executable files...which is good enough for lmi (though
it rejects a script named 'pre-commit'). What I most want is to flag added
files that cygwin makes executable by default.

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