{"thread":{"id":"18098","subject":"post-checkout hook not run on clone","startedAt":"2009-03-02T22:43:37Z","lastAt":"2009-03-04T05:01:09Z","messageCount":11,"participants":["layer","Jeff King","Shawn O. Pearce","Junio C Hamano","Jay Soffian","Nanako Shiraishi"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"106733","messageId":"20273.1236033817@relay.known.net","threadId":"18098","inReplyTo":null,"subject":"post-checkout hook not run on clone","fromName":"layer","fromEmail":"layer@known.net","sentAt":"2009-03-02T22:43:37Z","receivedAt":"2009-03-02T22:43:37Z","isPatch":false,"sender":{"key":"layer@known.net","avatar":null},"body":"I realize this might be a feature, but when I switch to the master\nbranch with \"git checkout master\" it is, and I would think that a\nclone that gets the master branch would also does a sort of \"checkout\nmaster\" and would run the hook.\n\nIn any case, I'd be happy of there was a post-clone hook, instead, but\nthere isn't.\n\nAny suggestions?\n\nThanks.\n\nKevin\n"},{"id":"106756","messageId":"20090303042848.GC18136@coredump.intra.peff.net","threadId":"18098","inReplyTo":"20273.1236033817@relay.known.net","subject":"Re: post-checkout hook not run on clone","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-03T04:28:49Z","receivedAt":"2009-03-03T04:28:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2009 at 02:43:37PM -0800, layer wrote:\n\n> I realize this might be a feature, but when I switch to the master\n> branch with \"git checkout master\" it is, and I would think that a\n> clone that gets the master branch would also does a sort of \"checkout\n> master\" and would run the hook.\n\nRight. Hooks are not cloned with the repo.\n\n> In any case, I'd be happy of there was a post-clone hook, instead, but\n> there isn't.\n\nThe general wisdom on the list is that it's a bad idea to run remote\ncode arbitrarily for security reasons (i.e., \"git clone\ngit://host/foo.git\" should not automatically run code from \"host\"). Even\nif you are going to build and run the contents of \"foo.git\", you at\nleast have a chance to inspect the changes via git.\n\nHowever, for situations where you are OK implicitly trusting the remote,\nit is obviously less convenient.\n\n> Any suggestions?\n\nMost suggestions I have seen involve shipping the hooks in your repo,\nand then having users copy them to their .git/hooks directory (and you\ncan even provide a script for that).\n\nUnfortunately, there is a chicken-and-egg problem there with the initial\ncheckout. You could do something like (assuming the hooks are in \"hooks\"\nin the repo) to bootstrap:\n\n git clone -n \n cd repo\n git archive --format tar HEAD hooks | tar -C .git/hooks -xf -\n git checkout master\n\n-Peff\n"},{"id":"106759","messageId":"23978.1236056549@relay.known.net","threadId":"18098","inReplyTo":"20090303042848.GC18136@coredump.intra.peff.net","subject":"Re: post-checkout hook not run on clone","fromName":"layer","fromEmail":"layer@known.net","sentAt":"2009-03-03T05:02:29Z","receivedAt":"2009-03-03T05:02:29Z","isPatch":false,"sender":{"key":"layer@known.net","avatar":null},"body":"Jeff King wrote:\n\n>> On Mon, Mar 02, 2009 at 02:43:37PM -0800, layer wrote:\n>> \n>> > I realize this might be a feature, but when I switch to the master\n>> > branch with \"git checkout master\" it is, and I would think that a\n>> > clone that gets the master branch would also does a sort of \"checkout\n>> > master\" and would run the hook.\n>> \n>> Right. Hooks are not cloned with the repo.\n\nThe hook in question was in /usr/share/git-core/templates/hooks/, so\nit would get setup on clone. That works fine. If I immediately\nswitch branches, the hook gets called. It's just the `post-clone'\n(when I assume something like `checkout' is done), the hook doesn't\nget called.\n\n>> The general wisdom on the list is that it's a bad idea to run remote\n>> code arbitrarily for security reasons...\n\nI agree, but not in this specific situation. All the users of the\ncode are trusted, as is the author of the hooks.\n"},{"id":"106760","messageId":"20090303050451.GA26459@spearce.org","threadId":"18098","inReplyTo":"20090303042848.GC18136@coredump.intra.peff.net","subject":"Re: post-checkout hook not run on clone","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-03T05:04:51Z","receivedAt":"2009-03-03T05:04:51Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jeff King wrote:\n> On Mon, Mar 02, 2009 at 02:43:37PM -0800, layer wrote:\n> \n> > I realize this might be a feature, but when I switch to the master\n> > branch with \"git checkout master\" it is, and I would think that a\n> > clone that gets the master branch would also does a sort of \"checkout\n> > master\" and would run the hook.\n> \n> Right. Hooks are not cloned with the repo.\n\nI think the original poster was talking about a hook installed via\ntheir template directory. In which case the hook is trusted, and is\ncoming from a known source, its just not being called during clone.\n \n-- \nShawn.\n"},{"id":"106761","messageId":"20090303051026.GA21101@coredump.intra.peff.net","threadId":"18098","inReplyTo":"23978.1236056549@relay.known.net","subject":"Re: post-checkout hook not run on clone","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-03T05:10:26Z","receivedAt":"2009-03-03T05:10:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2009 at 09:02:29PM -0800, layer wrote:\n\n> The hook in question was in /usr/share/git-core/templates/hooks/, so\n> it would get setup on clone. That works fine. If I immediately\n> switch branches, the hook gets called. It's just the `post-clone'\n> (when I assume something like `checkout' is done), the hook doesn't\n> get called.\n\nAh, OK. I misunderstood. Then that is a bug, IMHO. Patch in a minute.\n\n-Peff\n"},{"id":"106762","messageId":"20090303053750.GA30201@coredump.intra.peff.net","threadId":"18098","inReplyTo":"23978.1236056549@relay.known.net","subject":"[PATCH] clone: run post-checkout hook when checking out","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-03T05:37:51Z","receivedAt":"2009-03-03T05:37:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The mental model for clone is that the branch is \"checked\nout\" (and it even says this in Documentation/git-clone.txt:\n\"...creates and checks out an initial branch\"). Therefore it\nis reasonable for users to expect that any post-checkout\nhook would be run.\n\nSigned-off-by: Jeff King \n---\nOn Mon, Mar 02, 2009 at 09:02:29PM -0800, layer wrote:\n\n> The hook in question was in /usr/share/git-core/templates/hooks/, so\n> it would get setup on clone. That works fine. If I immediately\n> switch branches, the hook gets called. It's just the `post-clone'\n> (when I assume something like `checkout' is done), the hook doesn't\n> get called.\n\nThis should fix it.\n\nJunio, I'm not sure what you want to do with this. It is definitely a\nbehavior change; we have never respected post-checkout hooks in shell\ngit-clone.sh or in the builtin version. However, it seems like an\nomission rather than an intentional behavior, so I consider this a\nbugfix.\n\n builtin-clone.c | 7 ++++++-\n t/t5403-post-checkout-hook.sh | 12 ++++++++++++\n 2 files changed, 18 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 3146ca8..ceffecb 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -21,6 +21,7 @@\n #include \"pack-refs.h\"\n #include \"sigchain.h\"\n #include \"remote.h\"\n+#include \"run-command.h\"\n \n /*\n * Overall FIXMEs:\n@@ -341,6 +342,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;\n \tstruct transport *transport = NULL;\n \tchar *src_ref_prefix = \"refs/heads/\";\n+\tint err = 0;\n \n \tstruct refspec refspec;\n \n@@ -596,6 +598,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t commit_locked_index(lock_file))\n \t\t\tdie(\"unable to write new index file\");\n+\n+\t\terr |= run_hook(NULL, \"post-checkout\", sha1_to_hex(null_sha1),\n+\t\t\t\tsha1_to_hex(remote_head->old_sha1), \"1\", NULL);\n \t}\n \n \tstrbuf_release(&reflog_msg);\n@@ -603,5 +608,5 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&key);\n \tstrbuf_release(&value);\n \tjunk_pid = 0;\n-\treturn 0;\n+\treturn err;\n }\ndiff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh\nindex 9b2e1a9..4fdb418 100755\n--- a/t/t5403-post-checkout-hook.sh\n+++ b/t/t5403-post-checkout-hook.sh\n@@ -71,4 +71,16 @@ test_expect_success 'post-checkout receives the right args when not switching br\n test $old = $new -a $flag = 0\n '\n \n+mkdir -p templates/hooks\n+cat >templates/hooks/post-checkout <<'EOF'\n+#!/bin/sh\n+echo $@ > $GIT_DIR/post-checkout.args\n+EOF\n+chmod +x templates/hooks/post-checkout\n+\n+test_expect_success 'post-checkout hook is triggered by clone' '\n+\tgit clone --template=templates . clone3 &&\n+\ttest -f clone3/.git/post-checkout.args\n+'\n+\n test_done\n-- \n1.6.2.rc2.313.gce9b3.dirty\n"},{"id":"106765","messageId":"7vsklvhzpm.fsf@gitster.siamese.dyndns.org","threadId":"18098","inReplyTo":"20090303053750.GA30201@coredump.intra.peff.net","subject":"Re: [PATCH] clone: run post-checkout hook when checking out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-03T06:45:09Z","receivedAt":"2009-03-03T06:45:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Junio, I'm not sure what you want to do with this. It is definitely a\n> behavior change; we have never respected post-checkout hooks in shell\n> git-clone.sh or in the builtin version. However, it seems like an\n> omission rather than an intentional behavior, so I consider this a\n> bugfix.\n\nI do not mind queueing this to 'pu' and let people fight it out. I hate\nthe local side hooks that have no reason to be there, and consider that\nthe existence of the \"checkout hook\" to be a bug to begin with, but I\nreally don't care that deeply anymore.\n"},{"id":"106768","messageId":"76718490903022255sab126c7qeab2fc60321a928e@mail.gmail.com","threadId":"18098","inReplyTo":"7vsklvhzpm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] clone: run post-checkout hook when checking out","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-03T06:55:28Z","receivedAt":"2009-03-03T06:55:28Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Mar 3, 2009 at 1:45 AM, Junio C Hamano wrote:\n> I do not mind queueing this to 'pu' and let people fight it out.  I hate\n> the local side hooks that have no reason to be there, and consider that\n> the existence of the \"checkout hook\" to be a bug to begin with\n\nI just want to clarify: do you mean to say that none of the local side\nhooks have any reason to be there, or that some of the local hooks do\nnot belong, and checkout is one of those?\n\nj.\n"},{"id":"106769","messageId":"20090303070308.GA30609@coredump.intra.peff.net","threadId":"18098","inReplyTo":"7vsklvhzpm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] clone: run post-checkout hook when checking out","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-03T07:03:08Z","receivedAt":"2009-03-03T07:03:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2009 at 10:45:09PM -0800, Junio C Hamano wrote:\n\n> > Junio, I'm not sure what you want to do with this. It is definitely a\n> > behavior change; we have never respected post-checkout hooks in shell\n> > git-clone.sh or in the builtin version. However, it seems like an\n> > omission rather than an intentional behavior, so I consider this a\n> > bugfix.\n> \n> I do not mind queueing this to 'pu' and let people fight it out. I hate\n> the local side hooks that have no reason to be there, and consider that\n> the existence of the \"checkout hook\" to be a bug to begin with, but I\n> really don't care that deeply anymore.\n\nThat sounds fine to me. I don't personally care either way (I didn't\neven know we had a post-checkout hook until today).\n\n-Peff\n"},{"id":"106810","messageId":"20090303190702.6117@nanako3.lavabit.com","threadId":"18098","inReplyTo":"76718490903022255sab126c7qeab2fc60321a928e@mail.gmail.com","subject":"Re: [PATCH] clone: run post-checkout hook when checking out","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-03-03T10:07:02Z","receivedAt":"2009-03-03T10:07:02Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Jay Soffian :\n\n> On Tue, Mar 3, 2009 at 1:45 AM, Junio C Hamano wrote:\n>> I do not mind queueing this to 'pu' and let people fight it out.  I hate\n>> the local side hooks that have no reason to be there, and consider that\n>> the existence of the \"checkout hook\" to be a bug to begin with\n>\n> I just want to clarify: do you mean to say that none of the local side\n> hooks have any reason to be there, or that some of the local hooks do\n> not belong, and checkout is one of those?\n\nThere are five valid reasons you might want a hook to a git operation.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/70781/focus=71069\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"106942","messageId":"5444.1236142869@relay.known.net","threadId":"18098","inReplyTo":"20090303053750.GA30201@coredump.intra.peff.net","subject":"Re: [PATCH] clone: run post-checkout hook when checking out","fromName":"layer","fromEmail":"layer@known.net","sentAt":"2009-03-04T05:01:09Z","receivedAt":"2009-03-04T05:01:09Z","isPatch":true,"sender":{"key":"layer@known.net","avatar":null},"body":"Jeff,\n\nThanks very much for doing this.\n\nKevin\n"}]}