Subject: [PATCH v10 0/2] [GSOC] trailer: add new .cmd config option

From: "ZheNing Hu via GitGitGadget" <[email-masked]>
Date: Fri, 16 Apr 2021 08:47:53 +0000

In https://lore.kernel.org/git/[email-masked]/ Junio and
Christian talked about the problem of using strbuf_replace() to replace
$ARG:

 1. if the user's script has more than one $ARG, only the first one will be
    replaced, which is incorrected.
 2. $ARG is textually replaced without shell syntax, which may result a
    broken command when $ARG include some unmatching single quote, very
    unsafe.

At the same time, the trailer.<token>.command has another design
disadvantages: It will automatically execute with a empty value in <token>
<value> pair, which will create a trailer that we don’t expect.

Now pass trailer value as $1 to the trailer command with another
trailer.<token>.cmd config, to solve these above problems.

We are now writing documents that are more readable and correct than before.

ZheNing Hu (2):
  [GSOC] docs: correct descript of trailer.<token>.command
  [GSOC] trailer: add new .cmd config option

 Documentation/git-interpret-trailers.txt | 93 ++++++++++++++++++++----
 t/t7513-interpret-trailers.sh            | 84 +++++++++++++++++++++
 trailer.c                                | 35 ++++++---
 3 files changed, 186 insertions(+), 26 deletions(-)

base-commit: 142430338477d9d1bb25be66267225fb58498d92
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-913%2Fadlternative%2Ftrailer-pass-ARG-env-v10
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-913/adlternative/trailer-pass-ARG-env-v10
Pull-Request: https://github.com/gitgitgadget/git/pull/913

Range-diff vs v9:

 1:  8129ef6c476b = 1:  8129ef6c476b [GSOC] docs: correct descript of trailer.<token>.command
 2:  7f645ec95f48 ! 2:  daa889bd0ade [GSOC] trailer: add new .cmd config option
     @@ Commit message
          replaced with the value given to the `interpret-trailer`
          command for the token in a '--trailer <token>=<value>' argument.
      
     -    This has two downsides:
     +    This has three downsides:
      
          * The use of $ARG in the mechanism misleads the users that
          the value is passed in the shell variable, and tempt them
     @@ Commit message
          a broken command that is not syntactically correct (or
          worse).
      
     +    * The first occurrence of substring `$ARG` will be replaced
     +    with the empty string, in the command when the command is
     +    first called to add a trailer with the specified <token>.
     +    This is a bad design, the nature of automatic execution
     +    causes it to add a trailer that we don't expect.
     +
          Introduce a new `trailer.<token>.cmd` configuration that
          takes higher precedence to deprecate and eventually remove
          `trailer.<token>.command`, which passes the value as an
          argument to the command.  Instead of "$ARG", users can
          refer to the value as positional argument, $1, in their
     -    scripts.
     +    scripts. At the same time, in order to allow
     +    `git interpret-trailers` to better simulate the behavior
     +    of `git command -s`, 'trailer.<token>.cmd' will not
     +    automatically execute.
      
          Helped-by: Junio C Hamano <[email-masked]>
          Helped-by: Christian Couder <[email-masked]>
     @@ Documentation/git-interpret-trailers.txt: leading and trailing whitespace trimme
      -occurrence of substring `$ARG` in the command. This way the
      -command can produce a <value> computed from the <value> passed
      -in the '--trailer <token>=<value>' argument.
     -+of these arguments, if any, will be passed to the command as its
     -+first argument. This way the command can produce a <value> computed
     -+from the <value> passed in the '--trailer <token>=<value>' argument.
     - +
     +-+
      -For consistency, the first occurrence of substring `$ARG` is
      -also replaced, this time with the empty string, in the command
      -when the command is first called to add a trailer with the
      -specified <token>.
     -+For consistency, the $1 is also passed, this time with the empty string,
     -+in the command when the command is first called to add a trailer with
     -+the specified <token>.
     ++of these arguments, if any, will be passed to the command as its
     ++first argument. This way the command can produce a <value> computed
     ++from the <value> passed in the '--trailer <token>=<value>' argument.
       
       EXAMPLES
       --------
     @@ Documentation/git-interpret-trailers.txt: subject
      +#!/bin/sh
      +test -n "$1" && git shortlog -s --author="$1" HEAD || true
      +$ git config trailer.cnt.key "Commit-count: "
     -+$ git config trailer.cnt.ifExists "replace"
     ++$ git config trailer.cnt.ifExists "addIfDifferentNeighbor"
      +$ git config trailer.cnt.cmd "~/bin/gcount"
     -+$ git interpret-trailers --trailer="cnt:Junio" <<EOF
     ++$ git interpret-trailers --trailer="cnt:Junio" --trailer="cnt:Linus Torvalds"<<EOF
      +> subject
      +> 
      +> message
     @@ Documentation/git-interpret-trailers.txt: subject
      +message
      +
      +Commit-count: 22484     Junio C Hamano
     ++Commit-count: 1117      Linus Torvalds
      +------------
      +
      +* Configure a 'ref' trailer with a cmd use a global script `glog-grep`
     @@ t/t7513-interpret-trailers.sh: test_expect_success 'setup' '
      +	git config trailer.bug.cmd "echo \"maybe is\"" &&
      +	cat >expected2 <<-EOF &&
      +
     -+	Bug-maker: maybe is
      +	Bug-maker: maybe is him
      +	Bug-maker: maybe is me
      +	EOF
     @@ t/t7513-interpret-trailers.sh: test_expect_success 'setup' '
      +test_expect_success 'with cmd and $1' '
      +	test_when_finished "git config --remove-section trailer.bug" &&
      +	git config trailer.bug.key "Bug-maker: " &&
     -+	git config trailer.bug.ifExists "replace" &&
     ++	git config trailer.bug.ifExists "add" &&
      +	git config trailer.bug.cmd "echo \"\$1\" is" &&
      +	cat >expected2 <<-EOF &&
      +
     ++	Bug-maker: him is him
      +	Bug-maker: me is me
      +	EOF
      +	git interpret-trailers --trailer "bug: him" --trailer "bug:me" \
     @@ trailer.c: static int git_trailer_config(const char *conf_key, const char *value
       	case TRAILER_WHERE:
       		if (trailer_set_where(&conf->where, value))
       			warning(_("unknown value '%s' for key '%s'"), value, conf_key);
     -@@ trailer.c: static void process_command_line_args(struct list_head *arg_head,
     - 	/* Add an arg item for each configured trailer with a command */
     - 	list_for_each(pos, &conf_head) {
     - 		item = list_entry(pos, struct arg_item, list);
     --		if (item->conf.command)
     -+		if (item->conf.cmd || item->conf.command)
     - 			add_arg_item(arg_head,
     - 				     xstrdup(token_from_item(item, NULL)),
     - 				     xstrdup(""),

-- 
gitgitgadget