Re: Patchwork patch tracking system

On 4/29/2014 3:32 PM, Pedro Alves wrote:
> On 04/29/2014 08:33 PM, Breazeal, Don wrote:
>>
>>
>>> -----Original Message-----
>>> From: [email protected] [mailto:[email protected]] On
>>> Behalf Of Pedro Alves
>>> Sent: Tuesday, April 29, 2014 11:58 AM
>>> To: Tom Tromey
>>> Cc: Joel Brobecker; Gary Benson; Stan Shebs; [email protected]
>>> Subject: Re: Patchwork patch tracking system
>>>
>>> On 04/29/2014 06:07 PM, Tom Tromey wrote:
>>>
>>>> I've been trying the patchworks install as well.  I don't find it all
>>>> that useful myself, but maybe it would be better if more people were
>>>> using it.
>>>
>>> I've been trying it out too.  I've already found it useful to keep track
>>> of which of my own patches I have pending.
>>>
>>> I've been absent a little while from review, but I'm heading back, and
>>> I'm using patchwork to guide me.
>>>
>>> I like that it doesn't make the mailing list a second class citizen.
>>> I'd be willing to continue giving it a try, but indeed I think it'd be
>>> better if more people were using it.  That'll be true for any tool we
>>> end up with.
>>>
>>> In the past week, I've been cleaning it up whenever I see that patches
>>> have been pushed, even those that I didn't approve myself, but of course
>>> it'd be better if who approves the patch or the submitter themselves
>>> take care of their own patches.
>>>
>>> non-maintainers shouldn't hold back from creating an account and
>>> updating the state of their own patches.  Whatever helps bringing the
>>> load down from maintainers should help your own patches.  :-)
>>>
>>> Here's the current list of who-has-how-many-pending:
>>>
>>> $ ~/bin/pwclient-hacked list -s New | sort | uniq -c | sort -nr
>>>       35 Andy Wingo <[email protected]>
>>>       24 Andreas Arnez <[email protected]>
>>>       20 Yao Qi <[email protected]>
>>>       17 Jan Kratochvil <[email protected]>
>>>       16 Pedro Alves <[email protected]>
>>>       14 Siva Chandra <[email protected]>
>>>       13 Keith Seitz <[email protected]>
>>>       13 David Blaikie <[email protected]>
>>>       12 Andrew Burgess <[email protected]>
>>>        9 Doug Evans <[email protected]>
>>>        4 Kyle McMartin <[email protected]>
>>>        4 Hui Zhu <[email protected]>
>>>        3 Simon Marchi <[email protected]>
>>>        3 Eli Zaretskii <[email protected]>
>>>        3 Alan Modra <[email protected]>
>>>        2 Ulrich Weigand <[email protected]>
>>>        2 Mike Frysinger <[email protected]>
>>>        2 Doug Evans <[email protected]>
>>>        2 Alexander Smundak <[email protected]>
>>>        2 Agovic, Sanimir <[email protected]>
>>>        1 Vladimir Nikulichev <[email protected]>
>>>        1 Tom Tromey <[email protected]>
>>>        1 Sandra Loosemore <[email protected]>
>>>        1 Pierre Langlois <[email protected]>
>>>        1 Nick Clifton <[email protected]>
>>>        1 Mateusz Tabaka <[email protected]>
>>>        1 Mark Wielaard <[email protected]>
>>>        1 Marcus Shawcroft <[email protected]>
>>>        1 Marc Khouzam <[email protected]>
>>>        1 Maciej W. Rozycki <[email protected]>
>>>        1 Julian Brown <[email protected]>
>>>        1 John Marino <[email protected]>
>>>        1 Gary Benson <[email protected]>
>>>        1 David Taylor <[email protected]>
>>>        1 Daniel Gutson <[email protected]>
>>>
>>> As you see, most of the patches so far, since we began tracking a few
>>> weeks back, came from a small set of people.  And I suspect many of
>>> those are actually already in.
>>
>
>> A patch series that I posted to gdb-patches at the beginning of April doesn't seem to show up in patchwork.  It would be good to understand why that is and how to fix it.
>> https://sourceware.org/ml/gdb-patches/2014-04/msg00037.html
>> https://sourceware.org/ml/gdb-patches/2014-04/msg00040.html
>> https://sourceware.org/ml/gdb-patches/2014-04/msg00072.html
>> https://sourceware.org/ml/gdb-patches/2014-04/msg00071.html
>
> Well, I suspect it's the same reason your patch doesn't show
> inline in those urls -- follow the "raw" link and we see:
>
> --_002_DA279C53C4A5884A907135DFCD7A059A0E1D95BDNAMBX02mgcmento_
> Content-Type: application/octet-stream; name="0001-remote-exit.patch"
> Content-Description: 0001-remote-exit.patch
> Content-Disposition: attachment; filename="0001-remote-exit.patch"; size=9688;
> 	creation-date="Wed, 02 Apr 2014 21:17:10 GMT";
> 	modification-date="Wed, 02 Apr 2014 21:35:13 GMT"
> Content-Transfer-Encoding: base64
>
> You need to either inline the patch in the body of the email,
> or make Content-Type be some kind of "text".  For ".patch" files,
> that's usually text/x-patch.
>
> Compare with Sandra's, which is also sent as an attachment:
>
>   https://sourceware.org/ml/gdb-patches/2014-03/msg00602.html
>
>
> I don't have access to patchwork's logs, but looking around current
> git mainline patchwork's sources, I see,
> in apps/patchwork/bin/parsemail.py:
>
>   150 def find_content(project, mail):
>   151     patchbuf = None
>   152     commentbuf = ''
>   153     pullurl = None
>   154
>   155     for part in mail.walk():
>   156         if part.get_content_maintype() != 'text':
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   157             continue
>   158
>   159         payload = part.get_payload(decode=True)
>   160         charset = part.get_content_charset()
>   161         subtype = part.get_content_subtype()
>   162
>   163         # if we don't have a charset, assume utf-8
>   164         if charset is None:
>   165             charset = 'utf-8'
>   166
>   167         if not isinstance(payload, unicode):
>   168             payload = unicode(payload, charset)
>   169
>   170         if subtype in ['x-patch', 'x-diff']:
>   171             patchbuf = payload
>   172
>   173         elif subtype == 'plain':
>   174             c = payload
>   175
>   176             if not patchbuf:
>   177                 (patchbuf, c) = parse_patch(payload)
>   178
>   179             if not pullurl:
>   180                 pullurl = find_pull_request(payload)
>   181
>   182             if c is not None:
>   183                 commentbuf += c.strip() + '\n'
>   184
>   185     patch = None
>   186     comment = None
>   187
>   188     if pullurl or patchbuf:
>   189         name = clean_subject(mail.get('Subject'), [project.linkname])
>   190         patch = Patch(name = name, pull_url = pullurl, content = patchbuf,
>   191                     date = mail_date(mail), headers = mail_headers(mail))
>   192
>   193     if commentbuf:
>   194         if patch:
>   195             cpatch = patch
>   196         else:
>   197             cpatch = find_patch_for_comment(project, mail)
>   198             if not cpatch:
>   199                 return (None, None)
>   200         comment = Comment(patch = cpatch, date = mail_date(mail),
>   201                 content = clean_content(commentbuf),
>   202                 headers = mail_headers(mail))
>   203
>   204     return (patch, comment)
>
>
> So it looks like patchwork just skips your attachments, (rightfully) considering them blobs.
>
> Full source here:
>
>   http://git.ozlabs.org/?p=patchwork;a=blob;f=apps/patchwork/bin/parsemail.py;h=b6eb97ad827a8f499e763dc99c297e2c0b6e4a8f;hb=HEAD
>
> I suggest just using "git send-email" to send patches.  It makes sending
> patch series _so_ much easier, it enforces following good practices
> commit log practices, and makes sure the receiving end has it easy too -- one
> can just save the emails as mbox files (from the mail client, or patchwork's
> web frontend -- see e.g., the "mbox" link at http://patchwork.siddhesh.in/patch/660/)
> and then simply use "git am" to import the result.  Or using patchwork's
> command line tool, do that in one step with "pwclient git-am $patch_id".
>
Thanks Pedro.  I'll re-post, following your suggestions.
--Don