Ideas for Patchwork

[Don Zickus]

Hello,

My co-worker and I have been looking at using Patchwork for managing the
Red Hat kernel patches for RHEL.  But we have run into some design
problems and were looking to bounce some ideas off the list to see if we
could get feedback on them.

I personally have been following the project for awhile back in its perl
days.  It wasn't until the tool started using the Django framework and had
a xmlrpc interface did it get interesting.  But this is coming from a
person who doesn't like Perl, doesn't know much sql, and has very little
python experience (but growing) :-).

The current framework is nice because it is easy to follow and expand.
Considering our requirements and workflow are different from upstream's,
it is easy to hack on.

Our current workflow requires us to hold on to a patch for awhile, while
it goes through the process of getting peer reviewed acks, management
approval of the bugzilla, and integration testing.  Because of this our
current scripts use a bunch of meta data to track this info.  In addition,
management would love to be able to parse through all the patches being
posted to our internal mailing list to get a feel of the queued up
patches, what bugzillas are missing, who is doing all the work ;-), etc.
So having a web interface and a database backend makes sense for us (but
us kernel hackers are too lazy to implement such a thing, hence why I am
here).

Some of the immediate issues we found seemed to have made your TODO list, so I
thought I would throw out some of our ideas and see what people thought.

Those include:
* bundle ordering
* patch series grouping
* ability to update some meta info on the patch

### Bundle ordering ###
We hacked up a solution for this that included us using a Django keyword
'through' when creating the ManyToMany relation between patches and a
bundle, like this

patches = models.ManyToManyField(Patch, through='BundledPatch')

class BundledPatch(models.Model):
    bundle = models.ForeignKey(Bundle)
    patch = models.ForeignKey(Patch)
    order = models.PositiveIntegerField(unique = True)
    ...

this allows us to create a unique id that correlates to a bundle and a
patch that can be modified as needed.  But then that forces us to change
all the xmlrpc calls to bundle() to BundledPatch like so:

@@ -114,8 +114,9 @@ def set_bundle(user, project, action, data, patches, context):

     for patch in patches:
         if action == 'create' or action == 'add':
-            bundle.patches.add(patch)
-
+            b = BundledPatch(bundle = bundle, patch = patch)
+            b.save()
         elif action == 'remove':
             bundle.patches.remove(patch)

then moving a patch up/down one slot in a bundle can be done like this:
(ideally we would like a free-form movement rather than one slot at a
time, but baby steps first)

+    def change_order(self, increase):
+        try:
+           if increase < 0:
+              other = BundledPatch.objects.filter(id__lt = self.id).order_by('-id')[0]
+           else:
+              other = BundledPatch.objects.filter(id__gt = self.id).order_by('id')[0]
+        except:
+            return None
+
+        if not other:
+            return None
+
+       # to avoid problems updating the id which is the primary key, we just
+       # swap the _contents_. since the bundle is the same, we only need to
+       # change the patch. if more fields are added, one should make the
+       # change here too
+       tmp_patch = self.patch
+
+       self.patch = other.patch
+       other.patch = tmp_patch
+
+       self.save()
+       other.save()
+
+    def up(self):
+       self.change_order(-1)
+
+    def down(self):
+        self.change_order(+1)

I would post the whole patch but it is far from complete, we just have
little snippets working.  But I think you can get an idea of where we are
heading.  We would love to get feedback on this approach, especially
considering python isn't our strong suit.

### Patch series grouping ###
We haven't implemented anything, but talked a lot about this.  One
approach we came up with, but not sure how to implement is creating a
bundle within a bundle.  Where a particular patch series would be
contained in one bundle with the patch header containing the overall
status of the series.  Then the 'queued up' patch bundle could point to
that particular series bundle from the web view or when you download or
such.  This would allow us to modularize things better.

Not sure if that is easy to implement or the right approach.  Combined
with bundle ordering, this would allow us to 'correct' the order in which
the patch series showed up in our inbox. :-)  Also using the technique
below of the field 'in-reply-to', one could ack a series header and have
it apply to all the patches referenced by it.

Again looking for feedback.

### ability to update some meta info on the patch ###

We were trying to figure out within the constraints of the patchwork
framework, what the appropriate way to update some meta fields were while
still retaining the ability to download the original email (for review
purposes).

I stumbled upon a thought that would require changing the idea behind the
Patch class from the patch itself to a workspace.  And instead putting the
original patch inside a field in the Comment class.

The thought was the Comment class should be the self-contained original
thread that can be constructed easily if one wanted to download an mbox
(this mbox being different than patchwork's mbox).

The Patch class can then just be a workspace that is updated through
various comments whether it is meta-data, modifying the subject line,
patch description, or even refreshing the patch without worrying about the
fact you lose the original data from the email.

And to expand the idea further, one can then envision setting up _stub_
Comment blocks to reflect changes that were done by the maintainer (that
doesn't necessarily need to be reflected back on the mailing list).

This would give you a full history on how the patch was put together and
what the final output looks like.

I know this may be against one of the design philosophies of patchwork,
but I thought I would throw it out there anyway.

In addition, we wanted to solve dealing with reposted patches by pointing
the old patch to the new patch.  This can easily be done now because the
original patch is stored in the comment block and the only thing that needs
updating is the Patch field in the Comment block.  Now one can easily
download the entire history of the patch and not just the latest replies
to the reposted patch.

Which leads me to one more trick.  Because we wanted to link patches and
the history together (either through a reposted patch, malformated thread,
etc) and still view a full mbox of the bundle, we needed a way to connect
those threads together without losing that information in our mailer.  We
noticed mutt does this by modifying the 'In-reply-To' field.  So our trick
would be to add a field to the Comment class, in-reply-to, that would
override the original thread's in-reply-to, if need be, to link various
disjoint threads together.

Then again I am not sure how much interest there is to download the entire
unedited thread, but we would use that feature a lot for review purposes.
Then using mutt and a modified pwclient we could create shortcut
keystrokes to update the patchwork database with acks/naks it missed,
missing or update bugzilla numbers, or just to save concerns.


Sorry for being long winded.  As you can see the ideas tie together, so I
felt I had to express all of them to present the big picture.  I hope they
make sense.  We welcome any feedback or different solutions someone may
have as we are just getting our feet wet.

Cheers,
Don