Bug 98140 – check-webkit-style can't handle qt-arm, qt-win, qt-mac, qt-5.0, qt-5.0-wk1 and qt-5.0-wk2 TestExpecatations

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

We got the following non-sense:

https://bugs.webkit.org/show_bug.cgi?id=98136#c3
-------------------------------------------------------------------------------------------------------------------------------------
Comment #3 From WebKit Review Bot 2012-10-02 03:39:37 PST (-) [reply]

Attachment 166654 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
LayoutTests/platform/qt-5.0-wk2/TestExpectations:1:  No port uses path LayoutTests/platform/qt-5.0-wk2/TestExpectations for test_expectations  [test/expectations] [5]
Total errors found: 1 in 2 files

If any of these errors are false positives, please file a bug against check-webkit-style.
-------------------------------------------------------------------------------------------------------------------------------------

Have you got any idea what happened with the check-webkit-style?

[Michael Brüning]

Interesting enough, it was still able to handle it a day (a couple of hours) earlier (see https://bugs.webkit.org/show_bug.cgi?id=98037).

[Csaba Osztrogonác]

(In reply to comment #1)
> Interesting enough, it was still able to handle it a day (a couple of hours) earlier (see https://bugs.webkit.org/show_bug.cgi?id=98037).

Hmmm ... let me check the history and try bisecting.

[Csaba Osztrogonác]

bisecting finished - https://trac.webkit.org/changeset/127910
Hmmm ... it seems it can be a very old bug ... But I don't
have time to find who caused this annoying bug. :-/

[Csaba Osztrogonác]

It seems it handles only qt and qt-linux ... :-/

[Tony Chang]

Created attachment 166752 [details]
Patch

[Tony Chang]

I fixed one bug and suppressed another.  The problem is that when we use port_factory.all_port_names(), it doesn't know to return both qt-4.8 and qt-5.0 versions.  I'm not sure what the right way to fix this is, so I'm skipping style check (the check is doing more harm than good).

[Eric Seidel (no email)]

Comment on attachment 166752 [details]
Patch

Seems OK.  I found qt's layout test fallback confusing (and different from all the other ports) so I didn't implement support for it in the original NRWT conversion.  I (perhaps naively) hoped that they'd move to a naming system/fallback more similar to the other ports.  But it's also possible I simply mis-understood.  In any case, someone just needs to implement proper Qt port fallback for NRWT and then we can remove qt-arm's ORWT usage, and fix the bugs like this one. :)

[Eric Seidel (no email)]

(In reply to comment #6)
> I fixed one bug and suppressed another.  The problem is that when we use port_factory.all_port_names(), it doesn't know to return both qt-4.8 and qt-5.0 versions.  I'm not sure what the right way to fix this is, so I'm skipping style check (the check is doing more harm than good).

Presumably those are a lot like mac-leopard/mac-lion, etc.  If Qt uses the same style of fallback logic then we can just copy the logic from ApplePort/MacPort/WinPort or move it down to WebKitPort/Port for sharing.

[Rafael Brandao]

I believe the only reliable solution to figure out which version to useis to pass executor around and use "qmake --version" whenever you need to figure out the version, like we do on Tools/Scripts/webkitpy/layout_tests/port/qt.py (check qt_version()), but when I did it back there I felt like it was the wrong approach. Should we do this here as well?

[Tony Chang]

(In reply to comment #8)
> Presumably those are a lot like mac-leopard/mac-lion, etc.  If Qt uses the same style of fallback logic then we can just copy the logic from ApplePort/MacPort/WinPort or move it down to WebKitPort/Port for sharing.

Yeah, there are 2 things to fix:

1) Make qt.py's __init__ know how to parse something like qt-5.0-mac (it currently doesn't know how).

2) Update builders.py to specify qt versions (currently it just has qt-linux, qt-mac, qt-win and qt-wk2).


(In reply to comment #9)
> I believe the only reliable solution to figure out which version to useis to pass executor around and use "qmake --version" whenever you need to figure out the version, like we do on Tools/Scripts/webkitpy/layout_tests/port/qt.py (check qt_version()), but when I did it back there I felt like it was the wrong approach. Should we do this here as well?

That's what it's doing, but the EWS bots have Qt 4.8 installed (actually, probably no Qt, but that falls back to 4.8), but wants to check the TestExpectations file for Qt 5.0.  There's currently no way to override the Qt version when constructing a QtPort().

[WebKit Review Bot]

Comment on attachment 166752 [details]
Patch

Clearing flags on attachment: 166752

Committed r130220: <http://trac.webkit.org/changeset/130220>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dirk Pranke]

Note that there are several open Qt/webkitpy/nrwt-related bugs still open that relate to this, tracked here: https://bugs.webkit.org/show_bug.cgi?id=89880.