@matheusbsbs / matheus-bagliano / commits / c364b7ef51

trailer: add new .cmd config option

The `trailer.<token>.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 <token>=<value>' 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 <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 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.<token>.cmd' will not automatically execute. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Christian Couder <christian.couder@gmail.com> Signed-off-by: ZheNing Hu <adlternative@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

ZheNing Hu committed May 3, 2021 at 15:41 UTC c364b7ef51ec3af871754e7afdfd73e4bed6da56

3 files changed +175 -19

Documentation/git-interpret-trailers.txt

+66 -9

@@ -232,6 +232,20 @@ trailer.<token>.ifmissing::

232 that option for trailers with the specified <token>.

233

234 trailer.<token>.command::

235 + This option behaves in the same way as 'trailer.<token>.cmd', except

236 + that it doesn't pass anything as argument to the specified command.

237 + Instead the first occurrence of substring $ARG is replaced by the

238 + value that would be passed as argument.

239 ++

240 +The 'trailer.<token>.command' option has been deprecated in favor of

241 +'trailer.<token>.cmd' due to the fact that $ARG in the user's command is

242 +only replaced once and that the original way of replacing $ARG is not safe.

243 ++

244 +When both 'trailer.<token>.cmd' and 'trailer.<token>.command' are given

245 +for the same <token>, 'trailer.<token>.cmd' is used and

246 +'trailer.<token>.command' is ignored.

247 +

248 +trailer.<token>.cmd::

249 This option can be used to specify a shell command that will be called:

250 once to automatically add a trailer with the specified <token>, and then

251 each time a '--trailer <token>=<value>' argument to modify the <value> of

@@ -247,15 +261,9 @@ leading and trailing whitespace trimmed off.

261 If some '--trailer <token>=<value>' arguments are also passed

262 on the command line, the command is called again once for each

263 of these arguments with the same <token>. And the <value> part

250 -of these arguments, if any, will be used to replace the first

251 -occurrence of substring `$ARG` in the command. This way the

252 -command can produce a <value> computed from the <value> passed

253 -in the '--trailer <token>=<value>' argument.

254 -+

255 -For consistency, the first occurrence of substring `$ARG` is

256 -also replaced, this time with the empty string, in the command

257 -when the command is first called to add a trailer with the

258 -specified <token>.

264 +of these arguments, if any, will be passed to the command as its

265 +first argument. This way the command can produce a <value> computed

266 +from the <value> passed in the '--trailer <token>=<value>' argument.

267

268 EXAMPLES

269 --------

@@ -338,6 +346,55 @@ subject

346 Fix #42

347 ------------

348

349 +* Configure a 'help' trailer with a cmd use a script `glog-find-author`

350 + which search specified author identity from git log in git repository

351 + and show how it works:

352 ++

353 +------------

354 +$ cat ~/bin/glog-find-author

355 +#!/bin/sh

356 +test -n "$1" && git log --author="$1" --pretty="%an <%ae>" -1 || true

357 +$ git config trailer.help.key "Helped-by: "

358 +$ git config trailer.help.ifExists "addIfDifferentNeighbor"

359 +$ git config trailer.help.cmd "~/bin/glog-find-author"

360 +$ git interpret-trailers --trailer="help:Junio" --trailer="help:Couder" <<EOF

361 +> subject

362 +>

363 +> message

364 +>

365 +> EOF

366 +subject

367 +

368 +message

369 +

370 +Helped-by: Junio C Hamano <gitster@pobox.com>

371 +Helped-by: Christian Couder <christian.couder@gmail.com>

372 +------------

373 +

374 +* Configure a 'ref' trailer with a cmd use a script `glog-grep`

375 + to grep last relevant commit from git log in the git repository

376 + and show how it works:

377 ++

378 +------------

379 +$ cat ~/bin/glog-grep

380 +#!/bin/sh

381 +test -n "$1" && git log --grep "$1" --pretty=reference -1 || true

382 +$ git config trailer.ref.key "Reference-to: "

383 +$ git config trailer.ref.ifExists "replace"

384 +$ git config trailer.ref.cmd "~/bin/glog-grep"

385 +$ git interpret-trailers --trailer="ref:Add copyright notices." <<EOF

386 +> subject

387 +>

388 +> message

389 +>

390 +> EOF

391 +subject

392 +

393 +message

394 +

395 +Reference-to: 8bc9a0c769 (Add copyright notices., 2005-04-07)

396 +------------

397 +

398 * Configure a 'see' trailer with a command to show the subject of a

399 commit that is related, and show how it works:

400 +

t/t7513-interpret-trailers.sh

+84

@@ -51,6 +51,69 @@ test_expect_success 'setup' '

51 EOF

52 '

54 +test_expect_success 'with cmd' '

55 + test_when_finished "git config --remove-section trailer.bug" &&

56 + git config trailer.bug.key "Bug-maker: " &&

57 + git config trailer.bug.ifExists "add" &&

58 + git config trailer.bug.cmd "echo \"maybe is\"" &&

59 + cat >expected2 <<-EOF &&

60 +

61 + Bug-maker: maybe is him

62 + Bug-maker: maybe is me

63 + EOF

64 + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \

65 + >actual2 &&

66 + test_cmp expected2 actual2

67 +'

68 +

69 +test_expect_success 'with cmd and $1' '

70 + test_when_finished "git config --remove-section trailer.bug" &&

71 + git config trailer.bug.key "Bug-maker: " &&

72 + git config trailer.bug.ifExists "add" &&

73 + git config trailer.bug.cmd "echo \"\$1\" is" &&

74 + cat >expected2 <<-EOF &&

75 +

76 + Bug-maker: him is him

77 + Bug-maker: me is me

78 + EOF

79 + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \

80 + >actual2 &&

81 + test_cmp expected2 actual2

82 +'

83 +

84 +test_expect_success 'with cmd and $1 with sh -c' '

85 + test_when_finished "git config --remove-section trailer.bug" &&

86 + git config trailer.bug.key "Bug-maker: " &&

87 + git config trailer.bug.ifExists "replace" &&

88 + git config trailer.bug.cmd "sh -c \"echo who is \"\$1\"\"" &&

89 + cat >expected2 <<-EOF &&

90 +

91 + Bug-maker: who is me

92 + EOF

93 + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \

94 + >actual2 &&

95 + test_cmp expected2 actual2

96 +'

97 +

98 +test_expect_success 'with cmd and $1 with shell script' '

99 + test_when_finished "git config --remove-section trailer.bug" &&

100 + git config trailer.bug.key "Bug-maker: " &&

101 + git config trailer.bug.ifExists "replace" &&

102 + git config trailer.bug.cmd "./echoscript" &&

103 + cat >expected2 <<-EOF &&

104 +

105 + Bug-maker: who is me

106 + EOF

107 + cat >echoscript <<-EOF &&

108 + #!/bin/sh

109 + echo who is "\$1"

110 + EOF

111 + chmod +x echoscript &&

112 + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \

113 + >actual2 &&

114 + test_cmp expected2 actual2

115 +'

116 +

117 test_expect_success 'without config' '

118 sed -e "s/ Z\$/ /" >expected <<-\EOF &&

119

@@ -1274,6 +1337,27 @@ test_expect_success 'setup a commit' '

1337 git commit -m "Add file a.txt"

1338 '

1339

1340 +test_expect_success 'cmd takes precedence over command' '

1341 + test_when_finished "git config --unset trailer.fix.cmd" &&

1342 + git config trailer.fix.ifExists "replace" &&

1343 + git config trailer.fix.cmd "test -n \"\$1\" && git log -1 --oneline --format=\"%h (%aN)\" \

1344 + --abbrev-commit --abbrev=14 \"\$1\" || true" &&

1345 + git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" \

1346 + --abbrev-commit --abbrev=14 \$ARG" &&

1347 + FIXED=$(git log -1 --oneline --format="%h (%aN)" --abbrev-commit --abbrev=14 HEAD) &&

1348 + cat complex_message_body >expected2 &&

1349 + sed -e "s/ Z\$/ /" >>expected2 <<-EOF &&

1350 + Fixes: $FIXED

1351 + Acked-by= Z

1352 + Reviewed-by:

1353 + Signed-off-by: Z

1354 + Signed-off-by: A U Thor <author@example.com>

1355 + EOF

1356 + git interpret-trailers --trailer "review:" --trailer "fix=HEAD" \

1357 + <complex_message >actual2 &&

1358 + test_cmp expected2 actual2

1359 +'

1360 +

1361 test_expect_success 'with command using $ARG' '

1362 git config trailer.fix.ifExists "replace" &&

1363 git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG" &&

trailer.c

+25 -10

@@ -14,6 +14,7 @@ struct conf_info {

14 char *name;

15 char *key;

16 char *command;

17 + char *cmd;

18 enum trailer_where where;

19 enum trailer_if_exists if_exists;

20 enum trailer_if_missing if_missing;

@@ -127,6 +128,7 @@ static void free_arg_item(struct arg_item *item)

128 free(item->conf.name);

129 free(item->conf.key);

130 free(item->conf.command);

131 + free(item->conf.cmd);

132 free(item->token);

133 free(item->value);

134 free(item);

@@ -216,18 +218,24 @@ static int check_if_different(struct trailer_item *in_tok,

218 return 1;

219 }

220

219 -static char *apply_command(const char *command, const char *arg)

221 +static char *apply_command(struct conf_info *conf, const char *arg)

222 {

223 struct strbuf cmd = STRBUF_INIT;

224 struct strbuf buf = STRBUF_INIT;

225 struct child_process cp = CHILD_PROCESS_INIT;

226 char *result;

227

226 - strbuf_addstr(&cmd, command);

227 - if (arg)

228 - strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);

229 -

230 - strvec_push(&cp.args, cmd.buf);

228 + if (conf->cmd) {

229 + strbuf_addstr(&cmd, conf->cmd);

230 + strvec_push(&cp.args, cmd.buf);

231 + if (arg)

232 + strvec_push(&cp.args, arg);

233 + } else if (conf->command) {

234 + strbuf_addstr(&cmd, conf->command);

235 + if (arg)

236 + strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);

237 + strvec_push(&cp.args, cmd.buf);

238 + }

239 cp.env = local_repo_env;

240 cp.no_stdin = 1;

241 cp.use_shell = 1;

@@ -247,7 +255,7 @@ static char *apply_command(const char *command, const char *arg)

255

256 static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)

257 {

250 - if (arg_tok->conf.command) {

258 + if (arg_tok->conf.command || arg_tok->conf.cmd) {

259 const char *arg;

260 if (arg_tok->value && arg_tok->value[0]) {

261 arg = arg_tok->value;

@@ -257,7 +265,7 @@ static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg

265 else

266 arg = xstrdup("");

267 }

260 - arg_tok->value = apply_command(arg_tok->conf.command, arg);

268 + arg_tok->value = apply_command(&arg_tok->conf, arg);

269 free((char *)arg);

270 }

271 }

@@ -430,6 +438,7 @@ static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)

438 dst->name = xstrdup_or_null(src->name);

439 dst->key = xstrdup_or_null(src->key);

440 dst->command = xstrdup_or_null(src->command);

441 + dst->cmd = xstrdup_or_null(src->cmd);

442 }

443

444 static struct arg_item *get_conf_item(const char *name)

@@ -454,8 +463,8 @@ static struct arg_item *get_conf_item(const char *name)

463 return item;

464 }

465

457 -enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,

458 - TRAILER_IF_EXISTS, TRAILER_IF_MISSING };

466 +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_CMD,

467 + TRAILER_WHERE, TRAILER_IF_EXISTS, TRAILER_IF_MISSING };

468

469 static struct {

470 const char *name;

@@ -463,6 +472,7 @@ static struct {

472 } trailer_config_items[] = {

473 { "key", TRAILER_KEY },

474 { "command", TRAILER_COMMAND },

475 + { "cmd", TRAILER_CMD },

476 { "where", TRAILER_WHERE },

477 { "ifexists", TRAILER_IF_EXISTS },

478 { "ifmissing", TRAILER_IF_MISSING }

@@ -542,6 +552,11 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)

552 warning(_("more than one %s"), conf_key);

553 conf->command = xstrdup(value);

554 break;

555 + case TRAILER_CMD:

556 + if (conf->cmd)

557 + warning(_("more than one %s"), conf_key);

558 + conf->cmd = xstrdup(value);

559 + break;

560 case TRAILER_WHERE:

561 if (trailer_set_where(&conf->where, value))

562 warning(_("unknown value '%s' for key '%s'"), value, conf_key);