{"thread":{"id":"33576","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","startedAt":"2013-04-22T23:43:19Z","lastAt":"2013-04-25T16:50:24Z","messageCount":15,"participants":["Junio C Hamano","Joel Jacobson","Michael J Gruber","Sebastian Götte"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"215193","messageId":"7vip3em8rs.fsf@alter.siamese.dyndns.org","threadId":"33576","inReplyTo":"CAASwCXf3YHmdQ_eSkShyzn5VniO=ufm3VTqV1JVOUN610bzE_A@mail.gmail.com","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-22T23:43:19Z","receivedAt":"2013-04-22T23:43:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joel Jacobson writes:\n\n> Signed-off-by: Joel Jacobson \n> ---\n> builtin/commit.c | 4 +++-\n\nNo docs? No tests?\n\nAs to the design, any regular configuration variable settings must\nbe overridable from the command line for a single invocation. Please\ndesign an escape hatch in, for somebody who has this configuration\nvariable set, but does not want to sign this commit he is about to\nmake.\n\nAlso do we generally use dash in the configuration variable names?\nI thought the norm was section.CamelCase.\n\n> + if (!strcmp(k, \"commit.gpg-sign\"))\n> + return git_config_string(&sign_commit, k, v);\n> if (!strcmp(k, \"commit.cleanup\"))\n> return git_config_string(&cleanup_arg, k, v);\n>\n> --\n> --to=gitster@pobox.com\n"},{"id":"215195","messageId":"CAASwCXcfCNqiMXD5JasTRKWZgCNsxnUY7k9E=f86xsTZjk37CA@mail.gmail.com","threadId":"33576","inReplyTo":"7vip3em8rs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Joel Jacobson","fromEmail":"joel@trustly.com","sentAt":"2013-04-23T00:00:50Z","receivedAt":"2013-04-23T00:00:50Z","isPatch":true,"sender":{"key":"joel@trustly.com","avatar":null},"body":"On Tue, Apr 23, 2013 at 12:43 AM, Junio C Hamano wrote:\n> No docs? No tests?\n\nMaybe simply adding this text to git-commit.txt,\n\nThe default can be changed by the 'commit.gpg-sign' configuration\nvariable (see linkgit:git-config[1]).\n\nafter,\n\n-S[]::\n--gpg-sign[=]::\nGPG-sign commit.\n\nwould be sufficient?\n\nNot sure what the proper way to test this,\ncould you please suggest any other unit test I could look at for inspiration?\n\n> As to the design, any regular configuration variable settings must\n> be overridable from the command line for a single invocation. Please\n> design an escape hatch in, for somebody who has this configuration\n> variable set, but does not want to sign this commit he is about to\n> make.\n\nSomething like --no-gpg-sign?\n\n> Also do we generally use dash in the configuration variable names?\n> I thought the norm was section.CamelCase.\n\nSince the command line long option is \"gpg-sign\", I thought it was best\nto use exactly the same term in the configuration variable name to\navoid confusion. Is there any problem with dashes in variable names?\n"},{"id":"215206","messageId":"51767290.2010208@drmicha.warpmail.net","threadId":"33576","inReplyTo":"CAASwCXcfCNqiMXD5JasTRKWZgCNsxnUY7k9E=f86xsTZjk37CA@mail.gmail.com","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-04-23T11:37:52Z","receivedAt":"2013-04-23T11:37:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Joel Jacobson venit, vidit, dixit 23.04.2013 02:00:\n> On Tue, Apr 23, 2013 at 12:43 AM, Junio C Hamano wrote:\n>> No docs? No tests?\n> \n> Maybe simply adding this text to git-commit.txt,\n> \n> The default can be changed by the 'commit.gpg-sign' configuration\n> variable (see linkgit:git-config[1]).\n> \n> after,\n> \n> -S[]::\n> --gpg-sign[=]::\n> GPG-sign commit.\n> \n> would be sufficient?\n> \n> Not sure what the proper way to test this,\n> could you please suggest any other unit test I could look at for inspiration?\n> \n>> As to the design, any regular configuration variable settings must\n>> be overridable from the command line for a single invocation. Please\n>> design an escape hatch in, for somebody who has this configuration\n>> variable set, but does not want to sign this commit he is about to\n>> make.\n> \n> Something like --no-gpg-sign?\n> \n>> Also do we generally use dash in the configuration variable names?\n>> I thought the norm was section.CamelCase.\n> \n> Since the command line long option is \"gpg-sign\", I thought it was best\n> to use exactly the same term in the configuration variable name to\n> avoid confusion. Is there any problem with dashes in variable names?\n> \n\nNot really a problem, but as Junio writes, we don't use dashes in the\nconfig.\n\nAs for the command line override:\n\nDon't we have \"git -c commit.gpgsign=false\" and such these days, so that\nwe don't have to inflate options any more?\n\nMichael\n"},{"id":"215227","messageId":"7v4nexmjlc.fsf@alter.siamese.dyndns.org","threadId":"33576","inReplyTo":"7vip3em8rs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-23T14:01:51Z","receivedAt":"2013-04-23T14:01:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Joel Jacobson writes:\n>\n>> Signed-off-by: Joel Jacobson \n>> ---\n>> builtin/commit.c | 4 +++-\n>\n> No docs? No tests?\n>\n> As to the design, any regular configuration variable settings must\n> be overridable from the command line for a single invocation. Please\n> design an escape hatch in, for somebody who has this configuration\n> variable set, but does not want to sign this commit he is about to\n> make.\n>\n> Also do we generally use dash in the configuration variable names?\n> I thought the norm was section.CamelCase.\n\nAnother design issue you would need to consider before deciding to\nadd such a configuration is how it interacts with \"git rebase -i\".\n\nThe command internally runs \"git commit --amend\", etc. and I do not\nknow if _all_ users who set this configuration want to unconditionally\nalways re-sign all these commits.\n\nThere may be other commands that internally use \"git commit\" that\nmay be affected with such a configuration variable.\n"},{"id":"215264","messageId":"7va9opi163.fsf@alter.siamese.dyndns.org","threadId":"33576","inReplyTo":"51767290.2010208@drmicha.warpmail.net","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-23T17:53:24Z","receivedAt":"2013-04-23T17:53:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> As for the command line override:\n>\n> Don't we have \"git -c commit.gpgsign=false\" and such these days, so that\n> we don't have to inflate options any more?\n\nI would consider such use of \"git -c key=val\" a last-resort escape\nhatch to work around broken commands that do not implement a proper\nescape hatch designed in to help users, unless the \"key\" is for\nsomething very obscure and not meant for every-day use (read: not\ndeserving a proper command line override).\n"},{"id":"215265","messageId":"CAASwCXdhgNPzm51MfWTCTNYQwT0gC-UbV7xTNGcXhs6k9f4wsQ@mail.gmail.com","threadId":"33576","inReplyTo":"7va9opi163.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Joel Jacobson","fromEmail":"joel@trustly.com","sentAt":"2013-04-23T17:58:46Z","receivedAt":"2013-04-23T17:58:46Z","isPatch":true,"sender":{"key":"joel@trustly.com","avatar":null},"body":"On Tue, Apr 23, 2013 at 6:53 PM, Junio C Hamano wrote:\n> I would consider such use of \"git -c key=val\" a last-resort escape\n> hatch to work around broken commands that do not implement a proper\n> escape hatch designed in to help users, unless the \"key\" is for\n> something very obscure and not meant for every-day use (read: not\n> deserving a proper command line override).\n\nAgreed.\n\nWe already have --gpg-sign[=], so I suggest --no-gpg-sign to\noverride commit.gpgsign.\n\nSounds good?\n"},{"id":"215269","messageId":"7v1ua1hwx6.fsf@alter.siamese.dyndns.org","threadId":"33576","inReplyTo":"CAASwCXdhgNPzm51MfWTCTNYQwT0gC-UbV7xTNGcXhs6k9f4wsQ@mail.gmail.com","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-23T19:25:09Z","receivedAt":"2013-04-23T19:25:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joel Jacobson writes:\n\n> On Tue, Apr 23, 2013 at 6:53 PM, Junio C Hamano wrote:\n>> I would consider such use of \"git -c key=val\" a last-resort escape\n>> hatch to work around broken commands that do not implement a proper\n>> escape hatch designed in to help users, unless the \"key\" is for\n>> something very obscure and not meant for every-day use (read: not\n>> deserving a proper command line override).\n>\n> Agreed.\n>\n> We already have --gpg-sign[=], so I suggest --no-gpg-sign to\n> override commit.gpgsign.\n>\n> Sounds good?\n\nYup.\n\nAnd then we would need to add the same option to existing callers of\n\"git commit\" (such as \"git rebase\") to pass it down the callchain.\n\nBut stepping back a bit, I have a suspicion that your upstream\nproject _only_ cares about what you feed them (either by pushing\nyour work yourself to them, or telling them to pull from your\nrepository). There is no reason for you to be constantly signing\nyour commits you make during your exploratory development that you\nmay throw-away in the end.\n\nIt _might_ be a better option to just teach \"-S\" option to \"git\nrebase\" that tells it to replay all the commits with \"commit -S\",\ninstead of adding commit.gpgSign configuration.\n"},{"id":"215271","messageId":"CAASwCXcs8Q7qGWSNKttEuk0zPetDWNCse4J-KSm20r9h5XSo3Q@mail.gmail.com","threadId":"33576","inReplyTo":"7v1ua1hwx6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Joel Jacobson","fromEmail":"joel@trustly.com","sentAt":"2013-04-23T19:56:41Z","receivedAt":"2013-04-23T19:56:41Z","isPatch":true,"sender":{"key":"joel@trustly.com","avatar":null},"body":"On Tue, Apr 23, 2013 at 8:25 PM, Junio C Hamano wrote:\n> Yup.\n>\n> And then we would need to add the same option to existing callers of\n> \"git commit\" (such as \"git rebase\") to pass it down the callchain.\n\nGot it.\n\n> But stepping back a bit, I have a suspicion that your upstream\n> project _only_ cares about what you feed them (either by pushing\n> your work yourself to them, or telling them to pull from your\n> repository). There is no reason for you to be constantly signing\n> your commits you make during your exploratory development that you\n> may throw-away in the end.\n\nYour suspicions are correct.\nBut I'm a bit paranoid, so it feels better to sign even local commits.\n\n> It _might_ be a better option to just teach \"-S\" option to \"git\n> rebase\" that tells it to replay all the commits with \"commit -S\",\n> instead of adding commit.gpgSign configuration.\n\nIn my case, I don't do that much exploratory development locally,\nso I usually just commit, pull and push.\n\nAlways signing everything can't really hurt, can it? Takes a few clock\ncycles more, and a few more bytes, but apart from that I don't see any\nproblems?\n"},{"id":"215302","messageId":"51779DA1.7080606@physik.tu-berlin.de","threadId":"33576","inReplyTo":"CAASwCXcs8Q7qGWSNKttEuk0zPetDWNCse4J-KSm20r9h5XSo3Q@mail.gmail.com","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Sebastian Götte","fromEmail":"jaseg@physik.tu-berlin.de","sentAt":"2013-04-24T08:53:53Z","receivedAt":"2013-04-24T08:53:53Z","isPatch":true,"sender":{"key":"jaseg@physik.tu-berlin.de","avatar":null},"body":"On 04/23/2013 09:56 PM, Joel Jacobson wrote:\n>> But stepping back a bit, I have a suspicion that your upstream\n>> project _only_ cares about what you feed them (either by pushing\n>> your work yourself to them, or telling them to pull from your\n>> repository). There is no reason for you to be constantly signing\n>> your commits you make during your exploratory development that you\n>> may throw-away in the end.\n> \n> Your suspicions are correct.\n> But I'm a bit paranoid, so it feels better to sign even local commits.\n> \n>> It _might_ be a better option to just teach \"-S\" option to \"git\n>> rebase\" that tells it to replay all the commits with \"commit -S\",\n>> instead of adding commit.gpgSign configuration.\n> \n> In my case, I don't do that much exploratory development locally,\n> so I usually just commit, pull and push.\n> \n> Always signing everything can't really hurt, can it? Takes a few clock\n> cycles more, and a few more bytes, but apart from that I don't see any\n> problems?\nI have my GPG-keys password-protected, and I would be severely annoyed by\nGnuPG password prompts popping up on every commit. I think the -S option\nto rebase would be the more elegant way. What could be nice would be a\nconfig option that makes \"git push\" warn/abort in case I try to push an\nunsigned head commit to a repo where I want to have signed commits:\n> remote..abortUnsigned\nThis of course needs an command line override switch.\n\nSomething to be considered is whether \"git rebase -S\" should sign *every*\ncommit in the series or only the *head* commit.\n"},{"id":"215305","messageId":"5177AB10.30209@drmicha.warpmail.net","threadId":"33576","inReplyTo":"51779DA1.7080606@physik.tu-berlin.de","subject":"Re: [PATCH] Add .gitconfig variable commit.gpg-sign","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-04-24T09:51:12Z","receivedAt":"2013-04-24T09:51:12Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Sebastian Götte venit, vidit, dixit 24.04.2013 10:53:\n> On 04/23/2013 09:56 PM, Joel Jacobson wrote:\n>>> But stepping back a bit, I have a suspicion that your upstream\n>>> project _only_ cares about what you feed them (either by pushing\n>>> your work yourself to them, or telling them to pull from your\n>>> repository). There is no reason for you to be constantly signing\n>>> your commits you make during your exploratory development that you\n>>> may throw-away in the end.\n>>\n>> Your suspicions are correct.\n>> But I'm a bit paranoid, so it feels better to sign even local commits.\n>>\n>>> It _might_ be a better option to just teach \"-S\" option to \"git\n>>> rebase\" that tells it to replay all the commits with \"commit -S\",\n>>> instead of adding commit.gpgSign configuration.\n>>\n>> In my case, I don't do that much exploratory development locally,\n>> so I usually just commit, pull and push.\n>>\n>> Always signing everything can't really hurt, can it? Takes a few clock\n>> cycles more, and a few more bytes, but apart from that I don't see any\n>> problems?\n> I have my GPG-keys password-protected, and I would be severely annoyed by\n> GnuPG password prompts popping up on every commit. I think the -S option\n> to rebase would be the more elegant way. What could be nice would be a\n> config option that makes \"git push\" warn/abort in case I try to push an\n> unsigned head commit to a repo where I want to have signed commits:\n>> remote..abortUnsigned\n> This of course needs an command line override switch.\n\nThis appears to be more suited for a server side hook (update), or a new\npre-push hook.\n\n> Something to be considered is whether \"git rebase -S\" should sign *every*\n> commit in the series or only the *head* commit.\n\nThe idea is probably to sign a commit that used to signed?\n\nOtherwise, \"git commit --amend -S\" is your friend, either during rebase\n(for individual commits) or after (for the head commit).\n\nMichael\n"},{"id":"215336","messageId":"517816BD.3030203@physik.tu-berlin.de","threadId":"33576","inReplyTo":"5177AB10.30209@drmicha.warpmail.net","subject":"[PATCH 1/1] templates: pre-push hook: check for missing GPG signatures (was: Re: [PATCH] Add .gitconfig variable commit.gpg-sign)","fromName":"Sebastian Götte","fromEmail":"jaseg@physik.tu-berlin.de","sentAt":"2013-04-24T17:30:37Z","receivedAt":"2013-04-24T17:30:37Z","isPatch":true,"sender":{"key":"jaseg@physik.tu-berlin.de","avatar":null},"body":"\nOn 04/24/2013 11:51 AM, Michael J Gruber wrote:\n> Sebastian Götte venit, vidit, dixit 24.04.2013 10:53:\n>> What could be nice would be a\n>> config option that makes \"git push\" warn/abort in case I try to push an\n>> unsigned head commit to a repo where I want to have signed commits:\n>>> remote..abortUnsigned\n>> This of course needs a command line override switch.\n> \n> This appears to be more suited for a server side hook (update), or a new\n> pre-push hook.\nOk, here it is ;)\nI replaced the previous sample hook code because it did only check for commits\ncontaining \"WIP\" in their messages which I think is not terribly useful (and\ncan easily be added to this script. I also added a missing colon that caused my\nshell to complain about an empty if.\nThis patch applies to the current master as it requires the new GPG %G? pretty\nplaceholder output.\n\nSigned-off-by: Sebastian Götte \n---\n templates/hooks--pre-push.sample | 22 ++++++++++------------\n 1 file changed, 10 insertions(+), 12 deletions(-)\n\ndiff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample\nold mode 100644\nnew mode 100755\nindex 15ab6d8..08a72df\n--- a/templates/hooks--pre-push.sample\n+++ b/templates/hooks--pre-push.sample\n@@ -16,20 +16,19 @@\n #\n # \n #\n-# This sample shows how to prevent push of commits where the log message starts\n-# with \"WIP\" (work in progress).\n+# This sample shows how to prevent pushing commits without good GPG signatures\n \n remote=\"$1\"\n url=\"$2\"\n \n z40=0000000000000000000000000000000000000000\n+ec=0\n \n-IFS=' '\n while read local_ref local_sha remote_ref remote_sha\n do\n \tif [ \"$local_sha\" = $z40 ]\n \tthen\n-\t\t# Handle delete\n+\t\t: # Handle delete\n \telse\n \t\tif [ \"$remote_sha\" = $z40 ]\n \t\tthen\n@@ -40,14 +39,13 @@ do\n \t\t\trange=\"$remote_sha..$local_sha\"\n \t\tfi\n \n-\t\t# Check for WIP commit\n-\t\tcommit=`git rev-list -n 1 --grep '^WIP' \"$range\"`\n-\t\tif [ -n \"$commit\" ]\n-\t\tthen\n-\t\t\techo \"Found WIP commit in $local_ref, not pushing\"\n-\t\t\texit 1\n-\t\tfi\n+\t\tcommits=`git log --format=\"%G? %h\" \"$range\" | grep -v '^G' | cut -d\\ -f2`\n+\t\tfor commit in $commits\n+\t\tdo\n+\t\t\techo \"Commit $commit does not have a good GPG signature\"\n+\t\t\tec=1\n+\t\tdone\n \tfi\n done\n \n-exit 0\n+exit $ec\n-- \n1.8.2\n"},{"id":"215374","messageId":"7vppxjbt6t.fsf_-_@alter.siamese.dyndns.org","threadId":"33576","inReplyTo":"517816BD.3030203@physik.tu-berlin.de","subject":"Re: [PATCH 1/1] templates: pre-push hook: check for missing GPG signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-24T19:54:34Z","receivedAt":"2013-04-24T19:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Götte writes:\n\n> On 04/24/2013 11:51 AM, Michael J Gruber wrote:\n>> Sebastian Götte venit, vidit, dixit 24.04.2013 10:53:\n>>> What could be nice would be a\n>>> config option that makes \"git push\" warn/abort in case I try to push an\n>>> unsigned head commit to a repo where I want to have signed commits:\n>>>> remote..abortUnsigned\n>>> This of course needs a command line override switch.\n>> \n>> This appears to be more suited for a server side hook (update), or a new\n>> pre-push hook.\n> Ok, here it is ;)\n> I replaced the previous sample hook code because it did only check for commits\n> containing \"WIP\" in their messages which I think is not terribly useful (and\n> can easily be added to this script. I also added a missing colon that caused my\n> shell to complain about an empty if.\n> This patch applies to the current master as it requires the new GPG %G? pretty\n> placeholder output.\n\nNone of the above is part of a proper commit log message, is it?\n\n>\n> Signed-off-by: Sebastian Götte \n> ---\n> templates/hooks--pre-push.sample | 22 ++++++++++------------\n> 1 file changed, 10 insertions(+), 12 deletions(-)\n>\n> diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample\n> old mode 100644\n> new mode 100755\n\nWhy?\n\n> index 15ab6d8..08a72df\n> --- a/templates/hooks--pre-push.sample\n> +++ b/templates/hooks--pre-push.sample\n> @@ -16,20 +16,19 @@\n> #\n> # \n> #\n> -# This sample shows how to prevent push of commits where the log message starts\n> -# with \"WIP\" (work in progress).\n> +# This sample shows how to prevent pushing commits without good GPG signatures\n\nWhat justifies to remove existing demonstration? It is far easier\nfor the end users to remove parts that do not apply to their needs,\nthan coming up with a solution to add themselves without help from\nan example.\n\n> remote=\"$1\"\n> url=\"$2\"\n> \n> z40=0000000000000000000000000000000000000000\n> +ec=0\n\nI think it is more customary to call this kind of variable \"ret\" or\n\"retval\", not an abbreviation for \"european commission\" ;-).\n\n> -IFS=' '\n\nWhy?\n\n> while read local_ref local_sha remote_ref remote_sha\n> do\n> \tif [ \"$local_sha\" = $z40 ]\n> \tthen\n> -\t\t# Handle delete\n> +\t\t: # Handle delete\n> \telse\n> \t\tif [ \"$remote_sha\" = $z40 ]\n> \t\tthen\n> @@ -40,14 +39,13 @@ do\n> \t\t\trange=\"$remote_sha..$local_sha\"\n> \t\tfi\n> \n> -\t\t# Check for WIP commit\n> -\t\tcommit=`git rev-list -n 1 --grep '^WIP' \"$range\"`\n> -\t\tif [ -n \"$commit\" ]\n> -\t\tthen\n> -\t\t\techo \"Found WIP commit in $local_ref, not pushing\"\n> -\t\t\texit 1\n> -\t\tfi\n> +\t\tcommits=`git log --format=\"%G? %h\" \"$range\" | grep -v '^G' | cut -d\\ -f2`\n\nUseless use of cut. You could do\n\n\tgit log ... |\n while read sign commit\n do\n \ttest \"$sign\" = G && continue\n echo \"Found commit that is not properly signed: $commit\"\n\t\t...\n"},{"id":"215441","messageId":"51791F41.3040203@physik.tu-berlin.de","threadId":"33576","inReplyTo":"7vppxjbt6t.fsf_-_@alter.siamese.dyndns.org","subject":"[PATCH v2 0/1] templates: pre-push hook: check for missing GPG signatures","fromName":"Sebastian Götte","fromEmail":"jaseg@physik.tu-berlin.de","sentAt":"2013-04-25T12:19:13Z","receivedAt":"2013-04-25T12:19:13Z","isPatch":true,"sender":{"key":"jaseg@physik.tu-berlin.de","avatar":null},"body":"On 04/24/2013 09:54 PM, Junio C Hamano wrote:\n> None of the above is part of a proper commit log message, is it?\nFixed (I hope)\n\n>> Signed-off-by: Sebastian Götte \n>> diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample\n>> old mode 100644\n>> new mode 100755\n> Why?\nAccording to man githooks(5):\n>It is also a requirement for a given hook to be executable. However - in a\n>freshly initialized repository - the .sample files are executable by default.\nThis file is the only one in templates/ that was not executable, so I assume\nthis was a mistake.\n\n>> index 15ab6d8..08a72df\n>> --- a/templates/hooks--pre-push.sample\n>> +++ b/templates/hooks--pre-push.sample\n>> -# This sample shows how to prevent push of commits where the log message starts\n>> -# with \"WIP\" (work in progress).\n>> +# This sample shows how to prevent pushing commits without good GPG signatures\n> What justifies to remove existing demonstration? It is far easier\n> for the end users to remove parts that do not apply to their needs,\n> than coming up with a solution to add themselves without help from\n> an example.\nre-added it.\n\n>> +ec=0\n> I think it is more customary to call this kind of variable \"ret\" or\n> \"retval\", not an abbreviation for \"european commission\" ;-).\nrenamed it to \"exitcode\".\n\n>> -IFS=' '\n> Why?\nOtherwise in the for-loop below the output of the pipe chain is not correctly\nsplit by newlines. Also AFAIK, this is not needed: I think the default\n'' is just fine here.\n \n>> +\t\tcommits=`git log --format=\"%G? %h\" \"$range\" | grep -v '^G' | cut -d\\ -f2`\n> Useless use of cut. You could do\nI just tried this, but I really want the script to output a list of *all*\noffending commits (instead of exiting on the first problem). For this I need\nthe exitcode variable, but since at least bash executes the while loop in a\nsubshell due to the preceding pipe, I have some issues getting that out of the\nsubshell. This is what the code looked like without grep/cut:\n># Check for missing good GPG signatures\n>git log --format=\"%G? %h\" \"$range\" |\n>(\n> exitcode=0\n> while read sign commit\n> do\n> test \"$sign\" = G && continue\n> echo \"Commit $commit does not have a good GPG signature\"\n> exitcode=1\n> done\n> exit $exitcode\n>)\n>let exitcode=exitcode\\|$?\nThis is less readable and only spawns one process less.\n\nSebastian Götte (1):\n templates: pre-push hook: check for missing GPG signatures\n\n templates/hooks--pre-push.sample | 23 +++++++++++++++--------\n 1 file changed, 15 insertions(+), 8 deletions(-)\n mode change 100644 => 100755 templates/hooks--pre-push.sample\n\n-- \n1.8.2\n"},{"id":"215442","messageId":"51791F44.9090406@physik.tu-berlin.de","threadId":"33576","inReplyTo":"cover.1366890748.git.jaseg@physik-pool.tu-berlin.de","subject":"[PATCH v2 1/1] templates: pre-push hook: check for missing GPG signatures","fromName":"Sebastian Götte","fromEmail":"jaseg@physik.tu-berlin.de","sentAt":"2013-04-25T12:19:16Z","receivedAt":"2013-04-25T12:19:16Z","isPatch":true,"sender":{"key":"jaseg@physik.tu-berlin.de","avatar":null},"body":"Also added a missing colon that caused my bash 4.2.45(2)-release to\ncomplain about an empty if.\n\nSigned-off-by: Sebastian Götte \n---\n templates/hooks--pre-push.sample | 23 +++++++++++++++--------\n 1 file changed, 15 insertions(+), 8 deletions(-)\n\ndiff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample\nold mode 100644\nnew mode 100755\nindex 15ab6d8..a16283c\n--- a/templates/hooks--pre-push.sample\n+++ b/templates/hooks--pre-push.sample\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-# An example hook script to verify what is about to be pushed. Called by \"git\n+# An example hook script to verify what is about to be pushed.\tCalled by \"git\n # push\" after it has checked the remote status, but before anything has been\n # pushed. If this script exits with a non-zero status nothing will be pushed.\n #\n@@ -14,22 +14,22 @@\n # Information about the commits which are being pushed is supplied as lines to\n # the standard input in the form:\n #\n-# \n+#\t \n #\n-# This sample shows how to prevent push of commits where the log message starts\n-# with \"WIP\" (work in progress).\n+# This sample shows how to prevent pushing commits without good GPG signatures\n+# or where the log message starts with \"WIP\" (work in progress).\n \n remote=\"$1\"\n url=\"$2\"\n \n z40=0000000000000000000000000000000000000000\n+exitcode=0\n \n-IFS=' '\n while read local_ref local_sha remote_ref remote_sha\n do\n \tif [ \"$local_sha\" = $z40 ]\n \tthen\n-\t\t# Handle delete\n+\t\t: # Handle delete\n \telse\n \t\tif [ \"$remote_sha\" = $z40 ]\n \t\tthen\n@@ -45,9 +45,16 @@ do\n \t\tif [ -n \"$commit\" ]\n \t\tthen\n \t\t\techo \"Found WIP commit in $local_ref, not pushing\"\n-\t\t\texit 1\n+\t\t\texitcode=1\n \t\tfi\n+\n+\t\t# Check for missing good GPG signatures\n+\t\tfor commit in `git log --format=\"%G? %h\" \"$range\" | grep -v '^G' | cut -d\\ -f2`\n+\t\tdo\n+\t\t\techo \"Commit $commit does not have a good GPG signature\"\n+\t\t\texitcode=1\n+\t\tdone\n \tfi\n done\n \n-exit 0\n+exit $exitcode\n-- \n1.8.2\n"},{"id":"215453","messageId":"7vy5c64krz.fsf@alter.siamese.dyndns.org","threadId":"33576","inReplyTo":"51791F41.3040203@physik.tu-berlin.de","subject":"Re: [PATCH v2 0/1] templates: pre-push hook: check for missing GPG signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-25T16:50:24Z","receivedAt":"2013-04-25T16:50:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Götte writes:\n\n> On 04/24/2013 09:54 PM, Junio C Hamano wrote:\n>> None of the above is part of a proper commit log message, is it?\n> Fixed (I hope)\n\nDon't hope, instead do. ;-)\n\nThe questions I asked were not requests to explain them to _me_ in a\nresponse like this. They were the examples of what the proposed\ncommit log message should have explained what the patch attempts to\ndo.\n\n>>> -IFS=' '\n>> Why?\n> Otherwise in the for-loop below the output of the pipe chain is not correctly\n> split by newlines. Also AFAIK, this is not needed: I think the default\n> '' is just fine here.\n\nIt is not enough to make sure that IFS has SP so that existing code\nworks correctly; we also need to see if the existing code needs to\navoid cutting the tokens at HT or LF. I think in this case using\nthe default IFS is safe, as input to pre-push are SP separated refs\nand object names, none of which can have SP, HT or LF in it.\n\n>># Check for missing good GPG signatures\n>>git log --format=\"%G? %h\" \"$range\" |\n>>(\n>> exitcode=0\n>> while read sign commit\n>> do\n>> test \"$sign\" = G && continue\n>> echo \"Commit $commit does not have a good GPG signature\"\n>> exitcode=1\n>> done\n>> exit $exitcode\n>>)\n>>let exitcode=exitcode\\|$?\n\nDon't use bash-ism \"let\".\n\nThe above loop is a perfectly fine and readable way to write the\nlogic, by the way\n\nExcept that we tend to prefer $ret over $exitcode, but I've already\nsaid that.\n\nThanks.\n"}]}