[Bug 48034] New: Better detect which Wine-related project a patch applies to
https://bugs.winehq.org/show_bug.cgi?id=48034 Bug ID: 48034 Summary: Better detect which Wine-related project a patch applies to Product: Wine-Testbot Version: unspecified Hardware: x86 OS: Linux Status: NEW Severity: normal Priority: P2 Component: unknown Assignee: wine-bugs(a)winehq.org Reporter: fgouget(a)codeweavers.com Distribution: --- Currently the TestBot uses two pieces of information to detect whether a patch sent to the mailing list is a Wine patch or not: * The list of Wine source files (see bug 48033). * A pair of blacklists for source filenames that are too generic to be proof a patch is a Wine patch (see $AmbiguousPathsRe and $IgnoredPathsRe in testbot/lib/WineTestBot/PatchUtils.pm). A better method would be for the TestBot server to have access to all the Wine-related repositories. A patch would initially be considered as a match for all of them and the list of files it patches would then be analyzed. Every time it patches a (not-new) file not present in one of the repositories, that repository would be eliminated from the list of matches for that patch. At the end the remaining repositories are the ones the patch could apply to. Most of the time there should only be one and the TestBot would only create jobs if Wine is one of them. This would mostly eliminate the need for the hard-coded blacklists and may open the door to expanding support for other Wine-related projects since the TestBot would be able to identify patches for these projects too. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=48034 --- Comment #1 from Austin English <austinenglish(a)gmail.com> --- One consideration to keep in mind is common filenames. On mobile, but eg README or configure.in may apply to multiple trees. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=48034 --- Comment #2 from Henri Verbeet <hverbeet(a)gmail.com> --- Honestly, I think the testbot should just look at the subject prefix of the patch. (I.e., "[PATCH appdb]", "[PATCH vkd3d]", etc.) Some people will mess it up, but I think it's fine if that results in an "Apply failure" status. Arguably it already should. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=48034 --- Comment #3 from François Gouget <fgouget(a)codeweavers.com> --- Filenames like /Makefile.in are the reason behind $AmbiguousPathsRe. If instead the TestBot checked the patch against multiple repositories it would detect such a filename automatically since it would be present in multiple repositories. So a patch that only changes /Makefile.in would look like it could apply to multiple repositories and then what to do is a matter of heuristics. Random though: what about using 'git apply --check' on the server? If the patch is applicable then we'd have positive proof that it is a Wine patch. But if it does not we would not know because it's a bad patch or because it's not a Wine patch at all... unless we also test it against other Wine-related repositories. Also this would happen before the job is created so I'm not sure how the TestBot would report that the patch is bad. Maybe send a Marvin email as usual but not point to any job? We may still need something for source.winehq.org/patches. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=48034 --- Comment #4 from Austin English <austinenglish(a)gmail.com> --- (In reply to Henri Verbeet from comment #2)
Honestly, I think the testbot should just look at the subject prefix of the patch. (I.e., "[PATCH appdb]", "[PATCH vkd3d]", etc.) Some people will mess it up, but I think it's fine if that results in an "Apply failure" status. Arguably it already should.
I'd agree, with the suggestion that a lack of header defaults to Wine. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla