{"thread":{"id":"34769","subject":"[PATCH] git send-email: include [anything]-by: signatures","startedAt":"2013-08-26T16:57:47Z","lastAt":"2013-09-04T08:09:28Z","messageCount":10,"participants":["Michael S. Tsirkin","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"225909","messageId":"20130826165747.GA30788@redhat.com","threadId":"34769","inReplyTo":null,"subject":"[PATCH] git send-email: include [anything]-by: signatures","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-08-26T16:57:47Z","receivedAt":"2013-08-26T16:57:47Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Consider [anything]-by: a valid signature.\nThis includes Tested-by: Acked-by: Reviewed-by: etc.\n\nSigned-off-by: Michael S. Tsirkin \n---\n git-send-email.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex ecbf56f..bb9093b 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1359,7 +1359,7 @@ foreach my $t (@files) {\n \t# Now parse the message body\n \twhile(<$fh>) {\n \t\t$message .= $_;\n-\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n+\t\tif (/^([A-Za-z-]*-by|Cc): (.*)$/i) {\n \t\t\tchomp;\n \t\t\tmy ($what, $c) = ($1, $2);\n \t\t\tchomp $c;\n-- \nMST\n"},{"id":"226443","messageId":"20130831192250.GA3823@redhat.com","threadId":"34769","inReplyTo":"20130826165747.GA30788@redhat.com","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-08-31T19:22:50Z","receivedAt":"2013-08-31T19:22:50Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Mon, Aug 26, 2013 at 07:57:47PM +0300, Michael S. Tsirkin wrote:\n> Consider [anything]-by: a valid signature.\n> This includes Tested-by: Acked-by: Reviewed-by: etc.\n> \n> Signed-off-by: Michael S. Tsirkin \n\nPing.\nAny opinion on whether this change is acceptable?\n\n> ---\n> git-send-email.perl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index ecbf56f..bb9093b 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1359,7 +1359,7 @@ foreach my $t (@files) {\n> \t# Now parse the message body\n> \twhile(<$fh>) {\n> \t\t$message .= $_;\n> -\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n> +\t\tif (/^([A-Za-z-]*-by|Cc): (.*)$/i) {\n> \t\t\tchomp;\n> \t\t\tmy ($what, $c) = ($1, $2);\n> \t\t\tchomp $c;\n> -- \n> MST\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"226579","messageId":"20130903063535.GA3608@sigill.intra.peff.net","threadId":"34769","inReplyTo":"20130831192250.GA3823@redhat.com","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-03T06:35:35Z","receivedAt":"2013-09-03T06:35:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 31, 2013 at 10:22:50PM +0300, Michael S. Tsirkin wrote:\n\n> On Mon, Aug 26, 2013 at 07:57:47PM +0300, Michael S. Tsirkin wrote:\n> > Consider [anything]-by: a valid signature.\n> > This includes Tested-by: Acked-by: Reviewed-by: etc.\n> > \n> > Signed-off-by: Michael S. Tsirkin \n> \n> Ping.\n> Any opinion on whether this change is acceptable?\n\nI was left confused by your commit message, as it wasn't clear to me\nwhat a \"signature\" is. But the point of it seems to be that people\nmention others in commit messages using \"X-by:\" pseudo-headers besides\n\"signed-off-by\", and you want to cc them along with the usual S-O-B.\n\nThat seems like a reasonable goal, but I have two concerns.\n\nOne, I would think the utility of this would be per-project, depending\non what sorts of things people in a particular project put in\npseudo-headers. Grepping the kernel history shows that most X-by\nheaders have a person on the right-hand side, though quite often it is\nnot a valid email address (on the other hand, quite a few s-o-b lines in\nthe kernel do not have a valid email).\n\nAnd two, the existing options for enabling/disabling this code all\nexplicitly mention signed-off-by, which becomes awkward. You did not\nupdate the documentation in your patch, but I think you would end up\nhaving to explain that \"--supress-cc=sob\" and \"--signed-off-by-cc\"\nreally mean \"all pseudo-header lines ending in -by\".\n\nSo I think it might be a nicer approach to introduce a new \"suppress-cc\"\nclass that means \"all pseudo-header tokens ending in -by\" or similar.\nWe might even want the new behavior on by default, but it would at least\ngive the user an escape hatch if their project generates a lot of false\npositives.\n\n-Peff\n"},{"id":"226606","messageId":"20130903084454.GC18901@redhat.com","threadId":"34769","inReplyTo":"20130903063535.GA3608@sigill.intra.peff.net","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-09-03T08:44:54Z","receivedAt":"2013-09-03T08:44:54Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Tue, Sep 03, 2013 at 02:35:35AM -0400, Jeff King wrote:\n> On Sat, Aug 31, 2013 at 10:22:50PM +0300, Michael S. Tsirkin wrote:\n> \n> > On Mon, Aug 26, 2013 at 07:57:47PM +0300, Michael S. Tsirkin wrote:\n> > > Consider [anything]-by: a valid signature.\n> > > This includes Tested-by: Acked-by: Reviewed-by: etc.\n> > > \n> > > Signed-off-by: Michael S. Tsirkin \n> > \n> > Ping.\n> > Any opinion on whether this change is acceptable?\n> \n> I was left confused by your commit message, as it wasn't clear to me\n> what a \"signature\" is. But the point of it seems to be that people\n> mention others in commit messages using \"X-by:\" pseudo-headers besides\n> \"signed-off-by\", and you want to cc them along with the usual S-O-B.\n> \n> That seems like a reasonable goal, but I have two concerns.\n> \n> One, I would think the utility of this would be per-project, depending\n> on what sorts of things people in a particular project put in\n> pseudo-headers. Grepping the kernel history shows that most X-by\n> headers have a person on the right-hand side, though quite often it is\n> not a valid email address (on the other hand, quite a few s-o-b lines in\n> the kernel do not have a valid email).\n> \n> And two, the existing options for enabling/disabling this code all\n> explicitly mention signed-off-by, which becomes awkward. You did not\n> update the documentation in your patch, but I think you would end up\n> having to explain that \"--supress-cc=sob\" and \"--signed-off-by-cc\"\n> really mean \"all pseudo-header lines ending in -by\".\n> \n> So I think it might be a nicer approach to introduce a new \"suppress-cc\"\n> class that means \"all pseudo-header tokens ending in -by\" or similar.\n> We might even want the new behavior on by default, but it would at least\n> give the user an escape hatch if their project generates a lot of false\n> positives.\n> \n> -Peff\n\nI guess there's always cccmd, no?\n\n-- \nMST\n"},{"id":"226634","messageId":"xmqqmwntu96c.fsf@gitster.dls.corp.google.com","threadId":"34769","inReplyTo":"20130903084454.GC18901@redhat.com","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-03T17:06:19Z","receivedAt":"2013-09-03T17:06:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> On Tue, Sep 03, 2013 at 02:35:35AM -0400, Jeff King wrote:\n>> On Sat, Aug 31, 2013 at 10:22:50PM +0300, Michael S. Tsirkin wrote:\n>> \n>> > On Mon, Aug 26, 2013 at 07:57:47PM +0300, Michael S. Tsirkin wrote:\n>> > > Consider [anything]-by: a valid signature.\n>> > > This includes Tested-by: Acked-by: Reviewed-by: etc.\n>> > > \n>> > > Signed-off-by: Michael S. Tsirkin \n>> > \n>> > Ping.\n>> > Any opinion on whether this change is acceptable?\n>> \n>> I was left confused by your commit message, as it wasn't clear to me\n>> what a \"signature\" is. But the point of it seems to be that people\n>> mention others in commit messages using \"X-by:\" pseudo-headers besides\n>> \"signed-off-by\", and you want to cc them along with the usual S-O-B.\n>> \n>> That seems like a reasonable goal, but I have two concerns.\n>> \n>> One, I would think the utility of this would be per-project, depending\n>> on what sorts of things people in a particular project put in\n>> pseudo-headers. Grepping the kernel history shows that most X-by\n>> headers have a person on the right-hand side, though quite often it is\n>> not a valid email address (on the other hand, quite a few s-o-b lines in\n>> the kernel do not have a valid email).\n>> \n>> And two, the existing options for enabling/disabling this code all\n>> explicitly mention signed-off-by, which becomes awkward. You did not\n>> update the documentation in your patch, but I think you would end up\n>> having to explain that \"--supress-cc=sob\" and \"--signed-off-by-cc\"\n>> really mean \"all pseudo-header lines ending in -by\".\n>> \n>> So I think it might be a nicer approach to introduce a new \"suppress-cc\"\n>> class that means \"all pseudo-header tokens ending in -by\" or similar.\n>> We might even want the new behavior on by default, but it would at least\n>> give the user an escape hatch if their project generates a lot of false\n>> positives.\n>> \n>> -Peff\n>\n> I guess there's always cccmd, no?\n\nI am having a hard time deciphering what this response means. Are\nyou suggesting that people can use cccmd to do what your patch\nwants to do, so the patch is not needed?\n\nI tend to agree with Peff that it is a reasonable goal to allow more\nthan just the fixed set of trailers to be used as a source to decide\nwhom to Cc, and if it can be generic enough, it would make sense to\nsupply users such support so that various projects do not have to\ninvent their own.\n\nThe question of course is the first point Peff raised. I am not\nsure offhand what the right per-project customization interface\nwould be. A starting point might be something like:\n\n\t--cc-trailer=signed-off-by,acked-by,reviewed-by\n\nor even\n\n\t--cc-trailer='*-by'\n\nand an obvious configuration variable that gives the default for it.\nThat would eventually allow us not to special case any fixed set of\ntrailers like S-o-b like the current code does, which would be a big\nplus.\n"},{"id":"226674","messageId":"20130903210149.GA24480@redhat.com","threadId":"34769","inReplyTo":"xmqqmwntu96c.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-09-03T21:01:49Z","receivedAt":"2013-09-03T21:01:49Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Tue, Sep 03, 2013 at 10:06:19AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" writes:\n> \n> > On Tue, Sep 03, 2013 at 02:35:35AM -0400, Jeff King wrote:\n> >> On Sat, Aug 31, 2013 at 10:22:50PM +0300, Michael S. Tsirkin wrote:\n> >> \n> >> > On Mon, Aug 26, 2013 at 07:57:47PM +0300, Michael S. Tsirkin wrote:\n> >> > > Consider [anything]-by: a valid signature.\n> >> > > This includes Tested-by: Acked-by: Reviewed-by: etc.\n> >> > > \n> >> > > Signed-off-by: Michael S. Tsirkin \n> >> > \n> >> > Ping.\n> >> > Any opinion on whether this change is acceptable?\n> >> \n> >> I was left confused by your commit message, as it wasn't clear to me\n> >> what a \"signature\" is. But the point of it seems to be that people\n> >> mention others in commit messages using \"X-by:\" pseudo-headers besides\n> >> \"signed-off-by\", and you want to cc them along with the usual S-O-B.\n> >> \n> >> That seems like a reasonable goal, but I have two concerns.\n> >> \n> >> One, I would think the utility of this would be per-project, depending\n> >> on what sorts of things people in a particular project put in\n> >> pseudo-headers. Grepping the kernel history shows that most X-by\n> >> headers have a person on the right-hand side, though quite often it is\n> >> not a valid email address (on the other hand, quite a few s-o-b lines in\n> >> the kernel do not have a valid email).\n> >> \n> >> And two, the existing options for enabling/disabling this code all\n> >> explicitly mention signed-off-by, which becomes awkward. You did not\n> >> update the documentation in your patch, but I think you would end up\n> >> having to explain that \"--supress-cc=sob\" and \"--signed-off-by-cc\"\n> >> really mean \"all pseudo-header lines ending in -by\".\n> >> \n> >> So I think it might be a nicer approach to introduce a new \"suppress-cc\"\n> >> class that means \"all pseudo-header tokens ending in -by\" or similar.\n> >> We might even want the new behavior on by default, but it would at least\n> >> give the user an escape hatch if their project generates a lot of false\n> >> positives.\n> >> \n> >> -Peff\n> >\n> > I guess there's always cccmd, no?\n> \n> I am having a hard time deciphering what this response means. Are\n> you suggesting that people can use cccmd to do what your patch\n> wants to do, so the patch is not needed?\n> \n> I tend to agree with Peff that it is a reasonable goal to allow more\n> than just the fixed set of trailers to be used as a source to decide\n> whom to Cc, and if it can be generic enough, it would make sense to\n> supply users such support so that various projects do not have to\n> invent their own.\n> \n> The question of course is the first point Peff raised. I am not\n> sure offhand what the right per-project customization interface\n> would be. A starting point might be something like:\n> \n> \t--cc-trailer=signed-off-by,acked-by,reviewed-by\n\ntested-by, reported-by ...\n\n> or even\n> \n> \t--cc-trailer='*-by'\n> \n> and an obvious configuration variable that gives the default for it.\n> That would eventually allow us not to special case any fixed set of\n> trailers like S-o-b like the current code does, which would be a big\n> plus.\n\nWhat bothers me is that git normally uses gawk based patterns,\nbut send-email is in perl so it has a different syntax for regexp.\nWhat do you suggest? Make a small binary to do the matching for us?\n\n-- \nMST\n"},{"id":"226675","messageId":"20130903210352.GA27344@sigill.intra.peff.net","threadId":"34769","inReplyTo":"20130903210149.GA24480@redhat.com","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-03T21:03:52Z","receivedAt":"2013-09-03T21:03:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 04, 2013 at 12:01:49AM +0300, Michael S. Tsirkin wrote:\n\n> > The question of course is the first point Peff raised. I am not\n> > sure offhand what the right per-project customization interface\n> > would be. A starting point might be something like:\n> > \n> > \t--cc-trailer=signed-off-by,acked-by,reviewed-by\n> \n> tested-by, reported-by ...\n\nYeah, I think having the list customizable is nice, but not allowing\nsome pattern matching seems unfriendly, as it requires the user to\nenumerate a potentially long list.\n\n> > \t--cc-trailer='*-by'\n> > \n> > and an obvious configuration variable that gives the default for it.\n> > That would eventually allow us not to special case any fixed set of\n> > trailers like S-o-b like the current code does, which would be a big\n> > plus.\n> \n> What bothers me is that git normally uses gawk based patterns,\n> but send-email is in perl so it has a different syntax for regexp.\n> What do you suggest? Make a small binary to do the matching for us?\n\nWould fnmatch-style globbing (like \"*-by\") be enough? That should be\neasy to do in perl.\n\n-Peff\n"},{"id":"226677","messageId":"20130903212432.GC24480@redhat.com","threadId":"34769","inReplyTo":"20130903210352.GA27344@sigill.intra.peff.net","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-09-03T21:24:32Z","receivedAt":"2013-09-03T21:24:32Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Tue, Sep 03, 2013 at 05:03:52PM -0400, Jeff King wrote:\n> On Wed, Sep 04, 2013 at 12:01:49AM +0300, Michael S. Tsirkin wrote:\n> \n> > > The question of course is the first point Peff raised. I am not\n> > > sure offhand what the right per-project customization interface\n> > > would be. A starting point might be something like:\n> > > \n> > > \t--cc-trailer=signed-off-by,acked-by,reviewed-by\n> > \n> > tested-by, reported-by ...\n> \n> Yeah, I think having the list customizable is nice, but not allowing\n> some pattern matching seems unfriendly, as it requires the user to\n> enumerate a potentially long list.\n> \n> > > \t--cc-trailer='*-by'\n> > > \n> > > and an obvious configuration variable that gives the default for it.\n> > > That would eventually allow us not to special case any fixed set of\n> > > trailers like S-o-b like the current code does, which would be a big\n> > > plus.\n> > \n> > What bothers me is that git normally uses gawk based patterns,\n> > but send-email is in perl so it has a different syntax for regexp.\n> > What do you suggest? Make a small binary to do the matching for us?\n> \n> Would fnmatch-style globbing (like \"*-by\") be enough? That should be\n> easy to do in perl.\n> \n> -Peff\n\nIf you mean only support * - that would be easy.\nOnce you get into bracket expressions it gets messy quickly\nhttp://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_03_05\n\n-- \nMST\n"},{"id":"226678","messageId":"xmqqk3ixpoue.fsf@gitster.dls.corp.google.com","threadId":"34769","inReplyTo":"20130903210352.GA27344@sigill.intra.peff.net","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-03T21:39:05Z","receivedAt":"2013-09-03T21:39:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Sep 04, 2013 at 12:01:49AM +0300, Michael S. Tsirkin wrote:\n>\n>> > The question of course is the first point Peff raised. I am not\n>> > sure offhand what the right per-project customization interface\n>> > would be. A starting point might be something like:\n>> > \n>> > \t--cc-trailer=signed-off-by,acked-by,reviewed-by\n>> \n>> tested-by, reported-by ...\n>\n> Yeah, I think having the list customizable is nice, but not allowing\n> some pattern matching seems unfriendly, as it requires the user to\n> enumerate a potentially long list.\n>\n>> > \t--cc-trailer='*-by'\n>> > \n>> > and an obvious configuration variable that gives the default for it.\n>> > That would eventually allow us not to special case any fixed set of\n>> > trailers like S-o-b like the current code does, which would be a big\n>> > plus.\n>> \n>> What bothers me is that git normally uses gawk based patterns,\n>> but send-email is in perl so it has a different syntax for regexp.\n>> What do you suggest? Make a small binary to do the matching for us?\n>\n> Would fnmatch-style globbing (like \"*-by\") be enough? That should be\n> easy to do in perl.\n\nWeb query finds File::FnMatch; I do not know if that is the most\ncommonly used, or if it comes with the base distribution, though.\n"},{"id":"226710","messageId":"20130904080928.GA30491@redhat.com","threadId":"34769","inReplyTo":"xmqqk3ixpoue.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git send-email: include [anything]-by: signatures","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-09-04T08:09:28Z","receivedAt":"2013-09-04T08:09:28Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Tue, Sep 03, 2013 at 02:39:05PM -0700, Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > On Wed, Sep 04, 2013 at 12:01:49AM +0300, Michael S. Tsirkin wrote:\n> >\n> >> > The question of course is the first point Peff raised. I am not\n> >> > sure offhand what the right per-project customization interface\n> >> > would be. A starting point might be something like:\n> >> > \n> >> > \t--cc-trailer=signed-off-by,acked-by,reviewed-by\n> >> \n> >> tested-by, reported-by ...\n> >\n> > Yeah, I think having the list customizable is nice, but not allowing\n> > some pattern matching seems unfriendly, as it requires the user to\n> > enumerate a potentially long list.\n> >\n> >> > \t--cc-trailer='*-by'\n> >> > \n> >> > and an obvious configuration variable that gives the default for it.\n> >> > That would eventually allow us not to special case any fixed set of\n> >> > trailers like S-o-b like the current code does, which would be a big\n> >> > plus.\n> >> \n> >> What bothers me is that git normally uses gawk based patterns,\n> >> but send-email is in perl so it has a different syntax for regexp.\n> >> What do you suggest? Make a small binary to do the matching for us?\n> >\n> > Would fnmatch-style globbing (like \"*-by\") be enough? That should be\n> > easy to do in perl.\n> \n> Web query finds File::FnMatch; I do not know if that is the most\n> commonly used, or if it comes with the base distribution, though.\n\nIt's also just a wrapper for the system's fnmatch - so I expect\nit doesn't work in the mingw environment.\n\n-- \nMST\n"}]}