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 then 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. Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx> --- [GSOC] trailer: change $ARG to environment variable In https://lore.kernel.org/git/xmqqv99i4ck2.fsf@gitster.g/ Junio and Christian talked about the problem of using strbuf_replace() to replace $ARG. The current new solution is to pass $ARG as an environment variable into the command. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-913%2Fadlternative%2Ftrailer-pass-ARG-env-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-913/adlternative/trailer-pass-ARG-env-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/913 Documentation/git-interpret-trailers.txt | 3 ++- t/t7513-interpret-trailers.sh | 17 +++++++++++++++++ trailer.c | 9 ++++++--- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 96ec6499f001..7cf7c032a0e9 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -242,7 +242,8 @@ 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. + diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 6602790b5f4c..d303cf0e4b36 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -1291,6 +1291,23 @@ test_expect_success 'with command using $ARG' ' test_cmp expected actual ' +test_expect_success 'with command using more than one $ARG' ' + 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" && + FIXED=$(git log -1 --oneline --format="%h (%s)" --abbrev-commit --abbrev=14 HEAD) && + cat complex_message_body >expected && + sed -e "s/ Z\$/ /" >>expected <<-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 >actual && + test_cmp expected actual +' + test_expect_success 'with failing command using $ARG' ' git config trailer.fix.ifExists "replace" && git config trailer.fix.command "false \$ARG" && diff --git a/trailer.c b/trailer.c index be4e9726421c..42e3b818327a 100644 --- a/trailer.c +++ b/trailer.c @@ -44,7 +44,7 @@ static char *separators = ":"; static int configured; -#define TRAILER_ARG_STRING "$ARG" +#define TRAILER_ARG_STRING "ARG" static const char *git_generated_prefixes[] = { "Signed-off-by: ", @@ -222,13 +222,16 @@ static char *apply_command(const char *command, const char *arg) struct strbuf buf = STRBUF_INIT; struct child_process cp = CHILD_PROCESS_INIT; 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) - 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; + cp.no_stdin = 1; cp.use_shell = 1; base-commit: 142430338477d9d1bb25be66267225fb58498d92 -- gitgitgadget