post-checkout hook not run on clone

post-checkout hook not run on clone

[layer]

I realize this might be a feature, but when I switch to the master
branch with "git checkout master" it is, and I would think that a
clone that gets the master branch would also does a sort of "checkout
master" and would run the hook.

In any case, I'd be happy of there was a post-clone hook, instead, but
there isn't.

Any suggestions?

Thanks.

Kevin

Re: post-checkout hook not run on clone

[Jeff King]

On Mon, Mar 02, 2009 at 02:43:37PM -0800, layer wrote:

> I realize this might be a feature, but when I switch to the master
> branch with "git checkout master" it is, and I would think that a
> clone that gets the master branch would also does a sort of "checkout
> master" and would run the hook.

Right. Hooks are not cloned with the repo.

> In any case, I'd be happy of there was a post-clone hook, instead, but
> there isn't.

The general wisdom on the list is that it's a bad idea to run remote
code arbitrarily for security reasons (i.e., "git clone
git://host/foo.git" should not automatically run code from "host"). Even
if you are going to build and run the contents of "foo.git", you at
least have a chance to inspect the changes via git.

However, for situations where you are OK implicitly trusting the remote,
it is obviously less convenient.

> Any suggestions?

Most suggestions I have seen involve shipping the hooks in your repo,
and then having users copy them to their .git/hooks directory (and you
can even provide a script for that).

Unfortunately, there is a chicken-and-egg problem there with the initial
checkout. You could do something like (assuming the hooks are in "hooks"
in the repo) to bootstrap:

  git clone -n <repo.git>
  cd repo
  git archive --format tar HEAD hooks | tar -C .git/hooks -xf -
  git checkout master

-Peff

Re: post-checkout hook not run on clone

[layer]

Jeff King <peff@peff.net> wrote:

>> On Mon, Mar 02, 2009 at 02:43:37PM -0800, layer wrote:
>> 
>> > I realize this might be a feature, but when I switch to the master
>> > branch with "git checkout master" it is, and I would think that a
>> > clone that gets the master branch would also does a sort of "checkout
>> > master" and would run the hook.
>> 
>> Right. Hooks are not cloned with the repo.

The hook in question was in /usr/share/git-core/templates/hooks/, so
it would get setup on clone.  That works fine.  If I immediately
switch branches, the hook gets called.  It's just the `post-clone'
(when I assume something like `checkout' is done), the hook doesn't
get called.

>> The general wisdom on the list is that it's a bad idea to run remote
>> code arbitrarily for security reasons...

I agree, but not in this specific situation.  All the users of the
code are trusted, as is the author of the hooks.

Re: post-checkout hook not run on clone

[Shawn O. Pearce]

Jeff King <peff@peff.net> wrote:
> On Mon, Mar 02, 2009 at 02:43:37PM -0800, layer wrote:
> 
> > I realize this might be a feature, but when I switch to the master
> > branch with "git checkout master" it is, and I would think that a
> > clone that gets the master branch would also does a sort of "checkout
> > master" and would run the hook.
> 
> Right. Hooks are not cloned with the repo.

I think the original poster was talking about a hook installed via
their template directory.  In which case the hook is trusted, and is
coming from a known source, its just not being called during clone.
 
-- 
Shawn.

Re: post-checkout hook not run on clone

[Jeff King]

On Mon, Mar 02, 2009 at 09:02:29PM -0800, layer wrote:

> The hook in question was in /usr/share/git-core/templates/hooks/, so
> it would get setup on clone.  That works fine.  If I immediately
> switch branches, the hook gets called.  It's just the `post-clone'
> (when I assume something like `checkout' is done), the hook doesn't
> get called.

Ah, OK. I misunderstood. Then that is a bug, IMHO. Patch in a minute.

-Peff

[PATCH] clone: run post-checkout hook when checking out

[Jeff King]

The mental model for clone is that the branch is "checked
out" (and it even says this in Documentation/git-clone.txt:
"...creates and checks out an initial branch"). Therefore it
is reasonable for users to expect that any post-checkout
hook would be run.

Signed-off-by: Jeff King <peff@peff.net>
---
On Mon, Mar 02, 2009 at 09:02:29PM -0800, layer wrote:

> The hook in question was in /usr/share/git-core/templates/hooks/, so
> it would get setup on clone.  That works fine.  If I immediately
> switch branches, the hook gets called.  It's just the `post-clone'
> (when I assume something like `checkout' is done), the hook doesn't
> get called.

This should fix it.

Junio, I'm not sure what you want to do with this. It is definitely a
behavior change; we have never respected post-checkout hooks in shell
git-clone.sh or in the builtin version. However, it seems like an
omission rather than an intentional behavior, so I consider this a
bugfix.

 builtin-clone.c               |    7 ++++++-
 t/t5403-post-checkout-hook.sh |   12 ++++++++++++
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 3146ca8..ceffecb 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -21,6 +21,7 @@
 #include "pack-refs.h"
 #include "sigchain.h"
 #include "remote.h"
+#include "run-command.h"
 
 /*
  * Overall FIXMEs:
@@ -341,6 +342,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
 	struct transport *transport = NULL;
 	char *src_ref_prefix = "refs/heads/";
+	int err = 0;
 
 	struct refspec refspec;
 
@@ -596,6 +598,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		if (write_cache(fd, active_cache, active_nr) ||
 		    commit_locked_index(lock_file))
 			die("unable to write new index file");
+
+		err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
+				sha1_to_hex(remote_head->old_sha1), "1", NULL);
 	}
 
 	strbuf_release(&reflog_msg);
@@ -603,5 +608,5 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	strbuf_release(&key);
 	strbuf_release(&value);
 	junk_pid = 0;
-	return 0;
+	return err;
 }
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 9b2e1a9..4fdb418 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -71,4 +71,16 @@ test_expect_success 'post-checkout receives the right args when not switching br
         test $old = $new -a $flag = 0
 '
 
+mkdir -p templates/hooks
+cat >templates/hooks/post-checkout <<'EOF'
+#!/bin/sh
+echo $@ > $GIT_DIR/post-checkout.args
+EOF
+chmod +x templates/hooks/post-checkout
+
+test_expect_success 'post-checkout hook is triggered by clone' '
+	git clone --template=templates . clone3 &&
+	test -f clone3/.git/post-checkout.args
+'
+
 test_done
-- 
1.6.2.rc2.313.gce9b3.dirty

Re: [PATCH] clone: run post-checkout hook when checking out

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Junio, I'm not sure what you want to do with this. It is definitely a
> behavior change; we have never respected post-checkout hooks in shell
> git-clone.sh or in the builtin version. However, it seems like an
> omission rather than an intentional behavior, so I consider this a
> bugfix.

I do not mind queueing this to 'pu' and let people fight it out.  I hate
the local side hooks that have no reason to be there, and consider that
the existence of the "checkout hook" to be a bug to begin with, but I
really don't care that deeply anymore.

Re: [PATCH] clone: run post-checkout hook when checking out

[Jay Soffian]

On Tue, Mar 3, 2009 at 1:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I do not mind queueing this to 'pu' and let people fight it out.  I hate
> the local side hooks that have no reason to be there, and consider that
> the existence of the "checkout hook" to be a bug to begin with

I just want to clarify: do you mean to say that none of the local side
hooks have any reason to be there, or that some of the local hooks do
not belong, and checkout is one of those?

j.

Re: [PATCH] clone: run post-checkout hook when checking out

[Jeff King]

On Mon, Mar 02, 2009 at 10:45:09PM -0800, Junio C Hamano wrote:

> > Junio, I'm not sure what you want to do with this. It is definitely a
> > behavior change; we have never respected post-checkout hooks in shell
> > git-clone.sh or in the builtin version. However, it seems like an
> > omission rather than an intentional behavior, so I consider this a
> > bugfix.
> 
> I do not mind queueing this to 'pu' and let people fight it out.  I hate
> the local side hooks that have no reason to be there, and consider that
> the existence of the "checkout hook" to be a bug to begin with, but I
> really don't care that deeply anymore.

That sounds fine to me. I don't personally care either way (I didn't
even know we had a post-checkout hook until today).

-Peff

Re: [PATCH] clone: run post-checkout hook when checking out

[Nanako Shiraishi]

Quoting Jay Soffian <jaysoffian@gmail.com>:

> On Tue, Mar 3, 2009 at 1:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> I do not mind queueing this to 'pu' and let people fight it out.  I hate
>> the local side hooks that have no reason to be there, and consider that
>> the existence of the "checkout hook" to be a bug to begin with
>
> I just want to clarify: do you mean to say that none of the local side
> hooks have any reason to be there, or that some of the local hooks do
> not belong, and checkout is one of those?

There are five valid reasons you might want a hook to a git operation.

http://thread.gmane.org/gmane.comp.version-control.git/70781/focus=71069

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH] clone: run post-checkout hook when checking out

[layer]

Jeff,

Thanks very much for doing this.

Kevin