| Message ID | 20180823102502.28122-1-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | scripts: report on author emails that are mangled by the mailing list | expand |
On Thu, 23 Aug 2018 11:25:02 +0100 Daniel P. Berrangé <berrange@redhat.com> wrote: > In some cases the Author: email address in patches submitted to the > list gets mangled such that it says > > John Doe via Qemu-devel <qemu-devel@nongnu.org> > > This change is a result of workarounds for DMARC policies. > > Subsystem maintainers accepting patches need to catch these and fix > them before sending pull requests, so a checkpatch.pl test is highly > desirable. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > scripts/checkpatch.pl | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 42e1c50dd8..be2114a179 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1402,6 +1402,10 @@ sub process { > $is_patch = 1; > } > > + if ($line =~ /^Author: .*via Qemu-devel.*<qemu-devel\@nongnu.org>/) { > + ERROR("Author email address is mangled by the mailing list\n" . $herecurr); > + } > + > #check the patch for a signoff: > if ($line =~ /^\s*signed-off-by:/i) { > # This is a signoff, if ugly, so do not double report. Acked-by: Cornelia Huck <cohuck@redhat.com>
On 23 August 2018 at 11:25, Daniel P. Berrangé <berrange@redhat.com> wrote: > In some cases the Author: email address in patches submitted to the > list gets mangled such that it says > > John Doe via Qemu-devel <qemu-devel@nongnu.org> > > This change is a result of workarounds for DMARC policies. I do wonder if overall we'd be better off with the other workaround (don't change From lines in list emails, don't add "[Qemu-devel]" prefix to subject lines)... It would be a one-off annoyance to people who filter list mail on subject rather than other headers, but it would get rid of this annoying wart where a handful of senders end up with their mail with a from address that our patch management tools don't handle very well... thanks -- PMM
On Thu, Aug 23, 2018 at 11:41:20AM +0100, Peter Maydell wrote: > On 23 August 2018 at 11:25, Daniel P. Berrangé <berrange@redhat.com> wrote: > > In some cases the Author: email address in patches submitted to the > > list gets mangled such that it says > > > > John Doe via Qemu-devel <qemu-devel@nongnu.org> > > > > This change is a result of workarounds for DMARC policies. > > I do wonder if overall we'd be better off with the other > workaround (don't change From lines in list emails, don't > add "[Qemu-devel]" prefix to subject lines)... It would be > a one-off annoyance to people who filter list mail on subject > rather than other headers, but it would get rid of this annoying > wart where a handful of senders end up with their mail with > a from address that our patch management tools don't handle > very well... I kind of like seeing the qemu-devel prefix, since I don't tend to pre-filter email into separate folders per list. I only do per-list filtering (on headers) once at start of the day when reading the overnight backlog. For rest of the day have a single inbox and tend to rely on subject alone. Of course the prefix isn't entirely reliable regardless, as if you are CC'd on mails you'll probably see the copy delivered via the CC long before you get the copy via the list. AFAIK, the libvirt mailing list isn't mangling From lines, and it also has a subject prefix added, so I wonder why it doesn't have a need for the same workaround wrt DMARC ? Or maybe it does need it and we've never realized. Regards, Daniel
On 08/23/2018 05:25 AM, Daniel P. Berrangé wrote: > In some cases the Author: email address in patches submitted to the > list gets mangled such that it says > > John Doe via Qemu-devel <qemu-devel@nongnu.org> > > This change is a result of workarounds for DMARC policies. > > Subsystem maintainers accepting patches need to catch these and fix > them before sending pull requests, so a checkpatch.pl test is highly > desirable. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > scripts/checkpatch.pl | 4 ++++ > 1 file changed, 4 insertions(+) > Adding it here will let patchew flag such message; the workaround is for the submitter to resubmit with an explicit 'From: User Name <address@example.com>' line in the body of the email. Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Maydell <peter.maydell@linaro.org> writes: > On 23 August 2018 at 11:25, Daniel P. Berrangé <berrange@redhat.com> wrote: >> In some cases the Author: email address in patches submitted to the >> list gets mangled such that it says >> >> John Doe via Qemu-devel <qemu-devel@nongnu.org> >> >> This change is a result of workarounds for DMARC policies. > > I do wonder if overall we'd be better off with the other > workaround (don't change From lines in list emails, don't > add "[Qemu-devel]" prefix to subject lines)... It would be > a one-off annoyance to people who filter list mail on subject > rather than other headers, but it would get rid of this annoying > wart where a handful of senders end up with their mail with > a from address that our patch management tools don't handle > very well... Mangling e-mail headers is a bad idea. The fact that it's commonly held only makes it worse. If you can hack QEMU, filtering on List-Id should not be an insurmountable obstacle.
Copying Paolo, since this probably falls under the "misc patches" maintainer bucket. On Thu, Aug 23, 2018 at 11:25:02AM +0100, Daniel P. Berrangé wrote: > In some cases the Author: email address in patches submitted to the > list gets mangled such that it says > > John Doe via Qemu-devel <qemu-devel@nongnu.org> > > This change is a result of workarounds for DMARC policies. > > Subsystem maintainers accepting patches need to catch these and fix > them before sending pull requests, so a checkpatch.pl test is highly > desirable. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > scripts/checkpatch.pl | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 42e1c50dd8..be2114a179 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1402,6 +1402,10 @@ sub process { > $is_patch = 1; > } > > + if ($line =~ /^Author: .*via Qemu-devel.*<qemu-devel\@nongnu.org>/) { > + ERROR("Author email address is mangled by the mailing list\n" . $herecurr); > + } > + > #check the patch for a signoff: > if ($line =~ /^\s*signed-off-by:/i) { > # This is a signoff, if ugly, so do not double report. > -- > 2.17.1 > Regards, Daniel
On 19/10/2018 13:22, Daniel P. Berrangé wrote: > Copying Paolo, since this probably falls under the "misc patches" > maintainer bucket. I think we should define a "generic maintainer stuff" bucket where every maintainer is supposed to send patches with a Reviewed-by. Maintainers are probably good enough at getting reviews by an appropriate person. (I am slowly trying to remove stuff from the misc bucket). Paolo > On Thu, Aug 23, 2018 at 11:25:02AM +0100, Daniel P. Berrangé wrote: >> In some cases the Author: email address in patches submitted to the >> list gets mangled such that it says >> >> John Doe via Qemu-devel <qemu-devel@nongnu.org> >> >> This change is a result of workarounds for DMARC policies. >> >> Subsystem maintainers accepting patches need to catch these and fix >> them before sending pull requests, so a checkpatch.pl test is highly >> desirable. >> >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >> --- >> scripts/checkpatch.pl | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> index 42e1c50dd8..be2114a179 100755 >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -1402,6 +1402,10 @@ sub process { >> $is_patch = 1; >> } >> >> + if ($line =~ /^Author: .*via Qemu-devel.*<qemu-devel\@nongnu.org>/) { >> + ERROR("Author email address is mangled by the mailing list\n" . $herecurr); >> + } >> + >> #check the patch for a signoff: >> if ($line =~ /^\s*signed-off-by:/i) { >> # This is a signoff, if ugly, so do not double report. >> -- >> 2.17.1 >> > > Regards, > Daniel >
On Fri, Oct 19, 2018 at 02:00:21PM +0200, Paolo Bonzini wrote: > On 19/10/2018 13:22, Daniel P. Berrangé wrote: > > Copying Paolo, since this probably falls under the "misc patches" > > maintainer bucket. > > I think we should define a "generic maintainer stuff" bucket where every > maintainer is supposed to send patches with a Reviewed-by. Maintainers > are probably good enough at getting reviews by an appropriate person. > > (I am slowly trying to remove stuff from the misc bucket). Ah, so you mean, any maintainer would be expected to directly send a PULL for any "generic" patches they had collected acks for. That would makes sense to me, since we don't seem to be making much progress in getting all source files to have an explicit maintainer. Regards, Daniel
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 42e1c50dd8..be2114a179 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1402,6 +1402,10 @@ sub process { $is_patch = 1; } + if ($line =~ /^Author: .*via Qemu-devel.*<qemu-devel\@nongnu.org>/) { + ERROR("Author email address is mangled by the mailing list\n" . $herecurr); + } + #check the patch for a signoff: if ($line =~ /^\s*signed-off-by:/i) { # This is a signoff, if ugly, so do not double report.
In some cases the Author: email address in patches submitted to the list gets mangled such that it says John Doe via Qemu-devel <qemu-devel@nongnu.org> This change is a result of workarounds for DMARC policies. Subsystem maintainers accepting patches need to catch these and fix them before sending pull requests, so a checkpatch.pl test is highly desirable. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- scripts/checkpatch.pl | 4 ++++ 1 file changed, 4 insertions(+)