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
next prev parent 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).