[PATCH 1/2] dim: Workaround Outlook + Patchwork mess.

Fri May 18 23:36:03 UTC 2018

On Mon, Feb 19, 2018 at 12:42 AM, Arkadiusz Hiler
<arkadiusz.hiler at intel.com> wrote:
> On Fri, Feb 16, 2018 at 10:59:50AM -0800, Rodrigo Vivi wrote:
>> On Fri, Feb 16, 2018 at 10:52:11AM +0200, Arkadiusz Hiler wrote:
>> > On Thu, Feb 15, 2018 at 11:14:09AM -0800, Rodrigo Vivi wrote:
>> > > Patchwork has a own identity that doesn't respect the author configuration.
>> > > It automatically changes the author to whatever it was last seen on
>> > > the public mailing list.
>> > >
>> > > In case the response came from an outlook server the author identity
>> > > inside patchwork will get changed to Last, First.
>> > >
>> > > We have authors that respond emails to public mailing list using
>> > > Intel emails. Even non outlook users can end up having this trouble
>> > > because the Evolution can be setup with EWS, for instance.
>> > >
>> > > Since we could never fix patchwork for real to respect users
>> > > choice we can workaround it here when applying the patch.
>> >
>> > This statement is not true. It's not quite trivial, but definitely possible.
>>
>> I'm sorry for the way I wrote it. I didn't mean to do any critic.
>> I'd be glad in re-phrase that to:
>>
>> Since the solution on patchwork to respect user's name of choice
>> is not trivial, we can, for now, workaround this here when applying the patch.
>>
>> >
>> > This models needs additional field for email address:
>> > https://github.com/dlespiau/patchwork/blob/master/patchwork/models.py#L308
>> >
>> > This part of code needs to save the raw author information using the
>> > newly added field:
>> > https://github.com/dlespiau/patchwork/blob/master/patchwork/bin/parsemail.py#L759
>> > (now it's just updating the name in the association)
>>
>> It's the first time that I ever look to this code so please bare with me.
>> But is it possible here to differentiate from mail mail and patch mail?
>> On this case we could only save the name from the patch mail and never from mail mail.
>>
>> This would solve the issue since smtp itself used by git send-email doesn't change
>> any address. Only the EWS does.
>
> That's one way to solve this, but it would still be just a half of a
> solution.
>
> There are still edge cases that can bite us in the future - legal change
> of the name, someone who has a typo in the gitconfig just on one of the
> machines - that ends up propagating to other patches (I've seen this
> happening!).

I think we may be over complicating this. The only place in which this
bites us is when downloading the patch from patchwork. For that
download I don't want any mapping to any identity from Patchwork. I
want to have whatever the user set on their patch. Because if that's
wrong we can point the person to fix on their end. If he changed his
name, married, had a typo, etc, they are all things this person should
rather had be changed on their config, because when they are
committing directly to the git tree, that's the name that is going to
be used. Not anything patchwork decided to map to.

There are people in the kernel community who use e.g. "Foo (company)
foo at foo.org" and change the company to differentiate contributions
from companies that are sponsoring him (hint git log --author="Steven
Rostedt" to see one use case). Mangling whatever the person set on
*that* specific patch based on what was used for other patches or
replies is just wrong IMO.  So I don't think it's just an
outlook/exchange problem.

>
> That's why I am strongly in favor of keeping "From" with each of the
> patches - it matches intuitive understanding of patch being self
> contained.

We could do that for the patchwork interface, but just deliver
whatever was received in the "patch email" when someone downloads the
mbox.

Lucas De Marchi

>
>> > Then you need either create a fall-back mechanism in the model so
>> > patches that came in before the change will still use data from the
>> > foreign key to the Person model or create a migration that copies
>> > current authorship information onto the patches.
>> >
>> > And update how the mboxes are generated to use the name form the Patch:
>> > https://github.com/dlespiau/patchwork/blob/master/patchwork/views/__init__.py#L218
>> >
>> > There's no need of removing the "Person" concept, as it also serves
>> > other purposes (search and ownership management).
>>
>> I agree that that is useful.
>>
>> >
>> > Also write a few tests to make sure it works as expected.
>>
>> Do you have any example of tests case?
>
> Tests are in patchwork/tests/ subdirectories.
>
> You would quite likely want to implement new tests for "mboxviews" to
> make sure which person is used in the From line in the generated mbox.
>
> And another one for mail parser to make sure it parses and handles
> authorship correctly.
>
> Those are just regular django unit tests. You can look up details in
> their documentation.
>
> Please base you work of this branch:
> https://github.com/ivyl/patchwork/commits/django111
>
>> And any doc to show how to setup a local version to test it?
>
> Check out the docs/ directory. There is installation and development
> guide.
>
>> Is there a test version and production version?
>
> I am running one internal instance locally that I use to test the
> changes with real workloads from mailing lists. I would deploy it there
> for a couple of days before going with freedesktop deployment.
>
>> > Patches are welcome. Repository has CI set up. I have commit right to
>> > this project. I can review and accept pull requests. I can also deploy
>> > the update.
>>
>> Is Damien still maintaining this? Or should we move to somewhere else?
>
> Sadly I am the only contributor as of now, and I would love to have
> someone else to review my work especially if that person helps me with
> the development from time to time.
>
> I have my own fork on github, but I always push the changes to Damien's
> repo when they are "production ready".
>
>> Is there any plan to contribute back to "main" getpatchwork? Or it will
>> be a forever fork?
>
> Quite likely this is not going to happen in the foreseeable future.
> Freedesktop forked quite a while ago. I was told that back then the
> patchwork project was not very keen on the direction of changes - UI
> revamp, automatic grouping patches in series and REST API instead of
> just XMLRPC, hence the fork.
>
> Now they are starting to catch up with features, but IMO they are in
> much rougher shape, usability-wise. Series are a second-class concept
> and are not nearly as functional as what freedesktop has, and the
> REST API looks very different and misses some of the key pieces.
>
> IMHO it's less effort to maintain the fork.
>
> - Arek
> _______________________________________________
> dim-tools mailing list
> dim-tools at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools

-- 
Lucas De Marchi