{"thread":{"id":"41848","subject":"pre-push hook does not get input on non-fast-forward pushes","startedAt":"2016-03-28T20:56:52Z","lastAt":"2016-03-28T22:09:08Z","messageCount":4,"participants":["Stefan Tauner","Junio C Hamano","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"281987","messageId":"201603282056.u2SKuqDf031459@mail2.student.tuwien.ac.at","threadId":"41848","inReplyTo":null,"subject":"pre-push hook does not get input on non-fast-forward pushes","fromName":"Stefan Tauner","fromEmail":"stefan.tauner@alumni.tuwien.ac.at","sentAt":"2016-03-28T20:56:52Z","receivedAt":"2016-03-28T20:56:52Z","isPatch":false,"sender":{"key":"stefan.tauner@alumni.tuwien.ac.at","avatar":null},"body":"Hi,\n\nI noticed that without an additional --force the pre-push hook does not\nget any input on stdin if a push would result in non-fast-forward\nuploads. This is not a problem per se (although I don't get the\nrationale) but it is undocumented and the latter left me puzzled.\n\n(Please keep me in CC since I am not subscribed, thanks)\n\n-- \nKind regards/Mit freundlichen Grüßen, Stefan Tauner\n"},{"id":"281988","messageId":"xmqq1t6uxop7.fsf@gitster.mtv.corp.google.com","threadId":"41848","inReplyTo":"201603282056.u2SKuqDf031459@mail2.student.tuwien.ac.at","subject":"Re: pre-push hook does not get input on non-fast-forward pushes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-28T21:44:20Z","receivedAt":"2016-03-28T21:44:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Tauner writes:\n\n> I noticed that without an additional --force the pre-push hook does not\n> get any input on stdin if a push would result in non-fast-forward\n> uploads. This is not a problem per se (although I don't get the\n> rationale) but it is undocumented and the latter left me puzzled.\n>\n> (Please keep me in CC since I am not subscribed, thanks)\n\nThanks (no need to say \"please cc me\", as that is the norm here).\n\nThe pre-push hook is not the only thing that may prevent you from\npushing a ref update. As you noticed, non-fast-forward check may\ntrigger and decide that a ref is not going to be pushed, and that\nmay happen before we call the pre-push hook.\n\n Information about what is to be pushed is provided on the hook's\n standard input with lines of the form ...\n\nSo when the pre-push is called, the refs that will not fast-forward\nmay not be among the \"what is to be pushed\", hence not reported.\n\nWe _could_ add something like this to the documentation, but we do\nnot necessarily want to promise that the order of checks to stay\n\"internal checks like non-ff check first before pre-push hook\", so\nthis update may not be an improvement. The current text conveys\nenough information without making such a promise, but you need to\nread it carefully.\n\nSo I dunno.\n\n Documentation/githooks.txt | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 9ef2469..605ba4d 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. If a ref is to be\n deleted, the `` will be supplied as `(delete)` and the `` will be 40 `0`. If the local commit was specified by something other\n than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be\n-supplied as it was originally given.\n+supplied as it was originally given. A request to update remote ref that has\n+already been rejected for other reasons (e.g. failing to pass a fast-forward\n+test) does not appear in the input.\n \n If this hook exits with a non-zero status, 'git push' will abort without\n pushing anything. Information about why the push is rejected may be sent\n"},{"id":"281989","messageId":"CAGZ79kbua6zgS3wkd9UKASDknxostFcBb_EWsVQ2WNaGeKw=2w@mail.gmail.com","threadId":"41848","inReplyTo":"xmqq1t6uxop7.fsf@gitster.mtv.corp.google.com","subject":"Re: pre-push hook does not get input on non-fast-forward pushes","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-28T21:56:49Z","receivedAt":"2016-03-28T21:56:49Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Mar 28, 2016 at 2:44 PM, Junio C Hamano wrote:\n> Stefan Tauner writes:\n>\n>> I noticed that without an additional --force the pre-push hook does not\n>> get any input on stdin if a push would result in non-fast-forward\n>> uploads. This is not a problem per se (although I don't get the\n>> rationale) but it is undocumented and the latter left me puzzled.\n>>\n>> (Please keep me in CC since I am not subscribed, thanks)\n>\n> Thanks (no need to say \"please cc me\", as that is the norm here).\n>\n> The pre-push hook is not the only thing that may prevent you from\n> pushing a ref update. As you noticed, non-fast-forward check may\n> trigger and decide that a ref is not going to be pushed, and that\n> may happen before we call the pre-push hook.\n>\n> Information about what is to be pushed is provided on the hook's\n> standard input with lines of the form ...\n>\n> So when the pre-push is called, the refs that will not fast-forward\n> may not be among the \"what is to be pushed\", hence not reported.\n>\n> We _could_ add something like this to the documentation, but we do\n> not necessarily want to promise that the order of checks to stay\n> \"internal checks like non-ff check first before pre-push hook\", so\n> this update may not be an improvement. The current text conveys\n> enough information without making such a promise, but you need to\n> read it carefully.\n>\n> So I dunno.\n>\n> Documentation/githooks.txt | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 9ef2469..605ba4d 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. If a ref is to be\n> deleted, the `` will be supplied as `(delete)` and the ` SHA-1>` will be 40 `0`. If the local commit was specified by something other\n> than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be\n> -supplied as it was originally given.\n> +supplied as it was originally given. A request to update remote ref that has\n> +already been rejected for other reasons (e.g. failing to pass a fast-forward\n> +test) does not appear in the input.\n\nIf we don't want to give promises, s/does not/ may not/ ?\n\nAlthough that sounds like it is non-deterministic (some occur, some don't,\nmaybe it depends on $random_reason)\n\n>\n> If this hook exits with a non-zero status, 'git push' will abort without\n> pushing anything. Information about why the push is rejected may be sent\n>\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"281990","messageId":"201603282209.u2SM98Zb010448@mail2.student.tuwien.ac.at","threadId":"41848","inReplyTo":"xmqq1t6uxop7.fsf@gitster.mtv.corp.google.com","subject":"Re: pre-push hook does not get input on non-fast-forward pushes","fromName":"Stefan Tauner","fromEmail":"stefan.tauner@alumni.tuwien.ac.at","sentAt":"2016-03-28T22:09:08Z","receivedAt":"2016-03-28T22:09:08Z","isPatch":false,"sender":{"key":"stefan.tauner@alumni.tuwien.ac.at","avatar":null},"body":"On Mon, 28 Mar 2016 14:44:20 -0700\nJunio C Hamano wrote:\n\n> Stefan Tauner writes:\n> \n> The pre-push hook is not the only thing that may prevent you from\n> pushing a ref update. As you noticed, non-fast-forward check may\n> trigger and decide that a ref is not going to be pushed, and that\n> may happen before we call the pre-push hook.\n> \n> Information about what is to be pushed is provided on the hook's\n> standard input with lines of the form ...\n> \n> So when the pre-push is called, the refs that will not fast-forward\n> may not be among the \"what is to be pushed\", hence not reported.\n> \n> We _could_ add something like this to the documentation, but we do\n> not necessarily want to promise that the order of checks to stay\n> \"internal checks like non-ff check first before pre-push hook\", so\n> this update may not be an improvement. The current text conveys\n> enough information without making such a promise, but you need to\n> read it carefully.\n\nI understand but that behavior is quite surprising and the wording not\nthat clear (if one is not aware of the behavior already) IMHO. If the\nfast forward check is done beforehand and no refs remain, why is the\nscript called at all? Or put otherwise, why isn't git aborting before\nthat? That would seem way more logical... consistency is a good\nargument to call the hook with no refs anyway... but the abort/filter\norder is remarkable and should be documented in some way IMHO.\n\n> \n> Documentation/githooks.txt | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 9ef2469..605ba4d 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. If a ref is to be\n> deleted, the `` will be supplied as `(delete)` and the ` SHA-1>` will be 40 `0`. If the local commit was specified by something other\n> than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be\n> -supplied as it was originally given.\n> +supplied as it was originally given. A request to update remote ref that has\n> +already been rejected for other reasons (e.g. failing to pass a fast-forward\n> +test) does not appear in the input.\n> \n> If this hook exits with a non-zero status, 'git push' will abort without\n> pushing anything. Information about why the push is rejected may be sent\n\nLGTM... if you don't want to promise anything then maybe something like\n\"may not appear\" instead of \"does not appear\" is better. As long as the\nreason for no input is stated more explicitly than currently I am happy.\n\n-- \nKind regards/Mit freundlichen Grüßen, Stefan Tauner\n"}]}