[PATCH 10/10] models: Enable "auto-completion" of series patches

[Stephen Finucane]

Users can send a new revision of a single patch from a series. When
this happens there is no immediate context: one must grok the
previous series to understand that this new patch is in fact a minor
change to an existing series and not a new, standalone hanges. This is
particularly impactful for things like CI which _need_ to understand
this dependency trail in order to successfully apply and test a patch.

Resolve this issue through the "auto-completion" of series. This is
achieved by examining previous series and taking only the patches that
have not be replaced in the current version. For example, if a user
sends a three patch series and then submits a revision to patch two,
only patches one and three of the prior series will be used and the
resulting group presented as an entirely new series.

This is done entirely dynamically. The reason for this is that series
relationships themselves are dynamic: sometimes it may not be possible
to identify a relationship until the last patch of a new series is
submitted (say there's a three patch series and the first two parsed
patches are significantly reworked).

This won't catch situations where the order of a patch has been moved
or the patch has been significantly reworked. However, one can hope
users are not silly enough to do something like that.

Signed-off-by: Stephen Finucane 
---
 patchwork/models.py |   49 -
 1 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/patchwork/models.py b/patchwork/models.py
index d2d4079..ce8ac05 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -250,19 +250,50 @@ class SeriesRevision(models.Model):
 def complete(self):
 return self.total == self.actual_total
 
+@cached_property
+def ancestors(self):
+if self.group:
+# NOTE(stephenfin) Under normal usage dates should be more
+# than adequate. However, if patches were submitted in an
+# unusual order, e.g. using parsearchive tool, then this
+# can be invalid. This is a  corner case that we
+# unfortunately do not cover
+return self.group.revisions.filter(date__lt=self.date)
+
+@cached_property
+def ancestor(self):
+if self.ancestors:
+return self.ancestors.reverse()[0]
+
 @property
 def patches(self):
-"""Return patches associated with this series .
+"""Build a complete list of patches.
 
-Eventually this will "autofill" a series revision by pulling in
-missing patches from prior revisions, where possible. For now,
-however, this just lets us retrieve the patches created for
-this given revision.
-
-Returns:
-The patches in the revision.
+This works by recursively searching through ancestor series to
+"fill in the gaps".
 """
-return self.unique_patches.all()
+if self.complete or not self.ancestor:
+return self.unique_patches
+
+# TODO(stephenfin) We should compare on more than one field to
+# identify duplicates. The 'name' field itself is fairly useless
+# in the case of typos etc.
+# TODO(stephenfin) This won't handle different version numbers!
+# We need to strip the tags to ensure 'xxx' and '[v2] xxx' are
+# identified as different versions of the same patch. This will
+# fix series linking also
+ids = self.unique_patches.values('name')
+ancestor_patches = self.ancestor.patches
+
+# TODO(stephenfin) Is the below correct?
+patches_ = ancestor_patches.exclude(name__in=ids)
+return patches_ | self.unique_patches.all()
+
+@cached_property
+def actual_version(self):
+if not self.group:
+return 1
+return self.ancestors.count() + 1
 
 def __str__(self):
 return self.name
-- 
1.7.4.1

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

[PATCH 08/10] templates: Integrate series view into patches

[Stephen Finucane]

Patches for related series are listed on the patch page - this
provides a way to quickly grok a given patches location in a
series hierarchy.

Signed-off-by: Stephen Finucane 
---
 htdocs/css/style.css  |6 +++
 patchwork/templates/patchwork/submission.html |   50 +++-
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/htdocs/css/style.css b/htdocs/css/style.css
index f7f7b6a..a75a7fd 100644
--- a/htdocs/css/style.css
+++ b/htdocs/css/style.css
@@ -188,6 +188,12 @@ table.patchmeta tr th, table.patchmeta tr td {
vertical-align: middle;
 }
 
+#patchrelations ul {
+list-style-type: none;
+padding: 0;
+margin: 0;
+}
+
 .patchnav {
padding-left: 1em;
padding-top: 1em;
diff --git a/patchwork/templates/patchwork/submission.html 
b/patchwork/templates/patchwork/submission.html
index bda36f9..054726f 100644
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -9,7 +9,7 @@
 
 {% block body %}
 

[PATCH 02/10] models: Add 'Series' model and related models

[Stephen Finucane]

Add a series model. This model is intentionally very minimal to allow
as much dynaminism as possible. It is expected that patches will be
migrated between series as new data is provided.

Signed-off-by: Stephen Finucane 
---
v2:
- Rename 'SeriesGroup' to 'Series'
- Rename 'Series' to 'SeriesRevision'
---
 patchwork/admin.py |   55 +++-
 patchwork/migrations/0013_add_series_models.py |   56 
 patchwork/models.py|  110 +++-
 3 files changed, 216 insertions(+), 5 deletions(-)
 create mode 100644 patchwork/migrations/0013_add_series_models.py

diff --git a/patchwork/admin.py b/patchwork/admin.py
index 0f217d7..afb4af6 100644
--- a/patchwork/admin.py
+++ b/patchwork/admin.py
@@ -23,7 +23,8 @@ from django.contrib import admin
 
 from patchwork.models import (Project, Person, UserProfile, State, Submission,
   Patch, CoverLetter, Comment, Bundle, Tag, Check,
-  DelegationRule)
+  DelegationRule, Series, SeriesRevision,
+  SeriesReference)
 
 
 class DelegationRuleInline(admin.TabularInline):
@@ -76,8 +77,8 @@ admin.site.register(CoverLetter, CoverLetterAdmin)
 
 class PatchAdmin(admin.ModelAdmin):
 list_display = ('name', 'submitter', 'project', 'state', 'date',
-'archived', 'is_pull_request')
-list_filter = ('project', 'state', 'archived')
+'archived', 'is_pull_request', 'series')
+list_filter = ('project', 'state', 'archived', 'series')
 search_fields = ('name', 'submitter__name', 'submitter__email')
 date_hierarchy = 'date'
 
@@ -97,6 +98,54 @@ class CommentAdmin(admin.ModelAdmin):
 admin.site.register(Comment, CommentAdmin)
 
 
+class CoverLetterInline(admin.StackedInline):
+model = CoverLetter
+extra = 0
+
+
+class PatchInline(admin.StackedInline):
+model = Patch
+extra = 0
+
+
+class SeriesRevisionAdmin(admin.ModelAdmin):
+list_display = ('name', 'group', 'date', 'submitter', 'version', 'total',
+'actual_total', 'complete')
+readonly_fields = ('actual_total', 'complete')
+search_fields = ('submitter_name', 'submitter_email')
+inlines = [CoverLetterInline, PatchInline]
+
+def complete(self, series):
+return series.complete
+complete.boolean = True
+admin.site.register(SeriesRevision, SeriesRevisionAdmin)
+
+
+class SeriesRevisionInline(admin.StackedInline):
+model = SeriesRevision
+readonly_fields = ('date', 'submitter', 'version', 'total',
+   'actual_total', 'complete')
+ordering = ('-date', )
+show_change_link = True
+extra = 0
+
+def complete(self, series):
+return series.complete
+complete.boolean = True
+
+
+class SeriesAdmin(admin.ModelAdmin):
+list_display = ('name', )
+readonly_fields = ('name', )
+inlines = [SeriesRevisionInline]
+admin.site.register(Series, SeriesAdmin)
+
+
+class SeriesReferenceAdmin(admin.ModelAdmin):
+model = SeriesReference
+admin.site.register(SeriesReference, SeriesReferenceAdmin)
+
+
 class CheckAdmin(admin.ModelAdmin):
 list_display = ('patch', 'user', 'state', 'target_url',
 'description', 'context')
diff --git a/patchwork/migrations/0013_add_series_models.py 
b/patchwork/migrations/0013_add_series_models.py
new file mode 100644
index 000..03b7a22
--- /dev/null
+++ b/patchwork/migrations/0013_add_series_models.py
@@ -0,0 +1,56 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+dependencies = [
+('patchwork', '0012_add_coverletter_model'),
+]
+
+operations = [
+migrations.CreateModel(
+name='Series',
+fields=[
+('id', models.AutoField(verbose_name='ID', serialize=False, 
auto_created=True, primary_key=True)),
+],
+options={
+'verbose_name_plural': 'Series',
+},
+),
+migrations.CreateModel(
+name='SeriesReference',
+fields=[
+('id', models.AutoField(verbose_name='ID', serialize=False, 
auto_created=True, primary_key=True)),
+('msgid', models.CharField(unique=True, max_length=255)),
+],
+),
+migrations.CreateModel(
+name='SeriesRevision',
+fields=[
+('id', models.AutoField(verbose_name='ID', serialize=False, 
auto_created=True, primary_key=True)),
+('date', models.DateTimeField()),
+('version', models.IntegerField(default=1, help_text=b'Version 
of series revision as indicated by the subject prefix(es)')),
+('total', models.IntegerField(help_text=b'Number of patches in 
series as indicated by the subject 

[PATCH 06/10] templates: Generate and use a "handle"

[Stephen Finucane]

In the absence of a name, the email is currently used. Some emails are
long and rather unreadable. The likelihood of someone's localpart (i.e.
the bit before the '@') conflicting with that of someone else on the
same patchwork instance and project is very low, so form a "handle"
from just this.

Signed-off-by: Stephen Finucane 
---
 patchwork/bin/parsemail.py   |2 +-
 patchwork/models.py  |7 +++
 patchwork/templatetags/person.py |   12 
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index 8baacf6..ddcd9e7 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -284,7 +284,7 @@ def parse_version(subject, subject_prefixes):
 if m:
 return int(m.group(1))
 
-m = re.search(r'\(([vV]\d+)\)', subject)
+m = re.search(r'\([vV](\d+)\)', subject)
 if m:
 return int(m.group(1))
 
diff --git a/patchwork/models.py b/patchwork/models.py
index 67ea012..d2d4079 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -54,6 +54,13 @@ class Person(models.Model):
 self.name = user.profile.name
 self.user = user
 
+@property
+def handle(self):
+if self.user:
+return self.user.name
+else:
+return self.email.split('@')[0]
+
 def __str__(self):
 if self.name:
 return '%s <%s>' % (self.name, self.email)
diff --git a/patchwork/templatetags/person.py b/patchwork/templatetags/person.py
index 7af021f..98c0f2b 100644
--- a/patchwork/templatetags/person.py
+++ b/patchwork/templatetags/person.py
@@ -33,14 +33,10 @@ register = template.Library()
 @register.filter
 def personify(person, project):
 
-if person.name:
-linktext = escape(person.name)
-else:
-linktext = escape(person.email)
-
 url = reverse('patch-list',
   kwargs={'project_id': project.linkname})
-str = '%s' % (
-url, SubmitterFilter.param, escape(person.id), linktext)
+elem = '%s' % (
+url, SubmitterFilter.param, escape(person.id), str(person),
+person.name or person.handle)
 
-return mark_safe(str)
+return mark_safe(elem)
-- 
1.7.4.1

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

[PATCH 01/10] models: Convert functions to properties

[Stephen Finucane]

A number of models contain functions that are, semantically speaking,
actually properties. Mark them as such.

Signed-off-by: Stephen Finucane 
---
 patchwork/models.py |   15 ++-
 patchwork/views/__init__.py |2 +-
 patchwork/views/patch.py|4 ++--
 patchwork/views/user.py |2 +-
 patchwork/views/xmlrpc.py   |2 +-
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/patchwork/models.py b/patchwork/models.py
index 4d454c7..9ad2bcb 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -48,7 +48,7 @@ class Person(models.Model):
  on_delete=models.SET_NULL)
 
 def link_to_user(self, user):
-self.name = user.profile.name()
+self.name = user.profile.name
 self.user = user
 
 def __str__(self):
@@ -134,6 +134,7 @@ class UserProfile(models.Model):
 default=100, null=False, blank=False,
 help_text='Number of items to display per page')
 
+@property
 def name(self):
 if self.user.first_name or self.user.last_name:
 names = list(filter(
@@ -141,17 +142,19 @@ class UserProfile(models.Model):
 return ' '.join(names)
 return self.user.username
 
+@property
 def contributor_projects(self):
 submitters = Person.objects.filter(user=self.user)
 return Project.objects.filter(id__in=Submission.objects.filter(
 submitter__in=submitters).values('project_id').query)
 
-def sync_person(self):
-pass
-
+@property
 def n_todo_patches(self):
 return self.todo_patches().count()
 
+def sync_person(self):
+pass
+
 def todo_patches(self, project=None):
 # filter on project, if necessary
 if project:
@@ -165,7 +168,7 @@ class UserProfile(models.Model):
 return qs
 
 def __str__(self):
-return self.name()
+return self.name
 
 
 def _user_saved_callback(sender, created, instance, **kwargs):
@@ -278,6 +281,7 @@ class EmailMixin(models.Model):
 r'^(Tested|Reviewed|Acked|Signed-off|Nacked|Reported)-by: .*$',
 re.M | re.I)
 
+@property
 def patch_responses(self):
 if not self.content:
 return ''
@@ -379,6 +383,7 @@ class Patch(Submission):
 
 return self.project.is_editable(user)
 
+@property
 def filename(self):
 fname_re = re.compile(r'[^-_A-Za-z0-9\.]+')
 str = fname_re.sub('-', self.name)
diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index f63..c1fb8ff 100644
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -366,7 +366,7 @@ def patch_to_mbox(patch):
 body += patch.patch_responses()
 
 for comment in Comment.objects.filter(submission=patch):
-body += comment.patch_responses()
+body += comment.patch_responses
 
 if postscript:
 body += '---\n' + postscript + '\n'
diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 3346568..b48cf14 100644
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -113,7 +113,7 @@ def content(request, patch_id):
 response = HttpResponse(content_type="text/x-patch")
 response.write(patch.diff)
 response['Content-Disposition'] = 'attachment; filename=' + \
-patch.filename().replace(';', '').replace('\n', '')
+patch.filename.replace(';', '').replace('\n', '')
 return response
 
 
@@ -126,7 +126,7 @@ def mbox(request, patch_id):
 else:
 response.write(patch_to_mbox(patch).as_string(True))
 response['Content-Disposition'] = 'attachment; filename=' + \
-patch.filename().replace(';', '').replace('\n', '')
+patch.filename.replace(';', '').replace('\n', '')
 return response
 
 
diff --git a/patchwork/views/user.py b/patchwork/views/user.py
index dfbfde8..0eba67b 100644
--- a/patchwork/views/user.py
+++ b/patchwork/views/user.py
@@ -87,7 +87,7 @@ def register_confirm(request, conf):
 person = Person.objects.get(email__iexact=conf.user.email)
 except Person.DoesNotExist:
 person = Person(email=conf.user.email,
-name=conf.user.profile.name())
+name=conf.user.profile.name)
 person.user = conf.user
 person.save()
 
diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
index 1f48959..b65ae18 100644
--- a/patchwork/views/xmlrpc.py
+++ b/patchwork/views/xmlrpc.py
@@ -268,7 +268,7 @@ def patch_to_dict(obj):
 return {
 'id': obj.id,
 'date': six.text_type(obj.date).encode('utf-8'),
-'filename': obj.filename(),
+'filename': obj.filename,
 'msgid': obj.msgid,
 'name': obj.name,
 'project': six.text_type(obj.project).encode('utf-8'),
-- 
1.7.4.1

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

[PATCH 04/10] parsemail: Handle 'xxx (v2)' style messages

[Stephen Finucane]

Some emails are received in the above format. Might as well support
them.

Signed-off-by: Stephen Finucane 
---
 patchwork/bin/parsemail.py |   11 ---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index 1c7ce0f..f52776e 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -268,21 +268,26 @@ def parse_series_marker(subject_prefixes):
 return (None, None)
 
 
-def parse_version(subject_prefixes):
+def parse_version(subject, subject_prefixes):
 """Extract patch version.
 
 Args:
+subject: Main body of subject line
 subject_prefixes: List of subject prefixes to extract version
   from
 
 Returns:
 version if found, else 1
 """
-regex = re.compile('^v([0-9]+)$')
+regex = re.compile('^v(\d+)$')
 m = _parse_prefixes(subject_prefixes, regex)
 if m:
 return int(m.group(1))
 
+m = re.search(r'\(([vV]\d+)\)', subject)
+if m:
+return int(m.group(1))
+
 return 1
 
 
@@ -542,7 +547,7 @@ def parse_mail(mail, list_id=None):
 author = find_author(mail)
 name, prefixes = clean_subject(mail.get('Subject'), [project.linkname])
 x, n = parse_series_marker(prefixes)
-version = parse_version(prefixes)
+version = parse_version(name, prefixes)
 refs = find_references(mail)
 date = find_date(mail)
 headers = find_headers(mail)
-- 
1.7.4.1

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

[PATCH 00/10] Add series support

[Stephen Finucane]

Add support for series. Series are groups of patches sent as one
bundle. For example:

[PATCH 0/3] A cover letter
  [PATCH 1/3] The first patch
  [PATCH 2/3] The second patch
  [PATCH 3/3] The third patch

The following features are currently provided:

Parsing
===

* Creation of new series, and linking of patches/cover letters to
  existing series
* Rudimentary linking of different series revisions as one series

UI
==

* Series column in patch-list
* Series and related patches/cover letters links in patch-detail
* Filtering by series
* Integration with Django-admin

There are some important notes regarding how this is implemented:

* We *never* expose 'Series' to the end-user. Instead, "related
  series" should be exposed
* Patches form a one-to-one relationship with SeriesReference.
  Auto-completion of series should be done in the view code, rather
  than the database

Stephen Finucane (10):
  models: Convert functions to properties
  models: Add 'Series' model and related models
  parsemail: Add series parsing
  parsemail: Handle 'xxx (v2)' style messages
  parsemail: Handle series sent "in-reply-to"
  templates: Generate and use a "handle"
  templates: Integrate series support
  templates: Integrate series view into patches
  parsemail: Implement series linking
  models: Enable "auto-completion" of series patches

 htdocs/css/style.css   |6 +
 patchwork/admin.py |   55 +++-
 patchwork/bin/parsemail.py |  179 ++--
 patchwork/filters.py   |   56 +++-
 patchwork/migrations/0013_add_series_models.py |   56 
 patchwork/models.py|  163 +-
 patchwork/templates/patchwork/patch-list.html  |   12 ++
 patchwork/templates/patchwork/submission.html  |   50 ++-
 patchwork/templatetags/person.py   |   12 +-
 patchwork/views/__init__.py|2 +-
 patchwork/views/patch.py   |4 +-
 patchwork/views/user.py|2 +-
 patchwork/views/xmlrpc.py  |2 +-
 13 files changed, 561 insertions(+), 38 deletions(-)
 create mode 100644 patchwork/migrations/0013_add_series_models.py

-- 
1.7.4.1

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

[PATCH 05/10] parsemail: Handle series sent "in-reply-to"

[Stephen Finucane]

Take a series, "v2", sent as a reply to another, "v1", like so:

[PATCH 0/3] A cover letter
  [PATCH 1/3] The first patch
  ...
  [PATCH v2 0/3] A cover letter
[PATCH v2 1/3] The first patch
...

The current behavior for traversing references is oldest first. For
example, for "PATCH v2 1/3" above, the references field will look like
so:

["0/3", "v2 0/3", "v2 1/3"]

Where "0/3" corresponds to the message-id of "PATCH 0/3".

By traversing this way, patches sent in-reply-to an existing series
will always be linked to that series, instead of creating a new series,
as expected. Flip things around, and instead attempt to find series
using the most recent message-id first. This will ensure the most
recent series is always used.

Signed-off-by: Stephen Finucane 
---
 patchwork/bin/parsemail.py |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index f52776e..8baacf6 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -137,7 +137,7 @@ def find_series(mail):
 """
 series = None
 
-for ref in find_references(mail) + [mail.get('Message-ID').strip()]:
+for ref in [mail.get('Message-ID').strip()] + find_references(mail)[::-1]:
 # try parsing by RFC5322 fields first
 try:
 series_ref = SeriesReference.objects.get(msgid=ref)
@@ -575,6 +575,10 @@ def parse_mail(mail, list_id=None):
 series.save()
 
 for ref in refs + [msgid]:  # save references for series
+# prevent duplication
+if SeriesReference.objects.filter(msgid=ref).exists():
+continue
+
 series_ref = SeriesReference(series=series,
  msgid=ref)
 series_ref.save()
@@ -621,6 +625,10 @@ def parse_mail(mail, list_id=None):
 series.save()
 
 for ref in refs + [msgid]:  # save references for series
+# prevent duplication
+if SeriesReference.objects.filter(msgid=ref).exists():
+continue
+
 series_ref = SeriesReference(series=series,
  msgid=ref)
 series_ref.save()
-- 
1.7.4.1

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