{"thread":{"id":"47226","subject":"Documentation of post-receive hook","startedAt":"2017-11-16T22:40:39Z","lastAt":"2017-11-24T02:11:06Z","messageCount":11,"participants":["Christoph Michelbach","Junio C Hamano","Robert P. J. Day"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"332728","messageId":"1510872031.23230.7.camel@gmail.com","threadId":"47226","inReplyTo":null,"subject":"Documentation of post-receive hook","fromName":"Christoph Michelbach","fromEmail":"michelbach94@gmail.com","sentAt":"2017-11-16T22:40:31Z","receivedAt":"2017-11-16T22:40:39Z","isPatch":false,"sender":{"key":"michelbach94@gmail.com","avatar":null},"body":"Hi,\n\nI think the documentation of the post-receive hook is misleading. When reading\nit, it appears as though the post-receive hook is executed even when no commits\nare transferred by a git push because it isn't mentioned anywhere that this is\nnecessary for its execution.\n\nThis can easily be fixed by changing\n\n This hook is invoked by 'git-receive-pack' on the remote repository,\n which happens when a 'git push' is done on a local repository.\n\nto:\n\n This hook is invoked by 'git-receive-pack' on the remote repository,\n which happens when a 'git push' is done on a local repository and\n successfully transfers at least 1 commit.\n\nAlternatively,\n\n This hook executes once for the receive operation.\n\ncan be changed to\n\n This hook executes once for the receive operation, but only if at least\n 1 commit was successfully transferred.\n\nImho, the first option should be chosen as it informs the reader about this\nbehavior at the first convenient opportunity.\n\n-- \nChristoph Michelbach \n"},{"id":"332735","messageId":"xmqqpo8hu1fi.fsf@gitster.mtv.corp.google.com","threadId":"47226","inReplyTo":"1510872031.23230.7.camel@gmail.com","subject":"Re: Documentation of post-receive hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-17T01:41:21Z","receivedAt":"2017-11-17T01:41:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christoph Michelbach writes:\n\n> I think the documentation of the post-receive hook is misleading. When reading\n> it, it appears as though the post-receive hook is executed even when no commits\n> are transferred by a git push because it isn't mentioned anywhere that this is\n> necessary for its execution.\n\nIn other words, post-receive hook triggers only after it receives\nobjects. A mere action of running receive-pack command does not.\n\n> This can easily be fixed by changing\n>\n> This hook is invoked by 'git-receive-pack' on the remote repository,\n> which happens when a 'git push' is done on a local repository.\n\nSo the existing description is technically correct (i.e. it does\ncorrectly identify who invokes it) but lacks a more interesting and\nrelevant information (i.e. receive-pack invokes only after receiving\ndata).\n\n> This hook is invoked by 'git-receive-pack' on the remote repository,\n> which happens when a 'git push' is done on a local repository and\n> successfully transfers at least 1 commit.\n\nI am not sure \"at least 1 commit\" is a good phrase to use here.\nThere are transfer that sends objects but no commit object, and the\nabove makes it sound as if such a transfer will not trigger the\nhook. Would\n\n\tThis hook is run by 'git receive-pack' on the remote\n\trepository, after it receives objects sent by 'git push'.\n\nbe clear enough to teach readers that a no-op push that recieve-pack\ndoes not receive any object does not trigger the hook?\n\n"},{"id":"332740","messageId":"xmqq375dtwnr.fsf@gitster.mtv.corp.google.com","threadId":"47226","inReplyTo":"xmqqpo8hu1fi.fsf@gitster.mtv.corp.google.com","subject":"Re: Documentation of post-receive hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-17T03:24:24Z","receivedAt":"2017-11-17T03:24:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> This hook is invoked by 'git-receive-pack' on the remote repository,\n>> which happens when a 'git push' is done on a local repository and\n>> successfully transfers at least 1 commit.\n>\n> I am not sure \"at least 1 commit\" is a good phrase to use here.\n> There are transfer that sends objects but no commit object, and the\n> above makes it sound as if such a transfer will not trigger the\n> hook. Would\n>\n> \tThis hook is run by 'git receive-pack' on the remote\n> \trepository, after it receives objects sent by 'git push'.\n>\n> be clear enough to teach readers that a no-op push that recieve-pack\n> does not receive any object does not trigger the hook?\n\nActually I take this back. Your original observation \"only when at\nleast one commit is transferred\" is not even correct.\n\nYou can try what I just tried to make sure:\n\n $ git clone --no-local . ../victim\n $ cat >../victim/.git/hooks/post-receive <<\\EOF\n #!/bin/sh\n (\n echo \"post receive was here at $(date)\"\n cat \n ) >>../STAMP\n EOF\n $ chmod +x ../victim/.git/hooks/post-receive\n $ git push ../victim master:foo\n\nThe last \"push\" does not transfer any object (and obviously does not\nsatisfy your \"at least 1 commit\" requirement), but it does update\nthe STAMP file. This is because it updates a ref and that is what\npost-receive wants to react to, even if there is no new objects\nplaced in the receiving repository.\n\nSo an updated suggestion for the text would be:\n\n This hook is invoked by 'git-receive-pack' on the remote repository,\n which happens when a 'git push' is done on a local repository.\n\nOh, wait. That is what we already have ;-).\n\nHaving said all that, there is one case that running 'git push' does\n*NOT* cause 'receive-pack' to be invoked at the other end, and in\nthat scenario, obviously the hook cannot be run, simply because the\ncommand that would run the hook is not run in the first place.\n\nAfter the above sequence against the \"victim\" test repository, you\ncould try:\n\n $ git push ../victim master:foo\n\tEverything up-to-date\n\nand observe that the STAMP file is not updated.\n\nWhat is happening is that \"git push\" notices that there is nothing\ngained by invoking receive-pack on the other side, because the\nbranch 'foo' already points at the commit at the tip of our\n'master'.\n\nSo it might technically be an improvement to update the text to\nmention that 'git push' does not necessarily always lead to\ninvocation of receive-pack, something like:\n\n This hook is invoked by 'git-receive-pack' on the remote\n repository, which may happen when a 'git push' is done on a\n local repository.\n\nbut then that introduces the need to make the reader understand what\n\"may happen\" is trying to say, iow, when does a user 'push' and it\ndoes not trigger receive-pack?\n\nBut I do not think teaching that (i.e. when does receive-pack run?)\nis the job of this paragraph, whose primary objective is to teach\nabout this hook that is run when receive-pack is run. So...\n"},{"id":"332856","messageId":"1511112703.8109.5.camel@gmail.com","threadId":"47226","inReplyTo":"xmqq375dtwnr.fsf@gitster.mtv.corp.google.com","subject":"Re: Documentation of post-receive hook","fromName":"Christoph Michelbach","fromEmail":"michelbach94@gmail.com","sentAt":"2017-11-19T17:31:43Z","receivedAt":"2017-11-19T17:31:51Z","isPatch":false,"sender":{"key":"michelbach94@gmail.com","avatar":null},"body":"On Fri, 2017-11-17 at 12:24 +0900, Junio C Hamano wrote:\n> So an updated suggestion for the text would be:\n> \n>      This hook is invoked by 'git-receive-pack' on the remote repository,\n>      which happens when a 'git push' is done on a local repository.\n> \n> Oh, wait.  That is what we already have ;-).\n\nBut this text suggests that the hook is always invoked when a git push to that\nrepo is done, which is not the case. How about adding \"and updates are reference\nin the remote repository\"?\n"},{"id":"332861","messageId":"alpine.LFD.2.21.1711191241580.6781@localhost.localdomain","threadId":"47226","inReplyTo":"1511112703.8109.5.camel@gmail.com","subject":"Re: Documentation of post-receive hook","fromName":"Robert P. J. Day","fromEmail":"rpjday@crashcourse.ca","sentAt":"2017-11-19T17:42:15Z","receivedAt":"2017-11-19T17:43:39Z","isPatch":false,"sender":{"key":"rpjday@crashcourse.ca","avatar":"https://avatars.githubusercontent.com/u/226084077?v=4"},"body":"On Sun, 19 Nov 2017, Christoph Michelbach wrote:\n\n> On Fri, 2017-11-17 at 12:24 +0900, Junio C Hamano wrote:\n> > So an updated suggestion for the text would be:\n> > \n> >      This hook is invoked by 'git-receive-pack' on the remote repository,\n> >      which happens when a 'git push' is done on a local repository.\n> > \n> > Oh, wait.  That is what we already have ;-).\n>\n> But this text suggests that the hook is always invoked when a git\n> push to that repo is done, which is not the case. How about adding\n> \"and updates are reference in the remote repository\"?\n\n \"referenced\"?\n\nrday\n\n-- \n\n========================================================================\nRobert P. J. Day Ottawa, Ontario, CANADA\n http://crashcourse.ca\n\nTwitter: http://twitter.com/rpjday\nLinkedIn: http://ca.linkedin.com/in/rpjday\n========================================================================"},{"id":"332873","messageId":"1511117715.11184.0.camel@gmail.com","threadId":"47226","inReplyTo":"alpine.LFD.2.21.1711191241580.6781@localhost.localdomain","subject":"Re: Documentation of post-receive hook","fromName":"Christoph Michelbach","fromEmail":"michelbach94@gmail.com","sentAt":"2017-11-19T18:55:15Z","receivedAt":"2017-11-19T18:55:23Z","isPatch":false,"sender":{"key":"michelbach94@gmail.com","avatar":null},"body":"On Sun, 2017-11-19 at 12:42 -0500, Robert P. J. Day wrote:\n> On Sun, 19 Nov 2017, Christoph Michelbach wrote:\n> \n> > \n> > On Fri, 2017-11-17 at 12:24 +0900, Junio C Hamano wrote:\n> > > \n> > > So an updated suggestion for the text would be:\n> > >  \n> > >       This hook is invoked by 'git-receive-pack' on the remote repository,\n> > >       which happens when a 'git push' is done on a local repository.\n> > >  \n> > > Oh, wait.  That is what we already have ;-).\n> > But this text suggests that the hook is always invoked when a git\n> > push to that repo is done, which is not the case. How about adding\n> > \"and updates are reference in the remote repository\"?\n>   \"referenced\"?\n> \n> rday\n> \n\nSorry, meant to write \"a\" instead of \"are\".\n"},{"id":"332887","messageId":"xmqq1sktoiih.fsf@gitster.mtv.corp.google.com","threadId":"47226","inReplyTo":"1511112703.8109.5.camel@gmail.com","subject":"Re: Documentation of post-receive hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-20T01:17:58Z","receivedAt":"2017-11-20T01:18:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christoph Michelbach writes:\n\n> On Fri, 2017-11-17 at 12:24 +0900, Junio C Hamano wrote:\n>> So an updated suggestion for the text would be:\n>> \n>> This hook is invoked by 'git-receive-pack' on the remote repository,\n>> which happens when a 'git push' is done on a local repository.\n>> \n>> Oh, wait. That is what we already have ;-).\n>\n> But this text suggests that the hook is always invoked when a git push to that\n> repo is done, which is not the case. How about adding \"and updates are reference\n> in the remote repository\"?\n\nYeah, I think we are moving in the right direction. Let's step back\na bit and try not to say \"remote repository\" twice. \n\nFor that matter, I am not sure if local/remote is adding much value,\nto be honest. It has a certain value to clarify that the receive is\nhappening on the other end of the push (i.e. not in the repository\nyou push from), but we do not have to use local/remote for that---\nand it is better to avoid 'local', that is relative: if you are in\nthe mindset of thinking about what happens during a receive, the\nrepository the receive-pack and the hook operates in is 'local' and\nnot 'remote' (it is only 'remote' if you think you are the one who\nis pushing).\n\nHow about this rewrite? Would it consider all the points raised and\nmake it easier to understand?\n\n This hook is invoked by 'git-receive-pack' when it reacts to\n 'git push' and updates reference(s) in its repository.\n"},{"id":"333200","messageId":"A82275FD-C9AC-4632-A5E8-ED86CE2AC67D@gmail.com","threadId":"47226","inReplyTo":"xmqq1sktoiih.fsf@gitster.mtv.corp.google.com","subject":"Re: Documentation of post-receive hook","fromName":"Christoph Michelbach","fromEmail":"michelbach94@gmail.com","sentAt":"2017-11-21T23:44:47Z","receivedAt":"2017-11-21T23:44:31Z","isPatch":false,"sender":{"key":"michelbach94@gmail.com","avatar":null},"body":"\n\nOn November 20, 2017 2:17:58 AM GMT+01:00, \n>How about this rewrite? Would it consider all the points raised and\n>make it easier to understand?\n>\n> This hook is invoked by 'git-receive-pack' when it reacts to\n> 'git push' and updates reference(s) in its repository.\n\nI think it's much more intelligible but a hint as to when this happens wouldn't hurt. E.g.: \"This does not happen if the user receives the message 'Already up-to-date'.\" That is if this is correct, of course. \n"},{"id":"333208","messageId":"xmqqlgizyv0o.fsf@gitster.mtv.corp.google.com","threadId":"47226","inReplyTo":"A82275FD-C9AC-4632-A5E8-ED86CE2AC67D@gmail.com","subject":"Re: Documentation of post-receive hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-22T01:14:31Z","receivedAt":"2017-11-22T01:14:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christoph Michelbach writes:\n\n> On November 20, 2017 2:17:58 AM GMT+01:00, \n>>How about this rewrite? Would it consider all the points raised and\n>>make it easier to understand?\n>>\n>> This hook is invoked by 'git-receive-pack' when it reacts to\n>> 'git push' and updates reference(s) in its repository.\n>\n> I think it's much more intelligible but a hint as to when this\n> happens wouldn't hurt. E.g.: \"This does not happen if the user\n> receives the message 'Already up-to-date'.\" That is if this is\n> correct, of course.\n\nYour suggesting to mention that particular message hints at me that\nyou feel that the users may not necessarily understand that push did\nnot result in any update of references on the other side when they\nsee it. If the message was clear enough to them, \"when it reacts to\npush and updates\" ought to be clear enough description, too.\n\nAnd if that indeed is the case (and I would not be surprised if it\nis, but I suspect that most users are clueful enough), it is not the\ndocumentation, but the \"Already up-to-date\" message, that needs to\nbe clarified, no?\n\nBesides, we'd rather not cast the end-user facing message in stone\nin the documentation like that (especially when the message has\nknown room for improvement and will change).\n"},{"id":"333315","messageId":"1511377760.11458.0.camel@gmail.com","threadId":"47226","inReplyTo":"xmqqlgizyv0o.fsf@gitster.mtv.corp.google.com","subject":"Re: Documentation of post-receive hook","fromName":"Christoph Michelbach","fromEmail":"michelbach94@gmail.com","sentAt":"2017-11-22T19:09:20Z","receivedAt":"2017-11-22T19:09:28Z","isPatch":false,"sender":{"key":"michelbach94@gmail.com","avatar":null},"body":"On Wed, 2017-11-22 at 10:14 +0900, Junio C Hamano wrote:\n> Your suggesting to mention that particular message hints at me that\n> you feel that the users may not necessarily understand that push did\n> not result in any update of references on the other side when they\n> see it.  If the message was clear enough to them, \"when it reacts to\n> push and updates\" ought to be clear enough description, too.\n> \n> And if that indeed is the case (and I would not be surprised if it\n> is, but I suspect that most users are clueful enough), it is not the\n> documentation, but the \"Already up-to-date\" message, that needs to\n> be clarified, no?\n> \n> Besides, we'd rather not cast the end-user facing message in stone\n> in the documentation like that (especially when the message has\n> known room for improvement and will change).\n\nYou're right (with everything you wrote). Your proposed text sounds good.\n"},{"id":"333405","messageId":"xmqqlgiwpgst.fsf_-_@gitster.mtv.corp.google.com","threadId":"47226","inReplyTo":"xmqqlgizyv0o.fsf@gitster.mtv.corp.google.com","subject":"Re*: Documentation of post-receive hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-24T02:10:58Z","receivedAt":"2017-11-24T02:11:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Your suggesting to mention that particular message hints at me that\n> you feel that the users may not necessarily understand that push did\n> not result in any update of references on the other side when they\n> see it. If the message was clear enough to them, \"when it reacts to\n> push and updates\" ought to be clear enough description, too.\n>\n> And if that indeed is the case (and I would not be surprised if it\n> is, but I suspect that most users are clueful enough), it is not the\n> documentation, but the \"Already up-to-date\" message, that needs to\n> be clarified, no?\n\nI do not know if it helps to _also_ clarify the message, but at\nleast, let's tie the loose end by updating the documentation.\n\n-- >8 --\nSubject: hooks doc: clarify when receive-pack invokes its hooks\n\nThe text meant to say that receive-pack runs these hooks, and only\nbecause receive-pack is not a command the end users use every day\n(ever), as an explanation also meantioned that it is run in response\nto 'git push', which is an end-user facing command readers hopefully\nknow about.\n\nThis unfortunately gave an incorrect impression that 'git push'\nalways result in the hook to run. If the refs push wanted to update\nall already had the desired value, these hooks are not run. \n\nExplicitly mention \"... and updates reference(s)\" as a precondition\nto avoid this ambiguity.\n\nHelped-by: Christoph Michelbach \nSigned-off-by: Junio C Hamano \n---\n\n Documentation/githooks.txt | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 9565dc3fda..8f6a3cd63e 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -222,8 +222,8 @@ to the user by writing to standard error.\n pre-receive\n ~~~~~~~~~~~\n \n-This hook is invoked by 'git-receive-pack' on the remote repository,\n-which happens when a 'git push' is done on a local repository.\n+This hook is invoked by 'git-receive-pack' when it reacts to\n+'git push' and updates reference(s) in its repository.\n Just before starting to update refs on the remote repository, the\n pre-receive hook is invoked. Its exit status determines the success\n or failure of the update.\n@@ -260,8 +260,8 @@ will be set to zero, `GIT_PUSH_OPTION_COUNT=0`.\n update\n ~~~~~~\n \n-This hook is invoked by 'git-receive-pack' on the remote repository,\n-which happens when a 'git push' is done on a local repository.\n+This hook is invoked by 'git-receive-pack' when it reacts to\n+'git push' and updates reference(s) in its repository.\n Just before updating the ref on the remote repository, the update hook\n is invoked. Its exit status determines the success or failure of\n the ref update.\n@@ -305,8 +305,8 @@ unannotated tags to be pushed.\n post-receive\n ~~~~~~~~~~~~\n \n-This hook is invoked by 'git-receive-pack' on the remote repository,\n-which happens when a 'git push' is done on a local repository.\n+This hook is invoked by 'git-receive-pack' when it reacts to\n+'git push' and updates reference(s) in its repository.\n It executes on the remote repository once after all the refs have\n been updated.\n \n@@ -344,8 +344,8 @@ will be set to zero, `GIT_PUSH_OPTION_COUNT=0`.\n post-update\n ~~~~~~~~~~~\n \n-This hook is invoked by 'git-receive-pack' on the remote repository,\n-which happens when a 'git push' is done on a local repository.\n+This hook is invoked by 'git-receive-pack' when it reacts to\n+'git push' and updates reference(s) in its repository.\n It executes on the remote repository once after all the refs have\n been updated.\n \n@@ -375,8 +375,8 @@ for the user.\n push-to-checkout\n ~~~~~~~~~~~~~~~~\n \n-This hook is invoked by 'git-receive-pack' on the remote repository,\n-which happens when a 'git push' is done on a local repository, when\n+This hook is invoked by 'git-receive-pack' when it reacts to\n+'git push' and updates reference(s) in its repository, and when\n the push tries to update the branch that is currently checked out\n and the `receive.denyCurrentBranch` configuration variable is set to\n `updateInstead`. Such a push by default is refused if the working\n"}]}