Getting errors with Series support

[Stephen Finucane]

On 2016-11-16 07:00, Daniel Axtens wrote:
> Hi,
> 
>> In terms of your errors:
>>> (1062, "Duplicate entry '143-5' for key
>>> 'patchwork_seriespatch_series_id_1fc2c47f3eb85695_uniq'")
>> 
>> OK, so this is hitting our unique_together contstraint ('series',
>> 'patch'): somehow we're trying to add another entry for the same 
>> series
>> and the same patch. I wonder why this has not triggered duplicate
>> detection at an earlier stage? Let me look into this more and get back
>> to you.
>> 
> 
> Ah, I'm wrong - this could be hitting either the ('series', 'patch')
> constraint - which has duplicate detection - or the ('series', 
> 'number')
> constraint - which does not have duplicate detection. I will send
> another patch which fixes this (but probably not until tomorrow).
> 
> Feel free to not send the error logs at this point :)

Thanks for the patch, Daniel - I'll get that reviewed and applied this 
weekend.

 From what I can tell, there are two issues happening here, plus a third 
that we should fix. First, there's a patch being sent with a malformed 
version header in-reply-to an existing series. This looks something like 
this:

   [PATCH v2 1/2] My first patch
     [PATCH v2 2/2] My second patch
       [PATCH v2.1] My second patch

IMO, this is missing a number and we don't support series linking yet, 
so we should not add this to any series. The aforementioned patch should 
fix this.

Secondly, we're not handling series revisions correctly when sent 
without a cover letter in-reply-to an existing series. Take something 
like this:

   [PATCH 1/2] My first patch
     [PATCH 2/2] My second patch
       [PATCH v2 1/2] My first patch
         [PATCH v2 2/2] My second patch

When we receive these new patches, we'll traverse the references to find 
a stored SeriesReference objects [1]. This will match on a reference for 
the original patches and return the original series' Series object. 
Since this is actually a new series, the attempt to assign the revised 
patches to the original series hits the ('series', 'number') check and 
fails (note that this doesn't happen for series revisions sent *with* a 
cover letter because we don't traverse references for cover letters 
[2]). The easiest fix here would be to check the version of the Series 
returned by find_series. If it doesn't match, create a new series and 
store any new references against this new series, preventing further 
issues.

There's an issue we haven't seen yet but, as a variation of the above, 
is one likely to rear its malformed head sooner rather than later. Take 
the following series:

   [PATCH 1/2] My first patch
     [PATCH 2/2] My second patch
       [PATCH 1/2] My first patch
         [PATCH 2/2] My second patch

The user has forgotten to add a version but has sent the message 
in-reply-to an existing series. The version check I suggested using 
above wouldn't work as the versions are "the same". I've no idea how we 
can solve this right now, I'm afraid, but I'd welcome some ideas.

Stephen

[1] 
https://github.com/getpatchwork/patchwork/blob/c3137fe/patchwork/parser.py#L203-L208
[2] 
https://github.com/getpatchwork/patchwork/blob/c3137fe/patchwork/parser.py#L848-L851