{"thread":{"id":"58908","subject":"Skipping adding Signed-off-by even if it's not the last on git commit","startedAt":"2022-12-06T17:07:37Z","lastAt":"2022-12-09T20:40:00Z","messageCount":14,"participants":["David Caro","Taylor Blau","Junio C Hamano","Ævar Arnfjörð Bjarmason","Konstantin Ryabitsev","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"468628","messageId":"20221206170646.6lnpr6h7oprziy5b@vulcanus","threadId":"58908","inReplyTo":null,"subject":"Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"David Caro","fromEmail":"dcaro@wikimedia.org","sentAt":"2022-12-06T17:06:46Z","receivedAt":"2022-12-06T17:07:37Z","isPatch":false,"sender":{"key":"dcaro@wikimedia.org","avatar":"https://gravatar.com/avatar/c38df122f2f80dd69364cf54709edd67d85d305120b69bf14bf48df912fd8fc4?d=mp&s=160"},"body":"\nHi!\n\nI have noticed that when requesting git commit to add the Signed-off-by header, it will add it even if it already exists\nas long as it's not the last in the footer.\n\nIt seems that the functionality to do so has been there for a while (see [1]) and it's being used effectively on\nformat-patch, but it was never enabled for the rest of commands.\n\nI propose enabling it for commit, merge and am.\n\nI can easily send a patch for it if the change is welcome.\n\n\nThanks!\n\n[1] https://github.com/git/git/commit/bab4d1097c8be7d688a53b992232063dbf300fd4\n\n-- \nDavid Caro\nSRE - Cloud Services\nWikimedia Foundation <https://wikimediafoundation.org/>\nPGP Signature: 7180 83A2 AC8B 314F B4CE  1171 4071 C7E1 D262 69C3\n\n\"Imagine a world in which every single human being can freely share in the\nsum of all knowledge. That's our commitment.\"\n"},{"id":"468672","messageId":"Y4/xSObs9QXvE+xR@nand.local","threadId":"58908","inReplyTo":"20221206170646.6lnpr6h7oprziy5b@vulcanus","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-12-07T01:50:00Z","receivedAt":"2022-12-07T01:50:42Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi David,\n\nOn Tue, Dec 06, 2022 at 06:06:46PM +0100, David Caro wrote:\n> I have noticed that when requesting git commit to add the\n> Signed-off-by header, it will add it even if it already exists as long\n> as it's not the last in the footer.\n>\n> It seems that the functionality to do so has been there for a while\n> (see [1]) and it's being used effectively on format-patch, but it was\n> never enabled for the rest of commands.\n\nRight; bab4d1097c (sequencer.c: teach append_signoff how to detect\nduplicate s-o-b, 2013-02-12) introduced the APPEND_SIGNOFF_DEDUP flag,\nwhich instructs append_signoff() to avoid adding a Signed-off-by\ntrailer if one already exists anywhere in the trailers section of the\ntarget commit.\n\nAs bab4d1097c hints, this is desirable in some situations. However,\nadding duplicated S-o-b trailers is useful in other situations. For\nexample, if you and I exchange a patch back and forth on this list, it\nmight go something like:\n\n  1. I write up an initial patch, add my Singed-off-by, and send it to\n     the list.\n\n  2. You notice that some aspects can be improved, so you apply and\n     modify the commit locally, adding your own Signed-off-by.\n\n  3. I pick up your changes as part of a new version of the patch\n     series, and resubmit it to the list, adding my own Signed-off-by\n     again.\n\nThere are a couple of small things worth noting there. First, in (2),\nthe changes that you introduced were large enough to be worth crediting\n(e.g., you didn't suggest a typofix or to modify the amount of spacing\non a line or similar), but not large enough to change the author of the\npatch.\n\nLikewise, in (3), the second instance of my Signed-off-by indicates that\nI am OK with the changes that you wrote, and I certify the new version\nof the patch as my own work.\n\nOne such instance is in 0ca6ead81e (alias.c: reject too-long cmdline\nstrings in split_cmdline(), 2022-09-28). Another is in b3c5f5cb04\n(submodule: move core cmd_update() logic to C, 2022-03-15).\n\nIf you're curious to dig up more in your own project, here's a little\nshell snippet I wrote up to find some examples:\n\n    git rev-list --no-merges HEAD |\n    while read rev\n    do\n      git -P show -s -1 --format='%(trailers:key=Signed-off-by,unfold=true)' \"$rev\" |\n      grep -v '^$' | sort | uniq -c | grep -vq \"^      1 \" && echo \"$rev\"\n    done\n\n> I propose enabling it for commit, merge and am.\n\nSo I think that there are some legitimate uses outside of 'format-patch'\nthat we may want to keep the existing behavior. So I don't think we\nshould necessarily change the default to have other commands outside of\n'format-patch' start passing APPEND_SIGNOFF_DEDUP.\n\nBut I could see a future where we add a new option that controls whether\nor not we pass that flag, perhaps:\n\n    $ git commit --signoff[=[no-]dedup]\n\n?\n\nThanks,\nTaylor\n"},{"id":"468681","messageId":"xmqqlenj7t0b.fsf@gitster.g","threadId":"58908","inReplyTo":"Y4/xSObs9QXvE+xR@nand.local","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-07T04:11:16Z","receivedAt":"2022-12-07T04:11:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n>> I propose enabling it for commit, merge and am.\n>\n> So I think that there are some legitimate uses outside of 'format-patch'\n> that we may want to keep the existing behavior. So I don't think we\n> should necessarily change the default to have other commands outside of\n> 'format-patch' start passing APPEND_SIGNOFF_DEDUP.\n>\n> But I could see a future where we add a new option that controls whether\n> or not we pass that flag, perhaps:\n>\n>     $ git commit --signoff[=[no-]dedup]\n\nThat sounds sensible.\n"},{"id":"468685","messageId":"221207.86y1rjako8.gmgdl@evledraar.gmail.com","threadId":"58908","inReplyTo":"Y4/xSObs9QXvE+xR@nand.local","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-07T04:31:18Z","receivedAt":"2022-12-07T04:43:11Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Dec 06 2022, Taylor Blau wrote:\n\n> Hi David,\n>\n> On Tue, Dec 06, 2022 at 06:06:46PM +0100, David Caro wrote:\n>> I have noticed that when requesting git commit to add the\n>> Signed-off-by header, it will add it even if it already exists as long\n>> as it's not the last in the footer.\n>>\n>> It seems that the functionality to do so has been there for a while\n>> (see [1]) and it's being used effectively on format-patch, but it was\n>> never enabled for the rest of commands.\n>\n> Right; bab4d1097c (sequencer.c: teach append_signoff how to detect\n> duplicate s-o-b, 2013-02-12) introduced the APPEND_SIGNOFF_DEDUP flag,\n> which instructs append_signoff() to avoid adding a Signed-off-by\n> trailer if one already exists anywhere in the trailers section of the\n> target commit.\n>\n> As bab4d1097c hints, this is desirable in some situations. However,\n> adding duplicated S-o-b trailers is useful in other situations. For\n> example, if you and I exchange a patch back and forth on this list, it\n> might go something like:\n>\n>   1. I write up an initial patch, add my Singed-off-by, and send it to\n>      the list.\n>\n>   2. You notice that some aspects can be improved, so you apply and\n>      modify the commit locally, adding your own Signed-off-by.\n>\n>   3. I pick up your changes as part of a new version of the patch\n>      series, and resubmit it to the list, adding my own Signed-off-by\n>      again.\n>\n> There are a couple of small things worth noting there. First, in (2),\n> the changes that you introduced were large enough to be worth crediting\n> (e.g., you didn't suggest a typofix or to modify the amount of spacing\n> on a line or similar), but not large enough to change the author of the\n> patch.\n>\n> Likewise, in (3), the second instance of my Signed-off-by indicates that\n> I am OK with the changes that you wrote, and I certify the new version\n> of the patch as my own work.\n>\n> One such instance is in 0ca6ead81e (alias.c: reject too-long cmdline\n> strings in split_cmdline(), 2022-09-28). Another is in b3c5f5cb04\n> (submodule: move core cmd_update() logic to C, 2022-03-15).\n>\n> If you're curious to dig up more in your own project, here's a little\n> shell snippet I wrote up to find some examples:\n>\n>     git rev-list --no-merges HEAD |\n>     while read rev\n>     do\n>       git -P show -s -1 --format='%(trailers:key=Signed-off-by,unfold=true)' \"$rev\" |\n>       grep -v '^$' | sort | uniq -c | grep -vq \"^      1 \" && echo \"$rev\"\n>     done\n\nThis is pretty much what I would have said, i.e. that it is intentional,\nas the SOB has do do with the DCO chronology of events.\n\nThe SOB is a proxy for passing the copyrightable work around, and to\ncertify that you have permission to license the work per the DCO etc.\n\nBut it's also interesting in that context that we choose to omit this\nfor re-rolls. I.e.:\n\n 1. I write a patch, that has 10 lines of original work, add a SOB\n 2. You pick it up, add another 10 lines, add your SOB\n 3. I pick it up again, add another 10 lines, add my SOB\n\nSo at the end we have a SOB sequence of: Ævar, Taylor, Ævar, and 30\ncopyrightable lines of code.\n\nBut if I submit three versions of my own patch with the same growth\npattern over those three iterations shouldn't I have 3 of my own SOB\nlines: Ævar, Ævar, Ævar?\n\nI'm not suggesting that's a sane idea, but at the same time I can't\nreally square the internal logic of why we'd do one, but not the\nother.\n\nIf we assume that an earlier SOB from a v1 is carried forward to my own\nv2 and v3 re-rolls, shouldn't the same convention apply for my v1 SOB if\nyou the v2 re-roll was yours?\n"},{"id":"468686","messageId":"xmqq7cz37qux.fsf@gitster.g","threadId":"58908","inReplyTo":"221207.86y1rjako8.gmgdl@evledraar.gmail.com","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-07T04:57:42Z","receivedAt":"2022-12-07T04:57:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> But if I submit three versions of my own patch with the same growth\n> pattern over those three iterations shouldn't I have 3 of my own SOB\n> lines: Ævar, Ævar, Ævar?\n\nThe difference, I think, is that it is not worth recording if it is\njust you regurgitated the same idea a few times yourself without\ninput from others.\n"},{"id":"468699","messageId":"20221207084027.7dhyaatkzaawrg4g@vulcanus","threadId":"58908","inReplyTo":"xmqqlenj7t0b.fsf@gitster.g","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"David Caro","fromEmail":"dcaro@wikimedia.org","sentAt":"2022-12-07T08:40:27Z","receivedAt":"2022-12-07T08:40:36Z","isPatch":false,"sender":{"key":"dcaro@wikimedia.org","avatar":"https://gravatar.com/avatar/c38df122f2f80dd69364cf54709edd67d85d305120b69bf14bf48df912fd8fc4?d=mp&s=160"},"body":"On 12/07 13:11, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n> \n> >> I propose enabling it for commit, merge and am.\n> >\n> > So I think that there are some legitimate uses outside of 'format-patch'\n> > that we may want to keep the existing behavior. So I don't think we\n> > should necessarily change the default to have other commands outside of\n> > 'format-patch' start passing APPEND_SIGNOFF_DEDUP.\n> >\n> > But I could see a future where we add a new option that controls whether\n> > or not we pass that flag, perhaps:\n> >\n> >     $ git commit --signoff[=[no-]dedup]\n> \n> That sounds sensible.\n\nAgree, I will implement this then for commit for now, thanks!\n\n-- \nDavid Caro\nSRE - Cloud Services\nWikimedia Foundation <https://wikimediafoundation.org/>\nPGP Signature: 7180 83A2 AC8B 314F B4CE  1171 4071 C7E1 D262 69C3\n\n\"Imagine a world in which every single human being can freely share in the\nsum of all knowledge. That's our commitment.\"\n"},{"id":"468721","messageId":"20221207150014.zophrheptrz7456n@nitro.local","threadId":"58908","inReplyTo":"221207.86y1rjako8.gmgdl@evledraar.gmail.com","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2022-12-07T15:00:14Z","receivedAt":"2022-12-07T15:02:25Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"On Wed, Dec 07, 2022 at 05:31:18AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> The SOB is a proxy for passing the copyrightable work around, and to\n> certify that you have permission to license the work per the DCO etc.\n> \n> But it's also interesting in that context that we choose to omit this\n> for re-rolls. I.e.:\n> \n>  1. I write a patch, that has 10 lines of original work, add a SOB\n>  2. You pick it up, add another 10 lines, add your SOB\n>  3. I pick it up again, add another 10 lines, add my SOB\n> \n> So at the end we have a SOB sequence of: Ævar, Taylor, Ævar, and 30\n> copyrightable lines of code.\n> \n> But if I submit three versions of my own patch with the same growth\n> pattern over those three iterations shouldn't I have 3 of my own SOB\n> lines: Ævar, Ævar, Ævar?\n\nI'm not sure the trailer order matters much for the copyright info, because\nafter all this information is in the output of \"git blame\" and \"git log\". That\nsaid, \"trailer order\" is a touchy subject with kernel developers, because\nit means different things to pretty much every kernel maintainer. With b4, I'm\nsticking to the \"chain of custody\" (COC) approach, which treats \"S-o-b\"\ntrailers as markers for where the chain of custody boundaries are.\n\nFor example, consider the following patch:\n\n    | [PATCH] foo: implement libbar\n    |\n    | This patch implements libbar.\n    |\n\t| Suggested-by: Reporter 1 <...>\n    | Link: https://msgid.link/some-msgid\n\t| Signed-off-by: Developer 1 <...>     -- initial COC boundary\n\t| Reviewed-by: Reviewer 1 <...>\n\t| Tested-by: Tester 1 <...>\n\t| Signed-off-by: Submaintainer 1 <...> -- intermediate COC boundary\n\t| Acked-by: Submaintainer 2 <...>\n\t| Signed-off-by: Maintainer 1 <...>    -- final COC boundary\n\nIn terms of COC, this patch makes the following claims:\n\nDeveloper 1:\n - I am responsible for this code\n - It was suggested by Reporter 1\n - You can read about it at this URL\n\nSubmaintainer 1:\n - I am signing off on this code\n - I am the one who added the trailers from Reviewer 1 and Tester 1\n\nMaintainer 1:\n - I am signing off on this code\n - I am the one who added the trailer from Submaintainer 2\n\nIn the chain-of-custody scheme multiple identical trailers don't make sense,\nas far as I can tell. If the patch doesn't pass review and someone returns it\nback to the original developer, their \"S-o-b: Developer 1\" trailer simply\nmoves down to be the last entry in v2 of the patch, and the developer should\nremove any code review trailers that were issued for the previous version of\nthe patch.\n\nThis is really the only order that makes sense to me. :)\n\n-K\n"},{"id":"468737","messageId":"Y5EQCD4XCsN10HO+@nand.local","threadId":"58908","inReplyTo":"20221207084027.7dhyaatkzaawrg4g@vulcanus","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-12-07T22:13:28Z","receivedAt":"2022-12-07T22:13:33Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Dec 07, 2022 at 09:40:27AM +0100, David Caro wrote:\n> On 12/07 13:11, Junio C Hamano wrote:\n> > Taylor Blau <me@ttaylorr.com> writes:\n> >\n> > >> I propose enabling it for commit, merge and am.\n> > >\n> > > So I think that there are some legitimate uses outside of 'format-patch'\n> > > that we may want to keep the existing behavior. So I don't think we\n> > > should necessarily change the default to have other commands outside of\n> > > 'format-patch' start passing APPEND_SIGNOFF_DEDUP.\n> > >\n> > > But I could see a future where we add a new option that controls whether\n> > > or not we pass that flag, perhaps:\n> > >\n> > >     $ git commit --signoff[=[no-]dedup]\n> >\n> > That sounds sensible.\n>\n> Agree, I will implement this then for commit for now, thanks!\n\nThanks, I look forward to seeing your work. It would be nice to\nstandardize on this `--signoff[=[no-]dedup]` thing throughout all of the\ndifferent commands that support it.\n\nAccording to:\n\n    $ git grep -- '--signoff' -- Documentation/git-*.txt\n\nthat list is: `git am`, `git cherry-pick`, `git format-patch`, `git\nrebase`, and `git revert`. But there are others, too, via an include of\n`signoff-option.txt`, so that is `git commit`, `git merge`, and `git\npull`.\n\nPhew ;-).\n\nThanks,\nTaylor\n"},{"id":"468746","messageId":"xmqqtu266cj5.fsf@gitster.g","threadId":"58908","inReplyTo":"Y5EQCD4XCsN10HO+@nand.local","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-07T23:04:46Z","receivedAt":"2022-12-07T23:04:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n>> > >     $ git commit --signoff[=[no-]dedup]\n> ...\n> Thanks, I look forward to seeing your work. It would be nice to\n> standardize on this `--signoff[=[no-]dedup]` thing throughout all of the\n> different commands that support it.\n\nAlso, if I am not mistaken, each of trailers can be configured to\nhave its own semantics (e.g. .where and .ifExists).\n\n * Should we have similar override to these trailer tokens, not just\n   sign-off?\n\n * Should we offer not just [no-]dedup (which is equivalent to\n   switching from addIfDifferentNeighbor to addIfDifferent) but\n   other customization?  This affects what --signoff should be\n   allowed to take for consistency across trailers.\n\n\n"},{"id":"468771","messageId":"Y5GRx86i3ZIiVxb3@coredump.intra.peff.net","threadId":"58908","inReplyTo":"xmqqtu266cj5.fsf@gitster.g","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-08T07:27:03Z","receivedAt":"2022-12-08T07:27:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 08, 2022 at 08:04:46AM +0900, Junio C Hamano wrote:\n\n> Taylor Blau <me@ttaylorr.com> writes:\n> \n> >> > >     $ git commit --signoff[=[no-]dedup]\n> > ...\n> > Thanks, I look forward to seeing your work. It would be nice to\n> > standardize on this `--signoff[=[no-]dedup]` thing throughout all of the\n> > different commands that support it.\n> \n> Also, if I am not mistaken, each of trailers can be configured to\n> have its own semantics (e.g. .where and .ifExists).\n> \n>  * Should we have similar override to these trailer tokens, not just\n>    sign-off?\n\nThis made me curious about the opposite: is there config you can set to\nget this behavior for --signoff? I think the answer is \"no\". You can do:\n\n  git -c trailer.ifExists=addIfDifferent \\\n      commit --amend --trailer=\"Signed-off-by: Jeff King <peff@peff.net>\"\n\nto get the desired behavior, but using \"--signoff\" does not respect\ntrailer settings.\n\nI feel like config is probably a better match for the use cases here,\nbecause the decision about de-duping is not something you'd usually set\nfor one particular operation, but is more likely to be a project policy\nabout what the trailer means (and that includes Signed-off-by). So you'd\nwant to set it per-repo, not per-operation.\n\n>  * Should we offer not just [no-]dedup (which is equivalent to\n>    switching from addIfDifferentNeighbor to addIfDifferent) but\n>    other customization?  This affects what --signoff should be\n>    allowed to take for consistency across trailers.\n\nYeah, writing the above made me wonder if --signoff should behave\nexactly like:\n\n  --trailer=\"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\"\n\nFor the most part that would not change any behavior unless you set\ntrailer.signed-off-by.*, and setting those presumably signals intent\nthat you expect them to take effect. The one exception is that the\ngeneric trailer.ifExists, etc, would start affecting --signoff, which\n_might_ be a surprise. If we wanted to retain the behavior there, we\ncould say \"--signoff is special, and doesn't respect generic trailer\nconfig\".\n\nAlternatively, it would be nice if there was an easy way to put your\nident into a trailer (what I wrote above doesn't really work unless you\nhave those variables in your environment, and of course it's a lot of\ntyping). I think you can hack it up like:\n\n  git config trailer.sign.key Signed-off-by\n  git config trailer.sign.cmd \\\n    'git var GIT_COMMITTER_IDENT | sed \"s/>.*/>/\";:'\n  git commit --trailer=sign\n\nwhich is only a little more typing than --signoff, but it's not very\nergonomic.\n\n-Peff\n"},{"id":"468808","messageId":"xmqq5yel2wuv.fsf@gitster.g","threadId":"58908","inReplyTo":"Y5GRx86i3ZIiVxb3@coredump.intra.peff.net","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-09T01:23:52Z","receivedAt":"2022-12-09T01:24:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This made me curious about the opposite: is there config you can set to\n> get this behavior for --signoff? I think the answer is \"no\". You can do:\n>\n>   git -c trailer.ifExists=addIfDifferent \\\n>       commit --amend --trailer=\"Signed-off-by: Jeff King <peff@peff.net>\"\n>\n> to get the desired behavior, but using \"--signoff\" does not respect\n> trailer settings.\n\nA customization to allow addIfDifferent may be an interesting idea.\n\n> I feel like config is probably a better match for the use cases here,\n> because the decision about de-duping is not something you'd usually set\n> for one particular operation, but is more likely to be a project policy\n> about what the trailer means (and that includes Signed-off-by). So you'd\n> want to set it per-repo, not per-operation.\n\nSure.\n\n> ... The one exception is that the\n> generic trailer.ifExists, etc, would start affecting --signoff, which\n> _might_ be a surprise. If we wanted to retain the behavior there, we\n> could say \"--signoff is special, and doesn't respect generic trailer\n> config\".\n\nYeah, that may be safe, however it is very unsatisfying.\n\n> Alternatively, it would be nice if there was an easy way to put your\n> ident into a trailer (what I wrote above doesn't really work unless you\n> have those variables in your environment, and of course it's a lot of\n> typing). I think you can hack it up like:\n>\n>   git config trailer.sign.key Signed-off-by\n>   git config trailer.sign.cmd \\\n>     'git var GIT_COMMITTER_IDENT | sed \"s/>.*/>/\";:'\n>   git commit --trailer=sign\n>\n> which is only a little more typing than --signoff, but it's not very\n> ergonomic.\n\nIt does not look _too_ bad, though.\n"},{"id":"468810","messageId":"Y5KSfsxLO7com2f9@coredump.intra.peff.net","threadId":"58908","inReplyTo":"xmqq5yel2wuv.fsf@gitster.g","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-09T01:42:22Z","receivedAt":"2022-12-09T01:42:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 09, 2022 at 10:23:52AM +0900, Junio C Hamano wrote:\n\n> > ... The one exception is that the\n> > generic trailer.ifExists, etc, would start affecting --signoff, which\n> > _might_ be a surprise. If we wanted to retain the behavior there, we\n> > could say \"--signoff is special, and doesn't respect generic trailer\n> > config\".\n> \n> Yeah, that may be safe, however it is very unsatisfying.\n\nAgreed. ;)\n\n> > Alternatively, it would be nice if there was an easy way to put your\n> > ident into a trailer (what I wrote above doesn't really work unless you\n> > have those variables in your environment, and of course it's a lot of\n> > typing). I think you can hack it up like:\n> >\n> >   git config trailer.sign.key Signed-off-by\n> >   git config trailer.sign.cmd \\\n> >     'git var GIT_COMMITTER_IDENT | sed \"s/>.*/>/\";:'\n> >   git commit --trailer=sign\n> >\n> > which is only a little more typing than --signoff, but it's not very\n> > ergonomic.\n> \n> It does not look _too_ bad, though.\n\nWhat I don't like about it is:\n\n  - the external cmd is complicated and slow. It would be nice if you\n    could just set trailer.sign.ident=true or something, and it would\n    use your ident by default if no value is given (and maybe even do\n    completion similar to \"commit --author\" if a value is given).\n\n  - you have to know to be clever enough to define and use\n    --trailer=sign. If --signoff didn't exist, that's not too big a\n    stretch. But since it does, everyone will naturally reach for it\n    first.\n\nThe two could be solved independently, of course. And even if we did\nwant to solve both by making --signoff behave more like other trailers,\nhaving trailer.*.ident might be nice in general.\n\nI admit I just thought of this concept, though, so I don't have plans to\nwork up patches, and I make no claims there isn't a gotcha waiting\nsomewhere. ;)\n\n-Peff\n"},{"id":"468816","messageId":"xmqqcz8t1avz.fsf@gitster.g","threadId":"58908","inReplyTo":"Y5KSfsxLO7com2f9@coredump.intra.peff.net","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-09T04:03:44Z","receivedAt":"2022-12-09T04:03:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> >   git config trailer.sign.key Signed-off-by\n>> >   git config trailer.sign.cmd \\\n>> >     'git var GIT_COMMITTER_IDENT | sed \"s/>.*/>/\";:'\n>> >   git commit --trailer=sign\n>> >\n>> > which is only a little more typing than --signoff, but it's not very\n>> > ergonomic.\n>> \n>> It does not look _too_ bad, though.\n>\n> What I don't like about it is:\n>\n>   - the external cmd is complicated and slow. It would be nice if you\n>     could just set trailer.sign.ident=true or something, and it would\n>     use your ident by default if no value is given (and maybe even do\n>     completion similar to \"commit --author\" if a value is given).\n\nAh, \"trailer.sign.value\" to use the same value does not exist, and\nthe closest kludge we can use is the .cmd thing?  Then it is a shame\nbut it should be easy to correct?\n\n>   - you have to know to be clever enough to define and use\n>     --trailer=sign. If --signoff didn't exist, that's not too big a\n>     stretch. But since it does, everyone will naturally reach for it\n>     first.\n\nWe could reimplement --signoff to actually take attention to the\n\"trailer.sign.*\" thing, if we wanted to, and that makes it very easy\nto explain, I guess.\n"},{"id":"468841","messageId":"Y5OdHL4VFCIjgrk7@coredump.intra.peff.net","threadId":"58908","inReplyTo":"xmqqcz8t1avz.fsf@gitster.g","subject":"Re: Skipping adding Signed-off-by even if it's not the last on git commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-09T20:39:56Z","receivedAt":"2022-12-09T20:40:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 09, 2022 at 01:03:44PM +0900, Junio C Hamano wrote:\n\n> > What I don't like about it is:\n> >\n> >   - the external cmd is complicated and slow. It would be nice if you\n> >     could just set trailer.sign.ident=true or something, and it would\n> >     use your ident by default if no value is given (and maybe even do\n> >     completion similar to \"commit --author\" if a value is given).\n> \n> Ah, \"trailer.sign.value\" to use the same value does not exist, and\n> the closest kludge we can use is the .cmd thing?  Then it is a shame\n> but it should be easy to correct?\n\nYeah, I almost suggested trailer.sign.value at first, but on further\nthought, it would be kind of weird. When the user asks for a trailer to\nbe included, it usually already has a value along with it. This could be\nthought of as a default value, which might work like:\n\n  git config trailer.signed-off-by.default \"Jeff King <peff@peff.net>\"\n\nThat's pretty flexible, but it seems like what you really want is not\nsome string, but to say \"my usual committer ident\". And so we force you\nto repeat what's in user.email, etc, or we have to invent some new token\nthat means \"my usual ident\". So if we instead have a flag for \"this is\nan ident\", then it can just do the expected thing.\n\n> >   - you have to know to be clever enough to define and use\n> >     --trailer=sign. If --signoff didn't exist, that's not too big a\n> >     stretch. But since it does, everyone will naturally reach for it\n> >     first.\n> \n> We could reimplement --signoff to actually take attention to the\n> \"trailer.sign.*\" thing, if we wanted to, and that makes it very easy\n> to explain, I guess.\n\nYeah. My only reservation would be whether anybody would be surprised by\nthe change in behavior. I have the feeling that hardly anybody uses\ntrailer.* config, but I admit that's not based in any real data.\n\n-Peff\n"}]}