From c364b7ef51ec3af871754e7afdfd73e4bed6da56 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Mon, 3 May 2021 15:41:05 +0000 Subject: trailer: add new .cmd config option The `trailer..command` configuration variable specifies a command (run via the shell, so it does not have to be a single name or path to the command, but can be a shell script), and the first occurrence of substring $ARG is replaced with the value given to the `interpret-trailer` command for the token in a '--trailer =' argument. 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 to use $ARG more than once, but that would not work, as the second and subsequent $ARG are not replaced. * Because $ARG is textually replaced without regard to the shell language syntax, even '$ARG' (inside a single-quote pair), which a user would expect to stay intact, would be replaced, and worse, if the value had an unmatched single quote (imagine a name like "O'Connor", substituted into NAME='$ARG' to make it NAME='O'Connor'), it would result in 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 . 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..cmd` configuration that takes higher precedence to deprecate and eventually remove `trailer..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. At the same time, in order to allow `git interpret-trailers` to better simulate the behavior of `git command -s`, 'trailer..cmd' will not automatically execute. Helped-by: Junio C Hamano Helped-by: Christian Couder Signed-off-by: ZheNing Hu Signed-off-by: Junio C Hamano --- t/t7513-interpret-trailers.sh | 84 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) (limited to 't') diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 6602790b5f..04885d0a5e 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -51,6 +51,69 @@ test_expect_success 'setup' ' EOF ' +test_expect_success 'with cmd' ' + test_when_finished "git config --remove-section trailer.bug" && + git config trailer.bug.key "Bug-maker: " && + git config trailer.bug.ifExists "add" && + git config trailer.bug.cmd "echo \"maybe is\"" && + cat >expected2 <<-EOF && + + Bug-maker: maybe is him + Bug-maker: maybe is me + EOF + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ + >actual2 && + test_cmp expected2 actual2 +' + +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 "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" \ + >actual2 && + test_cmp expected2 actual2 +' + +test_expect_success 'with cmd and $1 with sh -c' ' + 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.cmd "sh -c \"echo who is \"\$1\"\"" && + cat >expected2 <<-EOF && + + Bug-maker: who is me + EOF + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ + >actual2 && + test_cmp expected2 actual2 +' + +test_expect_success 'with cmd and $1 with shell script' ' + 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.cmd "./echoscript" && + cat >expected2 <<-EOF && + + Bug-maker: who is me + EOF + cat >echoscript <<-EOF && + #!/bin/sh + 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 'without config' ' sed -e "s/ Z\$/ /" >expected <<-\EOF && @@ -1274,6 +1337,27 @@ test_expect_success 'setup a commit' ' git commit -m "Add file a.txt" ' +test_expect_success 'cmd takes precedence over command' ' + 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 (%aN)\" \ + --abbrev-commit --abbrev=14 \"\$1\" || true" && + git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" \ + --abbrev-commit --abbrev=14 \$ARG" && + FIXED=$(git log -1 --oneline --format="%h (%aN)" --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 + EOF + git interpret-trailers --trailer "review:" --trailer "fix=HEAD" \ + 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" && -- cgit v1.2.1