From: ZheNing Hu <adlternative@xxxxxxxxx>
In the original implementation of `trailer.<token>.command`,
use `strbuf_replace` to replace $ARG in the <value> of the
trailer, but it have a problem: `strbuf_replace` replace the
$ARG in command only once, If the user's trailer command have
used more than one $ARG, error will occur.
If directly modify the implementation of the original
`trailer.<token>.command`, The user’s previous `'$ARG'` in
trailer command will not be replaced. So now add new
config "trailer.<token>.cmd", pass trailer's value as
positional parameter 1 to the user's command, users can
use $1 as trailer's value, to implement original variable
replacement.
Original `trailer.<token>.command` can still be used until git
completely abandoned it.
Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
---
[GSOC]trailer: pass arg as positional parameter
In https://lore.kernel.org/git/xmqqv99i4ck2.fsf@gitster.g/ Junio and
Christian talked about the problem of using strbuf_replace() to replace
$ARG.
Now pass trailer value as $1 to the trailer command with another
trailer.<token>.cmd config.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-913%2Fadlternative%2Ftrailer-pass-ARG-env-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-913/adlternative/trailer-pass-ARG-env-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/913
Range-diff vs v1:
1: abc5b04d152f ! 1: 185356d6fc90 [GSOC]trailer: change $ARG to environment variable
@@ Metadata
Author: ZheNing Hu <adlternative@xxxxxxxxx>
## Commit message ##
- [GSOC]trailer: change $ARG to environment variable
+ [GSOC]trailer: pass arg as positional parameter
In the original implementation of `trailer.<token>.command`,
- use `strbuf_replace` to replace `$ARG` in the <value> of the trailer,
- but it have a problem:`strbuf_replace` replace the `$ARG` in command
- only once, If the user's trailer command have used more then one `$ARG`,
- error will occur.
+ use `strbuf_replace` to replace $ARG in the <value> of the
+ trailer, but it have a problem: `strbuf_replace` replace the
+ $ARG in command only once, If the user's trailer command have
+ used more than one $ARG, error will occur.
- So pass `$ARG` as an environment variable to the trailer command,
- all `$ARG` in the command will be replaced, which will fix this problem.
+ If directly modify the implementation of the original
+ `trailer.<token>.command`, The user’s previous `'$ARG'` in
+ trailer command will not be replaced. So now add new
+ config "trailer.<token>.cmd", pass trailer's value as
+ positional parameter 1 to the user's command, users can
+ use $1 as trailer's value, to implement original variable
+ replacement.
+
+ Original `trailer.<token>.command` can still be used until git
+ completely abandoned it.
Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
## Documentation/git-interpret-trailers.txt ##
-@@ Documentation/git-interpret-trailers.txt: line, where <value> is taken to be the standard output of the
- specified command with any leading and trailing whitespace trimmed
- off.
- +
--If the command contains the `$ARG` string, this string will be
-+If the command contains the `$ARG` string (`$ARG` is an exported
-+environment variable), this string will be
- replaced with the <value> part of an existing trailer with the same
- <token>, if any, before the command is launched.
- +
+@@ Documentation/git-interpret-trailers.txt: also be executed for each of these arguments. And the <value> part of
+ these arguments, if any, will be used to replace the `$ARG` string in
+ the command.
+
++trailer.<token>.cmd::
++ Similar to 'trailer.<token>.command'. But the difference is that
++ `$1` is used in the command to replace the value of the trailer
++ instead of the original `$ARG`, which means that we can quote the
++ trailer value multiple times in the command.
++ E.g. `trailer.sign.cmd="test -n \"$1\" && echo \"$1\" || true "`
++
+ EXAMPLES
+ --------
+
## t/t7513-interpret-trailers.sh ##
-@@ t/t7513-interpret-trailers.sh: test_expect_success 'with command using $ARG' '
- test_cmp expected actual
+@@ t/t7513-interpret-trailers.sh: test_expect_success 'setup a commit' '
+ git commit -m "Add file a.txt"
'
-+test_expect_success 'with command using more than one $ARG' '
++test_expect_success 'with cmd using $1' '
++ test_when_finished "git config --unset trailer.fix.cmd" &&
+ git config trailer.fix.ifExists "replace" &&
-+ git config trailer.fix.command "test -n $ARG && git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG || true" &&
++ git config trailer.fix.cmd "test -n \"\$1\" && git log -1 --oneline --format=\"%h (%s)\" \
++ --abbrev-commit --abbrev=14 \"\$1\" || true" &&
+ FIXED=$(git log -1 --oneline --format="%h (%s)" --abbrev-commit --abbrev=14 HEAD) &&
-+ cat complex_message_body >expected &&
-+ sed -e "s/ Z\$/ /" >>expected <<-EOF &&
++ cat complex_message_body >expected2 &&
++ sed -e "s/ Z\$/ /" >>expected2 <<-EOF &&
+ Fixes: $FIXED
+ Acked-by= Z
+ Reviewed-by:
@@ t/t7513-interpret-trailers.sh: test_expect_success 'with command using $ARG' '
+ Signed-off-by: A U Thor <author@xxxxxxxxxxx>
+ EOF
+ git interpret-trailers --trailer "review:" --trailer "fix=HEAD" \
-+ <complex_message >actual &&
-+ test_cmp expected actual
++ <complex_message >actual2 &&
++ test_cmp expected2 actual2
+'
+
- test_expect_success 'with failing command using $ARG' '
+ test_expect_success 'with command using $ARG' '
git config trailer.fix.ifExists "replace" &&
- git config trailer.fix.command "false \$ARG" &&
+- git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG" &&
++ git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" \
++ --abbrev-commit --abbrev=14 \$ARG" &&
+ FIXED=$(git log -1 --oneline --format="%h (%s)" --abbrev-commit --abbrev=14 HEAD) &&
+ cat complex_message_body >expected &&
+ sed -e "s/ Z\$/ /" >>expected <<-EOF &&
## trailer.c ##
-@@ trailer.c: static char *separators = ":";
-
- static int configured;
-
--#define TRAILER_ARG_STRING "$ARG"
-+#define TRAILER_ARG_STRING "ARG"
+@@ trailer.c: struct conf_info {
+ char *name;
+ char *key;
+ char *command;
++ int is_new_cmd;
+ enum trailer_where where;
+ enum trailer_if_exists if_exists;
+ enum trailer_if_missing if_missing;
+@@ trailer.c: static int check_if_different(struct trailer_item *in_tok,
+ return 1;
+ }
- static const char *git_generated_prefixes[] = {
- "Signed-off-by: ",
-@@ trailer.c: static char *apply_command(const char *command, const char *arg)
+-static char *apply_command(const char *command, const char *arg)
++static char *apply_command(const char *command, int is_new_cmd , const char *arg)
+ {
+ struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
- struct child_process cp = CHILD_PROCESS_INIT;
+@@ trailer.c: static char *apply_command(const char *command, const char *arg)
char *result;
-+ const char *const *var;
strbuf_addstr(&cmd, command);
-+ for (var = local_repo_env; *var; var++)
-+ strvec_push(&cp.env_array, *var);
- if (arg)
+- if (arg)
- strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);
-+ strvec_pushf(&cp.env_array, "%s=%s", TRAILER_ARG_STRING, arg);
-
+-
strvec_push(&cp.args, cmd.buf);
-- cp.env = local_repo_env;
-+
++ if (arg) {
++ if (is_new_cmd)
++ strvec_push(&cp.args, arg);
++ else
++ strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);
++ }
+ cp.env = local_repo_env;
cp.no_stdin = 1;
cp.use_shell = 1;
+@@ 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);
++ arg_tok->value = apply_command(arg_tok->conf.command, arg_tok->conf.is_new_cmd, arg);
+ free((char *)arg);
+ }
+ }
+@@ trailer.c: static struct arg_item *get_conf_item(const char *name)
+ return item;
+ }
+
+-enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,
+- TRAILER_IF_EXISTS, TRAILER_IF_MISSING };
++enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_CMD,
++ TRAILER_WHERE, TRAILER_IF_EXISTS, TRAILER_IF_MISSING };
+ static struct {
+ const char *name;
+@@ trailer.c: static struct {
+ } trailer_config_items[] = {
+ { "key", TRAILER_KEY },
+ { "command", TRAILER_COMMAND },
++ { "cmd", TRAILER_CMD },
+ { "where", TRAILER_WHERE },
+ { "ifexists", TRAILER_IF_EXISTS },
+ { "ifmissing", TRAILER_IF_MISSING }
+@@ trailer.c: static int git_trailer_config(const char *conf_key, const char *value, void *cb)
+ case TRAILER_COMMAND:
+ if (conf->command)
+ warning(_("more than one %s"), conf_key);
++ conf->is_new_cmd = 0;
++ conf->command = xstrdup(value);
++ break;
++ case TRAILER_CMD:
++ if (conf->command)
++ warning(_("more than one %s"), conf_key);
++ conf->is_new_cmd = 1;
+ conf->command = xstrdup(value);
+ break;
+ case TRAILER_WHERE:
Documentation/git-interpret-trailers.txt | 7 +++++++
t/t7513-interpret-trailers.sh | 22 +++++++++++++++++++-
trailer.c | 26 +++++++++++++++++-------
3 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 96ec6499f001..38656b1b3841 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -252,6 +252,13 @@ also be executed for each of these arguments. And the <value> part of
these arguments, if any, will be used to replace the `$ARG` string in
the command.
+trailer.<token>.cmd::
+ Similar to 'trailer.<token>.command'. But the difference is that
+ `$1` is used in the command to replace the value of the trailer
+ instead of the original `$ARG`, which means that we can quote the
+ trailer value multiple times in the command.
+ E.g. `trailer.sign.cmd="test -n \"$1\" && echo \"$1\" || true "`
+
EXAMPLES
--------
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 6602790b5f4c..7cb81201442a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -1274,9 +1274,29 @@ test_expect_success 'setup a commit' '
git commit -m "Add file a.txt"
'
+test_expect_success 'with cmd using $1' '
+ test_when_finished "git config --unset trailer.fix.cmd" &&
+ git config trailer.fix.ifExists "replace" &&
+ git config trailer.fix.cmd "test -n \"\$1\" && git log -1 --oneline --format=\"%h (%s)\" \
+ --abbrev-commit --abbrev=14 \"\$1\" || true" &&
+ FIXED=$(git log -1 --oneline --format="%h (%s)" --abbrev-commit --abbrev=14 HEAD) &&
+ cat complex_message_body >expected2 &&
+ sed -e "s/ Z\$/ /" >>expected2 <<-EOF &&
+ Fixes: $FIXED
+ Acked-by= Z
+ Reviewed-by:
+ Signed-off-by: Z
+ Signed-off-by: A U Thor <author@xxxxxxxxxxx>
+ EOF
+ git interpret-trailers --trailer "review:" --trailer "fix=HEAD" \
+ <complex_message >actual2 &&
+ test_cmp expected2 actual2
+'
+
test_expect_success 'with command using $ARG' '
git config trailer.fix.ifExists "replace" &&
- git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG" &&
+ git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" \
+ --abbrev-commit --abbrev=14 \$ARG" &&
FIXED=$(git log -1 --oneline --format="%h (%s)" --abbrev-commit --abbrev=14 HEAD) &&
cat complex_message_body >expected &&
sed -e "s/ Z\$/ /" >>expected <<-EOF &&
diff --git a/trailer.c b/trailer.c
index be4e9726421c..80f47657ff1a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -14,6 +14,7 @@ struct conf_info {
char *name;
char *key;
char *command;
+ int is_new_cmd;
enum trailer_where where;
enum trailer_if_exists if_exists;
enum trailer_if_missing if_missing;
@@ -216,7 +217,7 @@ static int check_if_different(struct trailer_item *in_tok,
return 1;
}
-static char *apply_command(const char *command, const char *arg)
+static char *apply_command(const char *command, int is_new_cmd , const char *arg)
{
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
@@ -224,10 +225,13 @@ static char *apply_command(const char *command, const char *arg)
char *result;
strbuf_addstr(&cmd, command);
- if (arg)
- strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);
-
strvec_push(&cp.args, cmd.buf);
+ if (arg) {
+ if (is_new_cmd)
+ strvec_push(&cp.args, arg);
+ else
+ strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);
+ }
cp.env = local_repo_env;
cp.no_stdin = 1;
cp.use_shell = 1;
@@ -257,7 +261,7 @@ 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);
+ arg_tok->value = apply_command(arg_tok->conf.command, arg_tok->conf.is_new_cmd, arg);
free((char *)arg);
}
}
@@ -454,8 +458,8 @@ static struct arg_item *get_conf_item(const char *name)
return item;
}
-enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,
- TRAILER_IF_EXISTS, TRAILER_IF_MISSING };
+enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_CMD,
+ TRAILER_WHERE, TRAILER_IF_EXISTS, TRAILER_IF_MISSING };
static struct {
const char *name;
@@ -463,6 +467,7 @@ static struct {
} trailer_config_items[] = {
{ "key", TRAILER_KEY },
{ "command", TRAILER_COMMAND },
+ { "cmd", TRAILER_CMD },
{ "where", TRAILER_WHERE },
{ "ifexists", TRAILER_IF_EXISTS },
{ "ifmissing", TRAILER_IF_MISSING }
@@ -540,6 +545,13 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
case TRAILER_COMMAND:
if (conf->command)
warning(_("more than one %s"), conf_key);
+ conf->is_new_cmd = 0;
+ conf->command = xstrdup(value);
+ break;
+ case TRAILER_CMD:
+ if (conf->command)
+ warning(_("more than one %s"), conf_key);
+ conf->is_new_cmd = 1;
conf->command = xstrdup(value);
break;
case TRAILER_WHERE:
base-commit: 142430338477d9d1bb25be66267225fb58498d92
--
gitgitgadget
