[PATCH] [GSOC] commit: provides multiple common signatures

From: ZheNing Hu <redacted>

Similar to "Helped-by", "Reported-by", "Reviewed-by", "Mentored-by"
these signatures are often seen in git commit messages. After
referring to the simple implementation of `commit --signoff`
and `send-email -cc=" commiter <email>"`, I am considering
whether to provide multiple signature parameters from the
command line. I think this might help maintainers and
developers directly uses the shell to provide these signatures
instead of multiple times repetitive writing those trailers
each time.

To achieve this goal, i refactored the `append_signoff` design and
provided `append_message` and `append_message_string_list` interfaces,
providing new ways to generate those various signatures.

Users now can use `commit -H "helper <eamil>"` to generate "Helped-by" trailer,
`commit -R "reviewer <eamil>"` to generate "Reviewed-by" trailer,
`commit -r "reporter <eamil> "`to generate "Reported-by" trailer,
`commit -M "mentor <eamil>"` to generate "Mentored-by" trailer.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC] commit: provides multiple signatures from command line
    
    I don’t know if my idea will satisfy everyone, I'm also thinking about
    whether we can provide a more generalized version (I think this idea can
    be extended: because users and developers have other signature methods
    that they want, such as "Based-on-patch-by") I hope someone can give me
    pointers (on the correctness of ideas or codes)

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/901

 Documentation/git-commit.txt |  24 +++++++-
 builtin/commit.c             |  63 +++++++++++++++++++++
 sequencer.c                  |  40 +++++++++----
 sequencer.h                  |   4 ++
 t/t7502-commit-porcelain.sh  | 106 +++++++++++++++++++++++++++++++++++
 5 files changed, 226 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..e1b528d70c1a 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -14,7 +14,9 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [-S[<keyid>]] [--] [<pathspec>...] [(-H|--helped-by)=<address>...]
+	   [(-R|--reviewed-by)=<address>...] [(-r|--reported-by)=<address>...]
+	   [(-M|--mentored)=<address>...]
 
 DESCRIPTION
 -----------

@@ -166,6 +168,26 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 
 include::signoff-option.txt[]
 
+-H=<address>...::
+--helped-by=<address>...::
+	Add one or more `Helped-by` trailer by the committer at the end of the commit
+	log message.
+
+-R=<address>...::
+--reviewed-by=<address>...::
+	Add one or more `Reviewed-by` trailer by the committer at the end of the commit
+	log message.
+
+-r=<address>...::
+--reported-by=<address>...::
+	Add one or more `Reported-by` trailer by the committer at the end of the commit
+	log message.
+
+-M=<address>...::
+--mentored-by=<address>...::
+	Add one or more `Mentored-by` trailer by the committer at the end of the commit
+	log message.
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..4b312af03181 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -113,6 +113,10 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+static struct string_list helped_by = STRING_LIST_INIT_NODUP;
+static struct string_list mentored_by = STRING_LIST_INIT_NODUP;
+static struct string_list reviewed_by = STRING_LIST_INIT_NODUP;
+static struct string_list reported_by = STRING_LIST_INIT_NODUP;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -829,6 +833,20 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (signoff)
 		append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);
 
+	if(helped_by.items)
+		append_message_string_list(&sb, "Helped-by: ", &helped_by, ignore_non_trailer(sb.buf, sb.len), 0);
+	if(reviewed_by.items)
+		append_message_string_list(&sb, "Reviewed-by: ", &reviewed_by, ignore_non_trailer(sb.buf, sb.len), 0);
+	if(reported_by.items)
+		append_message_string_list(&sb, "Reported-by: ", &reported_by, ignore_non_trailer(sb.buf, sb.len), 0);
+	if(mentored_by.items)
+		append_message_string_list(&sb, "Mentored-by: ", &mentored_by, ignore_non_trailer(sb.buf, sb.len), 0);
+
+	string_list_clear(&helped_by, 0);
+	string_list_clear(&reviewed_by, 0);
+	string_list_clear(&reported_by, 0);
+	string_list_clear(&mentored_by, 0);
+
 	if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
 		die_errno(_("could not write commit template"));
 

@@ -1490,6 +1508,42 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	return git_status_config(k, v, s);
 }
 
+static int help_callback(const struct option *opt, const char *arg, int unset)
+{
+	if (unset)
+		string_list_clear(&helped_by, 0);
+	else
+		string_list_append(&helped_by, arg);
+	return 0;
+}
+
+static int review_callback(const struct option *opt, const char *arg, int unset)
+{
+	if (unset)
+		string_list_clear(&reviewed_by, 0);
+	else
+		string_list_append(&reviewed_by, arg);
+	return 0;
+}
+
+static int report_callback(const struct option *opt, const char *arg, int unset)
+{
+	if (unset)
+		string_list_clear(&reported_by, 0);
+	else
+		string_list_append(&reported_by, arg);
+	return 0;
+}
+
+static int mentor_callback(const struct option *opt, const char *arg, int unset)
+{
+	if (unset)
+		string_list_clear(&mentored_by, 0);
+	else
+		string_list_append(&mentored_by, arg);
+	return 0;
+}
+
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
 	static struct wt_status s;

@@ -1507,6 +1561,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+		OPT_CALLBACK('H', "helped-by", NULL, N_("email"), N_("add a Helped-by trailer"), help_callback),
+		OPT_CALLBACK('r', "reported-by", NULL, N_("email"), N_("add a Reported-by trailer"), report_callback),
+		OPT_CALLBACK('R', "reviewed-by", NULL, N_("email"), N_("add a Reviewed-by trailer"), review_callback),
+		OPT_CALLBACK('M', "mentored-by", NULL, N_("email"), N_("add a Mentored-by trailer"), mentor_callback),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

@@ -1561,6 +1619,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct commit_extra_header *extra = NULL;
 	struct strbuf err = STRBUF_INIT;
 
+	helped_by.strdup_strings = 1;
+	reviewed_by.strdup_strings = 1;
+	reported_by.strdup_strings = 1;
+	mentored_by.strdup_strings = 1;
+
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_commit_usage, builtin_commit_options);
 

diff --git a/sequencer.c b/sequencer.c
index d2332d3e1787..528daf9df252 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -4668,16 +4668,12 @@ int sequencer_pick_revisions(struct repository *r,
 	return res;
 }
 
-void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
+void append_message(struct strbuf *msgbuf, struct strbuf *sob,
+			size_t ignore_footer, unsigned flag)
 {
 	unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
-	struct strbuf sob = STRBUF_INIT;
 	int has_footer;
 
-	strbuf_addstr(&sob, sign_off_header);
-	strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));
-	strbuf_addch(&sob, '\n');
-
 	if (!ignore_footer)
 		strbuf_complete_line(msgbuf);
 

@@ -4685,11 +4681,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
 	 * If the whole message buffer is equal to the sob, pretend that we
 	 * found a conforming footer with a matching sob
 	 */
-	if (msgbuf->len - ignore_footer == sob.len &&
-	    !strncmp(msgbuf->buf, sob.buf, sob.len))
+	if (msgbuf->len - ignore_footer == sob->len &&
+	    !strncmp(msgbuf->buf, sob->buf, sob->len))
 		has_footer = 3;
 	else
-		has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
+		has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);
 
 	if (!has_footer) {
 		const char *append_newlines = NULL;

@@ -4723,8 +4719,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
 
 	if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
 		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
-				sob.buf, sob.len);
+				sob->buf, sob->len);
+}
+
+void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
+{
+	struct strbuf sob = STRBUF_INIT;
+	strbuf_addstr(&sob, sign_off_header);
+	strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));
+	strbuf_addch(&sob, '\n');
+	append_message(msgbuf, &sob, ignore_footer, flag);
+	strbuf_release(&sob);
+}
 
+void append_message_string_list(struct strbuf *msgbuf, const char *header,
+		struct string_list *sobs, size_t ignore_footer, unsigned flag) {
+	int i;
+	struct strbuf sob = STRBUF_INIT;
+
+	for ( i = 0; i < sobs->nr; i++)
+	{
+		strbuf_addstr(&sob, header);
+		strbuf_addstr(&sob, sobs->items[i].string);
+		strbuf_addch(&sob, '\n');
+		append_message(msgbuf, &sob, ignore_footer, flag);
+		strbuf_reset(&sob);
+	}
 	strbuf_release(&sob);
 }
 

diff --git a/sequencer.h b/sequencer.h
index f8b2e4ab8527..b24e274f4c62 100644
--- a/sequencer.h
+++ b/sequencer.h

@@ -174,6 +174,10 @@ int todo_list_rearrange_squash(struct todo_list *todo_list);
  * and the new signoff will be spliced into the buffer before those bytes.
  */
 void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);
+void append_message(struct strbuf *msgbuf, struct strbuf *sob,
+		size_t ignore_footer, unsigned flag);
+void append_message_string_list(struct strbuf *msgbuf, const char*header,
+		struct string_list *sobs, size_t ignore_footer, unsigned flag);
 
 void append_conflicts_hint(struct index_state *istate,
 		struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..40823152a51c 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh

@@ -154,6 +154,112 @@ test_expect_success 'sign off' '
 
 '
 
+test_expect_success 'helped-by' '
+
+	>file1 &&
+	git add file1 &&
+	git commit --helped-by="foo <bar@frotz>" \
+	--helped-by="foo2 <bar2@frotz>" -m "thank you" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -ne "s/Helped-by: //p" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	foo <bar@frotz>
+	foo2 <bar2@frotz>
+	EOF
+	test_cmp expected actual
+
+'
+
+test_expect_success 'reported-by' '
+
+	>file2 &&
+	git add file2 &&
+	git commit --reported-by="foo <bar@frotz>" \
+	--reported-by="foo2 <bar2@frotz>" -m "thank you" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -ne "s/Reported-by: //p" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	foo <bar@frotz>
+	foo2 <bar2@frotz>
+	EOF
+	test_cmp expected actual
+
+'
+
+test_expect_success 'reviewed-by' '
+
+	>file3 &&
+	git add file3 &&
+	git commit --reviewed-by="foo <bar@frotz>" \
+	--reviewed-by="foo2 <bar2@frotz>" -m "thank you" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -ne "s/Reviewed-by: //p" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	foo <bar@frotz>
+	foo2 <bar2@frotz>
+	EOF
+	test_cmp expected actual
+
+'
+
+test_expect_success 'mentored-by' '
+
+	>file4 &&
+	git add file4 &&
+	git commit --mentored-by="foo <bar@frotz>" \
+	--mentored-by="foo2 <bar2@frotz>" -m "thank you" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -ne "s/Mentored-by: //p" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	foo <bar@frotz>
+	foo2 <bar2@frotz>
+	EOF
+	test_cmp expected actual
+
+'
+
+test_expect_success 'multiple signatures' '
+
+	>file5 &&
+	git add file5 &&
+	git commit --helped-by="foo <bar@frotz>" \
+	--reviewed-by="foo2 <bar2@frotz>" \
+	--mentored-by="foo3 <bar3@frotz>" \
+	--reported-by="foo4 <bar4@frotz>" -s -m "thank you" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,7d" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	Signed-off-by: C O Mitter <committer@example.com>
+	Helped-by: foo <bar@frotz>
+	Reviewed-by: foo2 <bar2@frotz>
+	Reported-by: foo4 <bar4@frotz>
+	Mentored-by: foo3 <bar3@frotz>
+	EOF
+	test_cmp expected actual
+
+'
+
+test_expect_success 'multiple signatures (use abbreviations)' '
+
+	>file6 &&
+	git add file6 &&
+	git commit -H "foo <bar@frotz>" \
+	-R "foo2 <bar2@frotz>" \
+	-M "foo3 <bar3@frotz>" \
+	-r "foo4 <bar4@frotz>" -s -m "thank you" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,7d" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	Signed-off-by: C O Mitter <committer@example.com>
+	Helped-by: foo <bar@frotz>
+	Reviewed-by: foo2 <bar2@frotz>
+	Reported-by: foo4 <bar4@frotz>
+	Mentored-by: foo3 <bar3@frotz>
+	EOF
+	test_cmp expected actual
+
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&

base-commit: 13d7ab6b5d7929825b626f050b62a11241ea4945
-- 
gitgitgadget

Hey!

The idea seems very useful to me though I am not sure what others feel
about this and whether Git is ready for such a thing or not. Keeping
this concern aside, I will still review the patch due to my own interest
in it as well.

On 11/03 07:16, ZheNing Hu via GitGitGadget wrote:

From: ZheNing Hu <redacted>

Similar to "Helped-by", "Reported-by", "Reviewed-by", "Mentored-by"
these signatures are often seen in git commit messages. After

I think it will be better to rephrase the line so that it is easier to
understand. It took me a couple of reads to figure out what you meant.

Something like:

Similar to "Signed-off-by", trailers such as "Reported-by", "Helped-by"
and "Mentored-by" are also seen in Git commits.

referring to the simple implementation of `commit --signoff`
and `send-email -cc=" commiter <email>"`, I am considering
whether to provide multiple signature parameters from the
command line. I think this might help maintainers and
developers directly uses the shell to provide these signatures
instead of multiple times repetitive writing those trailers
each time.

The above para is more appropriate for a cover letter than a commit
message. Your thought process for the patch you sent is equally valuable
but this does not belong in the commit message. Commit messages are more
about what you did and any nuances that follow. You get me?

Use 'git format-patch --cover-letter <...>' to create a cover letter for
your patch.

To achieve this goal, i refactored the `append_signoff` design and
provided `append_message` and `append_message_string_list` interfaces,
providing new ways to generate those various signatures.

s/i/I

Also, commit messages are generally written in the imperative tense when
desciribing what you have done in the commit.

Users now can use `commit -H "helper <eamil>"` to generate "Helped-by" trailer,
`commit -R "reviewer <eamil>"` to generate "Reviewed-by" trailer,
`commit -r "reporter <eamil> "`to generate "Reported-by" trailer,
`commit -M "mentor <eamil>"` to generate "Mentored-by" trailer.

Multiple typos.

Signed-off-by: ZheNing Hu <redacted>

So, an improved commit message could be:

-----8<-----
commit: support multiple trailers from the command line

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line. Extend
this list to include other commonly used trailers viz., "Helped-by",
"Reviewed-by", "Reported-by" and "Mentored-by". Introduce options '-H',
'-R', '-r' and '-M' corresponding to the aforementioned trailers.

While at it, add tests in the script 't7502-commit-porcelain.sh' and add
information regarding these options in the Documentation of 'git
commit'.
----->8-----

    [GSOC] commit: provides multiple signatures from command line
    
    I don’t know if my idea will satisfy everyone, I'm also thinking about
    whether we can provide a more generalized version (I think this idea can
    be extended: because users and developers have other signature methods
    that they want, such as "Based-on-patch-by") I hope someone can give me
    pointers (on the correctness of ideas or codes)

It will be great if we let the user add customised options for the
respective trailers but I think that trying to cater to such a large
audience will only complicate the whole process and decrease the support
avialable for the options/command. Apart from this, I think that the
trailer "Reviewed-by" may not be that widely used and it would be better
if we remove it from this patch.

Oh! I did not think you had added long forms of these options and was
about to comment on that. Good that you added. Do talk about them as
well in the commit message.

Good that you kept the variables as 'string_list's instead of a usual
'char*'.

I think this part will look prettier if you wrap around the text. For
code segments, the wrap around limit is 80 chars. For commit messages
its 72 chars. Wrap around means that as soon as you hit N number of
characters, you proceed to the next(new) line.

It would be lovely if you could line wrap these above additions too!

Its nice to see that you generalised the pre-exisiting function instead
of creating a new one(s) for the new trailers. Good.

It will be better to name 'struct strbuf *sob' to something more
generic, maybe 'struct strbuf *trailer' since 'sob' stands for
'Signed-off-by' and since the function is becoming more generic, its
contents should too.

Again, rename the variables.

I think it will be nice if you could use a flag to denote whether the
entity to be appended is a 'sign-off' or not. This way when say the
variable 'int signoff' is 1, then use the above part in the
'append_message()' function otherwise go with the flow that exists.

Similarly, here, if 'signoff = 0' then the above part goes on. Or
another alternative can be to create a 'append_message_helper()' and
shift the current contents of 'append_message()' into that and use the
above function to work as per the value of signoff.

So a possible flow can be:

void append_message_string_list(...params...) {

	if (signoff) {
		..excecute the necessary segments and call the
		'append_message_helper()' function...
	} else {
		..similarly here..
	}
}

This way we save ourselves some extra functions.

Then, these would have to be corrected too.

I feel that the tests deserve a commit of their own. A follow-up commit
in this patch.

Was this extra line feed intentional? It looks odd here and other test
(scripts) don't have this. Though, this test script seems to have many
tests which have an extra line feed. My suggestion would be to eliminate
it. Maybe this script is old and hasn't been reviewed for a cleanup in
a long time.

Similarly here and in the parts that follow.

<...>

This seems like a good patch to me but more experienced members will be
able to comment better I think. Good job anyways.

Regards,
Shourya Shukla

Shourya Shukla [off-list ref] 于2021年3月11日周四 下午11:05写道：

Hey!

Hi!

The idea seems very useful to me though I am not sure what others feel
about this and whether Git is ready for such a thing or not. Keeping
this concern aside, I will still review the patch due to my own interest
in it as well.

haha, thank you! I'm glad that my whims can be recognized by you and help
git itself.

On 11/03 07:16, ZheNing Hu via GitGitGadget wrote:

quoted

From: ZheNing Hu <redacted>

Similar to "Helped-by", "Reported-by", "Reviewed-by", "Mentored-by"
these signatures are often seen in git commit messages. After

I think it will be better to rephrase the line so that it is easier to
understand. It took me a couple of reads to figure out what you meant.

Something like:

Similar to "Signed-off-by", trailers such as "Reported-by", "Helped-by"
and "Mentored-by" are also seen in Git commits.

It's exactly what you said.
My lack of English sometimes limits my expression.

quoted

referring to the simple implementation of `commit --signoff`
and `send-email -cc=" commiter <email>"`, I am considering
whether to provide multiple signature parameters from the
command line. I think this might help maintainers and
developers directly uses the shell to provide these signatures
instead of multiple times repetitive writing those trailers
each time.

The above para is more appropriate for a cover letter than a commit
message. Your thought process for the patch you sent is equally valuable
but this does not belong in the commit message. Commit messages are more
about what you did and any nuances that follow. You get me?

Use 'git format-patch --cover-letter <...>' to create a cover letter for
your patch.

I understand it now, because I use GGG, I will put this content into the
GGG conversation.

quoted

To achieve this goal, i refactored the `append_signoff` design and
provided `append_message` and `append_message_string_list` interfaces,
providing new ways to generate those various signatures.

s/i/I

Also, commit messages are generally written in the imperative tense when
desciribing what you have done in the commit.

You're right.

quoted

Users now can use `commit -H "helper <eamil>"` to generate "Helped-by" trailer,
`commit -R "reviewer <eamil>"` to generate "Reviewed-by" trailer,
`commit -r "reporter <eamil> "`to generate "Reported-by" trailer,
`commit -M "mentor <eamil>"` to generate "Mentored-by" trailer.

Multiple typos.

quoted

Signed-off-by: ZheNing Hu <redacted>

So, an improved commit message could be:

-----8<-----
commit: support multiple trailers from the command line

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line. Extend
this list to include other commonly used trailers viz., "Helped-by",
"Reviewed-by", "Reported-by" and "Mentored-by". Introduce options '-H',
'-R', '-r' and '-M' corresponding to the aforementioned trailers.

While at it, add tests in the script 't7502-commit-porcelain.sh' and add
information regarding these options in the Documentation of 'git
commit'.

I think this paragraph may not be needed. Junio said in reply to one
of my previous patches that we may not need to add instructions about test
files and documents, because `git log -p --stat` can clearly see that what we
have done in testing and documentation.

----->8-----

quoted

    [GSOC] commit: provides multiple signatures from command line

    I don’t know if my idea will satisfy everyone, I'm also thinking about
    whether we can provide a more generalized version (I think this idea can
    be extended: because users and developers have other signature methods
    that they want, such as "Based-on-patch-by") I hope someone can give me
    pointers (on the correctness of ideas or codes)

It will be great if we let the user add customised options for the
respective trailers but I think that trying to cater to such a large
audience will only complicate the whole process and decrease the support
avialable for the options/command. Apart from this, I think that the
trailer "Reviewed-by" may not be that widely used and it would be better
if we remove it from this patch.

Okay, I will delete it.

quoted

 Documentation/git-commit.txt |  24 +++++++-
 builtin/commit.c             |  63 +++++++++++++++++++++
 sequencer.c                  |  40 +++++++++----
 sequencer.h                  |   4 ++
 t/t7502-commit-porcelain.sh  | 106 +++++++++++++++++++++++++++++++++++
 5 files changed, 226 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..e1b528d70c1a 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -14,7 +14,9 @@ SYNOPSIS
         [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
         [--date=<date>] [--cleanup=<mode>] [--[no-]status]
         [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-        [-S[<keyid>]] [--] [<pathspec>...]
+        [-S[<keyid>]] [--] [<pathspec>...] [(-H|--helped-by)=<address>...]
+        [(-R|--reviewed-by)=<address>...] [(-r|--reported-by)=<address>...]
+        [(-M|--mentored)=<address>...]

 DESCRIPTION
 -----------

@@ -166,6 +168,26 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.

 include::signoff-option.txt[]

+-H=<address>...::
+--helped-by=<address>...::
+     Add one or more `Helped-by` trailer by the committer at the end of the commit
+     log message.
+
+-R=<address>...::
+--reviewed-by=<address>...::
+     Add one or more `Reviewed-by` trailer by the committer at the end of the commit
+     log message.
+
+-r=<address>...::
+--reported-by=<address>...::
+     Add one or more `Reported-by` trailer by the committer at the end of the commit
+     log message.
+
+-M=<address>...::
+--mentored-by=<address>...::
+     Add one or more `Mentored-by` trailer by the committer at the end of the commit
+     log message.

Oh! I did not think you had added long forms of these options and was
about to comment on that. Good that you added. Do talk about them as
well in the commit message.

Yes, I forgot to mention these long formats in the commit message.

quoted

 -n::
 --no-verify::
      This option bypasses the pre-commit and commit-msg hooks.

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..4b312af03181 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -113,6 +113,10 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+static struct string_list helped_by = STRING_LIST_INIT_NODUP;
+static struct string_list mentored_by = STRING_LIST_INIT_NODUP;
+static struct string_list reviewed_by = STRING_LIST_INIT_NODUP;
+static struct string_list reported_by = STRING_LIST_INIT_NODUP;

Good that you kept the variables as 'string_list's instead of a usual
'char*'.

quoted

 /*
  * The default commit message cleanup mode will remove the lines

@@ -829,6 +833,20 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
      if (signoff)
              append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);

+     if(helped_by.items)
+             append_message_string_list(&sb, "Helped-by: ", &helped_by, ignore_non_trailer(sb.buf, sb.len), 0);
+     if(reviewed_by.items)
+             append_message_string_list(&sb, "Reviewed-by: ", &reviewed_by, ignore_non_trailer(sb.buf, sb.len), 0);
+     if(reported_by.items)
+             append_message_string_list(&sb, "Reported-by: ", &reported_by, ignore_non_trailer(sb.buf, sb.len), 0);
+     if(mentored_by.items)
+             append_message_string_list(&sb, "Mentored-by: ", &mentored_by, ignore_non_trailer(sb.buf, sb.len), 0);

I think this part will look prettier if you wrap around the text. For
code segments, the wrap around limit is 80 chars. For commit messages
its 72 chars. Wrap around means that as soon as you hit N number of
characters, you proceed to the next(new) line.

Fine.

quoted

+     string_list_clear(&helped_by, 0);
+     string_list_clear(&reviewed_by, 0);
+     string_list_clear(&reported_by, 0);
+     string_list_clear(&mentored_by, 0);
+
      if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
              die_errno(_("could not write commit template"));

@@ -1490,6 +1508,42 @@ static int git_commit_config(const char *k, const char *v, void *cb)
      return git_status_config(k, v, s);
 }

+static int help_callback(const struct option *opt, const char *arg, int unset)
+{
+     if (unset)
+             string_list_clear(&helped_by, 0);
+     else
+             string_list_append(&helped_by, arg);
+     return 0;
+}
+
+static int review_callback(const struct option *opt, const char *arg, int unset)
+{
+     if (unset)
+             string_list_clear(&reviewed_by, 0);
+     else
+             string_list_append(&reviewed_by, arg);
+     return 0;
+}
+
+static int report_callback(const struct option *opt, const char *arg, int unset)
+{
+     if (unset)
+             string_list_clear(&reported_by, 0);
+     else
+             string_list_append(&reported_by, arg);
+     return 0;
+}
+
+static int mentor_callback(const struct option *opt, const char *arg, int unset)
+{
+     if (unset)
+             string_list_clear(&mentored_by, 0);
+     else
+             string_list_append(&mentored_by, arg);
+     return 0;
+}
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
      static struct wt_status s;

@@ -1507,6 +1561,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
              OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
              OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
              OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+             OPT_CALLBACK('H', "helped-by", NULL, N_("email"), N_("add a Helped-by trailer"), help_callback),
+             OPT_CALLBACK('r', "reported-by", NULL, N_("email"), N_("add a Reported-by trailer"), report_callback),
+             OPT_CALLBACK('R', "reviewed-by", NULL, N_("email"), N_("add a Reviewed-by trailer"), review_callback),
+             OPT_CALLBACK('M', "mentored-by", NULL, N_("email"), N_("add a Mentored-by trailer"), mentor_callback),

It would be lovely if you could line wrap these above additions too!

quoted

              OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
              OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
              OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

@@ -1561,6 +1619,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
      struct commit_extra_header *extra = NULL;
      struct strbuf err = STRBUF_INIT;

+     helped_by.strdup_strings = 1;
+     reviewed_by.strdup_strings = 1;
+     reported_by.strdup_strings = 1;
+     mentored_by.strdup_strings = 1;
+
      if (argc == 2 && !strcmp(argv[1], "-h"))
              usage_with_options(builtin_commit_usage, builtin_commit_options);

diff --git a/sequencer.c b/sequencer.c
index d2332d3e1787..528daf9df252 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -4668,16 +4668,12 @@ int sequencer_pick_revisions(struct repository *r,
      return res;
 }

-void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
+void append_message(struct strbuf *msgbuf, struct strbuf *sob,
+                     size_t ignore_footer, unsigned flag)

Its nice to see that you generalised the pre-exisiting function instead
of creating a new one(s) for the new trailers. Good.

It will be better to name 'struct strbuf *sob' to something more
generic, maybe 'struct strbuf *trailer' since 'sob' stands for
'Signed-off-by' and since the function is becoming more generic, its
contents should too.

When I wrote it, I didn’t realize that sob refers to the abbreviation of
'Signed-off-by', I will change it.

quoted

 {
      unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
-     struct strbuf sob = STRBUF_INIT;
      int has_footer;

-     strbuf_addstr(&sob, sign_off_header);
-     strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));
-     strbuf_addch(&sob, '\n');
-
      if (!ignore_footer)
              strbuf_complete_line(msgbuf);

@@ -4685,11 +4681,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
       * If the whole message buffer is equal to the sob, pretend that we
       * found a conforming footer with a matching sob
       */
-     if (msgbuf->len - ignore_footer == sob.len &&
-         !strncmp(msgbuf->buf, sob.buf, sob.len))
+     if (msgbuf->len - ignore_footer == sob->len &&
+         !strncmp(msgbuf->buf, sob->buf, sob->len))
              has_footer = 3;
      else
-             has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
+             has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);

Again, rename the variables.

quoted

      if (!has_footer) {
              const char *append_newlines = NULL;

@@ -4723,8 +4719,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)

      if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
              strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
-                             sob.buf, sob.len);
+                             sob->buf, sob->len);
+}
+void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
+{
+     struct strbuf sob = STRBUF_INIT;
+     strbuf_addstr(&sob, sign_off_header);
+     strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));
+     strbuf_addch(&sob, '\n');
+     append_message(msgbuf, &sob, ignore_footer, flag);
+     strbuf_release(&sob);
+}

I think it will be nice if you could use a flag to denote whether the
entity to be appended is a 'sign-off' or not. This way when say the
variable 'int signoff' is 1, then use the above part in the
'append_message()' function otherwise go with the flow that exists.

quoted

+void append_message_string_list(struct strbuf *msgbuf, const char *header,
+             struct string_list *sobs, size_t ignore_footer, unsigned flag) {
+     int i;
+     struct strbuf sob = STRBUF_INIT;
+
+     for ( i = 0; i < sobs->nr; i++)
+     {
+             strbuf_addstr(&sob, header);
+             strbuf_addstr(&sob, sobs->items[i].string);
+             strbuf_addch(&sob, '\n');
+             append_message(msgbuf, &sob, ignore_footer, flag);
+             strbuf_reset(&sob);
+     }
      strbuf_release(&sob);
 }

Similarly, here, if 'signoff = 0' then the above part goes on. Or
another alternative can be to create a 'append_message_helper()' and
shift the current contents of 'append_message()' into that and use the
above function to work as per the value of signoff.

So a possible flow can be:

void append_message_string_list(...params...) {

        if (signoff) {
                ..excecute the necessary segments and call the
                'append_message_helper()' function...
        } else {
                ..similarly here..
        }
}

This way we save ourselves some extra functions.

May some thing like this:

if (signoff)
        append_message_string_list(&sb, "Signed-off-by: ", NULL,
                   ignore_non_trailer(sb.buf, sb.len), 0);

And then we could judge in `append_message_string_list`
if the arguement `string_list *trailers` set to NULL. If so,
we do something similar to `append_signoff`, otherwise,
we carry out other situations.

So that we can delete the `append_signoff` and user can only
need to call  `append_message_string_list` in any where, I don't
 know if it is better.

quoted

diff --git a/sequencer.h b/sequencer.h
index f8b2e4ab8527..b24e274f4c62 100644
--- a/sequencer.h
+++ b/sequencer.h

@@ -174,6 +174,10 @@ int todo_list_rearrange_squash(struct todo_list *todo_list);
  * and the new signoff will be spliced into the buffer before those bytes.
  */
 void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);
+void append_message(struct strbuf *msgbuf, struct strbuf *sob,
+             size_t ignore_footer, unsigned flag);
+void append_message_string_list(struct strbuf *msgbuf, const char*header,
+             struct string_list *sobs, size_t ignore_footer, unsigned flag);

 void append_conflicts_hint(struct index_state *istate,
              struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);

Then, these would have to be corrected too.

quoted

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..40823152a51c 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh

@@ -154,6 +154,112 @@ test_expect_success 'sign off' '

 '

I feel that the tests deserve a commit of their own. A follow-up commit
in this patch.

quoted

+test_expect_success 'helped-by' '
+
+     >file1 &&
+     git add file1 &&
+     git commit --helped-by="foo <bar@frotz>" \
+     --helped-by="foo2 <bar2@frotz>" -m "thank you" &&
+     git cat-file commit HEAD >commit.msg &&
+     sed -ne "s/Helped-by: //p" commit.msg >actual &&
+     cat >expected <<-\EOF &&
+     foo <bar@frotz>
+     foo2 <bar2@frotz>
+     EOF
+     test_cmp expected actual
+

Was this extra line feed intentional? It looks odd here and other test
(scripts) don't have this. Though, this test script seems to have many
tests which have an extra line feed. My suggestion would be to eliminate
it. Maybe this script is old and hasn't been reviewed for a cleanup in
a long time.

I might want to keep the same format with the last "sign off" test. Now it
seems that this is not necessary.

quoted

+'
+
+test_expect_success 'reported-by' '
+
+     >file2 &&
+     git add file2 &&
+     git commit --reported-by="foo <bar@frotz>" \
+     --reported-by="foo2 <bar2@frotz>" -m "thank you" &&
+     git cat-file commit HEAD >commit.msg &&
+     sed -ne "s/Reported-by: //p" commit.msg >actual &&
+     cat >expected <<-\EOF &&
+     foo <bar@frotz>
+     foo2 <bar2@frotz>
+     EOF
+     test_cmp expected actual
+

Similarly here and in the parts that follow.

<...>

This seems like a good patch to me but more experienced members will be
able to comment better I think. Good job anyways.

Your comments and encouragement are of great help to me :)

Regards,
Shourya Shukla

Regards,
ZheNing Hu

From: ZheNing Hu <redacted>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide richer trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Now use `--trailer <token>[(=|:)<value>]` pass the trailers to
`interpret-trailers` and generate trailers in commit messages.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC] commit: provides multiple signatures from command line
    
    Now maintainers or developers can also use commit
    --trailer="Signed-off-by:commiter<email>" from the command line to
    provide trailers to commit messages. This solution may be more
    generalized than v1.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/901

Range-diff vs v1:

 1:  e9389e72ac65 ! 1:  4c507d17db4f [GSOC] commit: provides multiple common signatures
     @@ Metadata
      Author: ZheNing Hu [off-list ref]
      
       ## Commit message ##
     -    [GSOC] commit: provides multiple common signatures
     +    [GSOC] commit: add trailer command
      
     -    Similar to "Helped-by", "Reported-by", "Reviewed-by", "Mentored-by"
     -    these signatures are often seen in git commit messages. After
     -    referring to the simple implementation of `commit --signoff`
     -    and `send-email -cc=" commiter <email>"`, I am considering
     -    whether to provide multiple signature parameters from the
     -    command line. I think this might help maintainers and
     -    developers directly uses the shell to provide these signatures
     -    instead of multiple times repetitive writing those trailers
     -    each time.
     +    Historically, Git has supported the 'Signed-off-by' commit trailer
     +    using the '--signoff' and the '-s' option from the command line.
     +    But users may need to provide richer trailer information from the
     +    command line such as "Helped-by", "Reported-by", "Mentored-by",
      
     -    To achieve this goal, i refactored the `append_signoff` design and
     -    provided `append_message` and `append_message_string_list` interfaces,
     -    providing new ways to generate those various signatures.
     -
     -    Users now can use `commit -H "helper <eamil>"` to generate "Helped-by" trailer,
     -    `commit -R "reviewer <eamil>"` to generate "Reviewed-by" trailer,
     -    `commit -r "reporter <eamil> "`to generate "Reported-by" trailer,
     -    `commit -M "mentor <eamil>"` to generate "Mentored-by" trailer.
     +    Now use `--trailer <token>[(=|:)<value>]` pass the trailers to
     +    `interpret-trailers` and generate trailers in commit messages.
      
          Signed-off-by: ZheNing Hu [off-list ref]
      
     @@ Documentation/git-commit.txt: SYNOPSIS
       	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
       	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
      -	   [-S[<keyid>]] [--] [<pathspec>...]
     -+	   [-S[<keyid>]] [--] [<pathspec>...] [(-H|--helped-by)=<address>...]
     -+	   [(-R|--reviewed-by)=<address>...] [(-r|--reported-by)=<address>...]
     -+	   [(-M|--mentored)=<address>...]
     ++	   [-S[<keyid>]] [--] [<pathspec>...] [(--trailer <token>[(=|:)<value>])...]
       
       DESCRIPTION
       -----------
     @@ Documentation/git-commit.txt: The `-m` option is mutually exclusive with `-c`, `
       
       include::signoff-option.txt[]
       
     -+-H=<address>...::
     -+--helped-by=<address>...::
     -+	Add one or more `Helped-by` trailer by the committer at the end of the commit
     -+	log message.
     -+
     -+-R=<address>...::
     -+--reviewed-by=<address>...::
     -+	Add one or more `Reviewed-by` trailer by the committer at the end of the commit
     -+	log message.
     -+
     -+-r=<address>...::
     -+--reported-by=<address>...::
     -+	Add one or more `Reported-by` trailer by the committer at the end of the commit
     -+	log message.
     -+
     -+-M=<address>...::
     -+--mentored-by=<address>...::
     -+	Add one or more `Mentored-by` trailer by the committer at the end of the commit
     -+	log message.
     ++--trailer <token>[(=|:)<value>]::
     ++	Specify a (<token>, <value>) pair that should be applied as a
     ++	trailer. (e.g.  `git commit --trailer "Signed-off-by:C O Mitter [off-list ref]" \
     ++	--trailer "Helped-by:C O Mitter [off-list ref]"`will add the "Signed-off" trailer
     ++	and the "Helped-by" trailer in the commit message.)
      +
       -n::
       --no-verify::
     @@ builtin/commit.c: static int config_commit_verbose = -1; /* unspecified */
       static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
       static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
       static char *sign_commit, *pathspec_from_file;
     -+static struct string_list helped_by = STRING_LIST_INIT_NODUP;
     -+static struct string_list mentored_by = STRING_LIST_INIT_NODUP;
     -+static struct string_list reviewed_by = STRING_LIST_INIT_NODUP;
     -+static struct string_list reported_by = STRING_LIST_INIT_NODUP;
     ++struct child_process run_trailer = CHILD_PROCESS_INIT;
     ++static const char *trailer;
       
       /*
        * The default commit message cleanup mode will remove the lines
     -@@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const char *prefix,
     - 	if (signoff)
     - 		append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);
     - 
     -+	if(helped_by.items)
     -+		append_message_string_list(&sb, "Helped-by: ", &helped_by, ignore_non_trailer(sb.buf, sb.len), 0);
     -+	if(reviewed_by.items)
     -+		append_message_string_list(&sb, "Reviewed-by: ", &reviewed_by, ignore_non_trailer(sb.buf, sb.len), 0);
     -+	if(reported_by.items)
     -+		append_message_string_list(&sb, "Reported-by: ", &reported_by, ignore_non_trailer(sb.buf, sb.len), 0);
     -+	if(mentored_by.items)
     -+		append_message_string_list(&sb, "Mentored-by: ", &mentored_by, ignore_non_trailer(sb.buf, sb.len), 0);
     -+
     -+	string_list_clear(&helped_by, 0);
     -+	string_list_clear(&reviewed_by, 0);
     -+	string_list_clear(&reported_by, 0);
     -+	string_list_clear(&mentored_by, 0);
     -+
     - 	if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
     - 		die_errno(_("could not write commit template"));
     +@@ builtin/commit.c: static struct strbuf message = STRBUF_INIT;
       
     -@@ builtin/commit.c: static int git_commit_config(const char *k, const char *v, void *cb)
     - 	return git_status_config(k, v, s);
     - }
     + static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
       
     -+static int help_callback(const struct option *opt, const char *arg, int unset)
     -+{
     -+	if (unset)
     -+		string_list_clear(&helped_by, 0);
     -+	else
     -+		string_list_append(&helped_by, arg);
     -+	return 0;
     -+}
     -+
     -+static int review_callback(const struct option *opt, const char *arg, int unset)
     ++static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
      +{
     -+	if (unset)
     -+		string_list_clear(&reviewed_by, 0);
     -+	else
     -+		string_list_append(&reviewed_by, arg);
     -+	return 0;
     -+}
     -+
     -+static int report_callback(const struct option *opt, const char *arg, int unset)
     -+{
     -+	if (unset)
     -+		string_list_clear(&reported_by, 0);
     -+	else
     -+		string_list_append(&reported_by, arg);
     -+	return 0;
     -+}
     -+
     -+static int mentor_callback(const struct option *opt, const char *arg, int unset)
     -+{
     -+	if (unset)
     -+		string_list_clear(&mentored_by, 0);
     -+	else
     -+		string_list_append(&mentored_by, arg);
     ++	if (unset) {
     ++		strvec_clear(&run_trailer.args);
     ++		return -1;
     ++	}
     ++	run_trailer.git_cmd = 1;
     ++	strvec_pushl(&run_trailer.args, "--trailer", arg, NULL);
      +	return 0;
      +}
      +
     - int cmd_commit(int argc, const char **argv, const char *prefix)
     + static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
       {
     - 	static struct wt_status s;
     + 	enum wt_status_format *value = (enum wt_status_format *)opt->value;
     +@@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const char *prefix,
     + 
     + 	fclose(s->fp);
     + 
     ++	run_command(&run_trailer);
     ++
     + 	/*
     + 	 * Reject an attempt to record a non-merge empty commit without
     + 	 * explicit --allow-empty. In the cherry-pick case, it may be
      @@ builtin/commit.c: int cmd_commit(int argc, const char **argv, const char *prefix)
       		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
       		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
       		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
     -+		OPT_CALLBACK('H', "helped-by", NULL, N_("email"), N_("add a Helped-by trailer"), help_callback),
     -+		OPT_CALLBACK('r', "reported-by", NULL, N_("email"), N_("add a Reported-by trailer"), report_callback),
     -+		OPT_CALLBACK('R', "reviewed-by", NULL, N_("email"), N_("add a Reviewed-by trailer"), review_callback),
     -+		OPT_CALLBACK('M', "mentored-by", NULL, N_("email"), N_("add a Mentored-by trailer"), mentor_callback),
     ++		OPT_CALLBACK(0, "trailer", &trailer, N_("trailer"), N_("trailer(s) to add"), opt_pass_trailer),
       		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
       		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
       		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
      @@ builtin/commit.c: int cmd_commit(int argc, const char **argv, const char *prefix)
     - 	struct commit_extra_header *extra = NULL;
     - 	struct strbuf err = STRBUF_INIT;
     - 
     -+	helped_by.strdup_strings = 1;
     -+	reviewed_by.strdup_strings = 1;
     -+	reported_by.strdup_strings = 1;
     -+	mentored_by.strdup_strings = 1;
     -+
     - 	if (argc == 2 && !strcmp(argv[1], "-h"))
     - 		usage_with_options(builtin_commit_usage, builtin_commit_options);
     - 
     -
     - ## sequencer.c ##
     -@@ sequencer.c: int sequencer_pick_revisions(struct repository *r,
     - 	return res;
     - }
     - 
     --void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
     -+void append_message(struct strbuf *msgbuf, struct strbuf *sob,
     -+			size_t ignore_footer, unsigned flag)
     - {
     - 	unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
     --	struct strbuf sob = STRBUF_INIT;
     - 	int has_footer;
     - 
     --	strbuf_addstr(&sob, sign_off_header);
     --	strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));
     --	strbuf_addch(&sob, '\n');
     --
     - 	if (!ignore_footer)
     - 		strbuf_complete_line(msgbuf);
     - 
     -@@ sequencer.c: void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
     - 	 * If the whole message buffer is equal to the sob, pretend that we
     - 	 * found a conforming footer with a matching sob
     - 	 */
     --	if (msgbuf->len - ignore_footer == sob.len &&
     --	    !strncmp(msgbuf->buf, sob.buf, sob.len))
     -+	if (msgbuf->len - ignore_footer == sob->len &&
     -+	    !strncmp(msgbuf->buf, sob->buf, sob->len))
     - 		has_footer = 3;
     - 	else
     --		has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
     -+		has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);
     - 
     - 	if (!has_footer) {
     - 		const char *append_newlines = NULL;
     -@@ sequencer.c: void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
     - 
     - 	if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
     - 		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
     --				sob.buf, sob.len);
     -+				sob->buf, sob->len);
     -+}
     -+
     -+void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
     -+{
     -+	struct strbuf sob = STRBUF_INIT;
     -+	strbuf_addstr(&sob, sign_off_header);
     -+	strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));
     -+	strbuf_addch(&sob, '\n');
     -+	append_message(msgbuf, &sob, ignore_footer, flag);
     -+	strbuf_release(&sob);
     -+}
     - 
     -+void append_message_string_list(struct strbuf *msgbuf, const char *header,
     -+		struct string_list *sobs, size_t ignore_footer, unsigned flag) {
     -+	int i;
     -+	struct strbuf sob = STRBUF_INIT;
     -+
     -+	for ( i = 0; i < sobs->nr; i++)
     -+	{
     -+		strbuf_addstr(&sob, header);
     -+		strbuf_addstr(&sob, sobs->items[i].string);
     -+		strbuf_addch(&sob, '\n');
     -+		append_message(msgbuf, &sob, ignore_footer, flag);
     -+		strbuf_reset(&sob);
     -+	}
     - 	strbuf_release(&sob);
     - }
     - 
     -
     - ## sequencer.h ##
     -@@ sequencer.h: int todo_list_rearrange_squash(struct todo_list *todo_list);
     -  * and the new signoff will be spliced into the buffer before those bytes.
     -  */
     - void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);
     -+void append_message(struct strbuf *msgbuf, struct strbuf *sob,
     -+		size_t ignore_footer, unsigned flag);
     -+void append_message_string_list(struct strbuf *msgbuf, const char*header,
     -+		struct string_list *sobs, size_t ignore_footer, unsigned flag);
     - 
     - void append_conflicts_hint(struct index_state *istate,
     - 		struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);
     + 			die(_("could not parse HEAD commit"));
     + 	}
     + 	verbose = -1; /* unspecified */
     ++	strvec_pushl(&run_trailer.args, "interpret-trailers",
     ++		"--in-place", "--where=end", git_path_commit_editmsg(), NULL);
     + 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
     + 					  builtin_commit_usage,
     + 					  prefix, current_head, &s);
      
       ## t/t7502-commit-porcelain.sh ##
      @@ t/t7502-commit-porcelain.sh: test_expect_success 'sign off' '
       
       '
       
     -+test_expect_success 'helped-by' '
     -+
     ++test_expect_success 'trailer' '
      +	>file1 &&
      +	git add file1 &&
     -+	git commit --helped-by="foo <bar@frotz>" \
     -+	--helped-by="foo2 <bar2@frotz>" -m "thank you" &&
     -+	git cat-file commit HEAD >commit.msg &&
     -+	sed -ne "s/Helped-by: //p" commit.msg >actual &&
     -+	cat >expected <<-\EOF &&
     -+	foo <bar@frotz>
     -+	foo2 <bar2@frotz>
     -+	EOF
     -+	test_cmp expected actual
     -+
     -+'
     -+
     -+test_expect_success 'reported-by' '
     -+
     -+	>file2 &&
     -+	git add file2 &&
     -+	git commit --reported-by="foo <bar@frotz>" \
     -+	--reported-by="foo2 <bar2@frotz>" -m "thank you" &&
     -+	git cat-file commit HEAD >commit.msg &&
     -+	sed -ne "s/Reported-by: //p" commit.msg >actual &&
     -+	cat >expected <<-\EOF &&
     -+	foo <bar@frotz>
     -+	foo2 <bar2@frotz>
     -+	EOF
     -+	test_cmp expected actual
     -+
     -+'
     -+
     -+test_expect_success 'reviewed-by' '
     -+
     -+	>file3 &&
     -+	git add file3 &&
     -+	git commit --reviewed-by="foo <bar@frotz>" \
     -+	--reviewed-by="foo2 <bar2@frotz>" -m "thank you" &&
     -+	git cat-file commit HEAD >commit.msg &&
     -+	sed -ne "s/Reviewed-by: //p" commit.msg >actual &&
     -+	cat >expected <<-\EOF &&
     -+	foo <bar@frotz>
     -+	foo2 <bar2@frotz>
     -+	EOF
     -+	test_cmp expected actual
     -+
     -+'
     -+
     -+test_expect_success 'mentored-by' '
     -+
     -+	>file4 &&
     -+	git add file4 &&
     -+	git commit --mentored-by="foo <bar@frotz>" \
     -+	--mentored-by="foo2 <bar2@frotz>" -m "thank you" &&
     -+	git cat-file commit HEAD >commit.msg &&
     -+	sed -ne "s/Mentored-by: //p" commit.msg >actual &&
     -+	cat >expected <<-\EOF &&
     -+	foo <bar@frotz>
     -+	foo2 <bar2@frotz>
     -+	EOF
     -+	test_cmp expected actual
     -+
     -+'
     -+
     -+test_expect_success 'multiple signatures' '
     -+
     -+	>file5 &&
     -+	git add file5 &&
     -+	git commit --helped-by="foo <bar@frotz>" \
     -+	--reviewed-by="foo2 <bar2@frotz>" \
     -+	--mentored-by="foo3 <bar3@frotz>" \
     -+	--reported-by="foo4 <bar4@frotz>" -s -m "thank you" &&
     ++	git commit -s --trailer "Signed-off-by:C O Mitter1 [off-list ref]" \
     ++	--trailer "Helped-by:C O Mitter2 [off-list ref]"  \
     ++	--trailer "Reported-by:C O Mitter3 [off-list ref]" \
     ++	--trailer "Mentored-by:C O Mitter4 [off-list ref]" \
     ++	-m "hello" &&
      +	git cat-file commit HEAD >commit.msg &&
      +	sed -e "1,7d" commit.msg >actual &&
      +	cat >expected <<-\EOF &&
      +	Signed-off-by: C O Mitter [off-list ref]
     -+	Helped-by: foo <bar@frotz>
     -+	Reviewed-by: foo2 <bar2@frotz>
     -+	Reported-by: foo4 <bar4@frotz>
     -+	Mentored-by: foo3 <bar3@frotz>
     ++	Signed-off-by: C O Mitter1 [off-list ref]
     ++	Helped-by: C O Mitter2 [off-list ref]
     ++	Reported-by: C O Mitter3 [off-list ref]
     ++	Mentored-by: C O Mitter4 [off-list ref]
      +	EOF
      +	test_cmp expected actual
     -+
     -+'
     -+
     -+test_expect_success 'multiple signatures (use abbreviations)' '
     -+
     -+	>file6 &&
     -+	git add file6 &&
     -+	git commit -H "foo <bar@frotz>" \
     -+	-R "foo2 <bar2@frotz>" \
     -+	-M "foo3 <bar3@frotz>" \
     -+	-r "foo4 <bar4@frotz>" -s -m "thank you" &&
     -+	git cat-file commit HEAD >commit.msg &&
     -+	sed -e "1,7d" commit.msg >actual &&
     -+	cat >expected <<-\EOF &&
     -+	Signed-off-by: C O Mitter [off-list ref]
     -+	Helped-by: foo <bar@frotz>
     -+	Reviewed-by: foo2 <bar2@frotz>
     -+	Reported-by: foo4 <bar4@frotz>
     -+	Mentored-by: foo3 <bar3@frotz>
     -+	EOF
     -+	test_cmp expected actual
     -+
      +'
      +
       test_expect_success 'multiple -m' '


 Documentation/git-commit.txt |  8 +++++++-
 builtin/commit.c             | 18 ++++++++++++++++++
 t/t7502-commit-porcelain.sh  | 20 ++++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..764513a3f287 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -14,7 +14,7 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [-S[<keyid>]] [--] [<pathspec>...] [(--trailer <token>[(=|:)<value>])...]
 
 DESCRIPTION
 -----------

@@ -166,6 +166,12 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 
 include::signoff-option.txt[]
 
+--trailer <token>[(=|:)<value>]::
+	Specify a (<token>, <value>) pair that should be applied as a
+	trailer. (e.g.  `git commit --trailer "Signed-off-by:C O Mitter <committer@example.com>" \
+	--trailer "Helped-by:C O Mitter <committer@example.com>"`will add the "Signed-off" trailer
+	and the "Helped-by" trailer in the commit message.)
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..abbd136b27f0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -113,6 +113,8 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+struct child_process run_trailer = CHILD_PROCESS_INIT;
+static const char *trailer;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -131,6 +133,17 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
+{
+	if (unset) {
+		strvec_clear(&run_trailer.args);
+		return -1;
+	}
+	run_trailer.git_cmd = 1;
+	strvec_pushl(&run_trailer.args, "--trailer", arg, NULL);
+	return 0;
+}
+
 static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
 {
 	enum wt_status_format *value = (enum wt_status_format *)opt->value;

@@ -958,6 +971,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	fclose(s->fp);
 
+	run_command(&run_trailer);
+
 	/*
 	 * Reject an attempt to record a non-merge empty commit without
 	 * explicit --allow-empty. In the cherry-pick case, it may be

@@ -1507,6 +1522,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+		OPT_CALLBACK(0, "trailer", &trailer, N_("trailer"), N_("trailer(s) to add"), opt_pass_trailer),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

@@ -1577,6 +1593,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			die(_("could not parse HEAD commit"));
 	}
 	verbose = -1; /* unspecified */
+	strvec_pushl(&run_trailer.args, "interpret-trailers",
+		"--in-place", "--where=end", git_path_commit_editmsg(), NULL);
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 					  builtin_commit_usage,
 					  prefix, current_head, &s);

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..4b9ac4587d17 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh

@@ -154,6 +154,26 @@ test_expect_success 'sign off' '
 
 '
 
+test_expect_success 'trailer' '
+	>file1 &&
+	git add file1 &&
+	git commit -s --trailer "Signed-off-by:C O Mitter1 <committer1@example.com>" \
+	--trailer "Helped-by:C O Mitter2 <committer2@example.com>"  \
+	--trailer "Reported-by:C O Mitter3 <committer3@example.com>" \
+	--trailer "Mentored-by:C O Mitter4 <committer4@example.com>" \
+	-m "hello" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,7d" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C O Mitter1 <committer1@example.com>
+	Helped-by: C O Mitter2 <committer2@example.com>
+	Reported-by: C O Mitter3 <committer3@example.com>
+	Mentored-by: C O Mitter4 <committer4@example.com>
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&

base-commit: 13d7ab6b5d7929825b626f050b62a11241ea4945
-- 
gitgitgadget

On Fri, Mar 12, 2021 at 4:59 PM ZheNing Hu via GitGitGadget
[off-list ref] wrote:

From: ZheNing Hu <redacted>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide richer trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Nit: not sure that "richer" is the proper word here. I would just use
"other" instead.

Now use `--trailer <token>[(=|:)<value>]` pass the trailers to
`interpret-trailers` and generate trailers in commit messages.

The subject says "add trailer command" while here you say "use". So
which one is it? Does "--trailer" already exist, and we are just going
to use it? Or will this patch series actually "add" it?

Looking at the existing options and the code of this patch series, the
patch series actually adds the "--trailer" (not "trailer") option, so
"add" or "implement" would be clearer than "use".

So maybe something like the following might be better:

"Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages."

Also something like "--trailer" is usually called an option (or
sometimes a flag), not a command (especially not when the word is not
a verb, and when the new feature isn't a new exclusive mode of
operation). So something like "commit: add --trailer option" might be
a better subject.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC] commit: provides multiple signatures from command line

It looks like this is using the subject of a patch that previously
attempted to add features with a similar purpose. I don't think you
need to put it there, or if you want to refer to it, I think it might
be better to be a bit more explicit, for example like:

"This patch replaces my previous attempt to provide similar features
in a patch called: [GSOC] commit: provides multiple signatures from
command line."

    Now maintainers or developers can also use commit
    --trailer="Signed-off-by:commiter<email>" from the command line to
    provide trailers to commit messages. This solution may be more
    generalized than v1.

Ok, I agree that it's a good idea to have a good generic solution
first, before having specialized options for specific trailers.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/901

Range-diff vs v1:

If this patch series has very few code and commit messages in common
with a previous attempt at implementing similar features, it might be
better to make it a new patch series rather than a v2. This could
avoid sending range-diffs that are mostly useless.

Christian Couder [off-list ref] 于2021年3月14日周日 下午12:19写道：

On Fri, Mar 12, 2021 at 4:59 PM ZheNing Hu via GitGitGadget
[off-list ref] wrote:

quoted

From: ZheNing Hu <redacted>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide richer trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Nit: not sure that "richer" is the proper word here. I would just use
"other" instead.

OK.

quoted

Now use `--trailer <token>[(=|:)<value>]` pass the trailers to
`interpret-trailers` and generate trailers in commit messages.

The subject says "add trailer command" while here you say "use". So
which one is it? Does "--trailer" already exist, and we are just going
to use it? Or will this patch series actually "add" it?

Looking at the existing options and the code of this patch series, the
patch series actually adds the "--trailer" (not "trailer") option, so
"add" or "implement" would be clearer than "use".

You're right. "add" will be more accurate in this situation.

So maybe something like the following might be better:

"Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages."

Also something like "--trailer" is usually called an option (or
sometimes a flag), not a command (especially not when the word is not
a verb, and when the new feature isn't a new exclusive mode of
operation). So something like "commit: add --trailer option" might be
a better subject.

quoted

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC] commit: provides multiple signatures from command line

It looks like this is using the subject of a patch that previously
attempted to add features with a similar purpose. I don't think you
need to put it there, or if you want to refer to it, I think it might
be better to be a bit more explicit, for example like:

"This patch replaces my previous attempt to provide similar features
in a patch called: [GSOC] commit: provides multiple signatures from
command line."

I may have thought that the effect of the two patch was closer so did not
change it.

quoted

    Now maintainers or developers can also use commit
    --trailer="Signed-off-by:commiter<email>" from the command line to
    provide trailers to commit messages. This solution may be more
    generalized than v1.

Ok, I agree that it's a good idea to have a good generic solution
first, before having specialized options for specific trailers.

Thanks. :)

quoted

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/901

Range-diff vs v1:

If this patch series has very few code and commit messages in common
with a previous attempt at implementing similar features, it might be
better to make it a new patch series rather than a v2. This could
avoid sending range-diffs that are mostly useless.

Thank you for these pertinent suggestions. I will pay more attention.

From: ZheNing Hu <redacted>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide other trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC] commit: add --trailer option
    
    Now maintainers or developers can also use commit
    --trailer="Signed-off-by:commiter<email>" from the command line to
    provide trailers to commit messages. This solution may be more
    generalized than v1.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/901

Range-diff vs v2:

 1:  4c507d17db4f ! 1:  b4e161a98f8b [GSOC] commit: add trailer command
     @@ Metadata
      Author: ZheNing Hu [off-list ref]
      
       ## Commit message ##
     -    [GSOC] commit: add trailer command
     +    [GSOC] commit: add --trailer option
      
          Historically, Git has supported the 'Signed-off-by' commit trailer
          using the '--signoff' and the '-s' option from the command line.
     -    But users may need to provide richer trailer information from the
     +    But users may need to provide other trailer information from the
          command line such as "Helped-by", "Reported-by", "Mentored-by",
      
     -    Now use `--trailer <token>[(=|:)<value>]` pass the trailers to
     -    `interpret-trailers` and generate trailers in commit messages.
     +    Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
     +    other trailers to `interpret-trailers` and insert them into commit
     +    messages.
      
          Signed-off-by: ZheNing Hu [off-list ref]
      


 Documentation/git-commit.txt |  8 +++++++-
 builtin/commit.c             | 18 ++++++++++++++++++
 t/t7502-commit-porcelain.sh  | 20 ++++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..764513a3f287 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -14,7 +14,7 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [-S[<keyid>]] [--] [<pathspec>...] [(--trailer <token>[(=|:)<value>])...]
 
 DESCRIPTION
 -----------

@@ -166,6 +166,12 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 
 include::signoff-option.txt[]
 
+--trailer <token>[(=|:)<value>]::
+	Specify a (<token>, <value>) pair that should be applied as a
+	trailer. (e.g.  `git commit --trailer "Signed-off-by:C O Mitter <committer@example.com>" \
+	--trailer "Helped-by:C O Mitter <committer@example.com>"`will add the "Signed-off" trailer
+	and the "Helped-by" trailer in the commit message.)
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..abbd136b27f0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -113,6 +113,8 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+struct child_process run_trailer = CHILD_PROCESS_INIT;
+static const char *trailer;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -131,6 +133,17 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
+{
+	if (unset) {
+		strvec_clear(&run_trailer.args);
+		return -1;
+	}
+	run_trailer.git_cmd = 1;
+	strvec_pushl(&run_trailer.args, "--trailer", arg, NULL);
+	return 0;
+}
+
 static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
 {
 	enum wt_status_format *value = (enum wt_status_format *)opt->value;

@@ -958,6 +971,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	fclose(s->fp);
 
+	run_command(&run_trailer);
+
 	/*
 	 * Reject an attempt to record a non-merge empty commit without
 	 * explicit --allow-empty. In the cherry-pick case, it may be

@@ -1507,6 +1522,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+		OPT_CALLBACK(0, "trailer", &trailer, N_("trailer"), N_("trailer(s) to add"), opt_pass_trailer),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

@@ -1577,6 +1593,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			die(_("could not parse HEAD commit"));
 	}
 	verbose = -1; /* unspecified */
+	strvec_pushl(&run_trailer.args, "interpret-trailers",
+		"--in-place", "--where=end", git_path_commit_editmsg(), NULL);
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 					  builtin_commit_usage,
 					  prefix, current_head, &s);

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..4b9ac4587d17 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh

@@ -154,6 +154,26 @@ test_expect_success 'sign off' '
 
 '
 
+test_expect_success 'trailer' '
+	>file1 &&
+	git add file1 &&
+	git commit -s --trailer "Signed-off-by:C O Mitter1 <committer1@example.com>" \
+	--trailer "Helped-by:C O Mitter2 <committer2@example.com>"  \
+	--trailer "Reported-by:C O Mitter3 <committer3@example.com>" \
+	--trailer "Mentored-by:C O Mitter4 <committer4@example.com>" \
+	-m "hello" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,7d" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C O Mitter1 <committer1@example.com>
+	Helped-by: C O Mitter2 <committer2@example.com>
+	Reported-by: C O Mitter3 <committer3@example.com>
+	Mentored-by: C O Mitter4 <committer4@example.com>
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&

base-commit: 13d7ab6b5d7929825b626f050b62a11241ea4945
-- 
gitgitgadget

"ZheNing Hu via GitGitGadget" [off-list ref] writes:

There is slight problem with running the command unconditionally.
If no --trailer is passed, then the opt_pass_trailer() backend
is never called, which consequently will not set the trailer
command ".git_cmd" option to 1.

This will lead the run_command() API to not interpret the command as git
internal, and attempt to launch as an usual command "interpret-trailers"
that will likely not exist or launch an unwanted command that is not
part of the GIT suite.

This can be seen by running `git commit` without any options:

     $ ./bin-wrappers/git -C /tmp/test commit
     error: cannot run interpret-trailers: No such file or directory
     ...

The `.git_cmd` should be set to true before running the command.

(the above output is from a built version with v2.31.0-rc2 + this patch
 for confirmation).


Furthermore, in my opinion, we shouldn't even bother to run the command
if no --trailer is passed, otherwise, we always be paying the cost of
launching an OS process regardless if the user doesn't want to add
trailers in theirs projects.

With that said and based on this current implementation, maybe an
improved version will look like:

        if (run_trailer.args.nr) {
                run_trailer.git_cmd = 1;
                run_command(&run_trailer);
        }

Naturally the `git_cmd = 1` will be removed from opt_pass_trailer()
function as it won't be necessary. As minor bonus, we don't end up
setting the value for every new --trailer :).

Style: The "--in-place" part should be aligned with the parentheses
much the like the following line with the "argc = parse_and_validate_options....".

For example:

	strvec_pushl(&run_trailer.args, "interpret-trailers",
		     "--in-place", "--where=end", .....

Perhaps here, the --trailer lines and "-m hello" option should be
indent in order to make it clear that these option are part of the
"git commit" from the above line, something like this:

	git commit -s --trailer "Signed-off-by:C O Mitter1 [off-list ref]" \
	        --trailer "Helped-by:C O Mitter2 [off-list ref]"  \
	        --trailer "Reported-by:C O Mitter3 [off-list ref]" \
	        --trailer "Mentored-by:C O Mitter4 [off-list ref]" \
	        -m "hello" &&

Hope these comments are useful.
-- 
Thanks
Rafael

Rafael Silva [off-list ref] 于2021年3月14日周日 下午9:46写道：

"ZheNing Hu via GitGitGadget" [off-list ref] writes:

quoted

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..abbd136b27f0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -113,6 +113,8 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+struct child_process run_trailer = CHILD_PROCESS_INIT;
+static const char *trailer;

 /*
  * The default commit message cleanup mode will remove the lines

@@ -131,6 +133,17 @@ static struct strbuf message = STRBUF_INIT;

 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;

+static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
+{
+     if (unset) {
+             strvec_clear(&run_trailer.args);
+             return -1;
+     }
+     run_trailer.git_cmd = 1;
+     strvec_pushl(&run_trailer.args, "--trailer", arg, NULL);
+     return 0;
+}
+
 static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
 {
      enum wt_status_format *value = (enum wt_status_format *)opt->value;

@@ -958,6 +971,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,

      fclose(s->fp);

+     run_command(&run_trailer);
+

There is slight problem with running the command unconditionally.
If no --trailer is passed, then the opt_pass_trailer() backend
is never called, which consequently will not set the trailer
command ".git_cmd" option to 1.

This will lead the run_command() API to not interpret the command as git
internal, and attempt to launch as an usual command "interpret-trailers"
that will likely not exist or launch an unwanted command that is not
part of the GIT suite.

This can be seen by running `git commit` without any options:

     $ ./bin-wrappers/git -C /tmp/test commit
     error: cannot run interpret-trailers: No such file or directory
     ...

The `.git_cmd` should be set to true before running the command.

(the above output is from a built version with v2.31.0-rc2 + this patch
 for confirmation).


Furthermore, in my opinion, we shouldn't even bother to run the command
if no --trailer is passed, otherwise, we always be paying the cost of
launching an OS process regardless if the user doesn't want to add
trailers in theirs projects.

With that said and based on this current implementation, maybe an
improved version will look like:

        if (run_trailer.args.nr) {
                run_trailer.git_cmd = 1;
                run_command(&run_trailer);
        }

Naturally the `git_cmd = 1` will be removed from opt_pass_trailer()
function as it won't be necessary. As minor bonus, we don't end up
setting the value for every new --trailer :).

Thank you, I didn't notice the problem before :).

quoted

      /*
       * Reject an attempt to record a non-merge empty commit without
       * explicit --allow-empty. In the cherry-pick case, it may be

@@ -1507,6 +1522,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
              OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
              OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
              OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+             OPT_CALLBACK(0, "trailer", &trailer, N_("trailer"), N_("trailer(s) to add"), opt_pass_trailer),
              OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
              OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
              OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

@@ -1577,6 +1593,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
                      die(_("could not parse HEAD commit"));
      }
      verbose = -1; /* unspecified */
+     strvec_pushl(&run_trailer.args, "interpret-trailers",
+             "--in-place", "--where=end", git_path_commit_editmsg(), NULL);

Style: The "--in-place" part should be aligned with the parentheses
much the like the following line with the "argc = parse_and_validate_options....".

Well, the CodingGuidelines say it's both vaild. But you are right,
try to keep the same format as nearby code will be better.

For example:

        strvec_pushl(&run_trailer.args, "interpret-trailers",
                     "--in-place", "--where=end", .....

quoted

      argc = parse_and_validate_options(argc, argv, builtin_commit_options,
                                        builtin_commit_usage,
                                        prefix, current_head, &s);

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..4b9ac4587d17 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh

@@ -154,6 +154,26 @@ test_expect_success 'sign off' '

 '

+test_expect_success 'trailer' '
+     >file1 &&
+     git add file1 &&
+     git commit -s --trailer "Signed-off-by:C O Mitter1 <committer1@example.com>" \
+     --trailer "Helped-by:C O Mitter2 <committer2@example.com>"  \
+     --trailer "Reported-by:C O Mitter3 <committer3@example.com>" \
+     --trailer "Mentored-by:C O Mitter4 <committer4@example.com>" \
+     -m "hello" &&

Perhaps here, the --trailer lines and "-m hello" option should be
indent in order to make it clear that these option are part of the
"git commit" from the above line, something like this:

        git commit -s --trailer "Signed-off-by:C O Mitter1 [off-list ref]" \
                --trailer "Helped-by:C O Mitter2 [off-list ref]"  \
                --trailer "Reported-by:C O Mitter3 [off-list ref]" \
                --trailer "Mentored-by:C O Mitter4 [off-list ref]" \
                -m "hello" &&

quoted

+     git cat-file commit HEAD >commit.msg &&
+     sed -e "1,7d" commit.msg >actual &&
+     cat >expected <<-\EOF &&
+     Signed-off-by: C O Mitter <committer@example.com>
+     Signed-off-by: C O Mitter1 <committer1@example.com>
+     Helped-by: C O Mitter2 <committer2@example.com>
+     Reported-by: C O Mitter3 <committer3@example.com>
+     Mentored-by: C O Mitter4 <committer4@example.com>
+     EOF
+     test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '

Hope these comments are useful.
--
Thanks
Rafael

 I am gratitude to you for helping me.
--
Thanks
ZheNing Hu

From: ZheNing Hu <redacted>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide other trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC] commit: add --trailer option
    
    Now maintainers or developers can also use commit
    --trailer="Signed-off-by:commiter<email>" from the command line to
    provide trailers to commit messages. This solution may be more
    generalized than v1.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/901

Range-diff vs v3:

 1:  b4e161a98f8b ! 1:  dc507553ef4f [GSOC] commit: add --trailer option
     @@ Documentation/git-commit.txt: The `-m` option is mutually exclusive with `-c`, `
       
      +--trailer <token>[(=|:)<value>]::
      +	Specify a (<token>, <value>) pair that should be applied as a
     -+	trailer. (e.g.  `git commit --trailer "Signed-off-by:C O Mitter [off-list ref]" \
     -+	--trailer "Helped-by:C O Mitter [off-list ref]"`will add the "Signed-off" trailer
     ++	trailer. (e.g. `git commit --trailer "Signed-off-by:C O Mitter \
     ++	[off-list ref]" --trailer "Helped-by:C O Mitter \
     ++	[off-list ref]"` will add the "Signed-off" trailer
      +	and the "Helped-by" trailer in the commit message.)
      +
       -n::
     @@ builtin/commit.c: static struct strbuf message = STRBUF_INIT;
      +		strvec_clear(&run_trailer.args);
      +		return -1;
      +	}
     -+	run_trailer.git_cmd = 1;
      +	strvec_pushl(&run_trailer.args, "--trailer", arg, NULL);
      +	return 0;
      +}
     @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha
       
       	fclose(s->fp);
       
     -+	run_command(&run_trailer);
     ++	if (run_trailer.args.nr != 4) {
     ++		run_trailer.git_cmd = 1;
     ++		run_command(&run_trailer);
     ++	} else
     ++		strvec_clear(&run_trailer.args);
      +
       	/*
       	 * Reject an attempt to record a non-merge empty commit without
     @@ builtin/commit.c: int cmd_commit(int argc, const char **argv, const char *prefix
       	}
       	verbose = -1; /* unspecified */
      +	strvec_pushl(&run_trailer.args, "interpret-trailers",
     -+		"--in-place", "--where=end", git_path_commit_editmsg(), NULL);
     ++		    "--in-place", "--where=end", git_path_commit_editmsg(), NULL);
       	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
       					  builtin_commit_usage,
       					  prefix, current_head, &s);
     @@ t/t7502-commit-porcelain.sh: test_expect_success 'sign off' '
      +	>file1 &&
      +	git add file1 &&
      +	git commit -s --trailer "Signed-off-by:C O Mitter1 [off-list ref]" \
     -+	--trailer "Helped-by:C O Mitter2 [off-list ref]"  \
     -+	--trailer "Reported-by:C O Mitter3 [off-list ref]" \
     -+	--trailer "Mentored-by:C O Mitter4 [off-list ref]" \
     -+	-m "hello" &&
     ++		--trailer "Helped-by:C O Mitter2 [off-list ref]"  \
     ++		--trailer "Reported-by:C O Mitter3 [off-list ref]" \
     ++		--trailer "Mentored-by:C O Mitter4 [off-list ref]" \
     ++		-m "hello" &&
      +	git cat-file commit HEAD >commit.msg &&
      +	sed -e "1,7d" commit.msg >actual &&
      +	cat >expected <<-\EOF &&


 Documentation/git-commit.txt |  9 ++++++++-
 builtin/commit.c             | 21 +++++++++++++++++++++
 t/t7502-commit-porcelain.sh  | 20 ++++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..73a7507db47f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -14,7 +14,7 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [-S[<keyid>]] [--] [<pathspec>...] [(--trailer <token>[(=|:)<value>])...]
 
 DESCRIPTION
 -----------

@@ -166,6 +166,13 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 
 include::signoff-option.txt[]
 
+--trailer <token>[(=|:)<value>]::
+	Specify a (<token>, <value>) pair that should be applied as a
+	trailer. (e.g. `git commit --trailer "Signed-off-by:C O Mitter \
+	<committer@example.com>" --trailer "Helped-by:C O Mitter \
+	<committer@example.com>"` will add the "Signed-off" trailer
+	and the "Helped-by" trailer in the commit message.)
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..b35ae5c9790d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -113,6 +113,8 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+struct child_process run_trailer = CHILD_PROCESS_INIT;
+static const char *trailer;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -131,6 +133,16 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
+{
+	if (unset) {
+		strvec_clear(&run_trailer.args);
+		return -1;
+	}
+	strvec_pushl(&run_trailer.args, "--trailer", arg, NULL);
+	return 0;
+}
+
 static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
 {
 	enum wt_status_format *value = (enum wt_status_format *)opt->value;

@@ -958,6 +970,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	fclose(s->fp);
 
+	if (run_trailer.args.nr != 4) {
+		run_trailer.git_cmd = 1;
+		run_command(&run_trailer);
+	} else
+		strvec_clear(&run_trailer.args);
+
 	/*
 	 * Reject an attempt to record a non-merge empty commit without
 	 * explicit --allow-empty. In the cherry-pick case, it may be

@@ -1507,6 +1525,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+		OPT_CALLBACK(0, "trailer", &trailer, N_("trailer"), N_("trailer(s) to add"), opt_pass_trailer),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

@@ -1577,6 +1596,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			die(_("could not parse HEAD commit"));
 	}
 	verbose = -1; /* unspecified */
+	strvec_pushl(&run_trailer.args, "interpret-trailers",
+		    "--in-place", "--where=end", git_path_commit_editmsg(), NULL);
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 					  builtin_commit_usage,
 					  prefix, current_head, &s);

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..0acf23799931 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh

@@ -154,6 +154,26 @@ test_expect_success 'sign off' '
 
 '
 
+test_expect_success 'trailer' '
+	>file1 &&
+	git add file1 &&
+	git commit -s --trailer "Signed-off-by:C O Mitter1 <committer1@example.com>" \
+		--trailer "Helped-by:C O Mitter2 <committer2@example.com>"  \
+		--trailer "Reported-by:C O Mitter3 <committer3@example.com>" \
+		--trailer "Mentored-by:C O Mitter4 <committer4@example.com>" \
+		-m "hello" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,7d" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C O Mitter1 <committer1@example.com>
+	Helped-by: C O Mitter2 <committer2@example.com>
+	Reported-by: C O Mitter3 <committer3@example.com>
+	Mentored-by: C O Mitter4 <committer4@example.com>
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&

base-commit: 13d7ab6b5d7929825b626f050b62a11241ea4945
-- 
gitgitgadget

From: ZheNing Hu <redacted>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide other trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC] commit: add --trailer option
    
    Now maintainers or developers can also use commit
    --trailer="Signed-off-by:commiter<email>" from the command line to
    provide trailers to commit messages. This solution may be more
    generalized than v1.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/901

Range-diff vs v4:

 1:  dc507553ef4f ! 1:  ca91accb2852 [GSOC] commit: add --trailer option
     @@ builtin/commit.c: static int config_commit_verbose = -1; /* unspecified */
       static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
       static char *sign_commit, *pathspec_from_file;
      +struct child_process run_trailer = CHILD_PROCESS_INIT;
     ++struct strvec trailer_args = STRVEC_INIT;
      +static const char *trailer;
       
       /*
     @@ builtin/commit.c: static struct strbuf message = STRBUF_INIT;
      +static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
      +{
      +	if (unset) {
     -+		strvec_clear(&run_trailer.args);
     ++		strvec_clear(&trailer_args);
      +		return -1;
      +	}
     -+	strvec_pushl(&run_trailer.args, "--trailer", arg, NULL);
     ++	strvec_pushl(&trailer_args, "--trailer", arg, NULL);
      +	return 0;
      +}
      +
     @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha
       
       	fclose(s->fp);
       
     -+	if (run_trailer.args.nr != 4) {
     ++	if (trailer_args.nr) {
     ++		strvec_pushl(&run_trailer.args, "interpret-trailers",
     ++			     "--in-place", "--where=end", git_path_commit_editmsg(), NULL);
     ++		strvec_pushv(&run_trailer.args, trailer_args.v);
      +		run_trailer.git_cmd = 1;
     -+		run_command(&run_trailer);
     -+	} else
     -+		strvec_clear(&run_trailer.args);
     ++		if (run_command(&run_trailer))
     ++			strvec_clear(&run_trailer.args);
     ++		strvec_clear(&trailer_args);
     ++	}
      +
       	/*
       	 * Reject an attempt to record a non-merge empty commit without
     @@ builtin/commit.c: int cmd_commit(int argc, const char **argv, const char *prefix
       		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
       		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
       		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
     -@@ builtin/commit.c: int cmd_commit(int argc, const char **argv, const char *prefix)
     - 			die(_("could not parse HEAD commit"));
     - 	}
     - 	verbose = -1; /* unspecified */
     -+	strvec_pushl(&run_trailer.args, "interpret-trailers",
     -+		    "--in-place", "--where=end", git_path_commit_editmsg(), NULL);
     - 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
     - 					  builtin_commit_usage,
     - 					  prefix, current_head, &s);
      
       ## t/t7502-commit-porcelain.sh ##
      @@ t/t7502-commit-porcelain.sh: test_expect_success 'sign off' '


 Documentation/git-commit.txt |  9 ++++++++-
 builtin/commit.c             | 24 ++++++++++++++++++++++++
 t/t7502-commit-porcelain.sh  | 20 ++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..73a7507db47f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -14,7 +14,7 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [-S[<keyid>]] [--] [<pathspec>...] [(--trailer <token>[(=|:)<value>])...]
 
 DESCRIPTION
 -----------

@@ -166,6 +166,13 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 
 include::signoff-option.txt[]
 
+--trailer <token>[(=|:)<value>]::
+	Specify a (<token>, <value>) pair that should be applied as a
+	trailer. (e.g. `git commit --trailer "Signed-off-by:C O Mitter \
+	<committer@example.com>" --trailer "Helped-by:C O Mitter \
+	<committer@example.com>"` will add the "Signed-off" trailer
+	and the "Helped-by" trailer in the commit message.)
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.

diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..24fccb561398 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -113,6 +113,9 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+struct child_process run_trailer = CHILD_PROCESS_INIT;
+struct strvec trailer_args = STRVEC_INIT;
+static const char *trailer;
 
 /*
  * The default commit message cleanup mode will remove the lines