scripts: report on author emails that are mangled by the mailing list

[Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Daniel P. Berrangé 7 years, 2 months ago

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

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Cornelia Huck 7 years, 2 months ago

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>

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Peter Maydell 7 years, 2 months ago

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

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Daniel P. Berrangé 7 years, 2 months ago

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Markus Armbruster 7 years, 2 months ago

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.

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Eric Blake 7 years, 2 months ago

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>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Daniel P. Berrangé 7 years ago

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Paolo Bonzini 7 years ago

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
>

Re: [Qemu-devel] [PATCH] scripts: report on author emails that are mangled by the mailing list

Posted by Daniel P. Berrangé 7 years ago

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|