Re: [PATCH v2 0/3] Store the 'actor' responsible for events

[Mauro Carvalho Chehab]

Em Tue,  8 Oct 2019 00:16:42 +0200
Johan Herland  escreveu:

> The V4L project (https://patchwork.linuxtv.org) uses patch states and
> delegates extensively to track progress. We want an audit log to keep
> track of the changes made to these patch fields. The Event model already
> records this information, but leaves out one crucial detail: which
> maintainer/user actually updated the patch state/delegate. The need for
> this enhancement is also documented in Issue #73.
> 
> This patch series adds an 'actor' field to the Event model, and - for
> applicable events - stores the user responsible for that event (i.e. the
> current/active user, if any) into this field.
> 
> This applies to the following events:
>  - patch-created
>  - patch-completed
>  - patch-state-changed
>  - patch-delegated
>  - series-completed
>  - check-created
> 
> For the other events (cover-created and series-created), I could not
> easily determine if there is a responsible user at all, as these events
> are typically generated in response to incoming emails. In these cases
> (and any other scenario where we cannot find the active/current user)
> the Event.actor field is left as null/None.
> 
> Finally, the new Event.actor field is exposed in the events view of the
> REST API (as of API version 1.2).
> 
> Changes since v1:
>  - Rename Event field from 'user' to 'actor'
>  - Add 'Closes: #73' to patch #2.
>  - Store actor for check-created as well. Assume actor == check.user
>  - Leave 'actor' out of API docs pre-v1.2
>  - Add test that delegates a Patch via the REST API, and verifies that
>the resulting event has the correct 'actor' set.
> 
> There is still an open discussion as to whether the strategy used in
> patch #2 to determine the actor and forward it to the event creation,
> is the way we want to go.
> 
> 
> Have fun!
> ...Johan

Hi Daniel/Stephen,

Any news with regards to this patchset? This is a long-waited feature that
we need on media, in order to allow more people to be able to change
status at our patchwork instance.

Regards,
Mauro

> 
> 
> Johan Herland (3):
>   models.Event: Add the user responsible for the event
>   Include the responsible actor in applicable events
>   /api/events: Add 'actor' field to generated JSON
> 
>  docs/api/schemas/latest/patchwork.yaml   |  6 ++
>  docs/api/schemas/patchwork.j2|  8 
>  docs/api/schemas/v1.2/patchwork.yaml |  6 ++
>  patchwork/api/event.py   | 10 +++---
>  patchwork/migrations/0037_event_actor.py | 21 +
>  patchwork/models.py  | 10 +-
>  patchwork/signals.py | 10 --
>  patchwork/tests/api/test_event.py| 24 
>  patchwork/tests/test_events.py   |  7 +++
>  9 files changed, 96 insertions(+), 6 deletions(-)
>  create mode 100644 patchwork/migrations/0037_event_actor.py
> 



Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: RFE: use patchwork to submit a patch

[Mauro Carvalho Chehab]

Em Mon, 14 Oct 2019 08:26:37 -0400
"Theodore Y. Ts'o"  escreveu:

> On Mon, Oct 14, 2019 at 12:42:36PM +0200, Toke Høiland-Jørgensen wrote:
> > It should be detectable, though, right?
> > 
> > Say you have two independently administered patchwork instances (or even
> > better, two different software packages entirely) that both subscribe to
> > the mailing lists, and compare patch content with each other. They
> > should at least be able to detect mismatches. Especially if you add a
> > sanity check before discarding duplicate message-ids.  
> 
> They don't even need to compare against each other; patchwork is about
> to add a feature where you can look up patches via message-id, right?
> That means it's easy enough to write a program which fetches patches
> from patchwork, and compares it to the patches found in
> lore.kernel.org.  If they don't match, then an alarm can be sounded.
> 
> Individuals who are reviewing patches can also compare the copy in
> their inbox with the copy from lore or some other public inbox.  And
> maintainers can compare copies from lore.kernel.org and patchwork
> before they apply a patch.  (99% of the time, I actually use the patch
> from my inbox, anyway.)

It can still have man-in-the-middle (MITM) attacks between the sender and
vger.kernel.org. Please notice that using https and adding the patch
via a web interface can also be subject to MITM, as companies and even some
Countries with strong policy enforcement may have some gateway on their
infra that will prevent end-to-end encryption[1], blocking direct
client-server https tunnels.

[1] They add an internal certificate to the browsers, so that the client
will see the connection as trustful, but the infra will actually do two
separate HTTPS encryption:

client  ---> Gateway
Gateway ---> Server

While unlikely, nothing prevents that the patch would be maliciously 
altered at the Gateway.

From security PoV, the only way to ensure that the patch was not
altered is to have it signed by the one who wrote it.

> 
> > This way you'd need to compromise multiple machines to achieve the kind
> > of compromise you're worried about. And you can add more independent
> > machines until you're satisfied that the risk is low enough :)  
> 
> Yep, exactly.  This is basically the theory behind Certificate
> Transparency[1], applied to patches.  For example, here's the
> certificate transparency report for kernel.org:
> 
>
> https://transparencyreport.google.com/https/certificates?cert_search=domain:kernel.org
> 
>   - Ted
> 
> [1] http://www.certificate-transparency.org/what-is-ct

That won't prevent companies/governments to require the manual
installation of the gateway's certificate on their browers, in order
to be able to navigate using https.

Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: RFE: use patchwork to submit a patch

[Mauro Carvalho Chehab]

Em Mon, 14 Oct 2019 09:53:58 -0400
"Theodore Y. Ts'o"  escreveu:

> On Mon, Oct 14, 2019 at 10:41:32AM -0300, Mauro Carvalho Chehab wrote:
> > It can still have man-in-the-middle (MITM) attacks between the sender and
> > vger.kernel.org. Please notice that using https and adding the patch
> > via a web interface can also be subject to MITM, as companies and even some
> > Countries with strong policy enforcement may have some gateway on their
> > infra that will prevent end-to-end encryption[1], blocking direct
> > client-server https tunnels.
> > 
> > [1] They add an internal certificate to the browsers, so that the client
> > will see the connection as trustful, but the infra will actually do two
> > separate HTTPS encryption:
> > 
> > client  ---> Gateway
> > Gateway ---> Server
> > 
> > While unlikely, nothing prevents that the patch would be maliciously 
> > altered at the Gateway.
> > 
> > From security PoV, the only way to ensure that the patch was not
> > altered is to have it signed by the one who wrote it.  
> 
> Well, sure, but the maintainer should be reviewing the patch looking
> for problems anyway.  There is the risk that people might slap a
> "Reviewed-by:" tag on a patch without sufficiently careful review if
> it's from a prominent kernel contributor, but we've always had that
> problem.  And so nothing, not even a digitally signed patch from a
> reviewer should absolve the maintainer from doing their own review.

Yeah, our current security model is based at the maintainer for
him to do his duties, properly reviewing the patch.

Yet, at the example that Daniel gave:

Instead of:

if ((permissions == allowed) && other_stuff) {
do_things();
}
do_more_stuff(permissions);

Patch was maliciously modified to:

if ((permission == allowed) && other_stuff) {
do_things();
}
do_more_stuff(permissions);

I suspect that a change like that might sleep though the maintainer's
review.

> 
> Now, one might argue that if there is a forged patch from "famous
> kernel developer A", followed up with a forged patch from "famous
> kernel developer B", that might cause a maintainer to happily take the
> patch without doing their own, independent review, for scaling
> reasons.  But that's a "vulernability" we've lived with for a long
> time, since today neither patches or "Reviewed-by" messages are
> usually signed.
> 
> And at least (as far as we know) no one has managed to sneak a
> malicious patch with a zero-day hidden with malice aforethought.  And
> perhaps that shouldn't be surprising.  We seem to be quite capable of
> introducing our own security vulererabilities without "help", so
> perhaps most malicious attackers wouldn't want to do something which
> could be so easily detected, when they can just pay money to a black
> hat hacker.

True, but, once we discover a patch introduced with a malicious code,
some action should be taken to eliminate the source of the bad code.

I mean, one thing is if "famous kernel developer A" maliciously
wrote a patch to violate security. The other thing is that the
infra used by "famous kernel developer A" is not safe, and has
some hidden back hat hacker in the middle of it.

The only way to identify that is by using signed patches/PRs.


Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: RFE: use patchwork to submit a patch

[Mauro Carvalho Chehab]

Em Fri, 11 Oct 2019 11:32:54 -0700 (PDT)
David Miller  escreveu:

> From: Steven Rostedt 
> Date: Fri, 11 Oct 2019 14:01:08 -0400
> 
> > Thus, if we want people to send us their fixes, we better keep just
> > an email with a patch the lowest bar for entry.  
> 
> I argue that for people coming into the software engineering world
> today, a PR is the lowest bar for entry.  And email is the exact
> opposite, _especially_ our way of doing email.
> 
> Because it IS NOT just an email with a patch.
> 
> It is an email with a specific Subject line format, with various
> fields.  Some field are optional and some are mandatory, and it also
> depends upon which tree you are targetting.
> 
> Then there is the commit message which has content and formatting
> requirements.
> 
> And then there are the tags, of various types and flavors, and the
> context (which is sometimes subtle) determines which of those tags are
> relevant and how they should be filled in.
> 
> The various other standard email fields like TO: and CC: have to
> be set a certain way otherwise the patch won't even be looked at.

I can't see how just adding a web interface like github/gitlab
would solve any of the above. I mean, the submitter will still
need to write a proper subject/description before submitting the
patch to the web interface, add the tags, etc. 

The only thing that would be different if we write a a Kernel-specific
web GUI would be to automate some sanity checks on it, like starting a CI
build for the patch series, run checkpatch and get_maintainers, but that 
could also happen if we write a script/submit_patches tool, that would
internally call checkpatch, get_maintainers and git send e-mail
(or use some other interface like patchwork REST to submit patches).

Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: RFE: use patchwork to submit a patch

[Mauro Carvalho Chehab]

Em Fri, 11 Oct 2019 14:44:01 -0400
Steven Rostedt  escreveu:

> On Fri, 11 Oct 2019 11:32:54 -0700 (PDT)
> David Miller  wrote:
> 
> > From: Steven Rostedt 
> > Date: Fri, 11 Oct 2019 14:01:08 -0400
> >   
> > > Thus, if we want people to send us their fixes, we better keep just
> > > an email with a patch the lowest bar for entry.
> > 
> > I argue that for people coming into the software engineering world
> > today, a PR is the lowest bar for entry.  And email is the exact
> > opposite, _especially_ our way of doing email.
> > 
> > Because it IS NOT just an email with a patch.  
> 
> That is if the maintainer wants to be anal about the submission. I've
> taken patches where I only asked the person to give me a signed off by.
> I also ask:
> 
> "Hi, do you intend on being a contributor, or do you only want to get
> this fix upstream? If the latter, I will handle the change log and
> other formatting for you, all I need is the signed-off-by. Otherwise, I
> will help you submit a proper patch."
> 
> Again, this is for one offs, where someone found something like an off
> by one error or other trivial bug to fix. I just want the fix in, but
> will let the submitter decide how strict I will be to get it in. I've
> had people say "I don't care, just get it fixed", and I do the
> formatting and all the grudge work, but still give the submitter the
> credit.
> 
> Not to mention, there's several times I get a patch where the solution
> is totally wrong, and I need to make the fix anyway. A simple
> Reported-by is what the submitter gets. Also, perfectly done by email.
> 

About the same here: if needed, I change patch descriptions, and
for trivial patches submitted by newbies, I even correct bad whitespacing,
if needed.

All it takes for a patch to be handled is to send it in a way that
patchwork will recognize as a patch - e. g. it should carry a properly
formatted diff.

> > 
> > You are not helping casual contributors with this "simple" email based
> > submission method.  It is understood and easy us, but nobody else.  
> 
> I'm not saying that email is the only way, this entire thread is about
> getting another tool to help. But I will scream very loudly if we
> eliminate email totally.

Yes, e-mail should stay.

Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: RFE: use patchwork to submit a patch

[Mauro Carvalho Chehab]

Em Thu, 10 Oct 2019 15:53:35 -0400
Konstantin Ryabitsev  escreveu:

> On Thu, Oct 10, 2019 at 03:07:29PM -0300, Mauro Carvalho Chehab wrote:
> >> - the patch submission screen has a succession of screens:
> >>
> >>   1. a screen with a single field allowing a user to paste a URL to
> >>  their fork of the git repository.  
> >
> >This will raise the bar, as it will force all developers to have a
> >public site to host the tree. I guess only a fraction of the 4k kernel
> >devs have it... In special, the ones that just want to send us a patch
> >fixing a bug may have serious troubles implementing that.  
> 
> I don't think this will raise the bar, as Github/Gitlab allow for very 
> easy forking of https://github.com/torvalds/linux. This is also not at 
> all aimed at "all developers" -- only those that don't want to use the 
> current CLI workflow and are more comfortable with web tools like 
> Github.

I guess people have different views about what's the goal.

If the goal is to attract more developers, the focus should be on
making something that would be simpler than what we current have.

What we currently have here is that just adding this to .git/config:

[sendemail]
  smtpserver = smtp.gmail.com
  smtpserverport = 587
  smtpencryption = tls
  smtpuser = 
  from = 

Is usually enough for the vast majority of the newcomers.

Using github/gitlab for Kernel development takes a lot more time
and a lot more steps than writing the above at .git/config, even
if the user already have an account there.

Also, instead of just doing:

git send-email origin..

Your proposal will require to do:

1) git push github_clone
2) open the browser
3) fill the forms, pointing to "github_clone" URL
4) click at the submission button

That is adding a lot additional complexity. I fail to see any gain
with that.

> >>   2. next screen asks the user to select the ref to work from using 
> >>  the
> >>  list obtained from the remote. Once submitted, patchwork performs a
> >>  `git clone --reference` to clone the repository locally using a
> >>  local fork of the same repo to minimize object transfer. This part
> >>  requires that:
> >>a. patchwork project is configured with a path to a local fork,
> >>   if this feature is enabled for a project
> >>b. that fork is kept current via some mechanism outside of
> >>   patchwork (e.g. with grokmirror)
> >>c. there is some sanity-checking during the clone process to
> >>   avoid abuse (e.g. a sane timeout, a tmpdir with limited size,
> >>   etc -- other suggestions welcome)  
> >
> >That would require a high bandwidth at the machine with as patchwork.
> >Also, doesn't sound a good idea to me, as the server may end by having
> >tons of open sections, most waiting for tens of minutes, in order to
> >complete git clone.  
> 
> This is actually really fast if you already have a local copy of the 
> repository with most objects. Try this yourself if you have 
> torvalds/linux.git locally:
> 
> git clone --bare -s torvalds/linux.git test
> cd test
> git remote add arm-soc 
> https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
> git fetch arm-soc for-next
> 
> The whole process takes a second or so and the resulting repo is 328K in 
> size.
> 
> Of course, this assumes that the remote repository isn't trying to do 
> something nasty, which is why I suggest anti-abuse precautions.

Well, one could, for example, send a pull request, let's say, from
a DRM development-based tree, into media (or vice versa), with would
require to sync the "alien" patches, just to get the ones that are
useful.

It can even be worse (still valid): the tree to be pulled could be
based on linux-next.

Here, I receive bull requests that are sometimes based on non-media
trees: it may take a few mins to get the patches.

-

Except if the idea is to have this only at kernel.org, and add an
alternates for every single other tree, even a non-nasty PR would
take a while, as not all kernel trees are hosted there.

Also, as you said, one could do something really nasty, like
sending a PR from a huge non-kernel repository into the Kernel.

Not sure how easy/hard would be to avoid that.

This could even happen by mistake, as, at least on some places
(like linuxtv.org) non-kernel trees are also hosted. Btw, on media,
our patchwork instance is also used by VDR, whose project is even 
hosted elsewhere.

> 
> >> I know this is a pretty big RFE, and I would like to hear your 
> >> thoughts
> >> about this. If there 

Re: RFE: use patchwork to submit a patch

[Mauro Carvalho Chehab]

Em Fri, 11 Oct 2019 11:20:17 -0600
Shuah Khan  escreveu:

> On 10/11/19 2:57 AM, Greg KH wrote:
> > On Thu, Oct 10, 2019 at 10:41:50AM -0400, Konstantin Ryabitsev wrote:  
> >> Hi, all:
> >>
> >> I would like to propose a new (large) feature to patchwork with the goal to
> >> make the process of submitting a patch easier for newbies and people
> >> generally less familiar with patch-based development. This was discussed
> >> previously on the workflows list:
> >> https://lore.kernel.org/workflows/20190930202451.GA14403@pure.paranoia.local/
> >>
> >> How I envision this would work:
> >>
> >> - user creates an account (which requires a mail confirmation) >> - they 
> >> choose a "submit patch" option from the menu
> >> - the patch submission screen has a succession of screens:
> >>
> >>   1. a screen with a single field allowing a user to paste a URL to 
> >> their
> >> fork of the git repository. Once submitted, patchwork does a "git
> >> ls-remote" to attempt to get a list of refs and to verify that this is
> >> indeed a valid git repository  
> > 
> > s/valid git repository/valid git repository based on the kernel git tree/
> > 
> > Otherwise you might be sending out lots of emails for other projects :)
> >   
> >>
> >>   2. next screen asks the user to select the ref to work from using the
> >> list obtained from the remote. Once submitted, patchwork performs a 
> >> `git
> >> clone --reference` to clone the repository locally using a local fork 
> >> of
> >> the same repo to minimize object transfer. This part requires that:
> >>a. patchwork project is configured with a path to a local fork,
> >> if this feature is enabled for a project
> >>b. that fork is kept current via some mechanism outside of
> >> patchwork (e.g. with grokmirror)
> >>c. there is some sanity-checking during the clone process to
> >> avoid abuse (e.g. a sane timeout, a tmpdir with limited size,  etc
> >> -- other suggestions welcome)
> >>
> >>   3. next screen asks the user to pick a starting commit from the log.
> >> Once submitted, patchwork generates the patch from the commit provided
> >> to the tip of the branch selected by the user earlier,
> >>  using git format-patch.
> >>
> >>   4. next screen asks the user to review the patch to make sure this is
> >> what they want to submit. Once confirmed, patchwork performs two
> >> admin-defined optional hooks:
> >>
> >>a. a hook to generate a list of cc's (e.g. get_maintainer.pl)
> >>b. a sanity check hook (e.g. checkpatch.pl)  
> > 
> > I will note that many "first patch" submissions are checkpatch.pl
> > cleanups for staging.  When doing that, I require that they do "one
> > logical change per patch", which means that many of the individual
> > patches themselves will not be checkpatch.pl clean, because many lines
> > have multiple issues with them (tabs, spaces, format, length, etc.)
> > 
> > So other than that minor thing, sounds interesting.  It's hard to
> > determine just how difficult the whole "set up git and send a patch out"
> > process is for people these days given the _huge_ numbers of new
> > contributions we keep getting, and the numerous good tutorials we have
> > created that spell out exactly how to do this.
> > 
> > So you might be "solving" a problem that we don't really have.  It's
> > hard to tell :(
> >   
> 
> I agree with this. I don't think this a problem that is worth solving.
> When a new developer wants to send a patch, they don't need to create
> any accounts. They setup their email client and send patch.
> 
> We have several resources that walk them through setting up email
> clients and sending patches. checkpatch.pl can be automated with
> git hooks.
> 
> >> I know this is a pretty big RFE, and I would like to hear your thoughts
> >> about this. If there is general agreement that this is doable/good idea, I
> >> may be able to come up with funding for this development as part of the
> >> overall tooling improvement proposal.  
> > 
> > The workflow seems sane, and matches what most people do today, with the
> > exception that it "solves" the git send-email issue, right?  Is that our
> > biggest barrier?
> > 
> > I would recommend interviewing some of the recent kernel mentor project
> > and outreachy applicants first, to try to determine exactly what their
> > problems, if any, were with our development process.  If they say that
> > this type of tool/workflow would have saved them hours of time and
> > energy, then that's a great indication that we should try to do this.
> >   
> 
> I would say considering the number of applicants to mentorship program
> and new developers it will be lot overhead to require them to create
> patchwork accounts, and it might even be hard overtime. A lot of them
> start out and drop out in the middle. With the current setup, nothing
> to cleanup.
> 
> Setting up email clients and git hooks is one time task. It is the
> easiest of the 

Re: RFE: use patchwork to submit a patch

[Mauro Carvalho Chehab]

Em Thu, 10 Oct 2019 10:41:50 -0400
Konstantin Ryabitsev  escreveu:

> Hi, all:
> 
> I would like to propose a new (large) feature to patchwork with the goal 
> to make the process of submitting a patch easier for newbies and people 
> generally less familiar with patch-based development. This was discussed 
> previously on the workflows list: 
> https://lore.kernel.org/workflows/20190930202451.GA14403@pure.paranoia.local/
> 
> How I envision this would work:
> 
> - user creates an account (which requires a mail confirmation)
> - they choose a "submit patch" option from the menu
> - the patch submission screen has a succession of screens:
> 
>   1. a screen with a single field allowing a user to paste a URL to 
>  their fork of the git repository. 

This will raise the bar, as it will force all developers to have a
public site to host the tree. I guess only a fraction of the 4k kernel
devs have it... In special, the ones that just want to send us a patch
fixing a bug may have serious troubles implementing that.

> Once submitted, patchwork does a 
>  "git ls-remote" to attempt to get a list of refs and to verify that 
>  this is indeed a valid git repository
> 
>   2. next screen asks the user to select the ref to work from using the 
>  list obtained from the remote. Once submitted, patchwork performs a 
>  `git clone --reference` to clone the repository locally using a 
>  local fork of the same repo to minimize object transfer. This part 
>  requires that:
>a. patchwork project is configured with a path to a local fork, 
>   if this feature is enabled for a project
>b. that fork is kept current via some mechanism outside of 
>   patchwork (e.g. with grokmirror)
>c. there is some sanity-checking during the clone process to 
>   avoid abuse (e.g. a sane timeout, a tmpdir with limited size, 
>   etc -- other suggestions welcome)

That would require a high bandwidth at the machine with as patchwork.

Also, doesn't sound a good idea to me, as the server may end by having
tons of open sections, most waiting for tens of minutes, in order to
complete git clone.

> 
>   3. next screen asks the user to pick a starting commit from the log.  
>  Once submitted, patchwork generates the patch from the commit 
>  provided to the tip of the branch selected by the user earlier,
>  using git format-patch.
> 
>   4. next screen asks the user to review the patch to make sure this is 
>  what they want to submit. Once confirmed, patchwork performs two 
>  admin-defined optional hooks:
> 
>a. a hook to generate a list of cc's (e.g. get_maintainer.pl)
>b. a sanity check hook (e.g. checkpatch.pl)
> 
>   5. if sanity checking is defined, next screen shows the output of the 
>  sanity check hook, asking confirmation to proceed.
> 
>   6. next screen shows the user three fields:
> 
>a. title of the patch/series
>b. cover letter for the patch/series
>c. message-id of the previous patch revision (can be picked from 
>   the list of previously submitted series by this user -- 
>   patchwork should have them already)
>d. number of the revision (can be auto-filled if previous field 
>   is provided) and other tags to include in []
> 
>   7. next screen shows final review of what would be sent out to the 
>  list (and cc's, if the hook to get cc's is defined and returned any 
>  results). Once submitted, patchwork sends the patch/series using 
>  patchwork's envelope-from and the user's own email in the From: 
>  header.
> 
>   8. once sent successfully, cleanups are performed (also needs to be 
>  done as part of the regular cron job, for any aborted attempts)
> 
> I know this is a pretty big RFE, and I would like to hear your thoughts 
> about this. If there is general agreement that this is doable/good idea, 
> I may be able to come up with funding for this development as part of 
> the overall tooling improvement proposal.

The procedure itself is not bad, but, if implemented, IMHO, it should,
instead, be something that will run at the machine of the one submitting
the patch. For instance, this could be a perl or python script inside 
Kernel's ./script directory that would handle everything locally, and
then submit the patch via patchwork's REST API.

By using the REST API, it would avoid the need of having to do special
e-mail setups for the casual developers.

Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Add an script to generate patchwork stats

[Mauro Carvalho Chehab]

It could be useful to be able to see how many patches a patchwork
instance has. This small script does that.

We're using it to maintain a page with includes the generated stats at:

https://linuxtv.org/patchwork_stats.php

It requires pandas and matplotlib to work. It assumes a database
using MySQL, but can easily changed to Postgres or to any other
DB.

Signed-off-by: Mauro Carvalho Chehab 

-

PS.: By purpose, I didn't use Django here. It is a way easier for me to
write SQL commands directly than try to guess how to produce the
same results with Django. Also, I don't expect this to be applied
upstream as-is, as, if we're doing some stats module at patchwork, it 
would require a lot more: templates, css changes, customizations, etc.

The goal here is just to have something really simple that would
allow to account the number of patches received/accepted/rejected/etc
and produce graphics in a way that we could later easily add/remove
stuff.

The script takes a 2 years window and includes the current month,
but it should be easy to change it to any other period of time.

The drawback is that this script will require changes if the database
model changes. It was produced against stable 2.1 data model.

Yet, as others may want to provide similar things, I'm providing
the code we use at linuxtv.org (with the DB connection details
sanitized).


+#!/usr/bin/env python
+
+import matplotlib
+matplotlib.use('Agg')
+
+import MySQLdb
+import pandas as pd
+import matplotlib.pyplot as plt
+
+conn = MySQLdb.connect(host="localhost", user="my_patchwork_user", 
passwd="my_password", db="patchwork_db_name")
+cursor = conn.cursor()
+
+# Total patches
+
+df = pd.read_sql('select DATE_FORMAT(date, "%Y-%m") AS date , count(*) AS 
patches from patchwork_submission WHERE date between 
DATE_SUB(DATE_FORMAT(CURRENT_DATE,"%Y-%m-01"),INTERVAL 2 YEAR) and NOW() group 
by DATE_FORMAT(date, "%Y-%m") ORDER BY YEAR(date), MONTH(DATE)', con=conn)
+df['date'] = pd.to_datetime(df.date, format='%Y-%m')
+
+fig = plt.figure(figsize=(10,6), facecolor='w', edgecolor='k')
+ax = fig.add_subplot(1,1,1)
+#ax.bar(df.date, df.patches)
+ax.plot_date(df.date, df.patches, 'ob')
+ax.patch.set_facecolor('lightgrey')
+ax.set_ylim(bottom=0)
+fig.autofmt_xdate()
+ax.set_title('Number of patches received by month')
+ax.autoscale_view()
+
+plt.grid()
+plt.savefig('htdocs/static/images/patches_per_date.svg')
+
+# Patches per state
+
+df = pd.read_sql('select DATE_FORMAT(date, "%Y-%m") as date, st.name, count(*) 
as patches from patchwork.patchwork_patch AS p, patchwork.patchwork_submission 
as s, patchwork_state as st where date between 
DATE_SUB(DATE_FORMAT(CURRENT_DATE,"%Y-%m-01"),INTERVAL 2 YEAR) and NOW() and 
s.id = p.submission_ptr_id and state_id = st.id group by DATE_FORMAT(date, 
"%Y-%m"), state_id', con=conn)
+df['date'] = pd.to_datetime(df.date, format='%Y-%m')
+df.set_index('date', inplace=True)
+
+fig, ax = plt.subplots(figsize=(14,6))
+
+df.groupby(['name'])['patches'].plot(ax=ax, legend=True)
+ax.patch.set_facecolor('lightgrey')
+ax.set_ylim(bottom=0)
+fig.autofmt_xdate()
+ax.set_title('Number of patches per state per month')
+ax.autoscale_view()
+
+plt.grid()
+plt.legend(loc="upper left", bbox_to_anchor=(1.05,1.0))
+plt.subplots_adjust(left=0.1, bottom=0.15, right=0.8)
+plt.savefig('htdocs/static/images/patches_per_state.svg')

Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH v2] filters: re-add the possibility of filtering undelegated patches

[Mauro Carvalho Chehab]

The filters.py redesign that happened for patchwork 1.1 removed
a functionality that we use a lot: to filter patches that weren't
delegated to anyone.

Also, it is a way harder to find someone to delegate with a free
text input. Use, instead a combo-box just like before.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/patchwork/filters.py b/patchwork/filters.py
index 79aaea437c6e..11d00390145a 100644
--- a/patchwork/filters.py
+++ b/patchwork/filters.py
@@ -385,6 +385,7 @@ class ArchiveFilter(Filter):
 class DelegateFilter(Filter):
 name = 'Delegate'
 param = 'delegate'
+no_delegate_str = 'Nobody'
 ANY_DELEGATE = object()
 
 def __init__(self, filters):
@@ -416,6 +417,11 @@ class DelegateFilter(Filter):
 if not key:
 return
 
+if key == self.no_delegate_str:
+self.delegate_match = key
+self.applied = True
+return
+
 try:
 self.delegate = User.objects.get(id=int(key))
 except (ValueError, User.DoesNotExist):
@@ -436,6 +442,9 @@ class DelegateFilter(Filter):
 if self.delegate:
 return {'delegate': self.delegate}
 
+if self.delegate_match == self.no_delegate_str:
+return {'delegate__username__isnull': True}
+
 if self.delegate_match:
 return {'delegate__username__icontains': self.delegate_match}
 
@@ -447,8 +456,33 @@ class DelegateFilter(Filter):
 return mark_safe('%s' % (
 self.param, self.condition))
 
-return mark_safe('')
+delegates = User.objects.filter(profile__maintainer_projects__isnull = 
False)
+
+str = ''
+
+selected = ''
+if not self.applied:
+selected = 'selected'
+
+str += '--' % selected
+
+selected = ''
+if self.applied and self.delegate is None:
+selected = 'selected'
+
+str += '%s' % (
+selected, self.no_delegate_str, self.no_delegate_str)
+
+for delegate in delegates:
+selected = ''
+if delegate == self.delegate:
+selected = ' selected'
+
+str += '%s' % (selected,
+delegate.id, delegate.username)
+str += ''
+
+return mark_safe(str)
 
 def set_status(self, *args, **kwargs):
 if 'delegate' in kwargs:
diff --git a/patchwork/templates/patchwork/partials/filters.html 
b/patchwork/templates/patchwork/partials/filters.html
index 41ed2c268e46..e89c4d0f6284 100644
--- a/patchwork/templates/patchwork/partials/filters.html
+++ b/patchwork/templates/patchwork/partials/filters.html
@@ -76,44 +76,6 @@ $(document).ready(function() {
 });
 }
 });
-
-$('#delegate_input').selectize({
-plugins: ['enter_key_submit'],
-maxItems: 1,
-persist: false,
-onInitialize: function() {
-this.on('submit', function() {
-if (!this.items.length)
-this.$input.val(this.lastValue);
-this.$input.closest('form').submit();
-}, this);
-},
-{% if "delegate" in filters.applied_filters %}
-{% with delegate_filter=filters.applied_filters.delegate %}
-options: [
-{
-value: "{{ delegate_filter.key }}",
-text: "{{ delegate_filter.condition }}",
-},
-],
-items: ["{{ delegate_filter.key }}"],
-{% endwith %}
-{% endif %}
-load: function(query, callback) {
-req = $.ajax({
-url: "{% url 'api-delegates' %}",
-data: {q: query, l: 10},
-error: function() {
-callback();
-},
-success: function(res) {
-callback($.map(res, function (obj) {
-return {value: obj.pk, text: obj.name};
-}));
-}
-});
-}
-});
 });
 
 
diff --git 
a/releasenotes/notes/Re-added-delegate-to-nobody-filter-and-use-select-for-delegated-people-04a81a4a914965d8.yaml
 
b/releasenotes/notes/Re-added-delegate-to-nobody-filter-and-use-select-for-delegated-people-04a81a4a914965d8.yaml
new file mode 100644
index ..eb4246e5600e
--- /dev/null
+++ 
b/releasenotes/notes/Re-added-delegate-to-nobody-filter-and-use-select-for-delegated-people-04a81a4a914965d8.yaml
@@ -0,0 +1,11 @@
+---
+prelude: >
+In the past, Patchwork used to support filtering patches that weren't
+delegated to anyone. This feature was removed in 2015, as part of a
+patch designed to support deletation to anyone. Yet, the feature didn't
+scale and got removed in 2016. So, re-introduce the old logic, fixing
+a regression.
+fixes:
+  - |
+Fix a regression introduced by changeset
+f439f5414206 ("Add delegate filter autocomplete support")


Thanks,
Mauro
___

Re: Add an script to generate patchwork stats

[Mauro Carvalho Chehab]

Em Fri, 7 Jun 2019 11:35:54 -0300
mche...@infradead.org escreveu:

> It could be useful to be able to see how many patches a patchwork
> instance has. This small script does that.
> 
> We're using it to maintain a page with includes the generated stats at:
> 
>   https://linuxtv.org/patchwork_stats.php
> 
> It requires pandas and matplotlib to work. It assumes a database
> using MySQL, but can easily changed to Postgres or to any other
> DB.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> -
> 
> PS.: By purpose, I didn't use Django here. It is a way easier for me to
> write SQL commands directly than try to guess how to produce the
> same results with Django. Also, I don't expect this to be applied
> upstream as-is, as, if we're doing some stats module at patchwork, it 
> would require a lot more: templates, css changes, customizations, etc.
> 
> The goal here is just to have something really simple that would
> allow to account the number of patches received/accepted/rejected/etc
> and produce graphics in a way that we could later easily add/remove
> stuff.
> 
> The script takes a 2 years window and includes the current month,
> but it should be easy to change it to any other period of time.
> 
> The drawback is that this script will require changes if the database
> model changes. It was produced against stable 2.1 data model.
> 
> Yet, as others may want to provide similar things, I'm providing
> the code we use at linuxtv.org (with the DB connection details
> sanitized).
> 
> 
> +#!/usr/bin/env python

That's what lack of caffeine brings... the patch is obviously
broken here :-)

-


[PATCH] Add an script to generate patchwork stats

It could be useful to be able to see how many patches a patchwork
instance has. This small script does that.

We're using it to maintain a page with includes the generated stats at:

https://linuxtv.org/patchwork_stats.php

It requires pandas and matplotlib to work. It assumes a database
using MySQL, but can easily changed to Postgres or to any other
DB.

Signed-off-by: Mauro Carvalho Chehab 

-

PS.: By purpose, I didn't use Django here. It is a way easier for me to
write SQL commands directly than try to guess how to produce the
same results with Django. Also, I don't expect this to be applied
upstream as-is, as, if we're doing some stats module at patchwork, it 
would require a lot more: templates, css changes, customizations, etc.

The goal here is just to have something really simple that would
allow to account the number of patches received/accepted/rejected/etc
and produce graphics in a way that we could later easily add/remove
stuff.

The script takes a 2 years window and includes the current month,
but it should be easy to change it to any other period of time.

The drawback is that this script will require changes if the database
model changes. It was produced against stable 2.1 data model.

Yet, as others may want to provide similar things, I'm providing
the code we use at linuxtv.org (with the DB connection details
sanitized).

diff --git a/stats.py b/stats.py
new file mode 100755
index ..d7878ce08c70
--- /dev/null
+++ b/stats.py
@@ -0,0 +1,49 @@
+#!/usr/bin/env python2
+
+import matplotlib
+matplotlib.use('Agg')
+
+import MySQLdb
+import pandas as pd
+import matplotlib.pyplot as plt
+
+conn = MySQLdb.connect(host="localhost", user="patchwork", passwd="yaicCoqui", 
db="patchwork")
+cursor = conn.cursor()
+
+# Total patches
+
+df = pd.read_sql('select DATE_FORMAT(date, "%Y-%m") AS date , count(*) AS 
patches from patchwork_submission WHERE date between 
DATE_SUB(DATE_FORMAT(CURRENT_DATE,"%Y-%m-01"),INTERVAL 2 YEAR) and NOW() group 
by DATE_FORMAT(date, "%Y-%m") ORDER BY YEAR(date), MONTH(DATE)', con=conn)
+df['date'] = pd.to_datetime(df.date, format='%Y-%m')
+
+fig = plt.figure(figsize=(10,6), facecolor='w', edgecolor='k')
+ax = fig.add_subplot(1,1,1)
+#ax.bar(df.date, df.patches)
+ax.plot_date(df.date, df.patches, 'ob')
+ax.patch.set_facecolor('lightgrey')
+ax.set_ylim(bottom=0)
+fig.autofmt_xdate()
+ax.set_title('Number of patches received by month')
+ax.autoscale_view()
+
+plt.grid()
+plt.savefig('/usr/local/patchwork/htdocs/static/images/patches_per_date.svg')
+
+# Patches per state
+
+df = pd.read_sql('select DATE_FORMAT(date, "%Y-%m") as date, st.name, count(*) 
as patches from patchwork.patchwork_patch AS p, patchwork.patchwork_submission 
as s, patchwork_state as st where date between 
DATE_SUB(DATE_FORMAT(CURRENT_DATE,"%Y-%m-01"),INTERVAL 2 YEAR) and NOW() and 
s.id = p.submission_ptr_id and state_id = st.id group by DATE_FORMAT(date, 
"%Y-%m"), state_id', con=conn)
+df['date'] = pd.to_datetime(df.date, format='%Y-%m')
+df.set_index('date', inplace=True)
+
+fig, ax = plt.subplots(figsize=(14,6))
+
+df.groupby(

Fw: [PATCH v2] filters: re-add the possibility of filtering undelegated patches

[Mauro Carvalho Chehab]

Forwarded message:

Date: Tue, 4 Jun 2019 15:07:58 -0300
From: Mauro Carvalho Chehab 
To: patchwork@lists.ozlabs.org
Subject: [PATCH v2] filters: re-add the possibility of filtering undelegated 
patches


The filters.py redesign that happened for patchwork 1.1 removed
a functionality that we use a lot: to filter patches that weren't
delegated to anyone.

Also, it is a way harder to find someone to delegate with a free
text input. Use, instead a combo-box just like before.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/patchwork/filters.py b/patchwork/filters.py
index 79aaea437c6e..11d00390145a 100644
--- a/patchwork/filters.py
+++ b/patchwork/filters.py
@@ -385,6 +385,7 @@ class ArchiveFilter(Filter):
 class DelegateFilter(Filter):
 name = 'Delegate'
 param = 'delegate'
+no_delegate_str = 'Nobody'
 ANY_DELEGATE = object()
 
 def __init__(self, filters):
@@ -416,6 +417,11 @@ class DelegateFilter(Filter):
 if not key:
 return
 
+if key == self.no_delegate_str:
+self.delegate_match = key
+self.applied = True
+return
+
 try:
 self.delegate = User.objects.get(id=int(key))
 except (ValueError, User.DoesNotExist):
@@ -436,6 +442,9 @@ class DelegateFilter(Filter):
 if self.delegate:
 return {'delegate': self.delegate}
 
+if self.delegate_match == self.no_delegate_str:
+return {'delegate__username__isnull': True}
+
 if self.delegate_match:
 return {'delegate__username__icontains': self.delegate_match}
 
@@ -447,8 +456,33 @@ class DelegateFilter(Filter):
 return mark_safe('%s' % (
 self.param, self.condition))
 
-return mark_safe('')
+delegates = User.objects.filter(profile__maintainer_projects__isnull = 
False)
+
+str = ''
+
+selected = ''
+if not self.applied:
+selected = 'selected'
+
+str += '--' % selected
+
+selected = ''
+if self.applied and self.delegate is None:
+selected = 'selected'
+
+str += '%s' % (
+selected, self.no_delegate_str, self.no_delegate_str)
+
+for delegate in delegates:
+selected = ''
+if delegate == self.delegate:
+selected = ' selected'
+
+str += '%s' % (selected,
+delegate.id, delegate.username)
+str += ''
+
+return mark_safe(str)
 
 def set_status(self, *args, **kwargs):
 if 'delegate' in kwargs:
diff --git a/patchwork/templates/patchwork/partials/filters.html 
b/patchwork/templates/patchwork/partials/filters.html
index 41ed2c268e46..e89c4d0f6284 100644
--- a/patchwork/templates/patchwork/partials/filters.html
+++ b/patchwork/templates/patchwork/partials/filters.html
@@ -76,44 +76,6 @@ $(document).ready(function() {
 });
 }
 });
-
-$('#delegate_input').selectize({
-plugins: ['enter_key_submit'],
-maxItems: 1,
-persist: false,
-onInitialize: function() {
-this.on('submit', function() {
-if (!this.items.length)
-this.$input.val(this.lastValue);
-this.$input.closest('form').submit();
-}, this);
-},
-{% if "delegate" in filters.applied_filters %}
-{% with delegate_filter=filters.applied_filters.delegate %}
-options: [
-{
-value: "{{ delegate_filter.key }}",
-text: "{{ delegate_filter.condition }}",
-},
-],
-items: ["{{ delegate_filter.key }}"],
-{% endwith %}
-{% endif %}
-load: function(query, callback) {
-req = $.ajax({
-url: "{% url 'api-delegates' %}",
-data: {q: query, l: 10},
-error: function() {
-callback();
-},
-success: function(res) {
-callback($.map(res, function (obj) {
-return {value: obj.pk, text: obj.name};
-}));
-}
-});
-}
-});
 });
 
 
diff --git 
a/releasenotes/notes/Re-added-delegate-to-nobody-filter-and-use-select-for-delegated-people-04a81a4a914965d8.yaml
 
b/releasenotes/notes/Re-added-delegate-to-nobody-filter-and-use-select-for-delegated-people-04a81a4a914965d8.yaml
new file mode 100644
index ..eb4246e5600e
--- /dev/null
+++ 
b/releasenotes/notes/Re-added-delegate-to-nobody-filter-and-use-select-for-delegated-people-04a81a4a914965d8.yaml
@@ -0,0 +1,11 @@
+---
+prelude: >
+In the past, Patchwork used to support filtering patches that weren't
+delegated to anyone. This feature was removed in 2015, as part of a
+patch designed to support deletation to anyone. Yet, the feature didn't
+scale and got removed in 2016. So, re-intr

Re: filters: re-add the possibility of filtering undelegated patches

[Mauro Carvalho Chehab]

Em Tue, 04 Jun 2019 12:39:49 +0100
Stephen Finucane  escreveu:

> On Mon, 2019-06-03 at 23:34 -0300, Mauro Carvalho Chehab wrote:
> > Em Tue, 04 Jun 2019 10:10:51 +1000
> > Daniel Axtens  escreveu:
> >   
> > > Mauro Carvalho Chehab  writes:
> > >   
> > > > The filters.py redesign that happened for patchwork 1.1 removed
> > > > a functionality that we use a lot: to filter patches that weren't
> > > > delegated to anyone.
> > > > 
> > > > Also, it is a way harder to find someone to delegate with a free
> > > > text input. Use, instead a combo-box just like before.
> > > > 
> > > > Signed-off-by: Mauro Carvalho Chehab 
> > > > 
> > > > ---
> > > > 
> > > > LinuxTV was using some pre-version 1.0 patchwork instance since
> > > > last week. Migrating was not easy, due to the delegation patches we
> > > > developed for 1.x, that caused the migration scripts to fail.
> > > > 
> > > > On Friday, we finally migrated to the latest stable version:
> > > > 
> > > > https://patchwork.linuxtv.org
> > > > 
> > > > After the migration, we noticed that one feature that we used a
> > > > lot got removed from patchwork: the filter was not allowing anymore
> > > > to filter not-delegated patches.
> > > > 
> > > > On media, we receive a large amount of patches per week. Just after
> > > > the migration, we received ~30 patches, and that's because it is
> > > > a weekend!
> > > > 
> > > > In order to handle such high volume, we have several sub-maintainers,
> > > > each one responsible for one part of the subsystem. While we do have
> > > > delegation rules on patchwork, we still do manual delegation. So,
> > > > being able to see what patches are not on someone's queue is very
> > > > important to us.
> > > > 
> > > > This patch restores such feature to patchwork. As a plus, it now
> > > > shows a combo-box with just the names of people that maintain
> > > > projects, instead of a free-text input that would otherwise seek
> > > > the entire user's database.
> > > 
> > > Right, that seems like a feature we probably should not have killed,
> > > sorry.
> > > 
> > > It looks like it went away in 2015:
> > > 
> > > commit f439f5414206c94d486c60801a86acc57ad3f273
> > > Author: Stephen Finucane 
> > > Date:   Mon Aug 24 11:05:47 2015 +0100
> > > 
> > > Add delegate filter autocomplete support
> > > 
> > > It would be helpful to provide autocomplete functionality for the
> > > delegate filter field, similar to that provided for the submitter
> > > field. This will provide a way to handle larger delegate lists without
> > > overloading the user with options.
> > > 
> > > Add this functionality through use of Selectize, which is already used
> > > to provide similar functionality for filtering of "submitters".
> > > 
> > > Signed-off-by: Stephen Finucane 
> > > 
> > > So perhaps reverting that all the way back might not be the way to go,
> > > but we should have something for your usecase.
> > > 
> > > Stephen, thoughts?  
> > 
> > Maybe the change was done in order to allow delegating patches to 
> > anyone:
> > 
> >   commit e0fd7cd91a5fbe0a0077c46bea870ccd09c8920d (v1.0.0-154-ge0fd7cd)
> >   Author: Stephen Finucane 
> >   Date:   Mon Aug 24 14:21:43 2015 +0100
> > 
> > Allow assigning of any user as delegate
> > 
> > With was reverted one year later:
> > 
> > commit 198139e4112cf337ffea403000441931b4ddad06 (v1.1.0-227-g198139e)
> >   Author: Stephen Finucane 
> >   Date:   Sun Sep 25 22:37:11 2016 +0100
> > 
> > Revert "Allow assigning of any user as delegate"
> >   
> > > >  def _form(self):
> > > > -return mark_safe(' > > > - 'id="delegate_input" class="form-control">')
> > > > +delegates = 
> > > > User.objects.filter(profile__maintainer_projects__isnull = False)
> > > 
> > > If we were to go down this route, I think we'd want to be filtering
> > > against the project as well?   
> 
> Yup, that was exactly why we'd done this. That's still a feature I'd
> like to have but I 

Re: filters: re-add the possibility of filtering undelegated patches

[Mauro Carvalho Chehab]

Em Tue, 04 Jun 2019 10:10:51 +1000
Daniel Axtens  escreveu:

> Mauro Carvalho Chehab  writes:
> 
> > The filters.py redesign that happened for patchwork 1.1 removed
> > a functionality that we use a lot: to filter patches that weren't
> > delegated to anyone.
> >
> > Also, it is a way harder to find someone to delegate with a free
> > text input. Use, instead a combo-box just like before.
> >
> > Signed-off-by: Mauro Carvalho Chehab 
> >
> > ---
> >
> > LinuxTV was using some pre-version 1.0 patchwork instance since
> > last week. Migrating was not easy, due to the delegation patches we
> > developed for 1.x, that caused the migration scripts to fail.
> >
> > On Friday, we finally migrated to the latest stable version:
> >
> > https://patchwork.linuxtv.org
> >
> > After the migration, we noticed that one feature that we used a
> > lot got removed from patchwork: the filter was not allowing anymore
> > to filter not-delegated patches.
> >
> > On media, we receive a large amount of patches per week. Just after
> > the migration, we received ~30 patches, and that's because it is
> > a weekend!
> >
> > In order to handle such high volume, we have several sub-maintainers,
> > each one responsible for one part of the subsystem. While we do have
> > delegation rules on patchwork, we still do manual delegation. So,
> > being able to see what patches are not on someone's queue is very
> > important to us.
> >
> > This patch restores such feature to patchwork. As a plus, it now
> > shows a combo-box with just the names of people that maintain
> > projects, instead of a free-text input that would otherwise seek
> > the entire user's database.  
> 
> Right, that seems like a feature we probably should not have killed,
> sorry.
> 
> It looks like it went away in 2015:
> 
> commit f439f5414206c94d486c60801a86acc57ad3f273
> Author: Stephen Finucane 
> Date:   Mon Aug 24 11:05:47 2015 +0100
> 
> Add delegate filter autocomplete support
> 
> It would be helpful to provide autocomplete functionality for the
> delegate filter field, similar to that provided for the submitter
> field. This will provide a way to handle larger delegate lists without
> overloading the user with options.
> 
> Add this functionality through use of Selectize, which is already used
> to provide similar functionality for filtering of "submitters".
> 
> Signed-off-by: Stephen Finucane 
> 
> So perhaps reverting that all the way back might not be the way to go,
> but we should have something for your usecase.
> 
> Stephen, thoughts?

Maybe the change was done in order to allow delegating patches to 
anyone:

  commit e0fd7cd91a5fbe0a0077c46bea870ccd09c8920d (v1.0.0-154-ge0fd7cd)
  Author: Stephen Finucane 
  Date:   Mon Aug 24 14:21:43 2015 +0100

Allow assigning of any user as delegate

With was reverted one year later:

commit 198139e4112cf337ffea403000441931b4ddad06 (v1.1.0-227-g198139e)
  Author: Stephen Finucane 
  Date:   Sun Sep 25 22:37:11 2016 +0100

Revert "Allow assigning of any user as delegate"

> 
> >  def _form(self):
> > -return mark_safe(' > - 'id="delegate_input" class="form-control">')
> > +delegates = 
> > User.objects.filter(profile__maintainer_projects__isnull = False)  
> 
> If we were to go down this route, I think we'd want to be filtering
> against the project as well? 

Before f439f5414206c94d486c60801a86acc57ad3f273, it used to check for
the project ID, but the code with was calling set_project() at the
Filter initialization, but this got removed. I don't know enough about
Django to re-add it.

> I'm thinking of OzLabs where there are a
> couple dozen different projects with completely different sets of
> people.

Either that or the Django's equivalent to:

SELECT DISTINCT(p.delegate_id) FROM `patchwork_patch` AS p, 
patchwork_submission AS s WHERE p.submission_ptr_id = s.id and project_id = 


That would, IMHO, be better, as it will only filter for the ones that
actually have patches delegated. As a plus, if we ever re-introduce a
feature to allow delegating to anyone, this will still work.

We're actually considering to have a way to delegate to driver
maintainers (with aren't project maintainers at Django).

> 
> Regards,
> Daniel
> 
> > +
> > +str = ''
> > +
> > +selected = ''
> > +if not self.applied:
> > +selected = 'selected'
> > +
> > +str += '--' % selected
> > +
> > +selected = '

filters: re-add the possibility of filtering undelegated patches

[Mauro Carvalho Chehab]

The filters.py redesign that happened for patchwork 1.1 removed
a functionality that we use a lot: to filter patches that weren't
delegated to anyone.

Also, it is a way harder to find someone to delegate with a free
text input. Use, instead a combo-box just like before.

Signed-off-by: Mauro Carvalho Chehab 

---

LinuxTV was using some pre-version 1.0 patchwork instance since
last week. Migrating was not easy, due to the delegation patches we
developed for 1.x, that caused the migration scripts to fail.

On Friday, we finally migrated to the latest stable version:

https://patchwork.linuxtv.org

After the migration, we noticed that one feature that we used a
lot got removed from patchwork: the filter was not allowing anymore
to filter not-delegated patches.

On media, we receive a large amount of patches per week. Just after
the migration, we received ~30 patches, and that's because it is
a weekend!

In order to handle such high volume, we have several sub-maintainers,
each one responsible for one part of the subsystem. While we do have
delegation rules on patchwork, we still do manual delegation. So,
being able to see what patches are not on someone's queue is very
important to us.

This patch restores such feature to patchwork. As a plus, it now
shows a combo-box with just the names of people that maintain
projects, instead of a free-text input that would otherwise seek
the entire user's database.

diff --git a/patchwork/filters.py b/patchwork/filters.py
index f6e483947b24..167f219810aa 100644
--- a/patchwork/filters.py
+++ b/patchwork/filters.py
@@ -375,6 +375,7 @@ class ArchiveFilter(Filter):
 
 class DelegateFilter(Filter):
 param = 'delegate'
+no_delegate_str = 'Nobody'
 AnyDelegate = 1
 
 def __init__(self, filters):
@@ -391,6 +392,11 @@ class DelegateFilter(Filter):
 if not key:
 return
 
+if key == self.no_delegate_str:
+self.delegate_match = key
+self.applied = True
+return
+
 try:
 self.delegate = User.objects.get(id=int(key))
 except (ValueError, User.DoesNotExist):
@@ -410,6 +416,9 @@ class DelegateFilter(Filter):
 if self.delegate:
 return {'delegate': self.delegate}
 
+if self.delegate_match == self.no_delegate_str:
+return {'delegate__username__isnull': True}
+
 if self.delegate_match:
 return {'delegate__username__icontains': self.delegate_match}
 return {}
@@ -422,8 +431,33 @@ class DelegateFilter(Filter):
 return ''
 
 def _form(self):
-return mark_safe('')
+delegates = User.objects.filter(profile__maintainer_projects__isnull = 
False)
+
+str = ''
+
+selected = ''
+if not self.applied:
+selected = 'selected'
+
+str += '--' % selected
+
+selected = ''
+if self.applied and self.delegate is None:
+selected = 'selected'
+
+str += '%s' % \
+(selected, self.no_delegate_str, self.no_delegate_str)
+
+for d in delegates:
+selected = ''
+if d == self.delegate:
+selected = ' selected'
+
+str += '%s' % (selected,
+d.id, d.username)
+str += ''
+
+return mark_safe(str)
 
 def key(self):
 if self.delegate:
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH 06/10] parser: Use full regexps for delegation rules paths

[Mauro Carvalho Chehab]

Em Sat, 26 Dec 2015 13:05:55 +0200
Laurent Pinchart <laurent.pinch...@ideasonboard.com> escreveu:

> Hi Johannes,
> 
> On Friday 25 December 2015 16:59:50 Johannes Berg wrote:
> > On Thu, 2015-12-24 at 14:06 +, Finucane, Stephen wrote:
> > >
> > > > Paths are validated by trying to compile it as a regexp using a
> > > > custom validator.
> > > > 
> > > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > > > Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
> > > 
> > > Some small nits that I can fix myself. Other than that,
> > 
> > A small comment that may or may not be relevant - but there's a bunch
> > of things one can do with regexes, from taking a lot of CPU to taking a
> > lot of memory.
> > 
> > What's the trust model for running regexes? I haven't found it in the
> > patches easily right now. If it's configured only in the config file it
> > should be OK, but if any kind of remote user can configure it then it
> > may need safeguards of some kind?
> > 
> > I'm just thinking of a use case like kernel.org where you don't really
> > even trust the people who are the typical delegates/admins in patchwork
> > for a given project, since they are pretty much just random people the
> > admin doesn't necessarily trust much.
> > 
> > (Or put another way - I'd hate for them to patch out/disable this
> > feature because of security concerns, since I'd want to use it on
> > kernel.org, but I'm not sure the admins would want me configuring
> > arbitrary regexes there)
> 
> I agree with your concerns but haven't given them a thought to be honest. 
> Right now only patchwork admins can changes the rules, but as you mention we 
> might not trust them.
> 
> I've used regexps for convenience, we could possibly replace them with a less 
> dangerous type of pattern. One option I was toying with was to create rules 
> automatically from MAINTAINERS, but I don't think that would be flexible 
> enough.
> 

IMHO, the real problem here is the need that everybody with write
access should be project maintainer in patchwork, and it lacks logs
when a patch is delegated or changed its status.

I would very much prefer to be able to delegate a patch to a driver
maintainer (with I don't have much trustee enough to promote it to
a Project Maintainer), but, in this case, I would need logs if such
person changes the patch status or delegates the patch to somebody 
else. I would also expect that the project maintainers would receive
any notification e-mails if such person changes the status.

Even better, I would like to be able to approve such changes for the
ones I don't trust enough, as I would need to confirm if the patch
change is associated with enough review emails at the ML, and, in
the case of patch acceptance, I would need to take the action of
adding the patch on my tree.

Regards,
Mauro


___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH 07/10] Report if a patch is delagated and to whom

[Mauro Carvalho Chehab]

Em Thu, 24 Dec 2015 13:57:08 +
"Finucane, Stephen" <stephen.finuc...@intel.com> escreveu:

> On 28 Nov 10:14, Mauro Carvalho Chehab wrote:
> > When a patch is delegated, it can be important to report it via
> > pwclient, as handing delegated patches may be different. So, add
> > an optional field at the email's body if this is happening.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
> > ---
> >  patchwork/models.py | 54 
> > +
> >  1 file changed, 54 insertions(+)
> > 
> > diff --git a/patchwork/models.py b/patchwork/models.py
> > index e552217a3cbc..024ff4d8d5e1 100644
> > --- a/patchwork/models.py
> > +++ b/patchwork/models.py
> > @@ -323,6 +323,60 @@ class Patch(models.Model):
> >  str = fname_re.sub('-', self.name)
> >  return str.strip('-') + '.patch'
> >  
> > +def mbox(self):
> > +postscript_re = re.compile('\n-{2,3} ?\n')
> > +
> > +comment = None
> > +try:
> > +comment = Comment.objects.get(patch = self, msgid = self.msgid)
> > +except Exception:
> > +pass
> > +
> > +body = ''
> > +if comment:
> > +body = comment.content.strip() + "\n"
> > +
> > +parts = postscript_re.split(body, 1)
> > +if len(parts) == 2:
> > +(body, postscript) = parts
> > +body = body.strip() + "\n"
> > +postscript = postscript.strip() + "\n"
> > +else:
> > +postscript = ''
> > +
> > +for comment in Comment.objects.filter(patch = self) \
> > +.exclude(msgid = self.msgid):
> > +body += comment.patch_responses()
> > +
> > +if body:
> > +body += '\n'
> > +
> > +if postscript:
> > +body += '---\n' + postscript.strip() + '\n'
> > +
> > +if self.content:
> > +body += '\n' + self.content
> > +
> > +mail = PatchMbox(body)
> > +mail['Subject'] = self.name
> > +mail['Date'] = email.utils.formatdate(
> > +time.mktime(self.date.utctimetuple()))
> > +mail['From'] = unicode(self.submitter)
> > +mail['X-Patchwork-Id'] = str(self.id)
> > +   if self.delegate:
> > +mail['X-Patchwork-Delegate'] = str(self.delegate.email)
> 
> [snip]
> 
> You seem to be missing a couple of dependencies here: it fails to run
> because of missing imports and the code looks like a minor modification
> of functionality found in 'patchwork/views/__init__.py'. 

Hmm.. it runs fine at patchwork, but perhaps due to some other
locale changes I did there, for patchwork to find the dependencies.

> Is there any
> reason we can't just include the above 'X-Patchwork-Delegate' lines in
> the 'patch_to_mbox' function found in the aforementioned file?

I remember I tried to add it on some other place (don't remember where
anymore, as it was done several years ago). It didn't work when the
patch were obtained via XMLRPC.

I need to receive this tag via pwclient, as my local scripts use it
to remove patches delegated to other developers from my own work
queue.

Anyway, feel free to either modify this patch or do to something else
that would provide the same functionality. I'm not a Pyhton expert.
So, I'm probably not taking the best approach ;)

> 
> Stephen
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 10/10] parsemail: Add print messages to help debugging email parsing

[Mauro Carvalho Chehab]

When things are broken at parsemail, we need to be able to
debug it. So, add some messages to it, in order to allow
checking what it actually did, and to let it return 1 if
an email got skipped by parsemail.

Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/bin/parsemail.py | 19 +++
 patchwork/bin/parsemail.sh |  2 +-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index 5d6ddf45d264..71d284b273b0 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -79,6 +79,9 @@ def find_project(mail):
 except Project.DoesNotExist:
 pass
 
+if project is None:
+   project = Project.objects.get(listid = 'linux-media.vger.kernel.org')
+
 return project
 
 def find_author(mail):
@@ -143,6 +146,9 @@ def find_pull_request(content):
 match = git_re.search(content)
 if match:
 return match.group(1)
+
+print "not a git pull request"
+
 return None
 
 def try_decode(payload, charset):
@@ -392,13 +398,16 @@ def parse_mail(mail):
 
 # some basic sanity checks
 if 'From' not in mail:
-return 0
+print "From: is missing"
+return 1
 
 if 'Subject' not in mail:
-return 0
+print "Subject: is missing"
+return 1
 
 if 'Message-Id' not in mail:
-return 0
+print "Message-Id: is missing"
+return 1
 
 hint = mail.get('X-Patchwork-Hint', '').lower()
 if hint == 'ignore':
@@ -407,7 +416,7 @@ def parse_mail(mail):
 project = find_project(mail)
 if project is None:
 print "no project found"
-return 0
+return 1
 
 msgid = mail.get('Message-Id').strip()
 
@@ -431,6 +440,7 @@ def parse_mail(mail):
 patch.delegate = delegate
 try:
 patch.save()
+   print "patch saved"
 except Exception, ex:
 print str(ex)
 
@@ -444,6 +454,7 @@ def parse_mail(mail):
 comment.msgid = msgid
 try:
 comment.save()
+   print "comment saved"
 except Exception, ex:
 print str(ex)
 
diff --git a/patchwork/bin/parsemail.sh b/patchwork/bin/parsemail.sh
index 9973392de9d4..c8220a799bba 100755
--- a/patchwork/bin/parsemail.sh
+++ b/patchwork/bin/parsemail.sh
@@ -26,4 +26,4 @@ 
PYTHONPATH="$PATCHWORK_BASE":"$PATCHWORK_BASE/lib/python:$PYTHONPATH" \
 DJANGO_SETTINGS_MODULE=patchwork.settings.production \
 "$PATCHWORK_BASE/patchwork/bin/parsemail.py"
 
-exit 0
+exit $@
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 01/10] models: Add DelegationRule object

[Mauro Carvalho Chehab]

From: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

Delegation rules are used to automatically set the delegate of a patch
based on the files touched by the patch.

Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/admin.py  | 9 -
 patchwork/models.py | 8 
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/patchwork/admin.py b/patchwork/admin.py
index eb8daa1eced2..e05c8bc7cf03 100644
--- a/patchwork/admin.py
+++ b/patchwork/admin.py
@@ -1,9 +1,16 @@
 from django.contrib import admin
 from patchwork.models import Project, Person, UserProfile, State, Patch, \
- Comment, Bundle, Tag
+ Comment, Bundle, Tag, DelegationRule
+
+class DelegationRuleInline(admin.TabularInline):
+model = DelegationRule
+fields = ('path', 'user')
 
 class ProjectAdmin(admin.ModelAdmin):
 list_display = ('name', 'linkname','listid', 'listemail')
+inlines = [
+DelegationRuleInline,
+]
 admin.site.register(Project, ProjectAdmin)
 
 class PersonAdmin(admin.ModelAdmin):
diff --git a/patchwork/models.py b/patchwork/models.py
index c2b8a9c9408d..1bd9af24b510 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -78,6 +78,14 @@ class Project(models.Model):
 ordering = ['linkname']
 
 
+class DelegationRule(models.Model):
+user = models.ForeignKey(User)
+path = models.CharField(max_length=255)
+project = models.ForeignKey(Project)
+
+def __unicode__(self):
+return self.path
+
 class UserProfile(models.Model):
 user = models.OneToOneField(User, unique = True, related_name='profile')
 primary_project = models.ForeignKey(Project, null = True, blank = True)
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 08/10] views: make X-Patchwork-Delegate: to work again

[Mauro Carvalho Chehab]

Something broke the output of X-Patchwork-Delegate: via xmlrpc.

Fix it by moving the code to the xmlrpc part.

Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/models.py | 2 --
 patchwork/views/__init__.py | 4 
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/patchwork/models.py b/patchwork/models.py
index 024ff4d8d5e1..676422d8aae5 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -363,8 +363,6 @@ class Patch(models.Model):
 time.mktime(self.date.utctimetuple()))
 mail['From'] = unicode(self.submitter)
 mail['X-Patchwork-Id'] = str(self.id)
-   if self.delegate:
-mail['X-Patchwork-Delegate'] = str(self.delegate.email)
 mail['Message-Id'] = self.msgid
 mail.set_unixfrom('From patchwork ' + self.date.ctime())
 
diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index 8df8920ce991..20122b733c3c 100644
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -205,6 +205,10 @@ def patch_to_mbox(patch):
 mail['Message-Id'] = patch.msgid
 mail.set_unixfrom('From patchwork ' + patch.date.ctime())
 
+try:
+mail['X-Patchwork-Delegate'] = str(patch.delegate.email)
+except:
+   pass
 
 copied_headers = ['To', 'Cc', 'Date']
 orig_headers = HeaderParser().parsestr(str(patch.headers))
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 03/10] parser: Set the delegate using Delegation rules

[Mauro Carvalho Chehab]

From: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/bin/parsemail.py | 47 +++---
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index e66b55715d8f..4f22c7f2d6a0 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -25,13 +25,14 @@ import datetime
 import time
 import operator
 import codecs
+from fnmatch import fnmatch
 from email import message_from_file
 from email.header import Header, decode_header
 from email.utils import parsedate_tz, mktime_tz
 
-from patchwork.parser import parse_patch
+from patchwork.parser import parse_patch, patch_get_filenames
 from patchwork.models import Patch, Project, Person, Comment, State, \
-get_default_initial_patch_state
+DelegationRule, get_default_initial_patch_state
 import django
 from django.contrib.auth.models import User
 
@@ -208,6 +209,10 @@ def find_content(project, mail):
 
 patch = None
 comment = None
+filenames = None
+
+if patchbuf:
+filenames = patch_get_filenames(patchbuf)
 
 if pullurl or patchbuf:
 name = clean_subject(mail.get('Subject'), [project.linkname])
@@ -225,12 +230,12 @@ def find_content(project, mail):
 else:
 cpatch = find_patch_for_comment(project, mail)
 if not cpatch:
-return (None, None)
+return (None, None, None)
 comment = Comment(patch = cpatch, date = mail_date(mail),
 content = clean_content(commentbuf),
 headers = mail_headers(mail))
 
-return (patch, comment)
+return (patch, comment, filenames)
 
 def find_patch_for_comment(project, mail):
 # construct a list of possible reply message ids
@@ -334,6 +339,31 @@ def get_state(state_name):
 pass
 return get_default_initial_patch_state()
 
+def auto_delegate(project, filenames):
+if not filenames:
+return None
+
+rules = list(DelegationRule.objects.filter(project = project))
+
+patch_delegate = None
+
+for filename in filenames:
+file_delegate = None
+for rule in rules:
+if fnmatch(filename, rule.path):
+file_delegate = rule.user
+break;
+
+if file_delegate is None:
+return None
+
+if patch_delegate is not None and file_delegate != patch_delegate:
+return None
+
+patch_delegate = file_delegate
+
+return patch_delegate
+
 def get_delegate(delegate_email):
 """ Return the delegate with the given email or None """
 if delegate_email:
@@ -368,9 +398,13 @@ def parse_mail(mail):
 
 (author, save_required) = find_author(mail)
 
-(patch, comment) = find_content(project, mail)
+(patch, comment, filenames) = find_content(project, mail)
 
 if patch:
+delegate = get_delegate(mail.get('X-Patchwork-Delegate', '').strip())
+   if not delegate:
+   delegate = auto_delegate(project, filenames)
+
 # we delay the saving until we know we have a patch.
 if save_required:
 author.save()
@@ -379,8 +413,7 @@ def parse_mail(mail):
 patch.msgid = msgid
 patch.project = project
 patch.state = get_state(mail.get('X-Patchwork-State', '').strip())
-patch.delegate = get_delegate(
-mail.get('X-Patchwork-Delegate', '').strip())
+patch.delegate = delegate
 try:
 patch.save()
 except Exception, ex:
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 07/10] Report if a patch is delagated and to whom

[Mauro Carvalho Chehab]

When a patch is delegated, it can be important to report it via
pwclient, as handing delegated patches may be different. So, add
an optional field at the email's body if this is happening.

Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/models.py | 54 +
 1 file changed, 54 insertions(+)

diff --git a/patchwork/models.py b/patchwork/models.py
index e552217a3cbc..024ff4d8d5e1 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -323,6 +323,60 @@ class Patch(models.Model):
 str = fname_re.sub('-', self.name)
 return str.strip('-') + '.patch'
 
+def mbox(self):
+postscript_re = re.compile('\n-{2,3} ?\n')
+
+comment = None
+try:
+comment = Comment.objects.get(patch = self, msgid = self.msgid)
+except Exception:
+pass
+
+body = ''
+if comment:
+body = comment.content.strip() + "\n"
+
+parts = postscript_re.split(body, 1)
+if len(parts) == 2:
+(body, postscript) = parts
+body = body.strip() + "\n"
+postscript = postscript.strip() + "\n"
+else:
+postscript = ''
+
+for comment in Comment.objects.filter(patch = self) \
+.exclude(msgid = self.msgid):
+body += comment.patch_responses()
+
+if body:
+body += '\n'
+
+if postscript:
+body += '---\n' + postscript.strip() + '\n'
+
+if self.content:
+body += '\n' + self.content
+
+mail = PatchMbox(body)
+mail['Subject'] = self.name
+mail['Date'] = email.utils.formatdate(
+time.mktime(self.date.utctimetuple()))
+mail['From'] = unicode(self.submitter)
+mail['X-Patchwork-Id'] = str(self.id)
+   if self.delegate:
+mail['X-Patchwork-Delegate'] = str(self.delegate.email)
+mail['Message-Id'] = self.msgid
+mail.set_unixfrom('From patchwork ' + self.date.ctime())
+
+
+copied_headers = ['To', 'Cc']
+orig_headers = HeaderParser().parsestr(str(self.headers))
+for header in copied_headers:
+if header in orig_headers:
+mail[header] = orig_headers[header]
+
+return mail
+
 @models.permalink
 def get_absolute_url(self):
 return ('patchwork.views.patch.patch', (), {'patch_id': self.id})
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 05/10] models: Add priority field to DelegationRule

[Mauro Carvalho Chehab]

From: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

The priority allows sorting delegation rules according to their
priorities. Higher priority rules will be evaluated first.

Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/admin.py  | 2 +-
 patchwork/models.py | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/patchwork/admin.py b/patchwork/admin.py
index e05c8bc7cf03..216cdf583968 100644
--- a/patchwork/admin.py
+++ b/patchwork/admin.py
@@ -4,7 +4,7 @@ from patchwork.models import Project, Person, UserProfile, 
State, Patch, \
 
 class DelegationRuleInline(admin.TabularInline):
 model = DelegationRule
-fields = ('path', 'user')
+fields = ('path', 'user', 'priority')
 
 class ProjectAdmin(admin.ModelAdmin):
 list_display = ('name', 'linkname','listid', 'listemail')
diff --git a/patchwork/models.py b/patchwork/models.py
index 1bd9af24b510..101a9af9746f 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -82,10 +82,15 @@ class DelegationRule(models.Model):
 user = models.ForeignKey(User)
 path = models.CharField(max_length=255)
 project = models.ForeignKey(Project)
+priority = models.IntegerField(default = 0)
 
 def __unicode__(self):
 return self.path
 
+class Meta:
+ordering = ['-priority', 'path']
+unique_together = (('path', 'project'))
+
 class UserProfile(models.Model):
 user = models.OneToOneField(User, unique = True, related_name='profile')
 primary_project = models.ForeignKey(Project, null = True, blank = True)
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 00/10] Patchwork Autodelegate patches

[Mauro Carvalho Chehab]

This patch series contain the non-LinuxTV specific patches,
mainly the auto-delegate patches written by Laurent Pinchart.

Those got ported to patchwork upstream.

Please notice that I'm still fighting to finish the migration
from Django 1.4 to Django 1.7. So, this patch series is not
fully tested.

The auto-delegation patches are authored by Laurent, and were
rebased by me to apply under the upstream branch. They're
working for a long time at LinuxTV.org, but the rebase might
cause some troubles on them.

So, please test.

There are some patches I wrote myself, to fix some bugs
and improve a few things:

- When a patch is delegated, it will add a new meta tag
(X-Patchwork-Delegate) via xmlrpc. This helps my local scripts
to detect when a patch is already being handled by somebody else.

- Added the new delegate table to migrations. The patch
was actually generated via ./manage makemigrations. Not sure
if this is correct, as I didn't need to do any migrations related
to it here ;)

- Added some prints that helps to debug troubles with parsemail.
From time to time, we have troubles with non-UTF8 messages on
different parts of the e-mail. Those prints helps a lot to
identify when a patch is not handled, giving some explanation
why it failed.

Laurent Pinchart (6):
  models: Add DelegationRule object
  parser: Add patch_get_filenames()
  parser: Set the delegate using Delegation rules
  forms: Allow the delegate field to keep its current value
  models: Add priority field to DelegationRule
  parser: Use full regexps for delegation rules paths

Mauro Carvalho Chehab (4):
  Report if a patch is delagated and to whom
  views: make X-Patchwork-Delegate: to work again
  Add delegate to migrations
  parsemail: Add print messages to help debugging email parsing

 patchwork/admin.py   |  9 +++-
 patchwork/bin/parsemail.py   | 81 +++-
 patchwork/bin/parsemail.sh   |  2 +-
 patchwork/forms.py   | 16 ---
 patchwork/migrations/0001_initial.py | 24 +++
 patchwork/models.py  | 72 
 patchwork/parser.py  | 34 +++
 patchwork/views/__init__.py  |  4 ++
 8 files changed, 223 insertions(+), 19 deletions(-)

-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 09/10] Add delegate to migrations

[Mauro Carvalho Chehab]

Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/migrations/0001_initial.py | 24 
 1 file changed, 24 insertions(+)

diff --git a/patchwork/migrations/0001_initial.py 
b/patchwork/migrations/0001_initial.py
index 65d1c3556414..5b71f3ef5385 100644
--- a/patchwork/migrations/0001_initial.py
+++ b/patchwork/migrations/0001_initial.py
@@ -165,6 +165,30 @@ class Migration(migrations.Migration):
 ('orig_state', models.ForeignKey(to='patchwork.State')),
 ],
 ),
+migrations.CreateModel(
+name='DelegationRule',
+fields=[
+('id', models.AutoField(verbose_name='ID', serialize=False, 
auto_created=True, primary_key=True)),
+('path', models.CharField(max_length=255, 
validators=[patchwork.models.validate_rule_path])),
+('priority', models.IntegerField(default=0)),
+('project', models.ForeignKey(to='patchwork.Project')),
+('user', models.ForeignKey(to=settings.AUTH_USER_MODEL)),
+],
+options={
+'ordering': ['-priority', 'path'],
+},
+bases=(models.Model,),
+),
+migrations.AlterUniqueTogether(
+name='delegationrule',
+unique_together=set([('path', 'project')]),
+),
+migrations.AlterField(
+model_name='patch',
+name='state',
+
field=models.ForeignKey(default=patchwork.models.get_default_initial_patch_state,
 to='patchwork.State'),
+preserve_default=True,
+),
 migrations.AddField(
 model_name='patchtag',
 name='patch',
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 06/10] parser: Use full regexps for delegation rules paths

[Mauro Carvalho Chehab]

From: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

Paths are validated by trying to compile it as a regexp using a custom
validator.

Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/bin/parsemail.py | 19 +--
 patchwork/models.py|  9 -
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index 4f22c7f2d6a0..5d6ddf45d264 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -339,18 +339,33 @@ def get_state(state_name):
 pass
 return get_default_initial_patch_state()
 
+class Rule(object):
+pass
+
 def auto_delegate(project, filenames):
 if not filenames:
 return None
 
-rules = list(DelegationRule.objects.filter(project = project))
+# Precompile the path regexps
+rules = []
+for rule in DelegationRule.objects.filter(project = project):
+r = Rule()
+r.user = rule.user
+
+try:
+r.path = re.compile(rule.path)
+except:
+print '%s is not a valid regular expression' % rule.path
+continue
+
+rules.append(r)
 
 patch_delegate = None
 
 for filename in filenames:
 file_delegate = None
 for rule in rules:
-if fnmatch(filename, rule.path):
+if rule.path.match(filename):
 file_delegate = rule.user
 break;
 
diff --git a/patchwork/models.py b/patchwork/models.py
index 101a9af9746f..e552217a3cbc 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -19,6 +19,7 @@
 
 from django.db import models
 from django.contrib.auth.models import User
+from django.core.exceptions import ValidationError
 from django.core.urlresolvers import reverse
 from django.contrib.sites.models import Site
 from django.conf import settings
@@ -78,9 +79,15 @@ class Project(models.Model):
 ordering = ['linkname']
 
 
+def validate_rule_path(value):
+try:
+re.compile(value)
+except:
+raise ValidationError(u'`%s\' is not a valid regulator expression' % 
value)
+
 class DelegationRule(models.Model):
 user = models.ForeignKey(User)
-path = models.CharField(max_length=255)
+path = models.CharField(max_length = 255, validators = 
[validate_rule_path])
 project = models.ForeignKey(Project)
 priority = models.IntegerField(default = 0)
 
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 04/10] forms: Allow the delegate field to keep its current value

[Mauro Carvalho Chehab]

From: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

When a patch is delegated at parse time (either through the
X-Patchwork-Hint mail header or through delegation rules), the delegate
might not be in the list of project maintainers.

Add the current delegate to the list of acceptable values for the
delegate field to allow the current value to be kept when editing the
patch.

Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/forms.py | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/patchwork/forms.py b/patchwork/forms.py
index 03279588a1ef..d82a3418fabc 100644
--- a/patchwork/forms.py
+++ b/patchwork/forms.py
@@ -19,6 +19,7 @@
 
 
 from django.contrib.auth.models import User
+from django.db.models.query_utils import Q
 from django import forms
 
 from patchwork.models import Patch, State, Bundle, UserProfile
@@ -88,11 +89,14 @@ class DeleteBundleForm(forms.Form):
 bundle_id = forms.IntegerField(widget = forms.HiddenInput)
 
 class DelegateField(forms.ModelChoiceField):
-def __init__(self, project, *args, **kwargs):
-queryset = User.objects.filter(profile__in = \
-UserProfile.objects \
-.filter(maintainer_projects = project) \
-.values('pk').query)
+def __init__(self, project, instance = None, *args, **kwargs):
+   q = Q(profile__in = \
+  UserProfile.objects \
+  .filter(maintainer_projects = project) \
+  .values('pk').query)
+if instance and instance.delegate:
+   q = q | Q(username = instance.delegate)
+queryset = User.objects.complex_filter(q)
 super(DelegateField, self).__init__(queryset, *args, **kwargs)
 
 
@@ -103,7 +107,7 @@ class PatchForm(forms.ModelForm):
 if not project:
 raise Exception("meep")
 super(PatchForm, self).__init__(instance = instance, *args, **kwargs)
-self.fields['delegate'] = DelegateField(project, required = False)
+self.fields['delegate'] = DelegateField(project, instance, required = 
False)
 
 class Meta:
 model = Patch
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 02/10] parser: Add patch_get_filenames()

[Mauro Carvalho Chehab]

From: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

The function returns the list of files touched by a patch, after
stripping the leading component of the path name.

Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
---
 patchwork/parser.py | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/patchwork/parser.py b/patchwork/parser.py
index 13b4466a4b14..2dbbf2b7b667 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -238,6 +238,34 @@ def extract_tags(content, tags):
 
 return counts
 
+def patch_get_filenames(str):
+# normalise spaces
+str = str.replace('\r', '')
+str = str.strip() + '\n'
+
+filenames = {}
+
+for line in str.split('\n'):
+
+if len(line) <= 0:
+continue
+
+filename_match = _filename_re.match(line)
+if not filename_match:
+continue
+
+filename = filename_match.group(2)
+if filename.startswith('/dev/null'):
+continue
+
+filename = '/'.join(filename.split('/')[1:])
+filenames[filename] = True
+
+filenames = filenames.keys()
+filenames.sort()
+
+return filenames
+
 def main(args):
 from optparse import OptionParser
 
@@ -248,6 +276,8 @@ def main(args):
 dest = 'print_comment', help = 'print parsed comment')
 parser.add_option('-#', '--hash', action = 'store_true',
 dest = 'print_hash', help = 'print patch hash')
+parser.add_option('-f', '--filenames', action = 'store_true',
+dest = 'print_filenames', help = 'print file names')
 
 (options, args) = parser.parse_args()
 
@@ -265,6 +295,10 @@ def main(args):
 if options.print_comment and comment:
 print "Comment: \n" + comment
 
+if options.print_filenames:
+filenames = patch_get_filenames(content)
+print "File names: \n" + '\n'.join(filenames)
+
 if __name__ == '__main__':
 import sys
 sys.exit(main(sys.argv))
-- 
2.5.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2 0/4] Remove support for Django 1.6

[Mauro Carvalho Chehab]

Hi Don,

Em Mon, 09 Nov 2015 11:22:50 -0500
Don Zickus  escreveu:

> On Fri, Nov 06, 2015 at 07:02:31PM +0100, Johannes Berg wrote:
> > +Konstantin, the kernel.org sysadmin. The context is that patchwork has
> > just removed (see subject) support for Django 1.6, but I've been told
> > that RHEL/EPEL7 which you run has only that version, so updating to the
> > latest patchwork would now be impossible (again)
> 
> Hi,
> 
> Just to throw another datapoint at folks.  RHEL-7 has this notion of
> software collections.  It allows customers to update a collection tools to a
> newer version (RH supported) in the /opt area.  Then using a script (which
> sets env variables), a program can easily use python3 and postgres9.2 on
> RHEL-7.
> 
> You can read about it here:
> 
> https://access.redhat.com/support/policy/updates/rhscl
> 
> and talk with your RH account manager about more of the details.
> 
> You would still need to pip install Django, but at least you could get a
> newer supported python and postgres on RHEL-7.  Not sure how much it helps.
> Again it was just another datapoint.

The big issue is with Django, and not with python/postgres. 

As far as I understood, the impact at the database is only a side
effect. It seems that each new version of Django uses a different logic
to build the SQL tables. So, the SQL tables generated by Django 1.6
are different than the ones generated by Django 1.8. So, the scripts
that allow upgrades between patchwork versions are Django-version
specific.

So, I guess that using RH Software Collections won't actually solve
the issue.

Additionally, not all projects that use patchwork uses RHEL7 for its LTS
distribution. LinuxTV, for example, uses Debian.

> 
> Cheers,
> Don
> 
> 
> > 
> > > We can't support Django 1.4: it's too long in the tooth and has been
> > > unsupported for too long to even consider.
> > 
> > It's also ancient and awful compared to newer versions :)
> > 
> > > So I think we need to answer the following question:
> > > 
> > > * Are deployers going to install from source/pip?
> > 
> > I don't really know. Perhaps Konstantin can chime in.
> > 
> > > If not, then we're going to have (a) roll back the Django 1.6 removal
> > > patches, (b) put together a roadmap for future Django version support and
> > > (c) avoid using non-stdlib libraries (outside of Django) going forward. As
> > > Damien pointed out, these actions come with some rather severe costs for
> > > us so I'd like to be absolutely certain of this before I take any actions.
> > > 
> > 
> > There's the obvious alternative of not caring about LTS installations
> > like kernel.org and simply continue developing the software as is
> > against upstream. *Eventually*, it's going to get to the users, perhaps
> > not as fast as the users (like me) would like.
> > 
> > And I'm not suggesting at all that you should consider "LTS distro"
> > support of huge importance.
> > 
> > I think in a way part of the problem is that your user audience is also
> > developers (in a different space), so we users might be interested in
> > working on tools improvements ourselves (like the regex auto-delegation 
> > feature that Mauro/Laurent have), but there's less incentive to do that
> > if we can't actually benefit from it in a fairly short time frame (and
> > we're more likely to script our way around it instead.)
> > 
> > johannes
> > ___
> > Patchwork mailing list
> > Patchwork@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2 0/4] Remove support for Django 1.6

[Mauro Carvalho Chehab]

Hi Stephen,

Em Thu, 5 Nov 2015 18:18:36 +
"Finucane, Stephen"  escreveu:

> The problem with continuing support for Django 1.6 is, as mentioned, that 
> it incurs a huge amount of overhead due to the SQL migrations (this affects 
> not only developers but also sysadmins, who must validate and apply these
> migrations). There's also the security impacts of using unsupported Django
> versions, though admittedly the extended support should mitigate these impact
> for another few months.
> 
> We want patchwork to be as usable as possible and 'kernel.org' is clearly a 
> hugely popular instance. I can revert these patches and continue to add
> migrations for the foreseeable future, but before doing so I'd like to know 
> if there is any way around this issue? 

Typically, LTS distributions don't upgrade packages that contain API
changes, as the hole point of LTS is the warranty that the APIs should
be stable.

For example, Debian 7 comes with Django 1.4, and will keep using this
version until May, 2018 (when Debian 7 ceases to be supported):
https://packages.debian.org/source/wheezy/python-django

That's by the way, the version we use at patchwork.linuxtv.org. So, we're
unable to do any patchwork update since the day the django 1.4
support was dropped from patchwork.

We have a plan to move to Debian 8, with comes with python 2 + Django 1.7:
https://packages.debian.org/source/jessie/python-django

> Django 1.8 is LTS so I imagine there should be support coming for this in
> EPEL, SLES etc. I'm little to no experience with these distros though, and 
> I have no idea how these folks plan/publish their roadmaps. Could you advise?

It doesn't matter whatever version Django maintainers decide to be their
LTS version if they don't sync it with the LTS distro releases.

The point is: it is a very high risk to run patchwork on any serious
projects like the Linux Kernel on some server that doesn't run a LTS
distribution.

It is also a serious risk to manually maintain its own manually
maintained packages on an LTS distro, as you may risk forgetting to do
some important security upgrade because the fix is not packaged by
inside the distribution.

Regards,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2 0/4] Remove support for Django 1.6

[Mauro Carvalho Chehab]

Em Fri, 06 Nov 2015 17:37:47 +
"Finucane, Stephen" <stephen.finuc...@intel.com> escreveu:

> > On Fri, Nov 06, 2015 at 12:25:18PM -0200, Mauro Carvalho Chehab wrote:
> > > > Django 1.8 is LTS so I imagine there should be support coming for this
> > in
> > > > EPEL, SLES etc. I'm little to no experience with these distros though,
> > and
> > > > I have no idea how these folks plan/publish their roadmaps. Could you
> > advise?
> > >
> > > It doesn't matter whatever version Django maintainers decide to be their
> > > LTS version if they don't sync it with the LTS distro releases.
> > 
> > > The point is: it is a very high risk to run patchwork on any serious
> > > projects like the Linux Kernel on some server that doesn't run a LTS
> > > distribution.
> > >
> > > It is also a serious risk to manually maintain its own manually
> > > maintained packages on an LTS distro, as you may risk forgetting to do
> > > some important security upgrade because the fix is not packaged by
> > > inside the distribution.
> > 
> > Just for the record, my opinion on this: there are definitely good
> > reasons to want to use distribution packages, but that's the usual
> > distro packages vs upstream versions/branches discussion. That's
> > especially true for even newer trendy things like Node.
> > 
> > I'd ask for a slice of empathy here. Demanding that a project
> > dependencies are based on the union of supported LTS releases across
> > distributions will hurt the project. Having constraints like "Patchwork
> > has to work from django 1.4 until 2018 because of Debian 7" makes
> > development a miserable experience.
> 
> We can't support Django 1.4: it's too long in the tooth and has been
> unsupported for too long to even consider. We only just removed Django
> 1.6 support so I can consider rolling that back, but unless someone
> else wants to contribute patches then it's not going to be viable to
> support anything older.

Yes, I know. I'm not asking to re-add Django 1.4 support, but just saying
that my legs got broken when django 1.4 support got removed.

> 
> > That's an even bigger problem when adding more dependencies into the
> > mix.
> 
> You've identified another issue here: non-Django dependencies. At the moment,
> we only rely on a very small set of requirements[1], most of which should be
> available via system packages. If we can't use pip then we're going to be
> very careful not to introduce dependencies that aren't provided by RHEL,
> Ubuntu LTS, Debian etc. This is going to severely hamper developer velocity
> so I'd like to know for sure that this is necessary before undertaking
> anything.
> 
> > I don't need to on, people have discussed at length the impedance
> > mismatch between distribution packages and upstream development. There
> > are a number of solutions people use to decouple their web applications
> > from the underlying system, I'd argue that's even one of the selling
> > point of interesting things like Docker: Build your app with its
> > dependencies, deploy!
> > 
> > So, I'll just ask if people could at least ponder the question on both
> > sides before being very assertive about the "right" way patchwork should
> > handle its dependencies and deployment. It's definitely not a black or
> > white answer, I have seen package updates in distributions break
> > applications as well for instance.
> > 
> > Security doesn't have to mean trusting the distribution (which in turns
> > more often than not trusts an upstream), there is something to say about
> > trusting an upstream directly.
> > 
> > --
> > Damien
> 
> So I think we need to answer the following question:
> 
> * Are deployers going to install from source/pip?

We won't. We don't want to increase the risk of attacks at linuxtv.org.
This is something that we've discussed already internally with the other
machine sysadmins. There, we'll be using only python/django/etc that are
provided by a LTS distribution (currently, Debian 7, but we'll likely
migrate to Debian 8 in some future).

I guess kernel.org also won't be installing Django from its sources,
but that's just my wild guess. I'm c/c Konstantin, as he's the one that
answers for kernel.org infrastructure.

> If not, then we're going to have (a) roll back the Django 1.6 removal
> patches, (b) put together a roadmap for future Django version support and
> (c) avoid using non-stdlib libraries (outside of Django) going forward. As
> Damien pointed out, these actions come with some rather severe costs for
> us so I'd like

Re: [PATCH v2 0/4] Remove support for Django 1.6

[Mauro Carvalho Chehab]

Em Fri, 6 Nov 2015 15:52:02 +
Damien Lespiau <damien.lesp...@intel.com> escreveu:

> On Fri, Nov 06, 2015 at 12:25:18PM -0200, Mauro Carvalho Chehab wrote:
> > > Django 1.8 is LTS so I imagine there should be support coming for this in
> > > EPEL, SLES etc. I'm little to no experience with these distros though, 
> > > and 
> > > I have no idea how these folks plan/publish their roadmaps. Could you 
> > > advise?
> > 
> > It doesn't matter whatever version Django maintainers decide to be their
> > LTS version if they don't sync it with the LTS distro releases.
> 
> > The point is: it is a very high risk to run patchwork on any serious
> > projects like the Linux Kernel on some server that doesn't run a LTS
> > distribution.
> > 
> > It is also a serious risk to manually maintain its own manually
> > maintained packages on an LTS distro, as you may risk forgetting to do
> > some important security upgrade because the fix is not packaged by
> > inside the distribution.
> 
> Just for the record, my opinion on this: there are definitely good
> reasons to want to use distribution packages, but that's the usual
> distro packages vs upstream versions/branches discussion. That's
> especially true for even newer trendy things like Node.
> 
> I'd ask for a slice of empathy here. Demanding that a project
> dependencies are based on the union of supported LTS releases across
> distributions will hurt the project. Having constraints like "Patchwork
> has to work from django 1.4 until 2018 because of Debian 7" makes
> development a miserable experience.

I'm not a python developer, but for me it sounds that the big issue here
is that Django (and Python) seem to have a very bad tradition of breaking
applications because they're always breaking their APIs.

At least where I sit (Kernel development), we try to do our best to
make sure that the APIs wouldn't break support for applications developed
using an older version.

I would say that, if Django has such huge problems of not allowing
apps to be backward compatible, maybe patchwork should try to use
something with a better design.

> That's an even bigger problem when adding more dependencies into the
> mix.
> 
> I don't need to on, people have discussed at length the impedance
> mismatch between distribution packages and upstream development. There
> are a number of solutions people use to decouple their web applications
> from the underlying system, I'd argue that's even one of the selling
> point of interesting things like Docker: Build your app with its
> dependencies, deploy!
> 
> So, I'll just ask if people could at least ponder the question on both
> sides before being very assertive about the "right" way patchwork should
> handle its dependencies and deployment. It's definitely not a black or
> white answer, I have seen package updates in distributions break
> applications as well for instance.

That should never happen with LTS distributions, as this is the selling
point of all LTS distros: APIs are stable, and applications should never
break when packages got updated.

> Security doesn't have to mean trusting the distribution (which in turns
> more often than not trusts an upstream), there is something to say about
> trusting an upstream directly.

You can't trust that upstream won't change APIs nor that upstream won't
be introducing new security bugs. After all, the goal of upstream
development is to introduce new features, and sometimes, things
accidentally break.

On the other hand, the goal of a LTS distribution is to provide a
stable and secure environment.

So, if one needs security and stability, the obvious choice is to use a
LTS distro. That's why kernel.org uses RHEL, and that's why and all other
servers at the Internet that aim to be reliable also use a LTS distro.

If you take other web apps like cgit, they're designed to work without
requiring the latest brand new libraries, as they rely on the APIs that
are known to be found at LTS distros. When new improvements are obtained
by using some specific newer version of some package, they have some
autoconfig logic that would detect the versions of the libraries and
select the corresponding options that would work best for the system
where it is being used.

Perhaps one solution would be to create separate branches for stable
versions of patchwork that could run with LTS distributions (and so
the version that kernel.org and other similar sites would be running)
and use the unstable "master" branch for those who are using patchwork
for some internal development and doesn't need the security provided by
a LTS distribution.

Regards,
Mauro

PS.: I don't work at kernel.org infrastructure, but I have a strong
feeling that they won't be changing from RHEL7 to a non-

Re: [PATCH v2 0/4] Remove support for Django 1.6

[Mauro Carvalho Chehab]

Em Fri, 06 Nov 2015 19:02:31 +0100
Johannes Berg  escreveu:

> > If not, then we're going to have (a) roll back the Django 1.6 removal
> > patches, (b) put together a roadmap for future Django version support and
> > (c) avoid using non-stdlib libraries (outside of Django) going forward. As
> > Damien pointed out, these actions come with some rather severe costs for
> > us so I'd like to be absolutely certain of this before I take any actions.
> > 
> 
> There's the obvious alternative of not caring about LTS installations
> like kernel.org and simply continue developing the software as is
> against upstream. *Eventually*, it's going to get to the users, perhaps
> not as fast as the users (like me) would like.

Well, for that to happen, we would need to either have a tarball or
(highly preferred) a branch with a patchwork version for us to pull
once we hit the requirements for running such version.

Also, that would mean that we'll be running our own fork of patchwork,
as we add some features that we may need to improve our workflow.

Right now, our tree is based on this changeset:
c4e5d96 docs: We're targetting django 1.5 now

But we have a bunch of patches we did our own on the top of that,
in order to have some features we need on our workflow (including
patches that restore backward compatibility with Django 1.4, and
a few fixup patches that I likely picked from upstream):

$ git lg origin/master..
3b2b253 (HEAD, master) Merge remote branch 'origin/django-1.4'
56e7b71 Read X-Patchwork-Delegate:
4876657 Restore compatibility with Django < 1.5
5edb8c3 Merge remote branch 'origin/master'
fce6d9e Report if a patch is delagated and to whom
2db2a1a parser: Use full regexps for delegation rules paths
ac66486 models: Add priority field to DelegationRule
7a4f2a6 forms: Allow the delegate field to keep its current value
42dca6c parser: Set the delegate using Delegation rules
8ecf299 parser: Add patch_get_filenames()
74062cc models: Add DelegationRule object
fdd7e24 docs: Update PostgreSQL database configuration example
f61ec38 Improve parsemail error handling logic a little bit
b6fdf69 Add media-specific comments to patchwork's template.
7fa02d0 Linuxtv-specific setup
51000a4 (origin/django-1.4) requestcontext: Initialise 'messages' context var
e36dbad settings: Use new class for auth context processor
e5dbc32 settings: Use class-based template loading API

> And I'm not suggesting at all that you should consider "LTS distro"
> support of huge importance.

I guess it is: at least every time a new requirement is added, patchwork
should have a "stable" branch with the older requirements. Otherwise,
big projects that run on LTS severs will never be able to benefit from
the changes done upstream.

> I think in a way part of the problem is that your user audience is also
> developers (in a different space), so we users might be interested in
> working on tools improvements ourselves (like the regex auto-delegation 
> feature that Mauro/Laurent have), but there's less incentive to do that
> if we can't actually benefit from it in a fairly short time frame (and
> we're more likely to script our way around it instead.)

True. The autodelegation patches were not sent upstream because upstream
has changed the Django's requirements, and we weren't unable to rebase
to the newer version, due to the LTS requirements.

Regards,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2 0/4] Remove support for Django 1.6

[Mauro Carvalho Chehab]

Em Fri, 06 Nov 2015 20:41:20 +
"Finucane, Stephen" <stephen.finuc...@intel.com> escreveu:

> On 06 Nov 15:57, Mauro Carvalho Chehab wrote:
> > Em Fri, 6 Nov 2015 15:52:02 +
> > Damien Lespiau <damien.lesp...@intel.com> escreveu:
> > 
> > > On Fri, Nov 06, 2015 at 12:25:18PM -0200, Mauro Carvalho Chehab wrote:
> > > > > Django 1.8 is LTS so I imagine there should be support coming for 
> > > > > this in
> > > > > EPEL, SLES etc. I'm little to no experience with these distros 
> > > > > though, and 
> > > > > I have no idea how these folks plan/publish their roadmaps. Could you 
> > > > > advise?
> > > > 
> > > > It doesn't matter whatever version Django maintainers decide to be their
> > > > LTS version if they don't sync it with the LTS distro releases.
> > > 
> > > > The point is: it is a very high risk to run patchwork on any serious
> > > > projects like the Linux Kernel on some server that doesn't run a LTS
> > > > distribution.
> > > > 
> > > > It is also a serious risk to manually maintain its own manually
> > > > maintained packages on an LTS distro, as you may risk forgetting to do
> > > > some important security upgrade because the fix is not packaged by
> > > > inside the distribution.
> > > 
> > > Just for the record, my opinion on this: there are definitely good
> > > reasons to want to use distribution packages, but that's the usual
> > > distro packages vs upstream versions/branches discussion. That's
> > > especially true for even newer trendy things like Node.
> > > 
> > > I'd ask for a slice of empathy here. Demanding that a project
> > > dependencies are based on the union of supported LTS releases across
> > > distributions will hurt the project. Having constraints like "Patchwork
> > > has to work from django 1.4 until 2018 because of Debian 7" makes
> > > development a miserable experience.
> > 
> > I'm not a python developer, but for me it sounds that the big issue here
> > is that Django (and Python) seem to have a very bad tradition of breaking
> > applications because they're always breaking their APIs.
> > 
> > At least where I sit (Kernel development), we try to do our best to
> > make sure that the APIs wouldn't break support for applications developed
> > using an older version.
> 
> I thought it was userspace that should never be broken? I'm familiar with
> Linus'...light-hearted discussions on the matter :) [1]

He meant to say that Kernel should never break userspace, meaning that
the APIs should not break support for binaries compiled with an older
Kernel.

> 
> > I would say that, if Django has such huge problems of not allowing
> > apps to be backward compatible, maybe patchwork should try to use
> > something with a better design.
> 
> We can't drop Django, if that's what you're suggesting. 

No, I'm not suggesting that. Just saying that it sucks that newer
versions break apps written to work with the previous ones.

> I have some quibbles
> with how it's designed (monolithically) but to change would in effect require
> a rewrite. Evolution, not revolution, is the name of the game here.
> 
> > > That's an even bigger problem when adding more dependencies into the
> > > mix.
> > > 
> > > I don't need to on, people have discussed at length the impedance
> > > mismatch between distribution packages and upstream development. There
> > > are a number of solutions people use to decouple their web applications
> > > from the underlying system, I'd argue that's even one of the selling
> > > point of interesting things like Docker: Build your app with its
> > > dependencies, deploy!
> > > 
> > > So, I'll just ask if people could at least ponder the question on both
> > > sides before being very assertive about the "right" way patchwork should
> > > handle its dependencies and deployment. It's definitely not a black or
> > > white answer, I have seen package updates in distributions break
> > > applications as well for instance.
> > 
> > That should never happen with LTS distributions, as this is the selling
> > point of all LTS distros: APIs are stable, and applications should never
> > break when packages got updated.
> > 
> > > Security doesn't have to mean trusting the distribution (which in turns
> > > more often than not trusts an upstream), there is something to say about
> > > trustin

Re: Delegate Vs Reviewer

[Mauro Carvalho Chehab]

Em Wed, 26 Nov 2014 13:42:21 +0800
Jeremy Kerr j...@ozlabs.org escreveu:

 Hi all,
 
  On Mon, Nov 24, 2014 at 05:35:11PM +, Damien Lespiau wrote:
  I'd like to be able to assign reviewers to patches. That seems very
  close to what a delegate is today. Would anyone have objections if I
  just use Delegate as Reviewer?
  
  I'm honestly not sure what Delegate is intended for. It seems like its
  purpose is open to your interpretation.
 
 One example: on the powerpc list, we have an overall maintainer, and
 several sub-maintainers, who are generally responsible for their own
 subsystem or set of hardware.
 
 If a patch for one of the sub-maintiners' areas appears, it can be
 delegated to the appropriate person to review, test, and/or merge. The
 change is then included in the next merge-request from the
 sub-maintainer to the main maintainer.

We do the same at the media subsystem. Laurent even wrote some patches
for patchwork to do patch auto-delegation based on the paths inside
the diffstat, using regex expressions.

Not sure if he sent those patches upstream.

 
 In this case, patch review is only one of the tasks of the delegate.
 
 Cheers,
 
 
 Jeremy
 ___
 Patchwork mailing list
 Patchwork@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/patchwork
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: Name missing?

[Mauro Carvalho Chehab]

Em Fri, 15 Feb 2013 18:34:16 +0100
Wolfram Sang w...@the-dreams.de escreveu:

 Doug,
 
  As far as I can tell, patchwork creates a submitter for each email
  address.  Once it has decided on the name associated with a submitter
  it doesn't change it.  So the problem was actually with James' first
  patch that patchwork saw:
 
 Thanks, that explains. I do think, though, it should take the name from
 the actual patch. Names might change due to marriage or someone might
 decide to add a second name, etc... Or?

I agree. There's a more common case: the submitter sends a patch with some
fantasy name, like media contributor. The maintainer asks the guy to
re-submit the patch with his real name (as required on Kernel rules).

The new patch then comes with the correct name, but patchwork insists
on preserving for the eternity the broken one, forcing the maintainer
to always fix patches from that contributor.

I got a few of those cases already.

Regards,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2] pwclient: Add heuristics to find a whole series of patches

[Mauro Carvalho Chehab]

Em Fri, 21 Dec 2012 09:23:30 +0800
Jeremy Kerr j...@ozlabs.org escreveu:

 Hi Doug,
 
  Add a new filter option '-r' that attempts to list all patches in a
  series.  Since there's no built-in way in patman to do this, we use
  some heuristics to try to find the series.
 
 Interesting idea, something I'm sure would be useful for many patchwork 
 users.
 
 Do you think it would be possible to do this in the actual server code 
 instead, so that the series relations are captured in the database? 
 This way, the series would be available through the web interface too, 
 and the pwclient method of accessing these series would be much more 
 straightforward.
 
 The difficult part of this is how to present the series relations in the 
 web UI, but I can work something out there.

Btw, there are two cases that should likely be handled as well:

1) on a patch series with a patch 0/x, patches 1/x to x/x refers to
patch 0/x;

2) sometimes, someone sends an ack/nack/... tag to the a hole patch series,
replying to patch 0/x.

It would be really nice if patchwork server/database could handle both.

Regards,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2] pwclient: Add heuristics to find a whole series of patches

[Mauro Carvalho Chehab]

Em Thu, 20 Dec 2012 13:06:07 -0800
Doug Anderson diand...@chromium.org escreveu:

 Add a new filter option '-r' that attempts to list all patches in a
 series.  Since there's no built-in way in patman to do this, we use
 some heuristics to try to find the series.

Instead of adding it at the client level, IMHO, it would be a way better
to add it at the patchwork server, allowing to show the patch group
as such and letting tag the entire patch group with the same tag
(as a v2 of a patch series superseeds a v1 of the same series).

Regards,
Mauro

 
 Signed-off-by: Doug Anderson diand...@chromium.org
 
 ---
 Changes in v2:
 - Handle more tag formats; use tags besides just version/num parts
   (like RFC, REPOST, etc) to identify a series
 
  apps/patchwork/bin/pwclient |  151 +-
  1 files changed, 147 insertions(+), 4 deletions(-)
 
 diff --git a/apps/patchwork/bin/pwclient b/apps/patchwork/bin/pwclient
 index 9588615..77d78c7 100755
 --- a/apps/patchwork/bin/pwclient
 +++ b/apps/patchwork/bin/pwclient
 @@ -23,11 +23,14 @@ import os
  import sys
  import xmlrpclib
  import getopt
 +import re
  import string
 +import time
  import tempfile
  import subprocess
  import base64
  import ConfigParser
 +import collections
  
  # Default Patchwork remote XML-RPC server URL
  # This script will check the PW_XMLRPC_URL environment variable
 @@ -79,6 +82,81 @@ class Filter:
  Return human-readable description of the filter.
  return str(self.d)
  
 +class Patch(object):
 +Nicer representation of a patch from the server.
 +
 +def __init__(self, patch_dict):
 +Patch constructor.
 +
 +@patch_dict: The dictionary version of the patch.
 +
 +# Make it easy to compare times of patches by getting an int.
 +self.time = time.mktime(time.strptime(patch_dict[date],
 +  %Y-%m-%d %H:%M:%S))
 +
 +self.version, self.part_num, self.num_parts, self.series_tags = \
 +self._parse_patch_name(patch_dict[name])
 +
 +# Add a few things to make it easier...
 +self.id = patch_dict[id]
 +self.project_id = patch_dict[project_id]
 +self.name = patch_dict[name]
 +self.submitter_id = patch_dict[submitter_id]
 +
 +# Keep the dict in case we need anything else...
 +self.dict = patch_dict
 +
 +@staticmethod
 +def _parse_patch_name(name):
 +Parse tags out of a patch name.
 +
 +
 +@name: The patch name.
 +@return: version: integer version of the patch
 +@return: part_num: integer part number of the patch
 +@return: num_parts: integer number of parts in the patch
 +@return: series_tags: A tuple of tags that should be shared by all
 + patches in this series.  Should be treated as opaque other
 + than comparing equality with other patches.
 +
 +version = 1
 +part_num = 1
 +num_parts = 1
 +series_tags = []
 +
 +# Pull out tags between []; bail if tags aren't found.
 +mo = re.match(r\[([^\]]*)\], name)
 +if mo:
 +tags = mo.group(1).split(',')
 +
 +# Work on one tag at a time
 +for tag in tags:
 +mo = re.match(r(\d*)/(\d*), tag)
 +if mo:
 +part_num = int(mo.group(1))
 +num_parts = int(mo.group(2))
 +continue
 +
 +mo = re.match(r[vV](\d*), tag)
 +if mo:
 +version = int(mo.group(1))
 +
 +series_tags.append(tag)
 +
 +# Add num_parts to the series tags
 +series_tags.append(%d parts % num_parts)
 +
 +# Turn series_tags into a tuple so it's hashable
 +series_tags = tuple(series_tags)
 +
 +return (version, part_num, num_parts, series_tags)
 +
 +def __str__(self):
 +return str(self.dict)
 +
 +def __repr__(self):
 +return repr(self.dict)
 +
  class BasicHTTPAuthTransport(xmlrpclib.SafeTransport):
  
  def __init__(self, username = None, password = None, use_https = False):
 @@ -128,7 +206,8 @@ def usage():
  -w who  : Filter by submitter (name, e-mail substring search)
  -d who  : Filter by delegate (name, e-mail substring search)
  -n max #: Restrict number of results
 --m messageid: Filter by Message-Id\n)
 +-m messageid: Filter by Message-Id
 +-r ID   : Filter by patches in the same series as ID\n)
  sys.stderr.write(\nActions that take an ID argument can also be \
  invoked with:
  -h hash : Lookup by patch hash\n)
 @@ -162,6 +241,56 @@ def person_ids_by_name(rpc, name):
  people = rpc.person_list(name, 0)
  return map(lambda x: x['id'], people)
  
 +def patch_id_to_series(rpc, patch_id):
 +Take a patch ID and return a list of patches in the same 

patchwork is not recognizing ssh (or git+ssh) url's on pull requests

[Mauro Carvalho Chehab]

Hi Jeremy,

The current rejex expression to catch pull requests are missing URL's
with ssh and git+ssh.

Could you please change it to also accept the other supported URL types?

If missing a patch is bad, it is even worse to miss a pull requests ;)

Thanks,
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [RFC] Store patch authors in the new Patch.author field

[Mauro Carvalho Chehab]

Em 29-04-2011 00:09, Jeremy Kerr escreveu:
 Hi Mauro,
 
 For ubuntu-kernel, when proposing an update patch, we forward the entire
 commit (possibly with comments before it, like why we're proposing the
 patch for that ubuntu kernel version). In this case, it'd be correct to
 parse the *last* From: line before the patch itself.

 This will do the wrong thing on several cases where someone keeps a From: 
 message
 from a comment received during the patch discussions. There are several cases
 where this happens.
 
 Sorry, not sure I understand this - you mean someone is sending a patch,
 but includes an un-quoted 'From:' line from existing discussion?

Yes.

 Could
 you point me to an example?

I don't have any example right now, as 99% of the patches I receive just have 
one
From: at the email headers. The remaining ones have a mix of the From: at the 
beginning
of the body (meaning the patch author) and From: or Subject: in the middle of 
the patch
used, for example, to point (or reply) to someone's email or to a patch that 
needs to
be reverted due to some problem.

The only case where I have a From: indicating patch author outside the first 
line in
body is when akpm sends me some patches from LKML that I might have missed, 
with a syntax
close to forward. My import script have an special rule to handle the 
forward special 
case. I don't think such special-case rules should be inside patchwork core. If 
someone
needs it, it should be done via some pwclient or external scripts at the XMLRPC 
client machine.

Mauro.
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: Patchwork user permissions

[Mauro Carvalho Chehab]

Em 30-03-2011 04:28, Jeremy Kerr escreveu:
 Hi Mauro,
 
 1) I'm using kernel.org facilities for Patchwork. As a project owner, I
 cannot give anyone else any extra permission. I'm forced to contact
 kernel.org manager, if I want do do it;
 
 This is what I'm planning to address: allow 'lead' maintainers to add/remove 
 other maintainers from a project.

Ok!

 However (as you say in your second point), all maintainers currently have 
 permissions to edit the state of all patches. To deal with this, it would be 
 feasible to (set by a per-project flag) allow non-lead maintainers to update 
 only the patches that are delegated to them.
 
 I'm not totally convinced that this is necessary though; do you not trust 
 your 
 sub-maintainers to refrain from editing patches that they shouldn't? Or is 
 there another reason for this protection?

When having a large number of sub-maintainers (20+), we should take some care
to avoid one to touch on somebody's else work. At drivers/media, there are some 
gray areas (like i2c drivers whose usage is shared by several other drivers,
and don't have its own maintainer) and core stuff that requires coordination to 
avoid 
the risk of regressions when used with other drivers.  Also, there are always 
the
risk of collisions (e. g. two maintainers touching the same patch at the same 
time).
So, protecting delegation address those issues.

 As for the comments on a set of patches - we'd need to define 'groups' of 
 patches to attach the comment to. Simply delegating the patches does not 
 create any association between them.
 
 Would it work to add a 'note' to bundles instead? This way, you could select 
 a 
 bunch of patches, create a (public) bundle, add a note to the bundle and then 
 delegate.

Yes, this should work, provided that I can touch at the bundle and add the
notes via pwclient.

Thanks!
Mauro
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: Patchwork user permissions

[Mauro Carvalho Chehab]

Em 30-03-2011 08:56, Alasdair G Kergon escreveu:
 On Wed, Mar 30, 2011 at 03:28:18PM +0800, Jeremy Kerr wrote:
 However (as you say in your second point), all maintainers currently have 
 permissions to edit the state of all patches. To deal with this, it would be 
 feasible to (set by a per-project flag) allow non-lead maintainers to update 
 only the patches that are delegated to them.
 I'm not totally convinced that this is necessary though; do you not trust 
 your 
 sub-maintainers to refrain from editing patches that they shouldn't?
 
 Rather than going down the we don't trust you to change this state
 route, personally I'd prefer to see an option to display the state
 history: who changed state from what to what and when.

As I said, it is not a matter of trust/not trust, but when there are lots of 
sub-maintainers,
it is better to have some coordination. Yet, the idea of having a state change 
history
sounds very nice to me.
  
 Alasdair
 
 ___
 Patchwork mailing list
 Patchwork@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/patchwork

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Patchwork user permissions

[Mauro Carvalho Chehab]

Hi Jeremy,

There is one thing at Patchwork that imposes a very high overload on
my knees: the lack of a proper way to delegate work to project collaborators.

I receive more than 1,000 patches per Kernel cycle, and manage about 300 
different
drivers, with 20+ driver maintainers. Yet, I have to manually work on all 
patches 
at Patchwork and have a separate control to delegate stuff, as the way it 
currently 
implements delegation is not ok, due to a few reasons:

1) I'm using kernel.org facilities for Patchwork. As a project owner, I cannot 
give anyone else any extra permission. I'm forced to contact kernel.org 
manager, 
if I want do do it;

2) the way delegates currently works is at all-or-nothing basis: or somebody
else will co-own the project, or will have only read-only access;

3) There's no way to store a comment about the current status.

What I'd like to have there is a way where I, as the project owner, can delegate
a patch set to one of the project contributors (general rule: the driver 
maintainer,
but it would be nice to be just allowed to delegate a patch to any of the 
project
contributors). At the time I delegate a patch to someone's else, I want to be 
able to
update some comment field that would allow me to later remind why I've 
delegated
that patch. Same when a patch is rejected, it would be nice to use the 
comment field
to indicate the reason why. The delegated contributor should be allowed to 
modify
the status only for the delegated patch.

Could you work on a way to implement those? I'm not a python developer, and with
all the overload I'm currently having, it is unlikely that I'll find any time 
soon
to propose a patchset for it.

Thanks,
Mauro

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: Using the From: address specified in the body

[Mauro Carvalho Chehab]

Em 16-03-2011 11:03, Guilherme Salgado escreveu:
 Hi there,
 
 I see that some emails with patches have a From: field in the body[1],
 and I'm wondering if there's any reason for not using that (when
 available, of course) as the patch submitter.  Well, now that I think of
 it, one could argue that the submitter is whoever sent the email, but
 maybe it would be useful to have a 'author' field on Patch so that we
 can properly represent cases where submitter != author?

When I apply patches, I prefer to have both information, as it is
currently provided. The scripts I use to apply patches detect when a
patch has more than one From and allow me to solve (in a matter of fact,
my scripts _force_ me to solve the dual From conflict).

It seems risky to me to let patchwork to automatically change the patch
submitter, especially since, sometimes, patch description could have a
From: that may not be the patch author.

For example, I've seem a few patches that have email references. It would
not be impossible to see a From:  or Author:  in the middle of such
references.

 
 [1] http://patchwork.ozlabs.org/patch/87084/
 
 
 
 
 ___
 Patchwork mailing list
 Patchwork@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/patchwork

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork