[PATCH] git-stash: Don't GPG sign when stashing changes

[PATCH] git-stash: Don't GPG sign when stashing changes

[Cameron Currie]

This is helpful for folks with commit.gpgsign = true in their .gitconfig.
---
 git-stash.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index c7c65e2..fcf01b9 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -85,7 +85,7 @@ create_stash () {
 	# state of the index
 	i_tree=$(git write-tree) &&
 	i_commit=$(printf 'index on %s\n' "$msg" |
-		git commit-tree $i_tree -p $b_commit) ||
+		git commit-tree --no-gpg-sign $i_tree -p $b_commit) ||
 		die "$(gettext "Cannot save the current index state")"
 
 	if test -n "$untracked"
@@ -99,7 +99,7 @@ create_stash () {
 				rm -f "$TMPindex" &&
 				git update-index -z --add --remove --stdin &&
 				u_tree=$(git write-tree) &&
-				printf 'untracked files on %s\n' "$msg" | git commit-tree $u_tree  &&
+				printf 'untracked files on %s\n' "$msg" | git commit-tree --no-gpg-sign $u_tree  &&
 				rm -f "$TMPindex"
 		) ) || die "Cannot save the untracked files"
 
@@ -153,7 +153,7 @@ create_stash () {
 		stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
 	fi
 	w_commit=$(printf '%s\n' "$stash_msg" |
-	git commit-tree $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
+	git commit-tree --no-gpg-sign $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
 	die "$(gettext "Cannot record working tree state")"
 }
 

--
https://github.com/git/git/pull/186

Re: [PATCH] git-stash: Don't GPG sign when stashing changes

[Junio C Hamano]

Cameron Currie <me@cameroncurrie.net> writes:

> This is helpful for folks with commit.gpgsign = true in their .gitconfig.
> ---
>  git-stash.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

I have to wonder if the right fix is to change "git commit-tree" to
ignore that configuration variable?  After all, the plumbing
commands are about stability of the interface, not convenience, and
it feels wrong for them to be affected by end-user configurations.

Patching git-stash.sh does not help other scripts (either in-tree or
third-party) that use commit-tree; they will also be broken by
over-eager users who set commit.gpgsign configuration.

>
> diff --git a/git-stash.sh b/git-stash.sh
> index c7c65e2..fcf01b9 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -85,7 +85,7 @@ create_stash () {
>  	# state of the index
>  	i_tree=$(git write-tree) &&
>  	i_commit=$(printf 'index on %s\n' "$msg" |
> -		git commit-tree $i_tree -p $b_commit) ||
> +		git commit-tree --no-gpg-sign $i_tree -p $b_commit) ||
>  		die "$(gettext "Cannot save the current index state")"
>  
>  	if test -n "$untracked"
> @@ -99,7 +99,7 @@ create_stash () {
>  				rm -f "$TMPindex" &&
>  				git update-index -z --add --remove --stdin &&
>  				u_tree=$(git write-tree) &&
> -				printf 'untracked files on %s\n' "$msg" | git commit-tree $u_tree  &&
> +				printf 'untracked files on %s\n' "$msg" | git commit-tree --no-gpg-sign $u_tree  &&
>  				rm -f "$TMPindex"
>  		) ) || die "Cannot save the untracked files"
>  
> @@ -153,7 +153,7 @@ create_stash () {
>  		stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
>  	fi
>  	w_commit=$(printf '%s\n' "$stash_msg" |
> -	git commit-tree $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
> +	git commit-tree --no-gpg-sign $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
>  	die "$(gettext "Cannot record working tree state")"
>  }
>  
>
> --
> https://github.com/git/git/pull/186

Re: [PATCH] git-stash: Don't GPG sign when stashing changes

[daurnimator]

Cameron Currie <me <at> cameroncurrie.net> writes:
> This is helpful for folks with commit.gpgsign = true in their .gitconfig.

> https://github.com/git/git/pull/186

I too would like this.
Bumping due to lack of attention.

Re: [PATCH] git-stash: Don't GPG sign when stashing changes

[Johannes Schindelin]

Hi,

you dropped the Cc: list. So most likely Cameron won't get your mail nor
any response to your mail.

On Thu, 7 Apr 2016, daurnimator wrote:

> Cameron Currie <me <at> cameroncurrie.net> writes:
> > This is helpful for folks with commit.gpgsign = true in their .gitconfig.
> 
> > https://github.com/git/git/pull/186
> 
> I too would like this.
> Bumping due to lack of attention.

It lacks a Sign-off, our convention is to continue in lower-case after the
colon in the commit's subject, and I think that it would be good to write
so much as one paragraph in the commit message.

Ciao,
Johannes

Re: [PATCH] git-stash: Don't GPG sign when stashing changes

[Daurnimator]

On 7 April 2016 at 18:19, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> you dropped the Cc: list. So most likely Cameron won't get your mail nor
> any response to your mail.

I originally replied via the gmane web interface, apparently it
doesn't CC the original sender.
CCd now.

> On Thu, 7 Apr 2016, daurnimator wrote:
>
>> Cameron Currie <me <at> cameroncurrie.net> writes:
>> > This is helpful for folks with commit.gpgsign = true in their .gitconfig.
>>
>> > https://github.com/git/git/pull/186
>>
>> I too would like this.
>> Bumping due to lack of attention.
>
> It lacks a Sign-off, our convention is to continue in lower-case after the
> colon in the commit's subject, and I think that it would be good to write
> so much as one paragraph in the commit message.
>
> Ciao,
> Johannes

Cameron, able you able to complete this?

Re: [PATCH] git-stash: Don't GPG sign when stashing changes

[Johannes Schindelin]

Hi Cameron,

On Wed, 13 Apr 2016, Cameron Currie wrote:

> > > On Thu, 7 Apr 2016, daurnimator wrote:
> > >
> > >> Cameron Currie <me <at> cameroncurrie.net> writes:
> > >> > This is helpful for folks with commit.gpgsign = true in their
> > >> > .gitconfig.
> > >>
> > >> > https://github.com/git/git/pull/186
> > >>
> > >> I too would like this.
> > >> Bumping due to lack of attention.
> > >
> > > It lacks a Sign-off, our convention is to continue in lower-case
> > > after the colon in the commit's subject, and I think that it would
> > > be good to write so much as one paragraph in the commit message.
>
> I don't think I can find the time right now. Feel free to rewrite the
> commit message to match convention.

I am afraid that it would be improper for anybody to add your Sign-off, as
it is *your* statement that you are indeed contributing this as Open
Source, and that you indeed are free to do so.

If you do not find the time to take care of these small changes, I fear
the Git maintainer will *have* to drop your patch to avoid hassle for
himself.

Ciao,
Johannes

Re: [PATCH] git-stash: Don't GPG sign when stashing changes

[Daurnimator]

On 15 April 2016 at 01:50, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Cameron,
>
> On Wed, 13 Apr 2016, Cameron Currie wrote:
>
>> > > On Thu, 7 Apr 2016, daurnimator wrote:
>> > >
>> > >> Cameron Currie <me <at> cameroncurrie.net> writes:
>> > >> > This is helpful for folks with commit.gpgsign = true in their
>> > >> > .gitconfig.
>> > >>
>> > >> > https://github.com/git/git/pull/186
>> > >>
>> > >> I too would like this.
>> > >> Bumping due to lack of attention.
>> > >
>> > > It lacks a Sign-off, our convention is to continue in lower-case
>> > > after the colon in the commit's subject, and I think that it would
>> > > be good to write so much as one paragraph in the commit message.
>>
>> I don't think I can find the time right now. Feel free to rewrite the
>> commit message to match convention.
>
> I am afraid that it would be improper for anybody to add your Sign-off, as
> it is *your* statement that you are indeed contributing this as Open
> Source, and that you indeed are free to do so.
>
> If you do not find the time to take care of these small changes, I fear
> the Git maintainer will *have* to drop your patch to avoid hassle for
> himself.
>
> Ciao,
> Johannes

Cameron, could you please add a sign off to your commit?
It should be as simple as running `git commit --amend -s`

[PATCH v2] git-stash: Don't GPG sign when stashing changes

[Cameron Currie]

This is helpful for folks with commit.gpgsign = true in their .gitconfig.

Signed-off-by: Cameron Currie <me@cameroncurrie.net>
---
 git-stash.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index c7c65e2..fcf01b9 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -85,7 +85,7 @@ create_stash () {
 	# state of the index
 	i_tree=$(git write-tree) &&
 	i_commit=$(printf 'index on %s\n' "$msg" |
-		git commit-tree $i_tree -p $b_commit) ||
+		git commit-tree --no-gpg-sign $i_tree -p $b_commit) ||
 		die "$(gettext "Cannot save the current index state")"
 
 	if test -n "$untracked"
@@ -99,7 +99,7 @@ create_stash () {
 				rm -f "$TMPindex" &&
 				git update-index -z --add --remove --stdin &&
 				u_tree=$(git write-tree) &&
-				printf 'untracked files on %s\n' "$msg" | git commit-tree $u_tree  &&
+				printf 'untracked files on %s\n' "$msg" | git commit-tree --no-gpg-sign $u_tree  &&
 				rm -f "$TMPindex"
 		) ) || die "Cannot save the untracked files"
 
@@ -153,7 +153,7 @@ create_stash () {
 		stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
 	fi
 	w_commit=$(printf '%s\n' "$stash_msg" |
-	git commit-tree $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
+	git commit-tree --no-gpg-sign $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
 	die "$(gettext "Cannot record working tree state")"
 }
 

--
https://github.com/git/git/pull/186

Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes

[Junio C Hamano]

Cameron Currie <me@cameroncurrie.net> writes:

> This is helpful for folks with commit.gpgsign = true in their .gitconfig.
>
> Signed-off-by: Cameron Currie <me@cameroncurrie.net>
> ---

I do not think this is particularly a good change.

There are a few other in-tree users of "git commit-tree",
e.g. quiltimport and filter-branch, and their users would be hurt
the same way if they set commit.gpgsign in the configuration.

I think it was a mistake that "commit-tree" was made to pay
attention to the configuration variable in the first place.
Allowing scripts that use commit-tree to explicitly pass -S to it is
perfectly fine (and these calling scripts are welcome to honor
commit.gpgsign by reading the configration themselves if their users
want it that way).

I'll send a pair of proposed alternative solutions shortly.

>  git-stash.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index c7c65e2..fcf01b9 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -85,7 +85,7 @@ create_stash () {
>  	# state of the index
>  	i_tree=$(git write-tree) &&
>  	i_commit=$(printf 'index on %s\n' "$msg" |
> -		git commit-tree $i_tree -p $b_commit) ||
> +		git commit-tree --no-gpg-sign $i_tree -p $b_commit) ||
>  		die "$(gettext "Cannot save the current index state")"
>  
>  	if test -n "$untracked"
> @@ -99,7 +99,7 @@ create_stash () {
>  				rm -f "$TMPindex" &&
>  				git update-index -z --add --remove --stdin &&
>  				u_tree=$(git write-tree) &&
> -				printf 'untracked files on %s\n' "$msg" | git commit-tree $u_tree  &&
> +				printf 'untracked files on %s\n' "$msg" | git commit-tree --no-gpg-sign $u_tree  &&
>  				rm -f "$TMPindex"
>  		) ) || die "Cannot save the untracked files"
>  
> @@ -153,7 +153,7 @@ create_stash () {
>  		stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
>  	fi
>  	w_commit=$(printf '%s\n' "$stash_msg" |
> -	git commit-tree $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
> +	git commit-tree --no-gpg-sign $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
>  	die "$(gettext "Cannot record working tree state")"
>  }
>  
>
> --
> https://github.com/git/git/pull/186

Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes

[Daurnimator]

On 3 May 2016 at 07:57, Junio C Hamano <gitster@pobox.com> wrote:
> I do not think this is particularly a good change.
>
> There are a few other in-tree users of "git commit-tree",
> e.g. quiltimport and filter-branch, and their users would be hurt
> the same way if they set commit.gpgsign in the configuration.

I agree quiltimport should not, but I think filter-branch possibly
should... at least for your *own* commits.
I often think of filter-branch as an "advanced" `git commit --amend`

Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes

[Junio C Hamano]

On Mon, May 2, 2016 at 4:56 PM, Daurnimator <quae@daurnimator.com> wrote:
> On 3 May 2016 at 07:57, Junio C Hamano <gitster@pobox.com> wrote:
>
> I agree quiltimport should not, but I think filter-branch possibly
> should... at least for your *own* commits.
> I often think of filter-branch as an "advanced" `git commit --amend`

But it does not do the gpgsign thing only for your own commits right now, and
even if it did, you would not necessarily want it to always use the configured
setting. That means that the command, if it wants to work well with the signed
commits, must learn to honor command line option to enable or disable passing
--gpgsign option to underlying commit-tree *anyway*.

So I think it is a right thing to fix commit-tree to ignore
commit.gpgsign at least
by default.

Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes

[Daurnimator]

On 3 May 2016 at 11:21, Junio C Hamano <gitster@pobox.com> wrote:
> On Mon, May 2, 2016 at 4:56 PM, Daurnimator <quae@daurnimator.com> wrote:
>> On 3 May 2016 at 07:57, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I agree quiltimport should not, but I think filter-branch possibly
>> should... at least for your *own* commits.
>> I often think of filter-branch as an "advanced" `git commit --amend`
>
> But it does not do the gpgsign thing only for your own commits right now, and
> even if it did, you would not necessarily want it to always use the configured
> setting. That means that the command, if it wants to work well with the signed
> commits, must learn to honor command line option to enable or disable passing
> --gpgsign option to underlying commit-tree *anyway*.
>
> So I think it is a right thing to fix commit-tree to ignore
> commit.gpgsign at least
> by default.

Okay, that makes sense.
Will you work on a patch?