Hey guys, I was on vacation for a little over a week, I'll be back on this this coming week (haven't forgotten). David On Wed, Mar 26, 2014 at 4:21 PM, Philip Oakley <philipoakley@xxxxxxx> wrote: > From: "Junio C Hamano" <gitster@xxxxxxxxx> >> >> David Cowden <dcow90@xxxxxxxxx> writes: >> >> >>> The documentation as-is does not mention that the pre-push hook is >>> executed even when there is nothing to push. This can lead a new >>> reader to believe there will always be lines fed to the script's >>> standard input and cause minor confusion as to what is happening >>> when there are no lines provided to the pre-push script. >>> >>> Signed-off-by: David Cowden <dcow90@xxxxxxxxx> >>> --- >>> >>> Notes: >>> I'm not sure if I've covered every case here. If there are more >>> cases to >>> consider, please let me know and I can update to include them. >> >> >> I do not think of any offhand, but a more important point that I was >> trying to get at was that we should not give an incorrect impression >> to the readers that the scenario that is described is the only case >> they need to be worried about by pretending to be exhaustive. >> >> The "may" in your wording "This may happen when" may be good enough >> to hint that these may not be the only cases. >> >>> c.f. >>> >>> http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin >>> >>> Documentation/githooks.txt | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt >>> index d954bf6..1fd6da9 100644 >>> --- a/Documentation/githooks.txt >>> +++ b/Documentation/githooks.txt >>> @@ -203,6 +203,15 @@ 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. >>> >>> +The hook is executed regardless of whether changes will actually be >>> pushed or >>> +not. This may happen when 'git push' is called and: >>> + >>> + - the remote ref is already up to date, or >>> + - pushing to the remote ref cannot be handled by a simple >>> fast-forward >>> + >>> +In other words, the script is called for every push. In the event >>> that nothing >>> +is to be pushed, no data will be provided on the script's standard >>> input. > > > Doesn't an 'in other words' indicate it could be further tightened? > Maybe > "If there is nothing to push, the hook will still run, but the input > line will be empty. > > Likewise the hook will still run for other cases such as: > > - the remote ref is already up to date, > - pushing to the remote ref cannot be handled by a simple > fast-forward, > - etc." > > >> >> When two things are to be pushed, the script will see the two >> things. When one thing is to be pushed, the script will see the one >> thing. When no thing is to be pushed, the script will see no thing >> on its standard input. >> >> But isn't that obvious? I still wonder if we really need to single >> out that "nothing" case. The more important thing is that it is >> invoked even in the "0-thing pushed" case, and "the list of things >> pushed that is given to the hook happens to be empty" is an obvious >> natural fallout. > > > Personally I think it should be mentioned in that paragraph, which is > covering all the various special cases. The 'nothing' case often causes > confusion when it's not specified in documentation. >> >> >>> 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 the user by writing to standard error. >> >> -- > > > It may be that the documentation should include the caveat > > "Hooks, when enabled, are executed unconditionally by their calling > functions. > Script writers should ensure they handle all conditions." > > somewhere near the top of the page to cover all hooks, which IIRC > started David's journey. That would allow my second paragraph > "Likewise.." to be dropped. > > Philip > -- > [apologies for any whitespace damage] > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html