Re: [PATCH 1/4] push options: {pre,post}-receive hook learns about push options

From: Jeff King <peff@peff.net>
Date: Jul 1, 2016, 18:11 UTC
Message-ID: <20160701181102.GA16695@sigill.intra.peff.net>
In-Reply-To: <CAPc5daWjSW5KM4uUyEBbb+765t50+dUsewF52uPrCiT1HW=NAQ@mail.gmail.com>

On Fri, Jul 01, 2016 at 11:03:06AM -0700, Junio C Hamano wrote:

Show 25 quoted lines

> On Fri, Jul 1, 2016 at 10:59 AM, Jeff King <peff@peff.net> wrote:
> > If you give up on having multiple incarnations of each variable, then I
> > think:
> >
> >   GIT_PUSH_VAR_foo=value_for_foo
> >   GIT_PUSH_VAR_bar=value_for_bar
> >
> > is quite elegant, and easy to use from hooks. It just cannot represent
> > multiple such "foo" variables.
> >
> >> If we did not have a GIT_PUSH_OPTIONS_COUNT and GIT_PUSH_OPTION_<N>
> >> but rather GIT_PUSH_OPTIONS_VARIABLES that contains the other variables,
> >> it may be easier to handle, but whether you read from a file or evaluate the
> >> environment variable is only a minor step, the indirection is there anyway
> >> and this would be very close to what we have above.
> >
> > It makes the server implementation a bit uglier. You have to create the
> > temporary file, and you have to clean it up. What process is responsible
> > for cleaning up stale files? Obviously receive-pack would try to clean
> > up after itself, but what happens when it is "kill -9"'d, or the system
> > goes down, etc? We clean up stale tmp files like tmp_obj_* in git-gc; I
> > think we'd want something like that here.
> 
> It still is not clear to me why the option to pass _COUNT and _VAR_<N> is
> rejected.

It's a little more unwieldy to parse in a shell hook, but not too bad, I guess:

  if test -n "$GIT_PUSH_VAR_COUNT"; then
    i=0
    while test "$i" -lt "$GIT_PUSH_VAR_COUNT"; do
      eval "value=\$GIT_PUSH_VAR_$i"
      case "$value" in
      force=*)
	force=${value#*=}
	;;
      ... and so on ...
      esac
    done
  fi

  ...
  if test "$force" = true
     ...

Compare to:

  if test "$GIT_PUSH_VAR_force" = true
     ...

The "count" method gives you the flexibility to parse multiple keys as lists, last-one-wins, or whatever scheme you want. But it also gives you the _responsibility_ to do the parsing yourself, which is a pain when you want to do the simple thing.

-Peff