Re: pre-push hook does not get input on non-fast-forward pushes
On Mon, 28 Mar 2016 14:44:20 -0700 Junio C Hamano wrote: > Stefan Tauner writes: > > The pre-push hook is not the only thing that may prevent you from > pushing a ref update. As you noticed, non-fast-forward check may > trigger and decide that a ref is not going to be pushed, and that > may happen before we call the pre-push hook. > > Information about what is to be pushed is provided on the hook's > standard input with lines of the form ... > > So when the pre-push is called, the refs that will not fast-forward > may not be among the "what is to be pushed", hence not reported. > > We _could_ add something like this to the documentation, but we do > not necessarily want to promise that the order of checks to stay > "internal checks like non-ff check first before pre-push hook", so > this update may not be an improvement. The current text conveys > enough information without making such a promise, but you need to > read it carefully. I understand but that behavior is quite surprising and the wording not that clear (if one is not aware of the behavior already) IMHO. If the fast forward check is done beforehand and no refs remain, why is the script called at all? Or put otherwise, why isn't git aborting before that? That would seem way more logical... consistency is a good argument to call the hook with no refs anyway... but the abort/filter order is remarkable and should be documented in some way IMHO. > > Documentation/githooks.txt | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt > index 9ef2469..605ba4d 100644 > --- a/Documentation/githooks.txt > +++ b/Documentation/githooks.txt > @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. > If a ref is to be > deleted, the `` will be supplied as `(delete)` and the ` SHA-1>` will be 40 `0`. If the local commit was specified by something other > than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be > -supplied as it was originally given. > +supplied as it was originally given. A request to update remote ref that has > +already been rejected for other reasons (e.g. failing to pass a fast-forward > +test) does not appear in the input. > > If this hook exits with a non-zero status, 'git push' will abort without > pushing anything. Information about why the push is rejected may be sent LGTM... if you don't want to promise anything then maybe something like "may not appear" instead of "does not appear" is better. As long as the reason for no input is stated more explicitly than currently I am happy. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pre-push hook does not get input on non-fast-forward pushes
On Mon, Mar 28, 2016 at 2:44 PM, Junio C Hamano wrote: > Stefan Tauner writes: > >> I noticed that without an additional --force the pre-push hook does not >> get any input on stdin if a push would result in non-fast-forward >> uploads. This is not a problem per se (although I don't get the >> rationale) but it is undocumented and the latter left me puzzled. >> >> (Please keep me in CC since I am not subscribed, thanks) > > Thanks (no need to say "please cc me", as that is the norm here). > > The pre-push hook is not the only thing that may prevent you from > pushing a ref update. As you noticed, non-fast-forward check may > trigger and decide that a ref is not going to be pushed, and that > may happen before we call the pre-push hook. > > Information about what is to be pushed is provided on the hook's > standard input with lines of the form ... > > So when the pre-push is called, the refs that will not fast-forward > may not be among the "what is to be pushed", hence not reported. > > We _could_ add something like this to the documentation, but we do > not necessarily want to promise that the order of checks to stay > "internal checks like non-ff check first before pre-push hook", so > this update may not be an improvement. The current text conveys > enough information without making such a promise, but you need to > read it carefully. > > So I dunno. > > Documentation/githooks.txt | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt > index 9ef2469..605ba4d 100644 > --- a/Documentation/githooks.txt > +++ b/Documentation/githooks.txt > @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. > If a ref is to be > deleted, the `` will be supplied as `(delete)` and the ` SHA-1>` will be 40 `0`. If the local commit was specified by something other > than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be > -supplied as it was originally given. > +supplied as it was originally given. A request to update remote ref that has > +already been rejected for other reasons (e.g. failing to pass a fast-forward > +test) does not appear in the input. If we don't want to give promises, s/does not/ may not/ ? Although that sounds like it is non-deterministic (some occur, some don't, maybe it depends on $random_reason) > > If this hook exits with a non-zero status, 'git push' will abort without > pushing anything. Information about why the push is rejected may be sent > > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pre-push hook does not get input on non-fast-forward pushes
Stefan Tauner writes: > I noticed that without an additional --force the pre-push hook does not > get any input on stdin if a push would result in non-fast-forward > uploads. This is not a problem per se (although I don't get the > rationale) but it is undocumented and the latter left me puzzled. > > (Please keep me in CC since I am not subscribed, thanks) Thanks (no need to say "please cc me", as that is the norm here). The pre-push hook is not the only thing that may prevent you from pushing a ref update. As you noticed, non-fast-forward check may trigger and decide that a ref is not going to be pushed, and that may happen before we call the pre-push hook. Information about what is to be pushed is provided on the hook's standard input with lines of the form ... So when the pre-push is called, the refs that will not fast-forward may not be among the "what is to be pushed", hence not reported. We _could_ add something like this to the documentation, but we do not necessarily want to promise that the order of checks to stay "internal checks like non-ff check first before pre-push hook", so this update may not be an improvement. The current text conveys enough information without making such a promise, but you need to read it carefully. So I dunno. Documentation/githooks.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 9ef2469..605ba4d 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. If a ref is to be deleted, the `` will be supplied as `(delete)` and the `` will be 40 `0`. If the local commit was specified by something other than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be -supplied as it was originally given. +supplied as it was originally given. A request to update remote ref that has +already been rejected for other reasons (e.g. failing to pass a fast-forward +test) does not appear in the input. If this hook exits with a non-zero status, 'git push' will abort without pushing anything. Information about why the push is rejected may be sent -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
pre-push hook does not get input on non-fast-forward pushes
Hi, I noticed that without an additional --force the pre-push hook does not get any input on stdin if a push would result in non-fast-forward uploads. This is not a problem per se (although I don't get the rationale) but it is undocumented and the latter left me puzzled. (Please keep me in CC since I am not subscribed, thanks) -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
