git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Josh England" <jjengla@sandia.gov>
To: "Dmitry Potapov" <deaptor@mail.ru>
Cc: "Junio C Hamano" <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] post-checkout hook, and related docs and tests
Date: Wed, 26 Sep 2007 13:23:53 -0600	[thread overview]
Message-ID: <1190834633.6078.139.camel@beauty> (raw)
In-Reply-To: <20070926145229.GA15300@potapov>

On Wed, 2007-09-26 at 18:52 +0400, Dmitry Potapov wrote:
> 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.

> 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.

>  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.

> 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

  reply	other threads:[~2007-09-26 19:22 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-21 20:27 [PATCH] post-checkout hook, and related docs and tests root
2007-09-21 20:35 ` Josh England
2007-09-22  0:15 ` Junio C Hamano
2007-09-24 17:14   ` Josh England
2007-09-24 18:34     ` Junio C Hamano
2007-09-24 19:33       ` Josh England
2007-09-24 21:07         ` Junio C Hamano
2007-09-24 22:05           ` Josh England
2007-09-24 23:54             ` Junio C Hamano
2007-09-25  4:42               ` Andreas Ericsson
2007-09-25 16:41               ` Josh England
2007-09-25 21:29                 ` Junio C Hamano
2007-09-25 21:47                   ` Josh England
2007-09-26 14:52           ` Dmitry Potapov
2007-09-26 19:23             ` Josh England [this message]
2007-09-24 17:58   ` Josh England

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1190834633.6078.139.camel@beauty \
    --to=jjengla@sandia.gov \
    --cc=deaptor@mail.ru \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).