I have some issues with the definitions of some Bugzilla keywords. Then they also have a lot of formatting inconsistencies and once we've come to a consensus I have no idea who can make changes (the definitions seem to be stored in some sort of database). Well. Here goes anyway:
* testcase | Bugs that have a testcase attached, in source form but not | necessarily integrated into the Wine test suite.
The definition does not say what a testcase is. Also, while I totally expect the source for most test cases to be available, I think the keyword should be usable even if the source code is not published. Specifying that the source is available should be the exclusive role of the 'source' keyword.
So I would use a definition like: | Bugs where a minimal program dedicated to reproducing the bug, aka | a testcase, has been attached. The testcase need not be integrated | into the Wine test suite. If the source is available (which is | strongly recommended) add the source keyword too.
* patch | Bugs in this keyword contain a patch that are waiting for approval.
What is a 'patch that is waiting for approval'? Is it a patch which is referenced in https://source.winehq.org/patches/ ? What happens if the patch is rejected? (or drops off the list) Should the patch keyword be removed? What if the patch is in wine-staging?
To me the patch keyword should make it possible to identify bugs where someone proposed a potential fix, even if it needs to be refined to be acceptable, may break other applications, etc. Essentially anything that's not a blatant hack that checks the executable filename or replaces a properly implemented function with a stub.
So I would use the following definition: | Bugs with this keyword contain a proposed fix. The fix may be | incomplete, break other applications or be otherwise unacceptable as | long as it does fix the symptoms described in the bug and is not a | pure workaround (such as altering Wine's behavior based on the | executable filename, replacing an implemented function with a stub, | etc.).
* regression | bugs relating to programs that were working at one time but stoped | working for some reason
Except for the obvious typo, capitalization and missing full stop I'm fine with this definition. I take 'programs' to mean applications that users would actually want to use with Wine, not Wine's tests.
So maybe the definition could be a bit more explicit:
| Bugs relating to applications (excluding Wine's tests) that were | working at one time but stopped working for some reason.
* Then there is the related 'Regression SHA1' property. I think it should be ok to use that property to identify the commit causing a bug, whether that bug is a regression or not.
First note that only bugs that have the regression *keyword* appear on the regressions page: https://source.winehq.org/regressions
So if a commit adds Wine tests that fail the 'regression' keyword is not applicable but I think it would still be valuable to set the "Regression SHA1" field: - The commit message may provide valuable insight into what was on the mind of the developer who wrote the tests. - Such commits can sometimes be a pain to track down when the test has seen a lot of churn. - The commit's author is likely to have insight into the bug. - Setting the 'Regression SHA1' field allows identifying the bugs for which the above information is available. - It also lets the patterns page put links to that commit and reference it as being the source of the test failures.
Maybe a better name for 'Regression SHA1' would be something like 'Bad SHA1' or 'Introduced by SHA1'. I'm not sure renaming the field is worth it but the description could be changed from
| SHA1 of the git commit that caused the regression. to | SHA1 of the git commit that causes the bug.
* hardware | Issues communicating with HW such as serial/parallel/cdrom/usb
s/HW/hardware/ + trailing full stop
* obfuscation It looks like there is an extra space in "management techniques".
* Integration | Bugs related to Wine integration into OS (menu, MIME-types, look)
s/integration into OS/integration with the OS/ And I'd even argue that the integration is with the desktop environment (GNOME, KDE, etc) rather than the operating system.
* dotnet | Bugs that appear in applications using dotNet and that are related | to this dotNet usage instead of being independent bugs.
s/dotNet/.NET/g
* The definitions for the following keywords also have formatting issues. Either the capitalization is wrong, or the trailing full stop is missing, or both: Abandoned? hardware integration localization performance source valgrind
Francois Gouget fgouget@free.fr wrote:
regression | bugs relating to programs that were working at one time but stoped | working for some reason
Except for the obvious typo, capitalization and missing full stop I'm fine with this definition. I take 'programs' to mean applications that users would actually want to use with Wine, not Wine's tests.
So maybe the definition could be a bit more explicit:
| Bugs relating to applications (excluding Wine's tests) that were | working at one time but stopped working for some reason.
It should be clarified that the application stopped working due to a change in Wine, not something external. For instance if the test case was broken by a Windows update that's clearly not a regression.
On Tue, 6 Jul 2021, Dmitry Timoshkov wrote:
Francois Gouget fgouget@free.fr wrote:
regression | bugs relating to programs that were working at one time but stoped | working for some reason
Except for the obvious typo, capitalization and missing full stop I'm fine with this definition. I take 'programs' to mean applications that users would actually want to use with Wine, not Wine's tests.
So maybe the definition could be a bit more explicit:
| Bugs relating to applications (excluding Wine's tests) that were | working at one time but stopped working for some reason.
It should be clarified that the application stopped working due to a change in Wine, not something external. For instance if the test case was broken by a Windows update that's clearly not a regression.
So unlike me you're arguing there are cases where the regression keyword can apply to Wine's test cases?
What's the use case?
Francois Gouget fgouget@free.fr wrote:
regression | bugs relating to programs that were working at one time but stoped | working for some reason
Except for the obvious typo, capitalization and missing full stop I'm fine with this definition. I take 'programs' to mean applications that users would actually want to use with Wine, not Wine's tests.
So maybe the definition could be a bit more explicit:
| Bugs relating to applications (excluding Wine's tests) that were | working at one time but stopped working for some reason.
It should be clarified that the application stopped working due to a change in Wine, not something external. For instance if the test case was broken by a Windows update that's clearly not a regression.
So unlike me you're arguing there are cases where the regression keyword can apply to Wine's test cases?
What's the use case?
Use case is the same as with any application: if the breakage is caused by a change in Wine source, i.e. a regression test could be performed, and the Regression SHA1 field could be set as a result.
On Tue, 6 Jul 2021, Dmitry Timoshkov wrote: [...]
So unlike me you're arguing there are cases where the regression keyword can apply to Wine's test cases?
What's the use case?
Use case is the same as with any application: if the breakage is caused by a change in Wine source, i.e. a regression test could be performed, and the Regression SHA1 field could be set as a result.
For Wine tests I'm arguing the Regression SHA1 field should be set but not the regression keyword.
Francois Gouget fgouget@free.fr wrote:
On Tue, 6 Jul 2021, Dmitry Timoshkov wrote: [...]
So unlike me you're arguing there are cases where the regression keyword can apply to Wine's test cases?
What's the use case?
Use case is the same as with any application: if the breakage is caused by a change in Wine source, i.e. a regression test could be performed, and the Regression SHA1 field could be set as a result.
For Wine tests I'm arguing the Regression SHA1 field should be set but not the regression keyword.
That doesn't make sense IMO.
On Tue, 6 Jul 2021 14:43:59 +0200 (CEST) Francois Gouget fgouget@free.fr wrote:
On Tue, 6 Jul 2021, Dmitry Timoshkov wrote:
It should be clarified that the application stopped working due to a change in Wine, not something external. For instance if the test case was broken by a Windows update that's clearly not a regression.
So unlike me you're arguing there are cases where the regression keyword can apply to Wine's test cases?
What's the use case?
I have no opinion on whether this should apply to test cases, but I do think it should be clarified that problem is due to a change in Wine and not an update to the app itself or a change to some other part of the user's system.
On Tue, 6 Jul 2021, Rosanne DiMesio wrote: [...]
I have no opinion on whether this should apply to test cases, but I do think it should be clarified that problem is due to a change in Wine and not an update to the app itself or a change to some other part of the user's system.
So something like this? | Bugs relating to applications (excluding Wine's tests) that were | working at one time but stopped working because of a change in | Wine which should ideally be specified in the Regression SHA1 field.
On Tue, 6 Jul 2021 16:42:24 +0200 (CEST) Francois Gouget fgouget@free.fr wrote:
On Tue, 6 Jul 2021, Rosanne DiMesio wrote: [...]
I have no opinion on whether this should apply to test cases, but I do think it should be clarified that problem is due to a change in Wine and not an update to the app itself or a change to some other part of the user's system.
So something like this? | Bugs relating to applications (excluding Wine's tests) that were | working at one time but stopped working because of a change in | Wine which should ideally be specified in the Regression SHA1 field.
Looks good to me.
On 7/6/21 6:20 AM, Dmitry Timoshkov wrote:
Francois Gouget fgouget@free.fr wrote:
regression | bugs relating to programs that were working at one time but stoped | working for some reason
Except for the obvious typo, capitalization and missing full stop I'm fine with this definition. I take 'programs' to mean applications that users would actually want to use with Wine, not Wine's tests.
So maybe the definition could be a bit more explicit:
| Bugs relating to applications (excluding Wine's tests) that were | working at one time but stopped working for some reason.
It should be clarified that the application stopped working due to a change in Wine, not something external. For instance if the test case was broken by a Windows update that's clearly not a regression.
Don't we already have RESOLVED NOTOURBUG for that?
Don't we already have RESOLVED NOTOURBUG for that?
No, this would be a case where a change outside of Wine causes the application to hit a failing case when it did not before.
Excluding Wine's test cases from the "regression" keyword makes the definition more complicated, so IMO there needs to be a specific reason to do that. Is there a reason we'd want to exclude Wine test case regressions from regression searches?
On Tue, 6 Jul 2021, Esme Povirk (she/they) wrote:
Excluding Wine's test cases from the "regression" keyword makes the definition more complicated, so IMO there needs to be a specific reason to do that. Is there a reason we'd want to exclude Wine test case regressions from regression searches?
It would be for people who are looking for things to fix for 'real applications'.
But maybe the open Wine test regressions are rare enough to not drown them out. Also one could presumably perform an advanced search for bugs with the regression keyword but not the testcase one.
On Tue, 6 Jul 2021, Francois Gouget wrote: [...]
patch | Bugs in this keyword contain a patch that are waiting for approval.
What is a 'patch that is waiting for approval'? Is it a patch which is referenced in https://source.winehq.org/patches/ ? What happens if the patch is rejected? (or drops off the list) Should the patch keyword be removed? What if the patch is in wine-staging?
Another question: should the patch be attached to the bug or is it sufficient to provide a link to a mailing list post?
I'm leaning towards the former in which case the definition could be amended as such: | Bugs with this keyword contain an attachment with a proposed fix. | The fix may be incomplete, break other applications or be otherwise | unacceptable as long as it does fix the symptoms described in the | bug and is not a pure workaround (such as altering Wine's behavior | based on the executable filename, replacing an implemented function | with a stub, etc.).
Op ma 19 jul. 2021 om 14:31 schreef Francois Gouget <fgouget@codeweavers.com
:
I'm leaning towards the former in which case the definition could be amended as such: | Bugs with this keyword contain an attachment with a proposed fix.
IMO a wine-devel or source.winhq.org/patches/data link should suffice. Not all patch submitters have bugzilla accounts and when someone other than the patch submitter attaches the patch, information could potentially get lost.
On Mon, 19 Jul 2021, Gijs Vermeulen wrote:
Op ma 19 jul. 2021 om 14:31 schreef Francois Gouget <fgouget@codeweavers.com
:
I'm leaning towards the former in which case the definition could be amended as such: | Bugs with this keyword contain an attachment with a proposed fix.
IMO a wine-devel or source.winhq.org/patches/data link should suffice.
The issue is dead links.
In particular, if I'm not mistaken, the expire script [1] removes patches from the source.winehq.org/patches site after at most 30 days [2] which I think is too short for Bugzilla.
Links to mailing list archives should be more durable. Maybe enough for them to be acceptable.
[1] https://source.winehq.org/git/tools.git/blob/HEAD:/patches/expire#l24
[2] Yet some patches defy the odds like 62608 which dates back to 2010! But it's an exception: the next oldest is 62707 which means the 99 intermediate patches were less lucky (then it's 62727, 62752, 62881, 62943, ... it's quite random).
Op ma 19 jul. 2021 om 15:37 schreef Francois Gouget <fgouget@codeweavers.com
:
In particular, if I'm not mistaken, the expire script [1] removes patches from the source.winehq.org/patches site after at most 30 days [2] which I think is too short for Bugzilla.
I can access a patch from May 18th [1] just fine, even as far back as February 16th [2]. I might be missing something, but doesn't the expiry code only hide them from the patches list and not actually delete the /data/ entries?
[1] https://source.winehq.org/patches/data/206000 [2] https://source.winehq.org/patches/data/200000
Francois Gouget fgouget@codeweavers.com writes:
On Mon, 19 Jul 2021, Gijs Vermeulen wrote:
Op ma 19 jul. 2021 om 14:31 schreef Francois Gouget <fgouget@codeweavers.com
:
I'm leaning towards the former in which case the definition could be amended as such: | Bugs with this keyword contain an attachment with a proposed fix.
IMO a wine-devel or source.winhq.org/patches/data link should suffice.
The issue is dead links.
In particular, if I'm not mistaken, the expire script [1] removes patches from the source.winehq.org/patches site after at most 30 days [2] which I think is too short for Bugzilla.
The entries are removed from the patches page, but the direct URL is valid forever.
[2] Yet some patches defy the odds like 62608 which dates back to 2010! But it's an exception: the next oldest is 62707 which means the 99 intermediate patches were less lucky (then it's 62727, 62752, 62881, 62943, ... it's quite random).
A few may have been lost in the very first versions of the patch tracker, but AFAICT everything from about 63250 should still be here.
On 7/19/21 15:30, Francois Gouget wrote:
On Tue, 6 Jul 2021, Francois Gouget wrote: [...]
patch | Bugs in this keyword contain a patch that are waiting for approval.
What is a 'patch that is waiting for approval'? Is it a patch which is referenced in https://source.winehq.org/patches/ ? What happens if the patch is rejected? (or drops off the list) Should the patch keyword be removed? What if the patch is in wine-staging?
I think the description does not match how this de facto used. AFAIK The keyword 'patch' used to be set now if the bug have just any patch attached which is supposed to be fixing the issue described in the bug report. This patch is not necessarily waiting for approval or is an upstream candidate at all.
If there is a related patch in Wine-staging there is currently 'Staged' status for that, so it looks like pretty much independent from the 'patch' keyword.
Another question: should the patch be attached to the bug or is it sufficient to provide a link to a mailing list post?
I'm leaning towards the former in which case the definition could be amended as such: | Bugs with this keyword contain an attachment with a proposed fix. | The fix may be incomplete, break other applications or be otherwise | unacceptable as long as it does fix the symptoms described in the | bug and is not a pure workaround (such as altering Wine's behavior | based on the executable filename, replacing an implemented function | with a stub, etc.).
If to choose one, I also think the former is a better option as quite often just a proof of concept hacky patch may be provided for illustration purposes which might have nothing in common to what makes sense to send upstream, or it is not going to be accepted upstream (unless the usage rules for 'patch' keyword are changed). I don't see why it can't also be a link to a mailing list if there is a patch sent, should it just one option to be left?
On Mon, 19 Jul 2021, Paul Gofman wrote:
On 7/19/21 15:30, Francois Gouget wrote:
On Tue, 6 Jul 2021, Francois Gouget wrote: [...]
- patch | Bugs in this keyword contain a patch that are waiting for approval.
[...]
I think the description does not match how this de facto used. AFAIK The keyword 'patch' used to be set now if the bug have just any patch attached which is supposed to be fixing the issue described in the bug report. This patch is not necessarily waiting for approval or is an upstream candidate at all.
Good. That matches what I think the definition should say.
[...]
Another question: should the patch be attached to the bug or is it sufficient to provide a link to a mailing list post?
I'm leaning towards the former in which case the definition could be amended as such: | Bugs with this keyword contain an attachment with a proposed fix. | The fix may be incomplete, break other applications or be otherwise | unacceptable as long as it does fix the symptoms described in the | bug and is not a pure workaround (such as altering Wine's behavior | based on the executable filename, replacing an implemented function | with a stub, etc.).
If to choose one, I also think the former is a better option as quite often just a proof of concept hacky patch may be provided for illustration purposes
I think your use of 'former' is different from mine. By "I'm leaning towards the former" I meant that I think it's better for the definition to explicitly require the patch to be attached to the bug (to avoid dead links as explained in another email).
It seems you meant something else but I got confused.
On 7/19/21 17:00, Francois Gouget wrote:
I think your use of 'former' is different from mine. By "I'm leaning towards the former" I meant that I think it's better for the definition to explicitly require the patch to be attached to the bug (to avoid dead links as explained in another email).
Yes, that is exactly what I prefer as well in the first place. Sorry for being unclear.
The fact that the patches got cleaned up after a while not just from the Web page but completely is a good reason not to replace the patches with the links at all, even if there is a patch sent.
-
Alexandre Julliard -
Dmitry Timoshkov -
Esme Povirk (she/they) -
Francois Gouget -
Francois Gouget -
Gijs Vermeulen -
Paul Gofman -
Rosanne DiMesio -
Zebediah Figura (she/her)