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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> On Fri, Jul 1, 2016 at 10:59 AM, Jeff King <peff@xxxxxxxx> 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
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]