git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: David Cowden <dcow90@gmail.com>
Cc: git@vger.kernel.org, philipoakley@iee.org, sunshine@sunshineco.com
Subject: Re: [PATCH v3] Clarify pre-push hook documentation
Date: Tue, 25 Mar 2014 10:11:09 -0700	[thread overview]
Message-ID: <xmqqmwgemceq.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1395705088-82216-1-git-send-email-dcow90@gmail.com> (David Cowden's message of "Mon, 24 Mar 2014 16:51:28 -0700")

David Cowden <dcow90@gmail.com> 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@gmail.com>
> ---
>
> 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.

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.

>  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.

  reply	other threads:[~2014-03-25 17:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-23 19:01 [PATCH] Clarify pre-push hook documentation David Cowden
2014-03-23 19:08 ` David Cowden
2014-03-23 20:12   ` Junio C Hamano
2014-03-23 19:18 ` David Cowden
2014-03-24  1:40 ` Eric Sunshine
2014-03-24 23:43 ` [PATCH v2] " David Cowden
2014-03-24 23:51   ` [PATCH v3] " David Cowden
2014-03-25 17:11     ` Junio C Hamano [this message]
2014-03-26 23:21       ` Philip Oakley
2014-04-08  8:43         ` David Cowden

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmwgemceq.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=dcow90@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=philipoakley@iee.org \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).