threads / patch / 40741

patchgit-stash: Don't GPG sign when stashing changes

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

## tl;dr

12 messages between Nov 6, 2015 and May 6, 2016. Diffs are folded; open one to read it.

replies: 11people: 4as markdown or json

Cameron Currie· Nov 6, 2015, 17:32 UTC · lore
This is helpful for folks with commit.gpgsign = true in their .gitconfig.
---
 git-stash.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to git-stash.sh +3 −4
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
Junio C Hamano· Nov 6, 2015, 18:56 UTC · re: Cameron Currie · lore

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

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.

Show 35 quoted lines
>
> 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
daurnimator· Apr 7, 2016, 02:24 UTC · re: Cameron Currie · lore

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

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.

Johannes Schindelin· Apr 7, 2016, 08:19 UTC · re: daurnimator · lore

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

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:
Show 7 quoted lines
> 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

Daurnimator· Apr 12, 2016, 02:46 UTC · re: Johannes Schindelin · lore

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

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.

Show 16 quoted lines
> 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?
Cameron Currie· May 2, 2016, 20:50 UTC · re: Cameron Currie · lore

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

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(-)
Show changes to git-stash.sh +3 −4
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
Junio C Hamano· May 2, 2016, 21:57 UTC · re: Cameron Currie · lore

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

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.
Show 37 quoted lines
>  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
Daurnimator· May 2, 2016, 23:56 UTC · re: Junio C Hamano · lore

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

On 3 May 2016 at 07:57, Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
> 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`

Junio C Hamano· May 3, 2016, 01:21 UTC · re: Daurnimator · lore

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

On Mon, May 2, 2016 at 4:56 PM, Daurnimator <quae@daurnimator.com> wrote:
Show 5 quoted lines
> 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.

Daurnimator· May 6, 2016, 06:06 UTC · re: Junio C Hamano · lore

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

On 3 May 2016 at 11:21, Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> 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?

Johannes Schindelin· Apr 14, 2016, 15:50 UTC · lore

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

Hi Cameron,
On Wed, 13 Apr 2016, Cameron Currie wrote:
Show 17 quoted lines
> > > 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

Daurnimator· May 2, 2016, 06:06 UTC · re: Johannes Schindelin · lore

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

On 15 April 2016 at 01:50, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 32 quoted lines
> 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`

← back to recent threads