Re: Patchwork patch tracking system

["Breazeal, Don" <donb@codesourcery.com>]

On 4/29/2014 3:32 PM, Pedro Alves wrote:
> On 04/29/2014 08:33 PM, Breazeal, Don wrote:
>>
>>
>>> -----Original Message-----
>>> From: gdb-owner@sourceware.org [mailto:gdb-owner@sourceware.org] On
>>> Behalf Of Pedro Alves
>>> Sent: Tuesday, April 29, 2014 11:58 AM
>>> To: Tom Tromey
>>> Cc: Joel Brobecker; Gary Benson; Stan Shebs; gdb@sourceware.org
>>> 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 <wingo@igalia.com>
>>>       24 Andreas Arnez <arnez@linux.vnet.ibm.com>
>>>       20 Yao Qi <yao@codesourcery.com>
>>>       17 Jan Kratochvil <jan.kratochvil@redhat.com>
>>>       16 Pedro Alves <palves@redhat.com>
>>>       14 Siva Chandra <sivachandra@google.com>
>>>       13 Keith Seitz <keiths@redhat.com>
>>>       13 David Blaikie <dblaikie@gmail.com>
>>>       12 Andrew Burgess <aburgess@broadcom.com>
>>>        9 Doug Evans <dje@google.com>
>>>        4 Kyle McMartin <kmcmarti@redhat.com>
>>>        4 Hui Zhu <hui_zhu@mentor.com>
>>>        3 Simon Marchi <simon.marchi@ericsson.com>
>>>        3 Eli Zaretskii <eliz@gnu.org>
>>>        3 Alan Modra <amodra@gmail.com>
>>>        2 Ulrich Weigand <uweigand@de.ibm.com>
>>>        2 Mike Frysinger <vapier@gentoo.org>
>>>        2 Doug Evans <xdje42@gmail.com>
>>>        2 Alexander Smundak <asmundak@google.com>
>>>        2 Agovic, Sanimir <sanimir.agovic@intel.com>
>>>        1 Vladimir Nikulichev <nvs@tbricks.com>
>>>        1 Tom Tromey <tromey@redhat.com>
>>>        1 Sandra Loosemore <sandra@codesourcery.com>
>>>        1 Pierre Langlois <pierre.langlois@embecosm.com>
>>>        1 Nick Clifton <nickc@redhat.com>
>>>        1 Mateusz Tabaka <8tab@wp.pl>
>>>        1 Mark Wielaard <mjw@redhat.com>
>>>        1 Marcus Shawcroft <marcus.shawcroft@arm.com>
>>>        1 Marc Khouzam <marc.khouzam@ericsson.com>
>>>        1 Maciej W. Rozycki <macro@codesourcery.com>
>>>        1 Julian Brown <julian@codesourcery.com>
>>>        1 John Marino <gnugcc@marino.st>
>>>        1 Gary Benson <gbenson@redhat.com>
>>>        1 David Taylor <dtaylor@emc.com>
>>>        1 Daniel Gutson <daniel.gutson@tallertechnologies.com>
>>>
>>> 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