All of lore.kernel.org
 help / color / mirror / Atom feed
From: Aaron Schrab <aaron@schrab.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/4] pre-push hook support
Date: Sat, 29 Dec 2012 09:50:25 -0500	[thread overview]
Message-ID: <20121229145025.GA3789@pug.qqx.org> (raw)
In-Reply-To: <7v1ue9hb06.fsf@alter.siamese.dyndns.org>

At 18:01 -0800 28 Dec 2012, Junio C Hamano <gitster@pobox.com> wrote:
>One lesson we learned long time ago while doing hooks is to avoid
>unbound number of command line arguments and instead feed them from
>the standard input.  I think this should do the same.

Good point.  I had been trying to keep the interface for this hook as 
close as possible to the ones for other client-side hooks on the theory 
that less development effort may go into those than for server-side 
hooks.  But thinking on that more I certainly see that this could easily 
run into limits on argument length on some systems; especially when it's 
likely that each of those arguments is likely to be over 100 bytes long.

I'll work on an updated version which sends the variable length 
information over a pipe, using the command-line arguments only to pass 
the remote name and URL.

>How does the hook communicate its decision to the calling Git?
>
>Will it be "all-or-none", or "I'll allow these but not those"?

Currently it just uses the exit code to communicate that back, so it's 
all-or-none.  I think I'll keep that in the updated version as well.

A future enhancement could modify the protocol to support reading from 
the hook's stdout the names of remote refs which are to be rejected, I 
think that just having the option for all-or-nothing is a good starting 
point.

  reply	other threads:[~2012-12-29 14:50 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-28 22:57 [PATCH 0/4] pre-push hook support Aaron Schrab
2012-12-28 22:57 ` [PATCH 1/4] hooks: Add function to check if a hook exists Aaron Schrab
2012-12-29  2:08   ` Junio C Hamano
2012-12-29 14:50     ` Aaron Schrab
2012-12-29 16:54       ` Junio C Hamano
2012-12-28 22:57 ` [PATCH 2/4] hooks: support variable number of parameters Aaron Schrab
2012-12-28 22:57 ` [PATCH 3/4] push: Add support for pre-push hooks Aaron Schrab
2012-12-28 22:57 ` [PATCH 4/4] Add sample pre-push hook script Aaron Schrab
2012-12-29  2:01 ` [PATCH 0/4] pre-push hook support Junio C Hamano
2012-12-29 14:50   ` Aaron Schrab [this message]
2012-12-29 16:48     ` Junio C Hamano
2013-01-13  5:17 ` [PATCH v2 0/3] " Aaron Schrab
2013-01-14 17:42   ` Junio C Hamano
2013-01-14 22:54     ` Junio C Hamano
2013-01-15  0:24       ` Junio C Hamano
2013-01-13  5:17 ` [PATCH v2 1/3] hooks: Add function to check if a hook exists Aaron Schrab
2013-01-13  5:17 ` [PATCH v2 2/3] push: Add support for pre-push hooks Aaron Schrab
2013-01-14 17:39   ` Junio C Hamano
2013-01-15  0:36   ` Junio C Hamano
2013-01-15  3:12     ` Junio C Hamano
2013-01-13  5:17 ` [PATCH v2 3/3] Add sample pre-push hook script Aaron Schrab
2013-01-14 17:42   ` Junio C Hamano

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=20121229145025.GA3789@pug.qqx.org \
    --to=aaron@schrab.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.