From: "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Christian Couder <christian.couder@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
ZheNing Hu <adlternative@gmail.com>
Subject: [PATCH v12 0/2] [GSOC] trailer: add new .cmd config option
Date: Mon, 03 May 2021 15:41:03 +0000 [thread overview]
Message-ID: <pull.913.v12.git.1620056466.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.913.v11.git.1618672417.gitgitgadget@gmail.com>
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@gmail.com>
## 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@pobox.com>
Helped-by: Christian Couder <christian.couder@gmail.com>
@@ 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@pobox.com>
++Helped-by: Christian Couder <christian.couder@gmail.com>
+------------
+
-+* 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
next prev parent reply other threads:[~2021-05-03 15:41 UTC|newest]
Thread overview: 101+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-23 14:53 [PATCH] [GSOC]trailer: change $ARG to environment variable ZheNing Hu via GitGitGadget
2021-03-24 15:42 ` [PATCH v2] [GSOC]trailer: pass arg as positional parameter ZheNing Hu via GitGitGadget
2021-03-24 20:18 ` Junio C Hamano
2021-03-25 1:43 ` ZheNing Hu
2021-03-25 11:53 ` [PATCH v3] " ZheNing Hu via GitGitGadget
2021-03-25 22:28 ` Junio C Hamano
2021-03-26 13:29 ` ZheNing Hu
2021-03-26 16:13 ` [PATCH v4] " ZheNing Hu via GitGitGadget
2021-03-27 18:04 ` Junio C Hamano
2021-03-27 19:53 ` Christian Couder
2021-03-28 10:46 ` ZheNing Hu
2021-03-29 9:04 ` Christian Couder
2021-03-29 13:43 ` ZheNing Hu
2021-03-30 8:45 ` Christian Couder
2021-03-30 11:22 ` ZheNing Hu
2021-03-30 15:07 ` ZheNing Hu
2021-03-30 17:14 ` Junio C Hamano
2021-03-31 5:14 ` ZheNing Hu
2021-03-31 18:19 ` Junio C Hamano
2021-03-31 18:29 ` Junio C Hamano
2021-04-01 3:56 ` ZheNing Hu
2021-04-01 19:49 ` Junio C Hamano
2021-04-02 2:08 ` ZheNing Hu
2021-04-01 3:39 ` ZheNing Hu
2021-03-31 10:05 ` [PATCH v5 0/2] " ZheNing Hu via GitGitGadget
2021-03-31 10:05 ` [PATCH v5 1/2] [GSOC] run-command: add shell_no_implicit_args option ZheNing Hu via GitGitGadget
2021-04-01 7:22 ` Christian Couder
2021-04-01 9:58 ` ZheNing Hu
2021-03-31 10:05 ` [PATCH v5 2/2] [GSOC]trailer: pass arg as positional parameter ZheNing Hu via GitGitGadget
2021-04-01 7:28 ` [PATCH v5 0/2] " Christian Couder
2021-04-01 10:02 ` ZheNing Hu
2021-04-02 13:26 ` [PATCH v6] [GSOC] trailer: add new trailer.<token>.cmd config option ZheNing Hu via GitGitGadget
2021-04-02 20:48 ` Junio C Hamano
2021-04-03 5:08 ` ZheNing Hu
2021-04-04 5:34 ` Junio C Hamano
2021-04-03 5:51 ` Christian Couder
2021-04-04 23:26 ` Junio C Hamano
2021-04-06 3:47 ` Christian Couder
2021-04-06 3:52 ` Christian Couder
2021-04-06 5:16 ` ZheNing Hu
2021-04-06 5:34 ` Junio C Hamano
2021-04-06 5:37 ` Junio C Hamano
2021-04-04 5:43 ` ZheNing Hu
2021-04-04 8:52 ` Christian Couder
2021-04-04 9:53 ` ZheNing Hu
2021-04-02 23:44 ` Junio C Hamano
2021-04-03 3:22 ` ZheNing Hu
2021-04-03 4:31 ` Junio C Hamano
2021-04-03 5:15 ` ZheNing Hu
2021-04-04 13:11 ` [PATCH v7] " ZheNing Hu via GitGitGadget
2021-04-06 16:23 ` Christian Couder
2021-04-07 4:51 ` ZheNing Hu
2021-04-09 13:37 ` [PATCH v8 0/2] [GSOC] trailer: add new .cmd " ZheNing Hu via GitGitGadget
2021-04-09 13:37 ` [PATCH v8 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-04-09 19:02 ` Christian Couder
2021-04-10 13:40 ` ZheNing Hu
2021-04-09 13:37 ` [PATCH v8 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-09 20:18 ` Christian Couder
2021-04-10 14:09 ` ZheNing Hu
2021-04-09 19:59 ` [PATCH v8 0/2] " Christian Couder
2021-04-12 16:39 ` [PATCH v9 " ZheNing Hu via GitGitGadget
2021-04-12 16:39 ` [PATCH v9 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-04-12 20:42 ` Junio C Hamano
2021-04-16 12:03 ` Christian Couder
2021-04-17 1:54 ` Junio C Hamano
2021-04-12 16:39 ` [PATCH v9 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-12 20:51 ` Junio C Hamano
2021-04-13 7:33 ` Christian Couder
2021-04-13 12:02 ` ZheNing Hu
2021-04-13 19:18 ` Junio C Hamano
2021-04-14 13:27 ` ZheNing Hu
2021-04-14 20:33 ` Junio C Hamano
2021-04-15 15:32 ` ZheNing Hu
2021-04-15 17:41 ` Junio C Hamano
2021-04-16 12:54 ` Christian Couder
2021-04-13 18:14 ` Junio C Hamano
2021-04-16 8:47 ` [PATCH v10 0/2] " ZheNing Hu via GitGitGadget
2021-04-16 8:47 ` [PATCH v10 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-04-16 19:11 ` Junio C Hamano
2021-04-16 8:47 ` [PATCH v10 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-16 19:13 ` Junio C Hamano
2021-04-16 19:21 ` Junio C Hamano
2021-04-16 19:25 ` Junio C Hamano
2021-04-17 2:58 ` Junio C Hamano
2021-04-17 3:36 ` Junio C Hamano
2021-04-17 7:41 ` ZheNing Hu
2021-04-17 8:11 ` Junio C Hamano
2021-04-17 15:13 ` [PATCH v11 0/2] " ZheNing Hu via GitGitGadget
2021-04-17 15:13 ` [PATCH v11 1/2] [GSOC] docs: correct description of .command ZheNing Hu via GitGitGadget
2021-04-17 15:13 ` [PATCH v11 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
2021-04-17 22:26 ` [PATCH v11 0/2] " Junio C Hamano
2021-04-18 7:47 ` ZheNing Hu
2021-04-21 0:09 ` Junio C Hamano
2021-04-21 5:47 ` ZheNing Hu
2021-04-21 23:40 ` Junio C Hamano
2021-04-22 9:20 ` ZheNing Hu
2021-04-27 6:49 ` Junio C Hamano
2021-04-27 12:24 ` ZheNing Hu
2021-05-03 15:41 ` ZheNing Hu via GitGitGadget [this message]
2021-05-03 15:41 ` [PATCH v12 1/2] [GSOC] docs: correct descript of trailer.<token>.command ZheNing Hu via GitGitGadget
2021-05-03 15:41 ` [PATCH v12 2/2] [GSOC] trailer: add new .cmd config option ZheNing Hu via GitGitGadget
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=pull.913.v12.git.1620056466.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=adlternative@gmail.com \
--cc=christian.couder@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).