{"thread":{"id":"51818","subject":"Re: [Qemu-devel] [PATCH v6 0/4] 9p: Fix file ID collisions","startedAt":"2019-09-09T14:05:50Z","lastAt":"2019-09-24T21:36:41Z","messageCount":7,"participants":["Eric Blake","Jeff King","Junio C Hamano","Christian Schoenebeck"],"isPatch":true,"patchVersion":6,"patchTotal":4},"messages":[{"id":"382064","messageId":"305577c2-709a-b632-4056-6582771176ac@redhat.com","threadId":"51818","inReplyTo":"1897173.eDCz7oYxVq@silver","subject":"Re: [Qemu-devel] [PATCH v6 0/4] 9p: Fix file ID collisions","fromName":"Eric Blake","fromEmail":"eblake@redhat.com","sentAt":"2019-09-09T14:05:45Z","receivedAt":"2019-09-09T14:05:50Z","isPatch":true,"sender":{"key":"eblake@redhat.com","avatar":"https://avatars.githubusercontent.com/u/32933908?v=4"},"body":"[adding git list]\n\nOn 9/5/19 7:25 AM, Christian Schoenebeck wrote:\n\n>>>>> How are you sending patches ? With git send-email ? If so, maybe you\n>>>>> can\n>>>>> pass something like --from='\"Christian Schoenebeck\"\n>>>>> <qemu_oss@crudebyte.com>'. Since this is a different string, git will\n>>>>> assume you're sending someone else's patch : it will automatically add\n>>>>> an\n>>>>> extra From: made out of the commit Author as recorded in the git tree.\n>>>\n>>> I think it is probably as simple as a 'git config' command to tell git\n>>> to always put a 'From:' in the body of self-authored patches when using\n>>> git format-patch; however, as I don't suffer from munged emails, I\n>>> haven't actually tested what that setting would be.\n> \n> Well, I tried that Eric. The expected solution would be enabling this git \n> setting:\n> \n> git config [--global] format.from true\n> https://git-scm.com/docs/git-config#Documentation/git-config.txt-formatfrom\n> \n> But as you can already read from the manual, the overall behaviour of git \n> regarding a separate \"From:\" line in the email body was intended solely for \n> the use case sender != author. So in practice (at least in my git version) git \n> always makes a raw string comparison between sender (name and email) string \n> and author string and only adds the separate From: line to the body if they \n> differ.\n> \n> Hence also \"git format-patch --from=\" only works here if you use a different \n> author string (name and email) there, otherwise on a perfect string match it \n> is simply ignored and you end up with only one \"From:\" in the email header.\n\ngit folks:\n\nHow hard would it be to improve 'git format-patch'/'git send-email' to\nhave an option to ALWAYS output a From: line in the body, even when the\nsender is the author, for the case of a mailing list that munges the\nmail headers due to DMARC/DKIM reasons?\n\n> \n> So eventually I added one extra character in my name for now and removed it \n> manually in the dumped emails subsequently (see today's\n> \"[PATCH v7 0/3] 9p: Fix file ID collisions\").\n> \n> Besides that direct string comparison restriction; git also seems to have a \n> bug here. Because even if you have sender != author, then git falsely uses \n> author as sender of the cover letter, whereas the emails of the individual \n> patches are encoded correctly.\n\nAt any rate, I'm glad that you have figured out a workaround, even if\npainful, while we wait for git to provide what we really need.\n\n\n-- \nEric Blake, Principal Software Engineer\nRed Hat, Inc.           +1-919-301-3226\nVirtualization:  qemu.org | libvirt.org\n\n"},{"id":"382066","messageId":"20190909142511.GA20726@sigill.intra.peff.net","threadId":"51818","inReplyTo":"305577c2-709a-b632-4056-6582771176ac@redhat.com","subject":"Re: [Qemu-devel] [PATCH v6 0/4] 9p: Fix file ID collisions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-09-09T14:25:12Z","receivedAt":"2019-09-09T14:25:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 09, 2019 at 09:05:45AM -0500, Eric Blake wrote:\n\n> > But as you can already read from the manual, the overall behaviour of git \n> > regarding a separate \"From:\" line in the email body was intended solely for \n> > the use case sender != author. So in practice (at least in my git version) git \n> > always makes a raw string comparison between sender (name and email) string \n> > and author string and only adds the separate From: line to the body if they \n> > differ.\n> > \n> > Hence also \"git format-patch --from=\" only works here if you use a different \n> > author string (name and email) there, otherwise on a perfect string match it \n> > is simply ignored and you end up with only one \"From:\" in the email header.\n> \n> git folks:\n> \n> How hard would it be to improve 'git format-patch'/'git send-email' to\n> have an option to ALWAYS output a From: line in the body, even when the\n> sender is the author, for the case of a mailing list that munges the\n> mail headers due to DMARC/DKIM reasons?\n\nIt wouldn't be very hard to ask format-patch to just handle this\nunconditionally. Something like:\n\ndiff --git a/pretty.c b/pretty.c\nindex e4ed14effe..9cf79d7874 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -451,7 +451,8 @@ void pp_user_info(struct pretty_print_context *pp,\n \t\tmap_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n \n \tif (cmit_fmt_is_mail(pp->fmt)) {\n-\t\tif (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {\n+\t\tif (pp->always_use_in_body_from ||\n+\t\t    (pp->from_ident && ident_cmp(pp->from_ident, &ident))) {\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n \n \t\t\tstrbuf_addstr(&buf, \"From: \");\n\nbut most of the work would be ferrying that option from the command line\ndown to the pretty-print code.\n\nThat would work in conjunction with \"--from\" to avoid a duplicate. It\nmight require send-email learning about the option to avoid doing its\nown in-body-from management. If you only care about send-email, it might\nbe easier to just add the option there.\n\n-Peff\n"},{"id":"382086","messageId":"xmqqd0g9jorg.fsf@gitster-ct.c.googlers.com","threadId":"51818","inReplyTo":"305577c2-709a-b632-4056-6582771176ac@redhat.com","subject":"Re: [Qemu-devel] [PATCH v6 0/4] 9p: Fix file ID collisions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-09T18:41:39Z","receivedAt":"2019-09-09T18:41:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Blake <eblake@redhat.com> writes:\n\n> How hard would it be to improve 'git format-patch'/'git send-email' to\n> have an option to ALWAYS output a From: line in the body, even when the\n> sender is the author, for the case of a mailing list that munges the\n> mail headers due to DMARC/DKIM reasons?\n\nI'd say that it shouldn't be so hard to implement than realizing\nwhat ahd why it is needed, designing what the end-user interaction\nwould be (i.e.  command line options?  configuration variables?\nshould it be per send-email destination?) and stating all of the\nabove clearly in the documentation and the proposed commit log\nmessage.\n\nThe reason you are asking is...?  Am I smelling a volunteer?\n"},{"id":"382764","messageId":"56046367.TiUlWITyhT@silver","threadId":"51818","inReplyTo":"20190909142511.GA20726@sigill.intra.peff.net","subject":"Re: [Qemu-devel] [PATCH v6 0/4] 9p: Fix file ID collisions","fromName":"Christian Schoenebeck","fromEmail":"qemu_oss@crudebyte.com","sentAt":"2019-09-23T11:19:18Z","receivedAt":"2019-09-23T11:36:31Z","isPatch":true,"sender":{"key":"qemu_oss@crudebyte.com","avatar":null},"body":"On Montag, 9. September 2019 16:25:12 CEST Jeff King wrote:\n> On Mon, Sep 09, 2019 at 09:05:45AM -0500, Eric Blake wrote:\n> > > But as you can already read from the manual, the overall behaviour of\n> > > git\n> > > regarding a separate \"From:\" line in the email body was intended solely\n> > > for\n> > > the use case sender != author. So in practice (at least in my git\n> > > version) git always makes a raw string comparison between sender (name\n> > > and email) string and author string and only adds the separate From:\n> > > line to the body if they differ.\n> > > \n> > > Hence also \"git format-patch --from=\" only works here if you use a\n> > > different author string (name and email) there, otherwise on a perfect\n> > > string match it is simply ignored and you end up with only one \"From:\"\n> > > in the email header.> \n> > git folks:\n> > \n> > How hard would it be to improve 'git format-patch'/'git send-email' to\n> > have an option to ALWAYS output a From: line in the body, even when the\n> > sender is the author, for the case of a mailing list that munges the\n> > mail headers due to DMARC/DKIM reasons?\n> \n> It wouldn't be very hard to ask format-patch to just handle this\n> unconditionally. Something like:\n> \n> diff --git a/pretty.c b/pretty.c\n> index e4ed14effe..9cf79d7874 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -451,7 +451,8 @@ void pp_user_info(struct pretty_print_context *pp,\n>  \t\tmap_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n> \n>  \tif (cmit_fmt_is_mail(pp->fmt)) {\n> -\t\tif (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {\n> +\t\tif (pp->always_use_in_body_from ||\n> +\t\t    (pp->from_ident && ident_cmp(pp->from_ident, &ident))) {\n>  \t\t\tstruct strbuf buf = STRBUF_INIT;\n> \n>  \t\t\tstrbuf_addstr(&buf, \"From: \");\n> \n> but most of the work would be ferrying that option from the command line\n> down to the pretty-print code.\n> \n> That would work in conjunction with \"--from\" to avoid a duplicate. It\n> might require send-email learning about the option to avoid doing its\n> own in-body-from management. If you only care about send-email, it might\n> be easier to just add the option there.\n\nWould it simplify the changes in git if that would be made a\n\"git config [--global]\" setting only? That is, would that probably simplify \nthat task to one simple function call there in pretty.c?\n\nOn the other hand, considering the already existing --from argument and \n\"format.from\" config option:\nhttps://git-scm.com/docs/git-config#Documentation/git-config.txt-formatfrom\n\nWouldn't it make sense to just drop the currently existing sender != author \nstring comparison in git and simply always add the \"From:\" line to the email's \nbody if \"format.from yes\" is used, instead of introducing a suggested 2nd \n(e.g. \"always-from\") option? I mean sure automatically removing redundant \ninformation in the generated emails if sender == author sounds nice on first \nthought, but does it address anything useful in practice to justify \nintroduction of a 2nd related option?\n\nBest regards,\nChristian Schoenebeck\n\n\n"},{"id":"382803","messageId":"20190923222415.GA22495@sigill.intra.peff.net","threadId":"51818","inReplyTo":"56046367.TiUlWITyhT@silver","subject":"Re: [Qemu-devel] [PATCH v6 0/4] 9p: Fix file ID collisions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-09-23T22:24:15Z","receivedAt":"2019-09-23T22:26:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 23, 2019 at 01:19:18PM +0200, Christian Schoenebeck wrote:\n\n> >  \tif (cmit_fmt_is_mail(pp->fmt)) {\n> > -\t\tif (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {\n> > +\t\tif (pp->always_use_in_body_from ||\n> > +\t\t    (pp->from_ident && ident_cmp(pp->from_ident, &ident))) {\n> >  \t\t\tstruct strbuf buf = STRBUF_INIT;\n> > \n> >  \t\t\tstrbuf_addstr(&buf, \"From: \");\n> > \n> > but most of the work would be ferrying that option from the command line\n> > down to the pretty-print code.\n> > \n> > That would work in conjunction with \"--from\" to avoid a duplicate. It\n> > might require send-email learning about the option to avoid doing its\n> > own in-body-from management. If you only care about send-email, it might\n> > be easier to just add the option there.\n> \n> Would it simplify the changes in git if that would be made a\n> \"git config [--global]\" setting only? That is, would that probably simplify \n> that task to one simple function call there in pretty.c?\n\nI think a config option would make sense, but we generally try to avoid\nadding a config option that doesn't have a matching command-line option.\n\nI also think saving implementation work there is orthogonal. You can as\neasily make a global \"always_use_in_body_from\" as you can call a global\nconfig_get_bool(\"format-patch.always_use_in_body_from\"). :)\n\nAnd anyway, it's not _that_ much work to pass it around. At least as\nmuch would go into writing documentation and tests. One of the reasons I\nleft the patch above as a sketch is that I'm not 100% convinced this is\na useful feature. Somebody caring enough about it to make a real patch\nwould send a signal there.\n\n> On the other hand, considering the already existing --from argument and \n> \"format.from\" config option:\n> https://git-scm.com/docs/git-config#Documentation/git-config.txt-formatfrom\n> \n> Wouldn't it make sense to just drop the currently existing sender != author \n> string comparison in git and simply always add the \"From:\" line to the email's \n> body if \"format.from yes\" is used, instead of introducing a suggested 2nd \n> (e.g. \"always-from\") option? I mean sure automatically removing redundant \n> information in the generated emails if sender == author sounds nice on first \n> thought, but does it address anything useful in practice to justify \n> introduction of a 2nd related option?\n\nYes, the resulting mail would be correct, in the sense that it could be\napplied just fine by git-am. But I think it would be uglier. IOW, I\nconsider the presence of the in-body From to be a clue that something\ninteresting is going on (like forwarding somebody else's patch). So from\nmy perspective, it would just be useless noise. Other communities may\nhave different opinions, though (I think I have seen some kernel folks\nalways including all of the possible in-body headers, including Date).\nBut it seems like it makes sense to keep both possibilities.\n\n-Peff\n"},{"id":"382838","messageId":"3312839.Zbq2WQg2AT@silver","threadId":"51818","inReplyTo":"20190923222415.GA22495@sigill.intra.peff.net","subject":"git format.from (was: 9p: Fix file ID collisions)","fromName":"Christian Schoenebeck","fromEmail":"qemu_oss@crudebyte.com","sentAt":"2019-09-24T09:03:38Z","receivedAt":"2019-09-24T09:03:48Z","isPatch":false,"sender":{"key":"qemu_oss@crudebyte.com","avatar":null},"body":"On Dienstag, 24. September 2019 00:24:15 CEST Jeff King wrote:\n> > On the other hand, considering the already existing --from argument and\n> > \"format.from\" config option:\n> > https://git-scm.com/docs/git-config#Documentation/git-config.txt-formatfro\n> > m\n> > \n> > Wouldn't it make sense to just drop the currently existing sender !=\n> > author\n> > string comparison in git and simply always add the \"From:\" line to the\n> > email's body if \"format.from yes\" is used, instead of introducing a\n> > suggested 2nd (e.g. \"always-from\") option? I mean sure automatically\n> > removing redundant information in the generated emails if sender ==\n> > author sounds nice on first thought, but does it address anything useful\n> > in practice to justify introduction of a 2nd related option?\n> \n> Yes, the resulting mail would be correct, in the sense that it could be\n> applied just fine by git-am. But I think it would be uglier. IOW, I\n> consider the presence of the in-body From to be a clue that something\n> interesting is going on (like forwarding somebody else's patch). So from\n> my perspective, it would just be useless noise. Other communities may\n> have different opinions, though (I think I have seen some kernel folks\n> always including all of the possible in-body headers, including Date).\n> But it seems like it makes sense to keep both possibilities.\n\nExactly, current git behaviour is solely \"prettier\" (at first thought only \nthough), but does not address anything useful in real life.\n\nCurrent git behaviour does cause real life problems though: Many email lists \nare munging emails of patch senders whose domain is configured for requiring \ndomain's emails being DKIM signed and/or being subject to SPF rules (a.k.a \nDMARC). So original sender's From: header is then automatically replaced by an \nalias (by e.g. mailman): https://en.wikipedia.org/wiki/DMARC#From:_rewriting\n\nFor instance the email header:\n\nFrom: \"Bob Bold\" <bold@foo.com>\n\nis automatically replaced by lists by something like\n\nFrom: \"Bob Bold via Somelist\" <somelist@gnu.org>\n\nAnd since git currently always drops the From: line from the email's body if\nsender == author, as a consequence maintainers applying patches from such \nlists, always need to rewrite git history subsequently and have to replace \npatch author's identity manually for each commit to have their correct, real \nemail address and real name in git history instead of something like\n\"Bob Bold via Somelist\" <somelist@gnu.org>\n\nSo what do you find \"uglier\"? I prefer key info not being lost as default \nbehaviour. :-)\n\nBest regards,\nChristian Schoenebeck\n\n\n"},{"id":"382876","messageId":"20190924213638.GE20858@sigill.intra.peff.net","threadId":"51818","inReplyTo":"3312839.Zbq2WQg2AT@silver","subject":"Re: git format.from (was: 9p: Fix file ID collisions)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-09-24T21:36:38Z","receivedAt":"2019-09-24T21:36:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 24, 2019 at 11:03:38AM +0200, Christian Schoenebeck wrote:\n\n> > Yes, the resulting mail would be correct, in the sense that it could be\n> > applied just fine by git-am. But I think it would be uglier. IOW, I\n> > consider the presence of the in-body From to be a clue that something\n> > interesting is going on (like forwarding somebody else's patch). So from\n> > my perspective, it would just be useless noise. Other communities may\n> > have different opinions, though (I think I have seen some kernel folks\n> > always including all of the possible in-body headers, including Date).\n> > But it seems like it makes sense to keep both possibilities.\n> \n> Exactly, current git behaviour is solely \"prettier\" (at first thought only \n> though), but does not address anything useful in real life.\n\nI wouldn't agree with that. By being pretty, it also is functionally\nmore useful (I can tell at a glance whether somebody is sending a patch\nfrom another author).\n\n> Current git behaviour does cause real life problems though: Many email lists \n> are munging emails of patch senders whose domain is configured for requiring \n> domain's emails being DKIM signed and/or being subject to SPF rules (a.k.a \n> DMARC). So original sender's From: header is then automatically replaced by an \n> alias (by e.g. mailman): https://en.wikipedia.org/wiki/DMARC#From:_rewriting\n> \n> For instance the email header:\n> \n> From: \"Bob Bold\" <bold@foo.com>\n> \n> is automatically replaced by lists by something like\n> \n> From: \"Bob Bold via Somelist\" <somelist@gnu.org>\n> \n> And since git currently always drops the From: line from the email's body if\n> sender == author, as a consequence maintainers applying patches from such \n> lists, always need to rewrite git history subsequently and have to replace \n> patch author's identity manually for each commit to have their correct, real \n> email address and real name in git history instead of something like\n> \"Bob Bold via Somelist\" <somelist@gnu.org>\n> \n> So what do you find \"uglier\"? I prefer key info not being lost as default \n> behaviour. :-)\n\nSure, for your list that munges From headers, always including an\nin-body From is way better. But for those of us _not_ on such lists, I'd\nmuch prefer not to force the in-body version on them.\n\n-Peff\n"}]}