Bug 64071

I believe the fallback orders in ORWT are: qt-arm, qt qt-4.8, qt But I suspect those aren't really right. From my investigation it appears that the qt-mac results aren't really used either. They're certainly very out of date.

(In reply to comment #1) > I believe the fallback orders in ORWT are: > > qt-arm, qt > qt-4.8, qt > > But I suspect those aren't really right. > > From my investigation it appears that the qt-mac results aren't really used either. They're certainly very out of date. Now qt, qt-arm, qt-4.8 and qt-mac platforms are used by buildbots run on http://build.webkit.sed.hu/waterfall . qt-linux and qt-win platforms aren't really used. The reference OS for Qt port is Linux, so all qt-* platforms should fallback to qt platform. qt-4.8 is for development, result must be same as on qt (4.7) platform when Qt-4.8 is released. AFAIK theses bugs are fixed in Qt trunk, but I'm going to check it in 1-2 days. On qt-mac platform we only use Skipped list now, because we haven't human resource to maintain expected files day by day. We always add failing tests to this skipped list with changeset number. I think making NRWT work with these platforms isn't priority for us now, because there are major problems with NRWT for Qt port now. But Szeged team will fix this bug later.

> I think making NRWT work with these platforms isn't priority for us now, > because there are major problems with NRWT for Qt port now. But Szeged team > will fix this bug later. What are those major problems exactly? I think it is a priority for us to use the test infrastructure that has become the official one for the WebKit project.

I disabled NRWT for qt-arm and qt-4.8 platforms until fix: http://trac.webkit.org/changeset/90746 Gábor started working on this bug.

It's easy to support these two platforms. But it would make the logic for generating these names cleaner if the platform directories were renamed to include the OS, so: qt-4.8-linux qt-linux-arm Which fit a more easy generated pattern of: PORT-VERSION-OS and PORT-OS-ARCH Where both OS and ARCH are optional. In either case it doesn't matter much, but we'll need to special case the fallback. chromium-linux-x86 definitely follows the PORT-OS-ARCH pattern, I don't really see any other ports which have a VERSION for their port, but not their OS. (mac-leopard, win-xp are 2 of many examples of using OSVERSION in the names). This is all part of our continuing effort to simplify our fallback rules. qt-4.8 and qt-arm didn't easily fit any existing fallback rules, so I didn't add support for them in the first-pass implementation of the QtPort for NRWT.

(In reply to comment #2) > Now qt, qt-arm, qt-4.8 and qt-mac platforms are used by buildbots run > on http://build.webkit.sed.hu/waterfall . qt-linux and qt-win platforms > aren't really used. Can we delete qt-linux and qt-win then?

(In reply to comment #2) > The reference OS for Qt port is Linux, so all qt-* platforms should > fallback to qt platform. qt-4.8 is for development, result must be > same as on qt (4.7) platform when Qt-4.8 is released. AFAIK theses > bugs are fixed in Qt trunk, but I'm going to check it in 1-2 days. The Apple folks have historically used "mac" to mean the "Development version" and specific numbered versions for older versions. I think Qt should consider doing similar (which means that some of the 4.8 results should move into qt/ and some of the 4.7-specific results in qt/ should move up into a qt-4.7.

Now the baseline search path on qt-wk platform: qt-wk2 -> qt-linux -> qt -> generic It should be: qt-wk2 -> ((qt-linux ->)) qt-4.8 -> qt -> generic If it is fixed, we can enable ~400 tests on qt-wk2 platform.

(In reply to comment #6) > Can we delete qt-linux and qt-win then? Not yet, they might be used in the future.

(In reply to comment #7) > (In reply to comment #2) > > The reference OS for Qt port is Linux, so all qt-* platforms should > > fallback to qt platform. qt-4.8 is for development, result must be > > same as on qt (4.7) platform when Qt-4.8 is released. AFAIK theses > > bugs are fixed in Qt trunk, but I'm going to check it in 1-2 days. > > The Apple folks have historically used "mac" to mean the "Development version" and specific numbered versions for older versions. > > I think Qt should consider doing similar (which means that some of the 4.8 results should move into qt/ and some of the 4.7-specific results in qt/ should move up into a qt-4.7. Now the Qt 4.7 is the default, and 4.8 exceptions are stored in qt-4.8. But after 4.8 is released we will merge qt-4.8 to qt and after then we can remove qt-4.8 platform. (Because qt will means Qt 4.8 and 4.7 won't be supported anymore.)

(In reply to comment #8) > Now the baseline search path on qt-wk platform: qt-wk2 -> qt-linux -> qt -> generic > > It should be: qt-wk2 -> ((qt-linux ->)) qt-4.8 -> qt -> generic > > If it is fixed, we can enable ~400 tests on qt-wk2 platform. I forgot to mention why we need this search path: Because to build WebKit2, the minimum Qt version is 5.0 (which based on 4.8).

It sounds like your troubles could very easily be fixed by moving qt-4.8 to qt and qt to qt-4.7, right?

The reason why I propose moving instead of hacking the tool is that there are long-term benefits to every project using similar fallback systems. We would still need to add 4.7 as an explicit version number (I think), but otherwise NRWT should just work "out of the box" including getting the wk2 fallback path correctly then, right?

(In reply to comment #13) > We would still need to add 4.7 as an explicit version number (I think), but otherwise NRWT should just work "out of the box" including getting the wk2 fallback path correctly then, right? Do you mean we should add a qt-4.7 platform too? And then the search paths would be in general: qt-wk2|none -> qt-(linux|win|mac) -> qt-(4.7|4.8) -> generic examples: qt-wk2 -> qt-linux -> qt-4.8 -> qt -> generic qt-linux -> qt-4.8 -> qt -> generic qt-linux -> qt-4.7 -> qt -> generic It sounds good to me. Now wk2 won't depends on Qt version. (You can't build it with Qt 4.7, only Qt 5.0, so version will be qt-48 always, because qt-4.7 == Qt-4.7.x and qt-4.8 == Qt-4.8.x or newer) We only need to add a Qt version checker function similar to in webkitdirs.pm: sub getQtVersion() { my $qtVersion = `$qmakebin --version`; $qtVersion =~ s/^(.*)Qt version (\d\.\d)(.*)/$2/s ; return $qtVersion; }

Removing qt-arm, and filed a new bug report about it: https://bugs.webkit.org/show_bug.cgi?id=67777

(In reply to comment #14) > (In reply to comment #13) > > We would still need to add 4.7 as an explicit version number (I think), but otherwise NRWT should just work "out of the box" including getting the wk2 fallback path correctly then, right? > > Do you mean we should add a qt-4.7 platform too? > And then the search paths would be in general: > qt-wk2|none -> qt-(linux|win|mac) -> qt-(4.7|4.8) -> generic > > examples: > qt-wk2 -> qt-linux -> qt-4.8 -> qt -> generic > qt-linux -> qt-4.8 -> qt -> generic > qt-linux -> qt-4.7 -> qt -> generic No, in order to match the other WebKit ports, the search paths would need to be: qt-wk2 -> qt -> qt-4.8 -> qt-4.7 -> generic That matches mac/win: mac-wk2 -> mac -> mac-lion -> mac-snow-leopard -> mac-leopard -> generic win-wk2 -> win -> win-win7sp0 -> win-xp -> generic I'm not sure where qt-linux vs. qt-mac, etc. falls in. We should look at how Chromium does their fallback.

(In reply to comment #16) > (In reply to comment #14) > > (In reply to comment #13) > > > We would still need to add 4.7 as an explicit version number (I think), but otherwise NRWT should just work "out of the box" including getting the wk2 fallback path correctly then, right? > > > > Do you mean we should add a qt-4.7 platform too? > > And then the search paths would be in general: > > qt-wk2|none -> qt-(linux|win|mac) -> qt-(4.7|4.8) -> generic > > > > examples: > > qt-wk2 -> qt-linux -> qt-4.8 -> qt -> generic > > qt-linux -> qt-4.8 -> qt -> generic > > qt-linux -> qt-4.7 -> qt -> generic > > No, in order to match the other WebKit ports, the search paths would need to be: > > qt-wk2 -> qt -> qt-4.8 -> qt-4.7 -> generic > > That matches mac/win: > mac-wk2 -> mac -> mac-lion -> mac-snow-leopard -> mac-leopard -> generic > win-wk2 -> win -> win-win7sp0 -> win-xp -> generic > > I'm not sure where qt-linux vs. qt-mac, etc. falls in. We should look at how Chromium does their fallback. Sorry, my comment was just wrong. I guess I should wake up more before commenting on bugs. Mac/Win work like this: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py#L128 http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py#L78 aka: mac-wk2 -> mac-lion -> mac-snow-leopard ... -> mac which is closer to what you said than what I said, with the exception of where you placed qt-4.7

Created attachment 108797 [details] proposed fix

Comment on attachment 108797 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=108797&action=review It's not immediately clear what the fallback path looks like here. But it seems that 4.8 results should fallback on 4.7 results, or vice versa, similar to how Mac/Win work. > Tools/Scripts/webkitpy/layout_tests/port/qt.py:93 > + def get_qt_version(self): I might instead make this qt_version(self) and have it be @memoized. We don't usually use "get" in function names in WebKit.

Comment on attachment 108797 [details] proposed fix r- now, because you missed to add the unit test change.

(In reply to comment #19) > (From update of attachment 108797 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108797&action=review > > It's not immediately clear what the fallback path looks like here. But it seems that 4.8 results should fallback on 4.7 results, or vice versa, similar to how Mac/Win work. Absolutely no. qt-4.8 shouldn't fallback to qt-4.7 or vice versa. qt-4.7 and qt-4.8 should fallback to qt and then generic. qt-4.7 and qt-4.8 doesn't depends on each other. There are failing tests on qt-4.7 which pass on qt-4.8 and vice-versa. > > Tools/Scripts/webkitpy/layout_tests/port/qt.py:93 > > + def get_qt_version(self): > > I might instead make this qt_version(self) and have it be @memoized. We don't usually use "get" in function names in WebKit. Good point.

You're welcome to have whatever fallback path you like, but the common way we do fallbacks in WebKit is to have older versions fall back to newer versions. For example, Leopard falls back to Snow Leopard, which falls back to Lion. Here's a diagram I drew a while back: https://docs.google.com/drawings/d/1z65SKkWrD4Slm6jobIphHwwRADyUtjOAxwGBVKBY8Kc/edit?hl=en_US I agree that it sounds goofy, but it seems to work pretty well.

Created attachment 109006 [details] proposed fix

Comment on attachment 109006 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=109006&action=review The development of Qt was faster than this patch's. :) So please update the patch. Now qt-wk2 should fallback to a new qt-5.0 platform. ( After Qt 4.8 will be releases, we are going to stop supporting qt-4.7 test results and then we're going to merge qt-4.8 results to general qt platform. But it is a mid-term plan.) Otherwise the patch LGTM. Is there any objection against landing the fixed patch? > Tools/Scripts/webkitpy/layout_tests/port/qt.py:33 > import logging > import sys > +import re style: alphabetic order > Tools/Scripts/webkitpy/layout_tests/port/qt.py:111 > + if '4.7' in version: > + search_paths.append('qt-4.7') > + elif version: > + search_paths.append('qt-4.8') if '4.7' in version: search_paths.append('qt-4.7') elif '4.8' in version: search_paths.append('qt-4.8') elif version: search_paths.append('qt-5.0') > Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py:36 > from webkitpy.layout_tests.port.qt import QtPort > from webkitpy.layout_tests.port import port_testcase > from webkitpy.tool.mocktool import MockOptions, MockUser, MockExecutive > +from webkitpy.common.system.executive_mock import MockExecutive2 style: alphabetic order > Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py:59 > + return 'QMake version 2.01a\nUsing Qt version 4.8.2 in /usr/local/Trolltech/Qt-4.8.2/lib' nit: s/4.8.2/4.8.0/g > Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py:78 > + self._assert_search_path(['qt-wk2', 'qt-mac', 'qt-4.8', 'qt'], 'darwin', use_webkit2=True, qt_version='5.0') > + self._assert_search_path(['qt-wk2', 'qt-win', 'qt-4.8', 'qt'], 'cygwin', use_webkit2=True, qt_version='5.0') > + self._assert_search_path(['qt-wk2', 'qt-linux', 'qt-4.8', 'qt'], 'linux2', use_webkit2=True, qt_version='5.0') s/qt-4.8/qt-5.0/g

Created attachment 109951 [details] updated fallback path

Comment on attachment 109951 [details] updated fallback path View in context: https://bugs.webkit.org/attachment.cgi?id=109951&action=review In general LGTM, but please fix these little things. > Tools/ChangeLog:9 > + * Scripts/webkitpy/layout_tests/port/qt_unittest.py: Removed. Removed ???? > Tools/Scripts/webkitpy/layout_tests/port/qt.py:93 > + def qt_version(self): @memoized is still missing > Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py:59 > + return 'QMake version 2.01a\nUsing Qt version 4.8.0 in /usr/local/Trolltech/Qt-4.8.2/lib' one more s/4.8.2/4.8.0 :)

(In reply to comment #26) > (From update of attachment 109951 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109951&action=review > > In general LGTM, but please fix these little things. > > > Tools/ChangeLog:9 > > + * Scripts/webkitpy/layout_tests/port/qt_unittest.py: Removed. > > Removed ???? > Uups, I had a little problem with the git. One moment, and I will fix it.

Created attachment 109960 [details] proposed fix

Comment on attachment 109960 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=109960&action=review I tested your patch, but I found one more thing. Skipped file in qt-<version> directory is ignored, but we need it. Could you override _skipped_file_search_paths too? > Tools/Scripts/webkitpy/layout_tests/port/qt.py:93 > + def qt_version(self): @memoized is still missing :-/

Created attachment 110135 [details] proposed fix I created also the Skipped files for qt-4.7 and qt-5.0

Comment on attachment 110135 [details] proposed fix r=me

Comment on attachment 110135 [details] proposed fix Landed in http://trac.webkit.org/changeset/97252

http://build.webkit.org/builders/SnowLeopard%20Intel%20Debug%20%28Tests%29/builds/2541/steps/webkitpy-test/logs/stdio You can't call out to qmake when determining the baseline paths: for line in self._executive.run_command(['qmake', '-v']).split('\n'): We need to be able to determine the baseline paths on all systems, even those without qmake. Mac and Win also need to solve this problem. It's probably a good idea to follow their approach.

We can't determine baseline path without calling qmake ... :((

> We can't determine baseline path without calling qmake ... :(( Can you explain why in more detail? I understand that you might not be able to determine which baseline path you'd like to use without calling qmake (or otherwise inspecting the environment), but you should be able to determine the universe of possible baseline paths without reading any information from the environment. Put another way, we have a requirement that you can enumerate the universe of baseline paths for all ports when running on any platform. For example, on a Mac without qmake installed, you should be able to know where the Qt port stores its baselines and what the fallback behavior is between them.

Fixed patch landed in http://trac.webkit.org/changeset/97461. It wasn't the best solution, but this patch was very important for us. I filed a new bug to fix Qt port for baselineoptimizer: https://bugs.webkit.org/show_bug.cgi?id=70108