On Tue, Jul 3, 2012 at 1:08 AM, Andrea Canciani ranma42@gmail.com wrote:
I tried to follow the http://wiki.winehq.org/SubmittingPatches guidelines, but this is my first submission to wine, so I guess the patches might need further improvement. Please point out any issues that need fixing.
On an administrivial note, you should send one patch per email to wine-patches.
Do you think you could write a test that (semi-)reliably causes the deadlock you're fixing?
The patches have been tested by Dan Kegel (in CC) and are currently being used by many MacOSX and Linux users to run LoL.
Our emails may have crossed - with these patches applied, dlls/ieframe/tests/webbrowser.ok seems to crash here:
Unhandled exception: privileged instruction in 32-bit code (0x02786624). Backtrace: =>0 0x02786624 (0x0032fcd8) 1 0x688e22db func_webbrowser+0x3ca() [dlls/ieframe/tests/webbrowser.c:3382]
Otherwise the tests seem to pass.
Thanks, Dan
On Tue, Jul 3, 2012 at 3:45 PM, Dan Kegel dank@kegel.com wrote:
On Tue, Jul 3, 2012 at 1:08 AM, Andrea Canciani ranma42@gmail.com wrote:
I tried to follow the http://wiki.winehq.org/SubmittingPatches guidelines, but this is my first submission to wine, so I guess the patches might need further improvement. Please point out any issues that need fixing.
On an administrivial note, you should send one patch per email to wine-patches.
Oops, sorry! Will do next time (which might be soon enough, given the regression in ieframe and the missing deadlock test)
Do you think you could write a test that (semi-)reliably causes the deadlock you're fixing?
Yes, I guess it should be possible. Could you point me to a test which triggers a deadlock (in particular, how should the deadlock be handled?) I tried git-grep'ing for deadlock in the tests, but I only found checks for deadlock-named constants/values.
The patches have been tested by Dan Kegel (in CC) and are currently being used by many MacOSX and Linux users to run LoL.
Our emails may have crossed - with these patches applied, dlls/ieframe/tests/webbrowser.ok seems to crash here:
Unhandled exception: privileged instruction in 32-bit code (0x02786624). Backtrace: =>0 0x02786624 (0x0032fcd8) 1 0x688e22db func_webbrowser+0x3ca() [dlls/ieframe/tests/webbrowser.c:3382]
Yes, I sent the e-mail just before finding out about this issue. I will probably need to set up a proper wine development environment to find out more about it (I have a ubuntu 12.04 x86 vm, but even wine/master fails the testsuite on it).
Otherwise the tests seem to pass.
Thanks, Dan
On Wed, Jul 4, 2012 at 12:15 AM, Andrea Canciani ranma42@gmail.com wrote:
Do you think you could write a test that (semi-)reliably causes the deadlock you're fixing?
Yes, I guess it should be possible. Could you point me to a test which triggers a deadlock (in particular, how should the deadlock be handled?)
http://bugs.winehq.org/show_bug.cgi?id=28362 says that the mshtml tests hang occasionally. The testsuite handles them by, well, hanging :-) Scripts that care about this add their own timeout.
Yes, I sent the e-mail just before finding out about this issue. I will probably need to set up a proper wine development environment to find out more about it (I have a ubuntu 12.04 x86 vm, but even wine/master fails the testsuite on it).
The testsuite does not, in general, fully pass on any machine but Alexandre's, so just generate a baseline by running the tests several times with "make -k test" without your patch, and saving the logs. Then repeat after applying your patch.
(https://code.google.com/p/winezeug/source/browse/trunk/buildbot/dotests_blac... has a list of all the tests that the buildbot failed on while it was running, and you can run just the known good or known bad tests with https://code.google.com/p/winezeug/source/browse/trunk/buildbot/dotests.sh but that's overkill (or underkill) for you, and slightly out of date anyway.) - Dan
Dan Kegel wrote on Wed, 04 Jul 2012:
(https://code.google.com/p/winezeug/source/browse/trunk/buildbot/dotests_blac... has a list of all the tests that the buildbot failed on while it was running
The ones annotated with http://xquartz.macosforge.org/trac/ticket/512 should work with the latest XQuartz, as that bug has been fixed.
Jonas