In https://lore.kernel.org/git/xmqqv99i4ck2.fsf@gitster.g/ 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. 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. Change from last version: Change docs example "Count-count" to "Helped-by". ZheNing Hu (2): [GSOC] docs: correct descript of trailer.<token>.command [GSOC] trailer: add new .cmd config option Documentation/git-interpret-trailers.txt | 94 ++++++++++++++++++++---- t/t7513-interpret-trailers.sh | 84 +++++++++++++++++++++ trailer.c | 35 ++++++--- 3 files changed, 187 insertions(+), 26 deletions(-) base-commit: 142430338477d9d1bb25be66267225fb58498d92 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-913%2Fadlternative%2Ftrailer-pass-ARG-env-v12 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-913/adlternative/trailer-pass-ARG-env-v12 Pull-Request: https://github.com/gitgitgadget/git/pull/913 Range-diff vs v11: 1: 34210e5bd3da ! 1: 8129ef6c476b [GSOC] docs: correct description of .command @@ Metadata Author: ZheNing Hu <adlternative@xxxxxxxxx> ## Commit message ## - [GSOC] docs: correct description of .command + [GSOC] docs: correct descript of trailer.<token>.command In the original documentation of `trailer.<token>.command`, some descriptions are easily misunderstood. So let's modify 2: 9c0fc91aba24 ! 2: edb7f1961ddb [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 @@ Commit message refer to the value as positional argument, $1, in their scripts. At the same time, in order to allow `git interpret-trailers` to better simulate the behavior - of `git command -s`, the first implicitly executed command - will not pass positional parameters, users can use this - feature to suppress its output. + of `git command -s`, 'trailer.<token>.cmd' will not + automatically execute. Helped-by: Junio C Hamano <gitster@xxxxxxxxx> Helped-by: Christian Couder <christian.couder@xxxxxxxxx> @@ 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>. -+It is worth mentioning that the command is first called to add a -+trailer with the specified <token> and without positional argument. -+Users can make use of this output when they need automatically add -+some trailers. On the other hand, users can use a trick to suppress -+this output by judging whether the number of positional parameters -+is equal to one, if it is true, execute the commands, otherwise exit -+with non-zero to suppress the output. ++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 Fix #42 ------------ -+* Configure a 'cnt' trailer with a cmd use a global script `gcount` -+to record commit counts of a specified author and show how it works: ++* Configure a 'help' trailer with a cmd use a script `glog-find-author` ++ which search specified author identity from git log in git repository ++ and show how it works: ++ +------------ -+$ cat ~/bin/gcount ++$ cat ~/bin/glog-find-author +#!/bin/sh -+if test "$#" != 1 -+then -+ exit 1 -+else -+ test -n "$1" && git shortlog -s --author="$1" HEAD || true -+fi -+$ git config trailer.cnt.key "Commit-count: " -+$ git config trailer.cnt.ifExists "addIfDifferentNeighbor" -+$ git config trailer.cnt.cmd "~/bin/gcount" -+$ git interpret-trailers --trailer="cnt:Junio" --trailer="cnt:Linus Torvalds"<<EOF ++test -n "$1" && git log --author="$1" --pretty="%an <%ae>" -1 || true ++$ git config trailer.help.key "Helped-by: " ++$ git config trailer.help.ifExists "addIfDifferentNeighbor" ++$ git config trailer.help.cmd "~/bin/glog-find-author" ++$ git interpret-trailers --trailer="help:Junio" --trailer="help:Couder" <<EOF +> subject -+> ++> +> message -+> ++> +> EOF +subject + +message + -+Commit-count: 22484 Junio C Hamano -+Commit-count: 1117 Linus Torvalds ++Helped-by: Junio C Hamano <gitster@xxxxxxxxx> ++Helped-by: Christian Couder <christian.couder@xxxxxxxxx> +------------ + -+* Configure a 'ref' trailer with a cmd use a global script `glog-grep` ++* Configure a 'ref' trailer with a cmd use a script `glog-grep` + to grep last relevant commit from git log in the git repository + and show how it works: ++ +------------ +$ cat ~/bin/glog-grep +#!/bin/sh -+if test "$#" != 1 -+then -+ exit 1 -+else -+ test -n "$1" && git log --grep "$1" --pretty=reference -1 || true -+fi ++test -n "$1" && git log --grep "$1" --pretty=reference -1 || true +$ git config trailer.ref.key "Reference-to: " +$ git config trailer.ref.ifExists "replace" +$ git config trailer.ref.cmd "~/bin/glog-grep" +$ git interpret-trailers --trailer="ref:Add copyright notices." <<EOF +> subject -+> ++> +> message -+> ++> +> EOF +subject + @@ 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' ' + git config trailer.bug.cmd "echo \"\$1\" is" && + cat >expected2 <<-EOF && + -+ Bug-maker: is + Bug-maker: him is him + Bug-maker: me is me + EOF @@ t/t7513-interpret-trailers.sh: test_expect_success 'setup' ' + EOF + cat >echoscript <<-EOF && + #!/bin/sh -+ if test "\$#" != 1 -+ then -+ exit 1 -+ else -+ echo who is "\$1" -+ fi ++ echo who is "\$1" + EOF + chmod +x echoscript && + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ + >actual2 && + test_cmp expected2 actual2 +' -+ -+test_expect_success 'with cmd, $1 and without --trailer' ' -+ test_when_finished "git config --remove-section trailer.bug" && -+ test_when_finished "git config --remove-section trailer.gub" && -+ git config trailer.bug.key "Bug-maker: " && -+ git config trailer.bug.ifExists "replace" && -+ git config trailer.bug.cmd "./echoscript" && -+ git config trailer.gub.key "Gub-maker: " && -+ git config trailer.gub.ifExists "replace" && -+ git config trailer.gub.cmd "./echoscript2" && -+ cat >expected2 <<-EOF && -+ -+ Gub-maker: si ohw -+ EOF -+ cat >echoscript <<-EOF && -+ #!/bin/sh -+ if test "\$#" != 1 -+ then -+ exit 1 -+ else -+ echo who is "\$1" -+ fi -+ EOF -+ cat >echoscript2 <<-EOF && -+ echo si ohw "\$1" -+ EOF -+ chmod +x echoscript && -+ chmod +x echoscript2 && -+ git interpret-trailers >actual2 && -+ test_cmp expected2 actual2 -+' + test_expect_success 'without config' ' sed -e "s/ Z\$/ /" >expected <<-\EOF && @@ trailer.c: static void free_arg_item(struct arg_item *item) free(item->conf.command); + free(item->conf.cmd); free(item->token); -- free(item->value); -+ if (item->value) -+ FREE_AND_NULL(item->value); + free(item->value); free(item); - } - @@ trailer.c: static int check_if_different(struct trailer_item *in_tok, return 1; } @@ trailer.c: static int check_if_different(struct trailer_item *in_tok, cp.env = local_repo_env; cp.no_stdin = 1; cp.use_shell = 1; - - if (capture_command(&cp, &buf, 1024)) { -- error(_("running trailer command '%s' failed"), cmd.buf); - strbuf_release(&buf); -- result = xstrdup(""); -+ if (!conf->cmd || arg) { -+ error(_("running trailer command '%s' failed"), cmd.buf); -+ result = xstrdup(""); -+ } else -+ result = NULL; - } else { - strbuf_trim(&buf); - result = strbuf_detach(&buf, NULL); @@ trailer.c: static char *apply_command(const char *command, const char *arg) static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok) { - if (arg_tok->conf.command) { -- const char *arg; -- if (arg_tok->value && arg_tok->value[0]) { + if (arg_tok->conf.command || arg_tok->conf.cmd) { -+ const char *arg = NULL; -+ -+ if ((arg_tok->value && arg_tok->value[0]) || -+ (arg_tok->conf.cmd && !arg_tok->value)) { + const char *arg; + if (arg_tok->value && arg_tok->value[0]) { arg = arg_tok->value; - } else { - if (in_tok && in_tok->value) @@ trailer.c: static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg else arg = xstrdup(""); } - arg_tok->value = apply_command(arg_tok->conf.command, arg); -- free((char *)arg); + arg_tok->value = apply_command(&arg_tok->conf, arg); -+ if (arg) -+ free((char *)arg); + free((char *)arg); } } - -@@ trailer.c: static void apply_arg_if_exists(struct trailer_item *in_tok, - break; - case EXISTS_REPLACE: - apply_item_command(in_tok, arg_tok); -+ if (!arg_tok->value) { -+ free_arg_item(arg_tok); -+ return; -+ } - add_arg_to_input_list(on_tok, arg_tok); - list_del(&in_tok->list); - free_trailer_item(in_tok); - break; - case EXISTS_ADD: - apply_item_command(in_tok, arg_tok); -+ if (!arg_tok->value) { -+ free_arg_item(arg_tok); -+ return; -+ } - add_arg_to_input_list(on_tok, arg_tok); - break; - case EXISTS_ADD_IF_DIFFERENT: - apply_item_command(in_tok, arg_tok); -+ if (!arg_tok->value) { -+ free_arg_item(arg_tok); -+ return; -+ } - if (check_if_different(in_tok, arg_tok, 1, head)) - add_arg_to_input_list(on_tok, arg_tok); - else -@@ trailer.c: static void apply_arg_if_exists(struct trailer_item *in_tok, - break; - case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR: - apply_item_command(in_tok, arg_tok); -+ if (!arg_tok->value) { -+ free_arg_item(arg_tok); -+ return; -+ } - if (check_if_different(on_tok, arg_tok, 0, head)) - add_arg_to_input_list(on_tok, arg_tok); - else -@@ trailer.c: static void apply_arg_if_missing(struct list_head *head, - case MISSING_ADD: - where = arg_tok->conf.where; - apply_item_command(NULL, arg_tok); -+ if (!arg_tok->value) { -+ free_arg_item(arg_tok); -+ return; -+ } - to_add = trailer_from_arg(arg_tok); - if (after_or_end(where)) - list_add_tail(&to_add->list, head); @@ trailer.c: static void duplicate_conf(struct conf_info *dst, const struct conf_info *src) dst->name = xstrdup_or_null(src->name); dst->key = xstrdup_or_null(src->key); @@ 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, - xstrdup(token_from_item(item, NULL)), - xstrdup(""), - &item->conf, NULL); -+ else if (item->conf.cmd) -+ add_arg_item(arg_head, -+ xstrdup(token_from_item(item, NULL)), -+ NULL, -+ &item->conf, NULL); - } - - /* Add an arg item for each trailer on the command line */ -- gitgitgadget
