https://bugs.winehq.org/show_bug.cgi?id=47042
Bug ID: 47042 Summary: Ignore replies to patchset parts Product: Wine-Testbot Version: unspecified Hardware: x86 OS: Linux Status: NEW Severity: normal Priority: P2 Component: unknown Assignee: wine-bugs@winehq.org Reporter: fgouget@codeweavers.com Distribution: ---
The TestBot treats every email as a potential patch.
This makes sense as replies to a patch could be a new version for that patch which means it should be tested.
But replies to parts of patches are different. Here is what happened due to the failure to recognize this.
I'm not sure why but the TestBot must have received the following emails in this order (this is not the order of the send times but it matches the order in which I received the emails in my inbox):
* [PATCH 1/5] ntoskrnl.exe: Implement PsLookupThreadByThreadId. https://www.winehq.org/pipermail/wine-devel/2019-April/144085.html
Part 1/5 in a patch series. The TestBot created a PendingPatchSet object to track the different parts.
* Re: [PATCH 1/5] ntoskrnl.exe: Implement PsLookupThreadByThreadId. https://www.winehq.org/pipermail/wine-devel/2019-April/144088.html
The TestBot decided this looked like a patch because the HTML version contains lines that start with "+++ ". Thus it treated this email as a new version of the previous patch, like it usually does, and replaced patch 1 in the PendingPatchSet.
* Re: [PATCH 2/5] ntoskrnl.exe: Implement KeAreApcsDisabled using critical region functions. https://www.winehq.org/pipermail/wine-devel/2019-April/144086.html
The TestBot created a job to test this patch but used the updated part 1 of the patch set which in fact was an HTML email. This resulted in an invalid patch and thus a failure to apply.
The TestBot probably did two things wrong:
1. It took a patch from an HTML attachment. The TestBot already rejects those but somehow the $Part->effective_type ne "text/html" test in Patch::NewPatch() did not work.
2. It replaced part 1 of a patchset with a new version. One cannot send a new version of a part in a patchset: the whole patchset must be resubmitted (*). So the TestBot should never replace a part in a patchset.
It looks like this check should be done in PendingPatchSets::NewSubmission() near the $Set->Parts->GetItem($PartNo) call.
The issue here is that we may receive emails out of order. So we could receive a reply to part 1, and thus add it as part 1, before we receive the actual email for part 1.
(*) Otherwise we'd have to keep the patchsets around and create new jobs for all parts that follow the replaced part. And if multiple parts are replaced... madness!
https://bugs.winehq.org/show_bug.cgi?id=47042
François Gouget fgouget@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|wine-bugs@winehq.org |fgouget@codeweavers.com