[PATCH] Allow cherry-pick (and revert) to add signoff line

[PATCH] Allow cherry-pick (and revert) to add signoff line

[Dan McGee]

I often find myself pulling patches off of other peoples trees using
cherry-pick, and following it with an immediate 'git commit --amend -s'
command. Eliminate the need for a double commit by allowing signoff on a
cherry-pick or revert.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---

This is something I have done in my workflow for a long time, and it seems
like a weird omission to me. Signoffs can be done on git-am without having
a second commit, and I often have a workflow where I am picking patches from
other users' topic branches and have reviewed the patch and would like to
signoff when I pull it into my tree.

I'm not particularly happy about the 4 case if statement at the end, so I'd
be happy to clean that up if anyone has suggestions.

 builtin-revert.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 607a2f0..433d0dd 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -33,7 +33,7 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline;
+static int edit, no_replay, no_commit, mainline, signoff;
 static enum { REVERT, CHERRY_PICK } action;
 static struct commit *commit;
 
@@ -53,6 +53,7 @@ static void parse_args(int argc, const char **argv)
 		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
 		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
+		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
 		OPT_END(),
 	};
@@ -404,10 +405,14 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	 */
 
 	if (!no_commit) {
-		if (edit)
+		if (edit && !signoff)
 			return execl_git_cmd("commit", "-n", NULL);
-		else
+		else if (edit)
+			return execl_git_cmd("commit", "-n", "-s", NULL);
+		else if (!signoff)
 			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
+		else
+			return execl_git_cmd("commit", "-n", "-s", "-F", defmsg, NULL);
 	}
 	free(reencoded_message);
 
-- 
1.5.5

Re: [PATCH] Allow cherry-pick (and revert) to add signoff line

[Björn Steinbrink]

On 2008.04.20 13:03:05 -0500, Dan McGee wrote:
> I often find myself pulling patches off of other peoples trees using
> cherry-pick, and following it with an immediate 'git commit --amend -s'
> command. Eliminate the need for a double commit by allowing signoff on a
> cherry-pick or revert.
> 
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
> ---
> 
> This is something I have done in my workflow for a long time, and it seems
> like a weird omission to me. Signoffs can be done on git-am without having
> a second commit, and I often have a workflow where I am picking patches from
> other users' topic branches and have reviewed the patch and would like to
> signoff when I pull it into my tree.
> 
> I'm not particularly happy about the 4 case if statement at the end, so I'd
> be happy to clean that up if anyone has suggestions.
> 
>  builtin-revert.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin-revert.c b/builtin-revert.c
> index 607a2f0..433d0dd 100644
> --- a/builtin-revert.c
> +++ b/builtin-revert.c
> @@ -33,7 +33,7 @@ static const char * const cherry_pick_usage[] = {
>  	NULL
>  };
>  
> -static int edit, no_replay, no_commit, mainline;
> +static int edit, no_replay, no_commit, mainline, signoff;
>  static enum { REVERT, CHERRY_PICK } action;
>  static struct commit *commit;
>  
> @@ -53,6 +53,7 @@ static void parse_args(int argc, const char **argv)
>  		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
>  		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
>  		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
> +		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),
>  		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
>  		OPT_END(),
>  	};
> @@ -404,10 +405,14 @@ static int revert_or_cherry_pick(int argc, const char **argv)
>  	 */
>  
>  	if (!no_commit) {
> -		if (edit)
> +		if (edit && !signoff)
>  			return execl_git_cmd("commit", "-n", NULL);
> -		else
> +		else if (edit)
> +			return execl_git_cmd("commit", "-n", "-s", NULL);
> +		else if (!signoff)
>  			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
> +		else
> +			return execl_git_cmd("commit", "-n", "-s", "-F", defmsg, NULL);

maybe like this?

	if (!no_commit) {
		char **argv[6] = { "commit", "-n" };
		int argc = 2;
		if (signoff)
			argv[argc++] = "-s";
		if (!edit) {
			argv[argc++] = "-F";
			argv[argc++] = defmsg;
		}
		argv[argc] = NULL;
		execv_git_command(argv);
	}

It duplicates what execl_git_cmd does, but other places already work
similar (eg. cmd_describe, when --contains is given), and IMHO it's
nicer than the if-else chain.

Björn

Re: [PATCH] Allow cherry-pick (and revert) to add signoff line

[Junio C Hamano]

Dan McGee <dpmcgee@gmail.com> writes:

> I often find myself pulling patches off of other peoples trees using
> cherry-pick, and following it with an immediate 'git commit --amend -s'
> command. Eliminate the need for a double commit by allowing signoff on a
> cherry-pick or revert.
>
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
> ---
>
> This is something I have done in my workflow for a long time, and it seems
> like a weird omission to me. Signoffs can be done on git-am without having
> a second commit, and I often have a workflow where I am picking patches from
> other users' topic branches and have reviewed the patch and would like to
> signoff when I pull it into my tree.

Yeah, when your workflow is heavily rely on cherry-picking, I can see how
this would be handy.

I see there was some improvement suggestion suggested on the list, so I
will wait until the dust settles and I see an appliable final submission.

Thanks.

[PATCH] Allow cherry-pick (and revert) to add signoff line

[Dan McGee]

I often find myself pulling patches off of other peoples trees using
cherry-pick, and following it with an immediate 'git commit --amend -s'
command. Eliminate the need for a double commit by allowing signoff on a
cherry-pick or revert.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---

Resubmit using execv_git_cmd() directly. This reduces some code duplication
and allows for any other commit options to be propagated forth in the future.

 builtin-revert.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 607a2f0..3f08cfe 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -33,7 +33,7 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline;
+static int edit, no_replay, no_commit, mainline, signoff;
 static enum { REVERT, CHERRY_PICK } action;
 static struct commit *commit;
 
@@ -53,6 +53,7 @@ static void parse_args(int argc, const char **argv)
 		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
 		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
+		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
 		OPT_END(),
 	};
@@ -404,10 +405,19 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	 */
 
 	if (!no_commit) {
-		if (edit)
-			return execl_git_cmd("commit", "-n", NULL);
-		else
-			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
+		/* 6 is max possible length of our args array including NULL */
+		const char *args[6];
+		int i = 0;
+		args[i++] = "commit";
+		args[i++] = "-n";
+		if (signoff)
+			args[i++] = "-s";
+		if (!edit) {
+			args[i++] = "-F";
+			args[i++] = defmsg;
+		}
+		args[i] = NULL;
+		return execv_git_cmd(args);
 	}
 	free(reencoded_message);
 
-- 
1.5.5.1

[PATCH resubmit] Allow cherry-pick (and revert) to add signoff line

[Dan McGee]

I often find myself pulling patches off of other peoples trees using
cherry-pick, and following it with an immediate 'git commit --amend -s'
command. Eliminate the need for a double commit by allowing signoff on a
cherry-pick or revert.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---

Documentation is a good thing too, so I've adjusted the patch to include it.

 Documentation/git-cherry-pick.txt |    5 ++++-
 Documentation/git-revert.txt      |    5 ++++-
 builtin-revert.c                  |   20 +++++++++++++++-----
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index f0beb41..ca048f4 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -7,7 +7,7 @@ git-cherry-pick - Apply the change introduced by an existing commit
 
 SYNOPSIS
 --------
-'git-cherry-pick' [--edit] [-n] [-m parent-number] [-x] <commit>
+'git-cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] <commit>
 
 DESCRIPTION
 -----------
@@ -64,6 +64,9 @@ OPTIONS
 This is useful when cherry-picking more than one commits'
 effect to your working tree in a row.
 
+-s|--signoff::
+	Add Signed-off-by line at the end of the commit message.
+
 
 Author
 ------
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 93e20f7..13ceabb 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -7,7 +7,7 @@ git-revert - Revert an existing commit
 
 SYNOPSIS
 --------
-'git-revert' [--edit | --no-edit] [-n] [-m parent-number] <commit>
+'git-revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>
 
 DESCRIPTION
 -----------
@@ -51,6 +51,9 @@ OPTIONS
 This is useful when reverting more than one commits'
 effect to your working tree in a row.
 
+-s|--signoff::
+	Add Signed-off-by line at the end of the commit message.
+
 
 Author
 ------
diff --git a/builtin-revert.c b/builtin-revert.c
index 607a2f0..3f08cfe 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -33,7 +33,7 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline;
+static int edit, no_replay, no_commit, mainline, signoff;
 static enum { REVERT, CHERRY_PICK } action;
 static struct commit *commit;
 
@@ -53,6 +53,7 @@ static void parse_args(int argc, const char **argv)
 		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
 		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
+		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
 		OPT_END(),
 	};
@@ -404,10 +405,19 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	 */
 
 	if (!no_commit) {
-		if (edit)
-			return execl_git_cmd("commit", "-n", NULL);
-		else
-			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
+		/* 6 is max possible length of our args array including NULL */
+		const char *args[6];
+		int i = 0;
+		args[i++] = "commit";
+		args[i++] = "-n";
+		if (signoff)
+			args[i++] = "-s";
+		if (!edit) {
+			args[i++] = "-F";
+			args[i++] = defmsg;
+		}
+		args[i] = NULL;
+		return execv_git_cmd(args);
 	}
 	free(reencoded_message);
 
-- 
1.5.5.1

Re: [PATCH resubmit] Allow cherry-pick (and revert) to add signoff line

[Junio C Hamano]

Dan McGee <dpmcgee@gmail.com> writes:

> +-s|--signoff::
> +	Add Signed-off-by line at the end of the commit message.
> +

Good.

> +		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),

Not so good ;-)

Will apply with an obvious "s/by: header/by:/" and merge to 'next'.

Thanks.

[PATCH] Remove 'header' from --signoff option description

[Dan McGee]

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 builtin-commit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index b41d4a3..256181a 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -90,7 +90,7 @@ static struct option builtin_commit_options[] = {
 	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
-	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),
+	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 	OPT_STRING('t', "template", &template_file, "FILE", "use specified template file"),
 	OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
 
-- 
1.5.5.1