Re: Patchwatcher online
On Fr, 2008-09-05 at 10:24 -0700, Dan Kegel wrote: The results page http://kegel.com/wine/patchwatcher/results/ looks nice and green; Opps, all developer send there Patches in September with 09 as minute, and in August with 08 ... :-) And it would be very nice, when you hide the Email-Address to block the bots, who collect spam targets. -- By by ... Detlef
Re: Patchwatcher online
On Tue, Sep 9, 2008 at 11:37 AM, Detlef Riekenberg [EMAIL PROTECTED] wrote: http://kegel.com/wine/patchwatcher/results/ Opps, all developer send there Patches in September with 09 as minute, and in August with 08 ... Whoops! And it would be very nice, when you hide the Email-Address to block the bots, who collect spam targets. Is that needed, given how the addresses are in the open on the mailing list and all its archives?
Re: Patchwatcher online
2008/9/9 Dan Kegel [EMAIL PROTECTED]: And it would be very nice, when you hide the Email-Address to block the bots, who collect spam targets. Is that needed, given how the addresses are in the open on the mailing list and all its archives? I doubt it, we're probably on every possible spam list anyway.
Re: Patchwatcher online
Dan Kegel wrote: On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming. True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it. I'd argue that testing just the affected dll is correct. What about things like patches to ntdll/kernel32/advapi32 (and the likes). They could influence far more tests then just the ones for it's own dll. -- Cheers, Paul.
Re: Patchwatcher online
On Tue, Aug 12, 2008 at 1:01 AM, Paul Vriens [EMAIL PROTECTED] wrote: Dan Kegel wrote: On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming. True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it. I'd argue that testing just the affected dll is correct. What about things like patches to ntdll/kernel32/advapi32 (and the likes). They could influence far more tests then just the ones for it's own dll. Which is an argument for why you should test the entire tree for each patch. If a patch to ntdll makes tests in another dll fail, the patch should be rejected just as if the ntdll tests failed. This is how Julliard's work flow goes, so the test bot should do the same. -- James Hawkins
Re: Patchwatcher online
Am Montag, den 11.08.2008, 17:34 -0700 schrieb Dan Kegel:
Yes. I already changed the success message to make more sense,
and added background colors of green and red for success and failure.
I dislike the implementation, while I like the idea. You now have:
a:visited { color: #FF; }
.fail { background color: #ff5050; }
At least on my laptop display, #ff5050 on #ff is quite hard to read.
Regards,
Michael Karcher
Re: patchwatcher online
On Tue, Aug 12, 2008 at 6:47 AM, [EMAIL PROTECTED] wrote: couldn't you instead when the patchwatcher takes the patch it assigns it a patch # and require if there is a patch dependency that the person put into a comment REQ_PATCH: 123456,15456, etc.. ? Yes, perhaps if patchwatcher catches on and becomes a central part of Wine's workflow, that would be worth it.
Re: Patchwatcher online
Yes. I already changed the success message to make more sense,
and added background colors of green and red for success and failure.
I dislike the implementation, while I like the idea. You now have:
a:visited { color: #FF; }
.fail { background color: #ff5050; }
At least on my laptop display, #ff5050 on #ff is quite hard to read.
I think making the a:visited link for pass or fail to be balck is good enough.
I think this should do the trick.
.pass a:visited { color: #00; }
.fail a:visited { color: #00; }
---
VJ
Re: Patchwatcher online
Michael Karcher [EMAIL PROTECTED] wrote:
Yes. I already changed the success message to make more sense,
and added background colors of green and red for success and failure.
I dislike the implementation, while I like the idea. You now have:
a:visited { color: #FF; }
.fail { background color: #ff5050; }
At least on my laptop display, #ff5050 on #ff is quite hard to read.
Yeah, I know. I fiddled with the colors for a while, but not very effectively.
I'm partly color-blind, and am not really the best person to
work on the look of the reports page. If somebody else would like
to get the colors right, I'd gladly accept patches.
Would using black for foreground uniformly be more acceptable?
Re: Patchwatcher online
Am Dienstag, den 12.08.2008, 08:26 -0700 schrieb Dan Kegel: Yeah, I know. I fiddled with the colors for a while, but not very effectively. I'm partly color-blind, and am not really the best person to work on the look of the reports page. If somebody else would like to get the colors right, I'd gladly accept patches. Would using black for foreground uniformly be more acceptable? Looks better now. UI suggestion: If tests fail, format the status column like this instead: a href=results/1.diff5 regressions/a in a href=results/1.logtests/a where results/1.diff just contains the diff output that is currently appended to the log file. I have the impression that the whole table just is too wide. For me, real name would definitely be shorter than e-mail address, but using Yet it would be even greater if my patches would get green. They seem to fail afoul flaky tests, but you are already aware of that problem. adresses might be advantageous because they are (a) unique and (b) definitely ASCII charset. Also subjects might be shortened by removing PATCH (I am sorry, but I didn't get git to number the series and not output PATCH, any hints welcome!) and cutting the subject after 40 characters. Still, besides all the critique: Great work! Regards, Michael Karcher PS: Yet patchwatcher would be even greater if my patches would get green. They seem to fail afoul flaky tests, but you are already aware of that problem.
Re: patchwatcher online
On Tue, Aug 12, 2008 at 10:29 AM, Reece Dunn [EMAIL PROTECTED] wrote: I use GMail to do something similar - tag mail I send to wine-patches with a 'wine-tracking' label, as well as the 'wine-patches' label it gets from the mailing list filter I have. This allows me to see all active patches I currently have. Once Alexandre commits them, I manually remove the 'wine-tracking' label; similarly if there are comments on it that mean I need to provide a revised patch. Potentially patchwatcher could monitor wine-devel for replies to the patch. The other patch monitoring systems out there do something similar; see http://bundlebuggy.aaronbentley.com/ and http://patchwork.ozlabs.org/linuxppc/ Where patchwatcher would be useful is to know whether Alexandre has looked at the patch yet (no more dly on #winehackers) and has rejected it - hopefully with comments as to why! Yes, but that's harder than: It should also help catch the patches that do not apply, or cause regressions in the tests, which should help Alexandre. which is my primary goal. It will also mean that patches are not lost in a flood (e.g. coming back off holiday). Perhaps, if it becomes a real workflow application, but resubmitting fresh patches is often required anyway... - Dan
Re: Patchwatcher online
On Mon, 2008-08-11 at 22:24 -0700, Dan Kegel wrote: Dmitry Timoshkov [EMAIL PROTECTED] wrote: Zachary Goldberg [EMAIL PROTECTED] wrote: Policy is that all patches should be independent, no? There is no such a policy. Dependent patches are marked as 1/xx, 2/xx, ... xx/xx. That's a patch series, and patchwatcher handles that ok. There's another kind of dependent patch, where somebody says This requires Harold's patch from yesterday. Patchwatcher probably isn't going to handle that ever. What about checking for a string in the email like patchwatchignore; if for some reason the patch is known to cause a failure the e-mail might read like: patchwatchignore This depends on Harald's patch from yesterday... [patch] This also might be useful for patches which update comments or README files, etc.
Re: Patchwatcher online
On Tue, Aug 12, 2008 at 1:39 PM, Adam Petaccia [EMAIL PROTECTED] wrote: What about checking for a string in the email like patchwatchignore; if for some reason the patch is known to cause a failure the e-mail might read like: patchwatchignore This depends on Harald's patch from yesterday... [patch] If patchwatcher becomes a gatekeeper, we will add a bypass like that. This also might be useful for patches which update comments or README files, etc. It's a lot easier to let the hardware run the tests, and as long as the hardware is fast enough to not get backlogged, the payoff for using the bypass for doc-only changes is small.
Re: patchwatcher online
2008/8/12 Dan Kegel [EMAIL PROTECTED]: On Tue, Aug 12, 2008 at 6:47 AM, [EMAIL PROTECTED] wrote: couldn't you instead when the patchwatcher takes the patch it assigns it a patch # and require if there is a patch dependency that the person put into a comment REQ_PATCH: 123456,15456, etc.. ? Yes, perhaps if patchwatcher catches on and becomes a central part of Wine's workflow, that would be worth it. I like the idea of patchwatcher. I use GMail to do something similar - tag mail I send to wine-patches with a 'wine-tracking' label, as well as the 'wine-patches' label it gets from the mailing list filter I have. This allows me to see all active patches I currently have. Once Alexandre commits them, I manually remove the 'wine-tracking' label; similarly if there are comments on it that mean I need to provide a revised patch. Where patchwatcher would be useful is to know whether Alexandre has looked at the patch yet (no more dly on #winehackers) and has rejected it - hopefully with comments as to why! It should also help catch the patches that do not apply, or cause regressions in the tests, which should help Alexandre. It will also mean that patches are not lost in a flood (e.g. coming back off holiday). So yeah, I like the idea. - Reece
Re: Patchwatcher online
On Sat, Aug 9, 2008 at 9:01 PM, Dan Kegel [EMAIL PROTECTED] wrote: For the moment, the results only go to a web page, http://kegel.com/wine/patchwatcher/results/ Most of the results there right now are just test messages so you can see how it will look when real patches with various problems are received. The scripts now run conformance tests and report regressions. It's pretty rough still - I haven't compensated for flaky tests properly yet. And I'm running them on a slowish machine, so it lags by twenty minutes or so. The source is still at http://code.google.com/p/winezeug/source/browse/#svn/trunk/patchwatcher if anyone else is curious, or wants to run it themselves.
Re: Patchwatcher online
Am Montag, den 11.08.2008, 09:45 -0700 schrieb Dan Kegel: The scripts now run conformance tests and report regressions. Does Ditto, but just the new error:end of output mean that there are no new errors? Regards, Michael Karcher
Re: Patchwatcher online
On Mon, Aug 11, 2008 at 10:13 AM, Michael Karcher [EMAIL PROTECTED] wrote: Am Montag, den 11.08.2008, 09:45 -0700 schrieb Dan Kegel: The scripts now run conformance tests and report regressions. Does Ditto, but just the new error:end of output mean that there are no new errors? Yes. Sorry, I'll tidy that up tonight, it really should say no regressions found.
Re: Patchwatcher online
Hi, I have one more concern. Its regarding running of tests. When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming. What will happen if we have patch barrage, like once when alexander comes from vacation. How much time will that take for verification of the entire patch tree submitted then. Just my $0.02 Thanks, VJ On Mon, Aug 11, 2008 at 3:05 PM, Dan Kegel [EMAIL PROTECTED] wrote: On Mon, Aug 11, 2008 at 10:13 AM, Michael Karcher [EMAIL PROTECTED] wrote: Am Montag, den 11.08.2008, 09:45 -0700 schrieb Dan Kegel: The scripts now run conformance tests and report regressions. Does Ditto, but just the new error:end of output mean that there are no new errors? Yes. Sorry, I'll tidy that up tonight, it really should say no regressions found.
Re: Patchwatcher online
On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming. True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it. What will happen if we have patch barrage, like once when alexander comes from vacation. It'll fall behind some. If need be, I can run it on a really fast machine. - dan
Re: Patchwatcher online
On Mon, Aug 11, 2008 at 7:42 PM, Dan Kegel [EMAIL PROTECTED] wrote: On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming. True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it. Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher. Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here) What will happen if we have patch barrage, like once when alexander comes from vacation. It'll fall behind some. If need be, I can run it on a really fast machine. It would better if we have a parallelized version of the tests also run on a fast m/c. Also can you improve the messages. If there are errors, Its possible to only show the test data that failed rather than the complete test run. Also put it in a public repository with you as sole commiter. So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) ) VJ - dan
Re: Patchwatcher online
Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher. It's running on a 1GHz single core machine right now. I'll probably put it on something rather faster. Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here) No. Probably should, but I'm not. It would better if we have a parallelized version of the tests also run on a fast m/c. I do have a patch that enables parallel execution of conformance tests, I hope Alexandre accepts it. That will help on multicore systems. Beyond that, I could fairly easily use multiple machines, e.g. assign all patches to machines based on md5sum. Also can you improve the messages. Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure. If there are errors, Its possible to only show the test data that failed rather than the complete test run. Yes. Also put it in a public repository with you as sole commiter. Already there, see http://code.google.com/p/winezeug/ So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) ) Please do. - Dan
Re: Patchwatcher online
On Mon, Aug 11, 2008 at 8:34 PM, Dan Kegel [EMAIL PROTECTED] wrote: Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher. It's running on a 1GHz single core machine right now. I'll probably put it on something rather faster. Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here) No. Probably should, but I'm not. It would better if we have a parallelized version of the tests also run on a fast m/c. I do have a patch that enables parallel execution of conformance tests, I hope Alexandre accepts it. That will help on multicore systems. Beyond that, I could fairly easily use multiple machines, e.g. assign all patches to machines based on md5sum. Also can you improve the messages. Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure. If there are errors, Its possible to only show the test data that failed rather than the complete test run. Yes. Also put it in a public repository with you as sole commiter. Already there, see http://code.google.com/p/winezeug/ So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) ) Please do. - Dan Dan, how are you handling the case when Alexandre floods the list with commits? One way I can think of to avoid those patches is to ignore any patches emailed by Alexandre but aren't written by him. Its not the worst thing in the world if the script doesn't skip commits, just some wasted perf.
Re: Patchwatcher online
On Mon, Aug 11, 2008 at 8:34 PM, Dan Kegel [EMAIL PROTECTED] wrote: Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher. It's running on a 1GHz single core machine right now. I'll probably put it on something rather faster. Something around 2GHz would be ok Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here) No. Probably should, but I'm not. I think you should do that, if there are some changes that involve wineserver and some change to registry. It would better if we have a parallelized version of the tests also run on a fast m/c. I do have a patch that enables parallel execution of conformance tests, I hope Alexandre accepts it. That will help on multicore systems. Beyond that, I could fairly easily use multiple machines, e.g. assign all patches to machines based on md5sum. Ok, it would be interesting how parallelization could affect wine :) Also can you improve the messages. Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure. Also add Yellow for ignored patches. For ignored patches /i would like to add a second pass, when have to check if the patch is generated by git or not if not patch is being ignored now, for that we need to process the patch to see whether we can extract some info to send the correct patch level. If there are errors, Its possible to only show the test data that failed rather than the complete test run. Yes. Also put it in a public repository with you as sole commiter. Already there, see http://code.google.com/p/winezeug/ Thanks So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) ) Please do. I will look into it now VJ
Re: Patchwatcher online
Zachary Goldberg [EMAIL PROTECTED] wrote: [much quoted text] Please trim the quotes down a bit when you reply... Dan, how are you handling the case when Alexandre floods the list with commits? See refresh_tree(), http://code.google.com/p/winezeug/source/browse/trunk/patchwatcher/patchwatcher.sh#105 After processing each patch (or patch series), patchwatcher check for git commits. If it finds any, it recomputes the expected test failures. This is slow, but it only happens once or twice a day, so it's ok. One way I can think of to avoid those patches is to ignore any patches emailed by Alexandre but aren't written by him. Alexandre never emails any patches at all, he just commits them directly. So patchwatcher will never give any feedback on his changes. - Dan
Re: Patchwatcher online
Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: Also add Yellow for ignored patches. Let me think on that a bit. Probably. For ignored patches /i would like to add a second pass, when have to check if the patch is generated by git or not if not patch is being ignored now, for that we need to process the patch to see whether we can extract some info to send the correct patch level. Yes, I plan to add a little code to try to guess the right argument to -p. - Dan
Re: Patchwatcher online
On Mon, Aug 11, 2008 at 11:02 PM, Dan Kegel [EMAIL PROTECTED] wrote: Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: Also add Yellow for ignored patches. Let me think on that a bit. Probably. For ignored patches /i would like to add a second pass, when have to check if the patch is generated by git or not if not patch is being ignored now, for that we need to process the patch to see whether we can extract some info to send the correct patch level. Yes, I plan to add a little code to try to guess the right argument to -p. Ahh... I forgot how to handle dependent patches, if they are not in a patch series Sorry, I always seem to get all kinda of weird ideas about how things can go wrong VJ
Re: Patchwatcher online
Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: Ahh... I forgot how to handle dependent patches, if they are not in a patch series I don't know if there's a good way to handle those. Maybe just encourage people not to send them :-)
Re: Patchwatcher online
On Mon, Aug 11, 2008 at 11:52 PM, Dan Kegel [EMAIL PROTECTED] wrote: Vijay Kiran Kamuju [EMAIL PROTECTED] wrote: Ahh... I forgot how to handle dependent patches, if they are not in a patch series I don't know if there's a good way to handle those. Maybe just encourage people not to send them :-) Policy is that all patches should be independent, no? (Sorry about big quoting, gmail just compresses it so nicely)
Re: Patchwatcher online
Zachary Goldberg [EMAIL PROTECTED] wrote: Policy is that all patches should be independent, no? There is no such a policy. Dependent patches are marked as 1/xx, 2/xx, ... xx/xx. -- Dmitry.
Re: Patchwatcher online
Dmitry Timoshkov [EMAIL PROTECTED] wrote: Zachary Goldberg [EMAIL PROTECTED] wrote: Policy is that all patches should be independent, no? There is no such a policy. Dependent patches are marked as 1/xx, 2/xx, ... xx/xx. That's a patch series, and patchwatcher handles that ok. There's another kind of dependent patch, where somebody says This requires Harold's patch from yesterday. Patchwatcher probably isn't going to handle that ever.
Re: Patchwatcher online
Dan Kegel [EMAIL PROTECTED] wrote: That's a patch series, and patchwatcher handles that ok. There's another kind of dependent patch, where somebody says This requires Harold's patch from yesterday. Patchwatcher probably isn't going to handle that ever. Well, that happens not that often, so this can be safely ignored for now I think. -- Dmitry.
Re: Patchwatcher online
On Sat, Aug 9, 2008 at 9:01 PM, Dan Kegel [EMAIL PROTECTED] wrote: For the moment, the results only go to a web page, http://kegel.com/wine/patchwatcher/results/ Most of the results there right now are just test messages so you can see how it will look when real patches with various problems are received. The scripts are a bit ugly, so expect problems to linger for the next week or so while I work out the kinks. I just noticed Ambroz actually came through with the chroot: http://article.gmane.org/gmane.comp.emulators.wine.devel/62314 I had already moved to using a separate userid, and switched to a very low-privilege way of uploading results, so my security story is probably ok for a little while, but I'll try to integrate the chroot one of these days soon for even more protection.
