Wrong series name applied to a patch series

[Bin Meng]

Hi Daniel,

On Fri, Aug 30, 2019 at 11:23 PM Daniel Axtens <dja at axtens.net> wrote:
>
> Hi Bin Meng,
>
> > As you can see below:
> > http://patchwork.ozlabs.org/project/qemu-devel/list/?series=127576
> >
> > The series name is wrongly set to the title of the patch [01/30]:
> > riscv: hw: Remove superfluous "linux, phandle" property
> >
> > But the series name should really be set to the tile of the cover
> > letter's [00/30]:
> > riscv: sifive_u: Improve the emulation fidelity of sifive_u machine
> >
> > See the QEMU mailing list archive for the above series:
> > https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg05765.html
> >
> > A similar issue happened with v4 of the same patch series.
> >
> > http://patchwork.ozlabs.org/project/qemu-devel/list/?series=125843
> > This is the patch [01/28] and its series name is set to the patch name itself
> >
> > http://patchwork.ozlabs.org/project/qemu-devel/list/?series=125844
> > This is the reset of the v4 patch series, and the series name is set
> > to the cover letter [00/28].
> > However they all belong to the same patch series.
> >
> > So the parsing logic of patchwork has something wrong.
>
> Thanks for pointing this out. I'll have a more detailed look when I get
> a minute, but we do have some known issues around parallelism: if
> multiple messages are being parsed at the same time, we can end up
> getting things wrong, and this happens from time to time on
> OzLabs. That's probably what went wrong... although it is weird for it
> to happen to multiple revisions of the same series... anyway as I say
> I'll take a look when I get a chance.

Thanks for looking into this.

I've just seen almost the exact the same issue as v4. Please check the
following example (happens to be the same series, but v8)

http://patchwork.ozlabs.org/project/qemu-devel/list/?series=129494
whose series name was set to: [v8,01/32] riscv: hw: Remove duplicated
"hw/hw.h" inclusion, as it was parsed by patchwork as a single patch
series.

http://patchwork.ozlabs.org/project/qemu-devel/list/?series=129493
whose series name was set to: riscv: sifive_u: Improve the emulation
fidelity of sifive_u machine, which is the cover letter's name
[00/32].

A little difference from the v4 example before, is that the [01/32]
was given a series ID 129494 and the reset of the patches was given a
series ID 129493, but v4 is on the contrary.

I was refreshing the QEMU patchwork immediately after I sent the
series, and I saw at first all patches in series 129493 was given a
series name "Untitled series #129493", at the time when series 129494
was already given a name "[v8,01/32] riscv: hw: Remove duplicated
"hw/hw.h" inclusion". I suspect what happened when patchwork parser
parsed the v8 series:

* The cover letter arrived very late than [v8, 01/32] and some other
patches in series 129493
* When patchwork saw patch [v8, 01/32], it did not see the cover
letter [v8, 00/32], and since it's perfectly OK for a patch series
without a cover letter, hence patchwork assigned the patch [v8, 01/32]
name as its series name
* Finally when the cover letter arrived at the mailing list, patchwork
fixed up the series 129493 name, replacing "Untitled series #129493"
with the cover letter name

I am not sure if this is a bug that could be categorized as
"parallelism", because I think when patchwork parsing the very first
patch in a series, like [v8, 01/32], it should check its "In-Reply-To"
and when it refers to a patch email that has not arrived, patchwork
should wait instead of setting the series name blindly to the first
patch name.

>
> > For the latter case, what I might guess is happening is that [01/28]
> > hit the list before [00/28], so there was no series to append to.
> >
> > But even if that's the case, I think patchwork has all the information
> > and can still correct the mistake later when it sees all other
> > message's "In-Reply-To"s are referring to the same cover letter and
> > should group them into one series.
> ... and we do have code to do this, which is what makes me a bit
> suspicious that you've just lost a race.

Regards,
Bin