{"thread":{"id":"43961","subject":"git am and duplicate signatures","startedAt":"2016-08-30T16:12:47Z","lastAt":"2016-08-30T19:01:03Z","messageCount":11,"participants":["Joe Perches","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"300568","messageId":"1472573556.5512.93.camel@perches.com","threadId":"43961","inReplyTo":null,"subject":"git am and duplicate signatures","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-30T16:12:36Z","receivedAt":"2016-08-30T16:12:47Z","isPatch":false,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"git-am -s will avoid duplicating the last signature\nin a patch.\n\nBut given a developer creates a patch, send it around for\nacks/other signoffs, collects signatures and then does\na git am -s on a different branch, this sort of sign-off\nchain is possible:\n\n\tSigned-off-by: Original Developer \n\tAcked-by: Random Developer \n\tSigned-off-by: Original Developer \n\nShould there be an option to avoid duplicate signatures\nin a sequence where an author can git-am the same patch?\n\nsequencer.c:append_signoff() has a flag for APPEND_SIGNOFF_DEDUP\n\nsequencer.c:void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)\n\nbut\n\nbuiltin/commit.c:               append_signoff(&sb, ignore_non_trailer(&sb), 0);\n\ndoesn't have an optional use mechanism available.\n\n"},{"id":"300572","messageId":"xmqqwpiy5haf.fsf@gitster.mtv.corp.google.com","threadId":"43961","inReplyTo":"1472573556.5512.93.camel@perches.com","subject":"Re: git am and duplicate signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-30T16:54:16Z","receivedAt":"2016-08-30T16:54:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches writes:\n\n> git-am -s will avoid duplicating the last signature\n> in a patch.\n>\n> But given a developer creates a patch, send it around for\n> acks/other signoffs, collects signatures and then does\n> a git am -s on a different branch, this sort of sign-off\n> chain is possible:\n>\n> \tSigned-off-by: Original Developer \n> \tAcked-by: Random Developer \n> \tSigned-off-by: Original Developer \n\nBoth correct and allowing the earlier one duplicated as long as\nthere is somebody/something else in between is deliberate.\n\n> Should there be an option to avoid duplicate signatures\n> in a sequence where an author can git-am the same patch?\n\nI dunno. The way \"Signed-off-by\" is handled is designed\nspecifically to support the meaning of that footer, namely to record\nwhere it originated and whose hands it passed, used in the kernel\nand Git land. Other projects certainly may have need for footers\nthat denote different things that want different semantics (e.g. Who\nauthored it and who cheered on it), but that is outside the scope of\nthe \"Signed-off-by\" supported by \"am -s\" and \"commit -s\".\n\nSupport for more generic footers was supposed to come when the\n\"interpret-trailers\" topic started, but the author of the topic\nseems to have lost interest before the mechanism has become ready to\nbe integrated in the workflow commands like \"am\", \"commit\", \"rebase\"\netc., which is unfortunate.\n\n> sequencer.c:append_signoff() has a flag for APPEND_SIGNOFF_DEDUP\n\nYes, I think this is one of the warts we talked about getting rid of\nbut haven't got around to it. It is there because \"format-patch -s\"\nwas incorrectly written to dedup Signed-off-by: from anywhere in its\nearly implementation and to keep the same behaviour. We should drop\nthat flag from append_signoff() function.\n"},{"id":"300574","messageId":"1472576869.5512.98.camel@perches.com","threadId":"43961","inReplyTo":"xmqqwpiy5haf.fsf@gitster.mtv.corp.google.com","subject":"Re: git am and duplicate signatures","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-30T17:07:49Z","receivedAt":"2016-08-30T17:08:12Z","isPatch":false,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"(adding lkml)\n\nOn Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote:\n> Joe Perches writes:\n> > git-am -s will avoid duplicating the last signature\n> > in a patch.\n> > \n> > But given a developer creates a patch, send it around for\n> > acks/other signoffs, collects signatures and then does\n> > a git am -s on a different branch, this sort of sign-off\n> > chain is possible:\n> > \n> > \tSigned-off-by: Original Developer \n> > \tAcked-by: Random Developer \n> > \tSigned-off-by: Original Developer \n> Both correct and allowing the earlier one duplicated as long as\n> there is somebody/something else in between is deliberate.\n\nlinux-kernel has a script (scripts/checkpatch.pl) that\nlooks for duplicate signatures (-by: [name]
)\n\nShould the last Signed-off-by: in the commit log be\nexcluded from this check?\n\n> > Should there be an option to avoid duplicate signatures\n> > in a sequence where an author can git-am the same patch?\n> I dunno.  The way \"Signed-off-by\" is handled is designed\n> specifically to support the meaning of that footer, namely to record\n> where it originated and whose hands it passed, used in the kernel\n> and Git land.  Other projects certainly may have need for footers\n> that denote different things that want different semantics (e.g. Who\n> authored it and who cheered on it), but that is outside the scope of\n> the \"Signed-off-by\" supported by \"am -s\" and \"commit -s\".\n> \n> Support for more generic footers was supposed to come when the\n> \"interpret-trailers\" topic started, but the author of the topic\n> seems to have lost interest before the mechanism has become ready to\n> be integrated in the workflow commands like \"am\", \"commit\", \"rebase\"\n> etc., which is unfortunate.\n> \n> > \n> > sequencer.c:append_signoff() has a flag for APPEND_SIGNOFF_DEDUP\n> Yes, I think this is one of the warts we talked about getting rid of\n> but haven't got around to it.  It is there because \"format-patch -s\"\n> was incorrectly written to dedup Signed-off-by: from anywhere in its\n> early implementation and to keep the same behaviour.  We should drop\n> that flag from append_signoff() function.\n"},{"id":"300577","messageId":"1472577999.5512.107.camel@perches.com","threadId":"43961","inReplyTo":"xmqqwpiy5haf.fsf@gitster.mtv.corp.google.com","subject":"Re: git am and duplicate signatures","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-30T17:26:39Z","receivedAt":"2016-08-30T17:26:47Z","isPatch":false,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote:\n> Support for more generic footers was supposed to come when the\n> \"interpret-trailers\" topic started, but the author of the topic\n> seems to have lost interest before the mechanism has become ready to\n> be integrated in the workflow commands like \"am\", \"commit\", \"rebase\"\n> etc., which is unfortunate.\n\nI think adding at least an option to git send-email\nallowing auto-cc's for all\n\t\"-by: [name]
\"\nlines in the commit log would be useful.\n\nToday, only \"Signed-off-by\" and \"CC\" lines are\nadded to cc's.\n\nI've always called these lines \"-by:\" lines\n\"signatures\", but perhaps there's a better name.\n\nAny preference?\n\nfrom git send-email --help\n       --suppress-cc=\n           Specify an additional category of recipients to suppress the\n           auto-cc of:\n\n           ·   author will avoid including the patch author\n\n           ·   self will avoid including the sender\n\n           ·   cc will avoid including anyone mentioned in Cc lines in the\n               patch header except for self (use self for that).\n\n           ·   bodycc will avoid including anyone mentioned in Cc lines in the\n               patch body (commit message) except for self (use self for\n               that).\n\n           ·   sob will avoid including anyone mentioned in Signed-off-by\n               lines except for self (use self for that).\n\n           ·   cccmd will avoid running the --cc-cmd.\n\n           ·   body is equivalent to sob + bodycc\n\n           ·   all will suppress all auto cc values.\n> > \n> > sequencer.c:append_signoff() has a flag for APPEND_SIGNOFF_DEDUP\n> Yes, I think this is one of the warts we talked about getting rid of\n> but haven't got around to it.  It is there because \"format-patch -s\"\n> was incorrectly written to dedup Signed-off-by: from anywhere in its\n> early implementation and to keep the same behaviour.  We should drop\n> that flag from append_signoff() function.\n"},{"id":"300579","messageId":"xmqq8tve5fet.fsf@gitster.mtv.corp.google.com","threadId":"43961","inReplyTo":"1472577999.5512.107.camel@perches.com","subject":"Re: git am and duplicate signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-30T17:34:50Z","receivedAt":"2016-08-30T17:37:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches writes:\n\n> On Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote:\n>> Support for more generic footers was supposed to come when the\n>> \"interpret-trailers\" topic started, but the author of the topic\n>> seems to have lost interest before the mechanism has become ready to\n>> be integrated in the workflow commands like \"am\", \"commit\", \"rebase\"\n>> etc., which is unfortunate.\n>\n> I think adding at least an option to git send-email\n> allowing auto-cc's for all\n> \t\"-by: [name]
\"\n> lines in the commit log would be useful.\n>\n> Today, only \"Signed-off-by\" and \"CC\" lines are\n> added to cc's.\n>\n> I've always called these lines \"-by:\" lines\n> \"signatures\", but perhaps there's a better name.\n\nI think we casually call them footers (as they are counter-part to\n\"headers\"), or trailers.\n"},{"id":"300581","messageId":"xmqq4m625fax.fsf@gitster.mtv.corp.google.com","threadId":"43961","inReplyTo":"1472576869.5512.98.camel@perches.com","subject":"Re: git am and duplicate signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-30T17:37:10Z","receivedAt":"2016-08-30T17:38:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches writes:\n\n> (adding lkml)\n>\n> On Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote:\n>> Joe Perches writes:\n>> > git-am -s will avoid duplicating the last signature\n>> > in a patch.\n>> > \n>> > But given a developer creates a patch, send it around for\n>> > acks/other signoffs, collects signatures and then does\n>> > a git am -s on a different branch, this sort of sign-off\n>> > chain is possible:\n>> > \n>> > \tSigned-off-by: Original Developer \n>> > \tAcked-by: Random Developer \n>> > \tSigned-off-by: Original Developer \n>> Both correct and allowing the earlier one duplicated as long as\n>> there is somebody/something else in between is deliberate.\n>\n> linux-kernel has a script (scripts/checkpatch.pl) that\n> looks for duplicate signatures (-by: [name]
)\n>\n> Should the last Signed-off-by: in the commit log be\n> excluded from this check?\n\nThat is left for the kernel folks to decide, but excluding only \"the\nlast\" does not make much sense to me. If you look for only \"two\nconsecutive same signatures\" and barf, that would be in line with\nwhat we have been shooting for to support the above \"original then\nrandom then back to original\" example you gave us above.\n\n\n"},{"id":"300583","messageId":"1472578879.5512.111.camel@perches.com","threadId":"43961","inReplyTo":"xmqq8tve5fet.fsf@gitster.mtv.corp.google.com","subject":"Re: git am and duplicate signatures","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-30T17:41:19Z","receivedAt":"2016-08-30T17:41:28Z","isPatch":false,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Tue, 2016-08-30 at 10:34 -0700, Junio C Hamano wrote:\n> Joe Perches writes:\n> > On Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote:\n> > > \n> > > Support for more generic footers was supposed to come when the\n> > > \"interpret-trailers\" topic started, but the author of the topic\n> > > seems to have lost interest before the mechanism has become ready to\n> > > be integrated in the workflow commands like \"am\", \"commit\", \"rebase\"\n> > > etc., which is unfortunate.\n> > I think adding at least an option to git send-email\n> > allowing auto-cc's for all\n> > \t\"-by: [name]
\"\n> > lines in the commit log would be useful.\n> > \n> > Today, only \"Signed-off-by\" and \"CC\" lines are\n> > added to cc's.\n> > \n> > I've always called these lines \"-by:\" lines\n> > \"signatures\", but perhaps there's a better name.\n> I think we casually call them footers (as they are counter-part to\n> \"headers\"), or trailers.\n\nI think they are neither footers, which would relate\nmore to the email headers, nor trailers.\n\nMaybe something like traces or chains.\n\nbtw: I submitted this awhile ago\n\nhttp://www.spinics.net/lists/git/msg162269.html\n\n"},{"id":"300584","messageId":"1472579363.5512.113.camel@perches.com","threadId":"43961","inReplyTo":"1472578879.5512.111.camel@perches.com","subject":"Re: git am and duplicate signatures","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-30T17:49:23Z","receivedAt":"2016-08-30T17:49:34Z","isPatch":false,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Tue, 2016-08-30 at 10:41 -0700, Joe Perches wrote:\n> Maybe something like traces or chains.\n\nOr \"taggers\" or \"tagged-bys\"\n\n"},{"id":"300589","messageId":"xmqqshtm3yur.fsf@gitster.mtv.corp.google.com","threadId":"43961","inReplyTo":"1472579363.5512.113.camel@perches.com","subject":"Re: git am and duplicate signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-30T18:17:48Z","receivedAt":"2016-08-30T18:17:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches writes:\n\n> On Tue, 2016-08-30 at 10:41 -0700, Joe Perches wrote:\n>> Maybe something like traces or chains.\n>\n> Or \"taggers\" or \"tagged-bys\"\n\nI am afraid that you are way too late; the ship has already sailed a\nfew years ago, if not earlier, I think.\n\n"},{"id":"300592","messageId":"1472581284.5512.117.camel@perches.com","threadId":"43961","inReplyTo":"xmqqshtm3yur.fsf@gitster.mtv.corp.google.com","subject":"Re: git am and duplicate signatures","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-30T18:21:24Z","receivedAt":"2016-08-30T18:21:33Z","isPatch":false,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Tue, 2016-08-30 at 11:17 -0700, Junio C Hamano wrote:\n> Joe Perches writes:\n> \n> > \n> > On Tue, 2016-08-30 at 10:41 -0700, Joe Perches wrote:\n> > > \n> > > Maybe something like traces or chains.\n> > Or \"taggers\" or \"tagged-bys\"\n> I am afraid that you are way too late; the ship has already sailed a\n> few years ago, if not earlier, I think.\n\nWhat's the ship's name?  Is it footers or trailers?\n\n"},{"id":"300598","messageId":"xmqq7fay3wux.fsf@gitster.mtv.corp.google.com","threadId":"43961","inReplyTo":"1472581284.5512.117.camel@perches.com","subject":"Re: git am and duplicate signatures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-30T19:00:54Z","receivedAt":"2016-08-30T19:01:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches writes:\n\n> On Tue, 2016-08-30 at 11:17 -0700, Junio C Hamano wrote:\n>> Joe Perches writes:\n>> \n>> > \n>> > On Tue, 2016-08-30 at 10:41 -0700, Joe Perches wrote:\n>> > > \n>> > > Maybe something like traces or chains.\n>> > Or \"taggers\" or \"tagged-bys\"\n>> I am afraid that you are way too late; the ship has already sailed a\n>> few years ago, if not earlier, I think.\n>\n> What's the ship's name? Is it footers or trailers?\n\nI think we casually call them footers (as they are counter-part to\n\"headers\"), or trailers (probably more official as that is half of\nthe name of the subsystem that is supposed to deal with them).\n\n"}]}