Re: [PATCH] post-checkout hook, and related docs and tests

From: Josh England <jjengla@sandia.gov>
Date: Sep 26, 2007, 19:23 UTC
Message-ID: <1190834633.6078.139.camel@beauty>
In-Reply-To: <20070926145229.GA15300@potapov>

On Wed, 2007-09-26 at 18:52 +0400, Dmitry Potapov wrote:

Show 22 quoted lines

> On Mon, Sep 24, 2007 at 02:07:36PM -0700, Junio C Hamano wrote:
> > "Josh England" <jjengla@sandia.gov> writes:
> > 
> > > ...  Granted, the
> > > branch (and HEAD) does not change for this operation, but that shouldn't
> > > matter.  It is somewhat in line with the principle of 'least-surprise':
> > > if the hook runs for 'git checkout otherbranch', but not 'git checkout
> > > otherbranch path.c', this could cause confusion and distress to the
> > > user.  IMO, it is a 'checkout' so the post-checkout hook should run.
> > > Why is that so insane?  
> > 
> > Because I find it would be surprising if the following commands
> > behave differently:
> > 
> > 	$ git cat-file blob otherbranch:path.c >path.c
> >         $ git show otherbranch:path.c >path.c
> >         $ git diff -R otherbranch path.c | git apply
> >         $ git checkout otherbranch path.c
> 
> Actually, they already act differently even without any hook.
> If path.c is a symbol link then 1 and 2 will give a different
> result than commands 3 and 4.

Moroever, with respect to permissions, the first 2 retain the permissions of the file if it already exists in the worktree, whereas the other variations actually recreate the file with a default umask and wipe out existing permissions.

Show 9 quoted lines

> On the other hand, while the difference in above commands
> understandable (in case 1 and 2, the shell creates path.c; and
> in 3 and 4, git creates it), I really dislike the idea of 
> "checkout is magical." I believe that command 3 and 4 should
> always give the same result or Git is broken.
> 
> Another reason, why I dislike the post-checkout hook is that it
> is prone to abuse like as not so smart user trying to put some
> content modification here.

Content modification is not among the intended uses for this hook. Any and all hooks can be abused/misused in this way. I just want to give the user a tool -- if he wants to hit himself in the face with it that's his prerogative.

Show 5 quoted lines

>  Moreover, it appears to be excessive
> to me, because if you want to run something after git-checkout,
> you can write a simple shell script for that that first runs
> git-checkout with the given arguments and then run whatever you
> want. I don't see why we should modify Git for that.

The same could be said for pre-commit and other hooks. The whole reason for the hook system is to provide a useful interface so that users are *not* required to write their own wrapper scripts to get the job done. In this case, providing the hook is by far the *more* consistent way of doing things.

Show 17 quoted lines

> Perhaps, it would be better to have a hook on modification,
> which is invoked every time when Git wants to try to change
> anything in the working directory. The hook could receives on
> the input something that looks like 'git-diff --name-status'
> output and can do any work on creation files, etc. It is much
> more flexible, because you can do additional stuff here like
> creating one directory in the path as a symbol link somewhere
> else or something like that. But what is much more important
> is that everything work _consistently_ and you get the same
> results whether you type:
> git diff -R otherbranch path.c | git apply
> or
> git checkout otherbranch path.c
> 
> If you start with one "magical interface" then eventually you
> will end up with everything being so magical that no one can
> make sense of it. Please, stay consistent.

I don't know why you think this is so magical. git-checkout can run a post-checkout hook, if enabled. Plain and simple. No magic here. As for the universal 'worktree-updated' hook, I look forward to seeing a sane implementation, but in the meantime post-merge and post-checkout suit my needs just fine.

-JE