[RFC] git: make signoff header configurable

[RFC] git: make signoff header configurable

[Arend van Spriel]

When using -s for commit, revert, and format-patch (there
may be more) command the comments message gets a generated
and hard-coded signed-off-by message. For some projects a
different header is used, eg. HostAP. Adding a config parameter
named 'signoff.label'.

Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
I had an itch to scratch. Like the -s command line parameter to
get the signed-off message added, but not all projects use the
same signature format. So let me know what you think about this
idea. Never contributed to git before so decided to make it an
RFC first as this solution may be a bit hack-ish.

Regards,
Arend
---
 builtin/commit.c |    5 +++--
 config.c         |   12 ++++++++++++
 sequencer.c      |   15 +++++++++++++--
 sequencer.h      |    3 ++-
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d2f30d9..6e28325 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -848,6 +848,7 @@ static int rest_is_empty(struct strbuf *sb, int start)
 {
 	int i, eol;
 	const char *nl;
+	char *so_hdr = get_signoff_header();
 
 	/* Check if the rest is just whitespace and Signed-of-by's. */
 	for (i = start; i < sb->len; i++) {
@@ -857,8 +858,8 @@ static int rest_is_empty(struct strbuf *sb, int start)
 		else
 			eol = sb->len;
 
-		if (strlen(sign_off_header) <= eol - i &&
-		    !prefixcmp(sb->buf + i, sign_off_header)) {
+		if (strlen(so_hdr) <= eol - i &&
+		    !prefixcmp(sb->buf + i, so_hdr)) {
 			i = eol;
 			continue;
 		}
diff --git a/config.c b/config.c
index aefd80b..110fa04 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,7 @@
 #include "exec_cmd.h"
 #include "strbuf.h"
 #include "quote.h"
+#include "sequencer.h"
 
 typedef struct config_file {
 	struct config_file *prev;
@@ -892,6 +893,17 @@ int git_default_config(const char *var, const char *value, void *dummy)
 		pack_size_limit_cfg = git_config_ulong(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "signoff.label")) {
+		const char *label;
+
+		if (!git_config_string(&label, var, value)) {
+			size_t len = strlen(label) + 3;
+			sign_off_header = xcalloc(1, len);
+			snprintf(sign_off_header, len, "%s: ", label);
+			free((void *)label);
+		}
+		return 0;
+	}
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
 }
diff --git a/sequencer.c b/sequencer.c
index cf8fbeb..7e9a0dd 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -17,9 +17,20 @@
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
-const char sign_off_header[] = "Signed-off-by: ";
+static const char def_signoff_header[] = "Signed-off-by";
+char *sign_off_header = NULL;
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";
 
+char *get_signoff_header(void)
+{
+	if (!sign_off_header) {
+		size_t len = strlen(def_signoff_header) + 3;
+		sign_off_header = xcalloc(1, len);
+		snprintf(sign_off_header, len, "%s: ", def_signoff_header);
+	}
+	return sign_off_header;
+}
+
 static int is_rfc2822_line(const char *buf, int len)
 {
 	int i;
@@ -1130,7 +1141,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
 	struct strbuf sob = STRBUF_INIT;
 	int has_footer;
 
-	strbuf_addstr(&sob, sign_off_header);
+	strbuf_addstr(&sob, get_signoff_header());
 	strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
 				getenv("GIT_COMMITTER_EMAIL")));
 	strbuf_addch(&sob, '\n');
diff --git a/sequencer.h b/sequencer.h
index 1fc22dc..5a91105 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -48,8 +48,9 @@ struct replay_opts {
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-extern const char sign_off_header[];
+extern char* sign_off_header;
 
+char *get_signoff_header(void);
 void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
 
 #endif
-- 
1.7.10.4

Re: [RFC] git: make signoff header configurable

[Junio C Hamano]

"Arend van Spriel" <arend@broadcom.com> writes:

> I had an itch to scratch. Like the -s command line parameter to
> get the signed-off message added, but not all projects use the
> same signature format. So let me know what you think about this
> idea. Never contributed to git before so decided to make it an
> RFC first as this solution may be a bit hack-ish.

It is customary to declare things like "char *get_signoff_header()"
that are meant to be available pretty much everywhere in cache.h
(look for /* Environment bits from configuration mechanism */) and
not in an unrelated header like sequencer.h (which builtin/commit.c
or config.c have no business including).  Also, default-config may
be a bit too generic place to read this information; it is used by
many read-only commands like "log", "diff", etc. that have no reason
to know this custom trailer setting.

Other than these, I do not see anything glaringly "hack-ish" in the
implementation itself.  Of course, a non-RFC patch would come with
documentation and test script updates.

By the way, what you are adding is a trailer, not a header, as it
comes at the very end ;-).

To projects that adopt the S-o-b convention from the kernel, the act
of signing off has a very specific legal meaning (I know it is not
an electronic signature, but the intention counts in court); I am
not sure if it is even a good idea to make "-s" mean something
different depending on the configuration in the first place, though.

Wouldn't a commit template a better alternative for appending a
random stuff in the log message, I wonder.

>  builtin/commit.c |    5 +++--
>  config.c         |   12 ++++++++++++
>  sequencer.c      |   15 +++++++++++++--
>  sequencer.h      |    3 ++-
>  4 files changed, 30 insertions(+), 5 deletions(-)
> diff --git a/sequencer.h b/sequencer.h
> index 1fc22dc..5a91105 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -48,8 +48,9 @@ struct replay_opts {
>  
>  int sequencer_pick_revisions(struct replay_opts *opts);
>  
> -extern const char sign_off_header[];
> +extern char* sign_off_header;

The asterisk sticks to the identifier, not type, i.e.

	extern char *signoff_label;

But I suspect you do not need to make this extern (for the same
reason we can keep def_signoff_header[] a static), as long as the
public facing API "get-signoff-header" is extern.

> +char *get_signoff_header(void);
>  void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
>  
>  #endif