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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2007-09-24 at 14:07 -0700, Junio C Hamano wrote:
> "Josh England" <jjengla@xxxxxxxxxx> 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

For all intents and purposes, these would still behave the same.  The
existence of a post-checkout hook does not at all affect the outcome of
the checkout.  Sure there is potential for someone to do something
stupid inside the hook script, but that is true of any hook.

Most git users would never even enable the hook, but for those that do I
would assume that they'd want the hook to run for all 'git-checkout'
variants -- as I do.

> These are all talking about various ways to _edit_ working tree
> files, and not about switching between revisions.
> 
> That's why I said I found that what the second sentence from
> your original description implied ("the hook gets old and new
> commit object name" which means we are talking about switching
> between revisions) was sensible, but it needs to be stressed a
> bit.
> 
> If you want to spacial case 
> 
>         $ git checkout otherbranch path.c
> 
> it raises another issue.  Which commit should supply the
> "extended attribute description" for path.c?  Should it be taken
> from the current commit (aka HEAD), otherbranch, or the index?

This already is a special case and your question is valid but not one
that git should necessary care about.  Since extended attributes are not
built into git the only way to handle them is through hooks.  A such, it
is up to the hook to worry about these kinds of issues.  The hook I have
written handles this by updating path.c attributes to match whatever
exists in the (tracked) attributes file of the current HEAD.

This 'git-checkout otherbranch path.c' is a corner case, but one that
can result in broken behavior when handling metadata unless the hook
runs.  I just want to close all the holes.  I can change the description
of the hook to try to dispel any confusion if that would help.

-JE



-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux