# [PATCH] git-stash: Don't GPG sign when stashing changes 12 messages from 2015-11-06 to 2016-05-06. Participants: Cameron Currie, Junio C Hamano, daurnimator, Johannes Schindelin, Daurnimator. Thread: https://gitlist.dev/t/40741 ## Cameron Currie, 2015-11-06 17:32 Subject: [PATCH] git-stash: Don't GPG sign when stashing changes Message-ID: <00000150dddb0eeb-b77240fb-1b63-4676-ac4b-1220b8d011ca-000000@eu-west-1.amazonses.com> URL: https://gitlist.dev/e/00000150dddb0eeb-b77240fb-1b63-4676-ac4b-1220b8d011ca-000000%40eu-west-1.amazonses.com ``` 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 ``` ## Junio C Hamano, 2015-11-06 18:56 Subject: Re: [PATCH] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/xmqqio5f2bz6.fsf%40gitster.mtv.corp.google.com In-Reply-To: <00000150dddb0eeb-b77240fb-1b63-4676-ac4b-1220b8d011ca-000000@eu-west-1.amazonses.com> ``` Cameron Currie 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 ``` ## daurnimator, 2016-04-07 02:24 Subject: Re: [PATCH] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/loom.20160407T042319-468%40post.gmane.org In-Reply-To: <00000150dddb0eeb-b77240fb-1b63-4676-ac4b-1220b8d011ca-000000@eu-west-1.amazonses.com> ``` Cameron Currie 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, 2016-04-07 08:19 Subject: Re: [PATCH] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.20.1604071017510.2967%40virtualbox In-Reply-To: ``` 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 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, 2016-04-12 02:46 Subject: Re: [PATCH] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/CAEnbY%2BeaReDYOH8azpSG7n7MOrvqORkC0ar80pd1m8wR4vv20A%40mail.gmail.com In-Reply-To: ``` On 7 April 2016 at 18:19, Johannes Schindelin 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 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? ``` ## Johannes Schindelin, 2016-04-14 15:50 Subject: Re: [PATCH] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.20.1604141748370.2967%40virtualbox In-Reply-To: ``` Hi Cameron, On Wed, 13 Apr 2016, Cameron Currie wrote: > > > On Thu, 7 Apr 2016, daurnimator wrote: > > > > > >> Cameron Currie 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, 2016-05-02 06:06 Subject: Re: [PATCH] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/CAEnbY%2BfG60mZSKs_d8Lpyb2Czf%3DUfA9cKOBTtEjEXeUfGE9zLg%40mail.gmail.com In-Reply-To: ``` On 15 April 2016 at 01:50, Johannes Schindelin wrote: > Hi Cameron, > > On Wed, 13 Apr 2016, Cameron Currie wrote: > >> > > On Thu, 7 Apr 2016, daurnimator wrote: >> > > >> > >> Cameron Currie 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` ``` ## Cameron Currie, 2016-05-02 20:50 Subject: [PATCH v2] git-stash: Don't GPG sign when stashing changes Message-ID: <01020154733c27e9-deaa7a20-1de6-416a-a7d4-3229854117eb-000000@eu-west-1.amazonses.com> URL: https://gitlist.dev/e/01020154733c27e9-deaa7a20-1de6-416a-a7d4-3229854117eb-000000%40eu-west-1.amazonses.com In-Reply-To: <00000150dddb0eeb-b77240fb-1b63-4676-ac4b-1220b8d011ca-000000@eu-west-1.amazonses.com> ``` This is helpful for folks with commit.gpgsign = true in their .gitconfig. Signed-off-by: Cameron Currie --- 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 ``` ## Junio C Hamano, 2016-05-02 21:57 Subject: Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/xmqqoa8oqg19.fsf%40gitster.mtv.corp.google.com In-Reply-To: <01020154733c27e9-deaa7a20-1de6-416a-a7d4-3229854117eb-000000@eu-west-1.amazonses.com> ``` Cameron Currie writes: > This is helpful for folks with commit.gpgsign = true in their .gitconfig. > > Signed-off-by: Cameron Currie > --- 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 ``` ## Daurnimator, 2016-05-02 23:56 Subject: Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/CAEnbY%2Bcu9E8DYGcg8FoK37NGZXbhSKvzVwMpLUEspXRYpPUdaA%40mail.gmail.com In-Reply-To: ``` On 3 May 2016 at 07:57, Junio C Hamano 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` ``` ## Junio C Hamano, 2016-05-03 01:21 Subject: Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/CAPc5daXVBJ%3D2KJfqcdwyH1Eq54vw-QeOEBF1aGj2dn%2BYihcy9A%40mail.gmail.com In-Reply-To: ``` On Mon, May 2, 2016 at 4:56 PM, Daurnimator wrote: > On 3 May 2016 at 07:57, Junio C Hamano 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, 2016-05-06 06:06 Subject: Re: [PATCH v2] git-stash: Don't GPG sign when stashing changes Message-ID: URL: https://gitlist.dev/e/CAEnbY%2BcAzLKw9GJTWy5ZP4rGsve161qSX5DgqWimcC5kYRqkyA%40mail.gmail.com In-Reply-To: ``` On 3 May 2016 at 11:21, Junio C Hamano wrote: > On Mon, May 2, 2016 at 4:56 PM, Daurnimator wrote: >> On 3 May 2016 at 07:57, Junio C Hamano 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? ```