98140 2012-10-02 03:46:15 -0700 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 2012-10-02 15:58:17 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P1 Critical --- 1 ossy tony abarth dpranke eric levin michael.bruning ojan ossy rafael.lobo tony webkit.review.bot oldest_to_newest 732489 0 ossy 2012-10-02 03:46:15 -0700 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? 732508 1 michael.bruning 2012-10-02 04:15:56 -0700 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). 732509 2 ossy 2012-10-02 04:17:20 -0700 (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. 732519 3 ossy 2012-10-02 04:25:06 -0700 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. :-/ 732529 4 ossy 2012-10-02 04:30:29 -0700 It seems it handles only qt and qt-linux ... :-/ 733087 5 166752 tony 2012-10-02 14:37:56 -0700 Created attachment 166752 Patch 733090 6 tony 2012-10-02 14:40:10 -0700 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). 733091 7 166752 eric 2012-10-02 14:41:09 -0700 Comment on attachment 166752 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. :) 733093 8 eric 2012-10-02 14:42:32 -0700 (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. 733103 9 rafael.lobo 2012-10-02 14:52:56 -0700 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? 733124 10 tony 2012-10-02 15:09:50 -0700 (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(). 733144 11 166752 webkit.review.bot 2012-10-02 15:29:29 -0700 Comment on attachment 166752 Patch Clearing flags on attachment: 166752 Committed r130220: <http://trac.webkit.org/changeset/130220> 733145 12 webkit.review.bot 2012-10-02 15:29:33 -0700 All reviewed patches have been landed. Closing bug. 733165 13 dpranke 2012-10-02 15:58:17 -0700 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. 166752 2012-10-02 14:37:56 -0700 2012-10-02 15:29:29 -0700 Patch bug-98140-20121002143709.patch text/plain 4355 tony U3VidmVyc2lvbiBSZXZpc2lvbjogMTMwMTgxCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMmY3NzBhOTMwODhjMDYwZDU3ZjI4MmM2ZWM4OGYwMDNl Mjk0ZTY4MS4uMTA0MGE1NmViYjUwZTk1OGNmNTgwOGRhYmZmMWQzYWYxMjI0MTFhZiAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1 IEBACisyMDEyLTEwLTAyICBUb255IENoYW5nICA8dG9ueUBjaHJvbWl1bS5vcmc+CisKKyAgICAg ICAgY2hlY2std2Via2l0LXN0eWxlIGNhbid0IGhhbmRsZSBxdC1hcm0sIHF0LXdpbiwgcXQtbWFj LCBxdC01LjAsIHF0LTUuMC13azEgYW5kIHF0LTUuMC13azIgVGVzdEV4cGVjYXRhdGlvbnMKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk4MTQwCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4IGEgYnVnIHdo ZXJlIHdlIHdlcmUgYWx3YXlzIHVzaW5nIHRoZSBob3N0IE9TIHJhdGhlciB0aGFuIHVzaW5nIHdo YXQgd2FzIHBhc3NlZCBpbiBhcyBwYXJ0IG9mIHRoZQorICAgICAgICBwb3J0IG5hbWUgb24gUXQu CisKKyAgICAgICAgVGhlcmUncyBhbHNvIGEgYnVnIHdpdGggYmVpbmcgYWJsZSB0byB0ZXN0IGZv ciBxdC01LjAgZXhwZWN0YXRpb25zIGZpbGVzIHdoZW4gb24gYSBzeXN0ZW0gd2l0aAorICAgICAg ICBRdCA0LjggaW5zdGFsbGVkLiBJJ20gbm90IHN1cmUgaG93IHRvIHNvbHZlIHRoYXQsIHNvIGZv ciBub3csIHN0b3AgZW1pdHRpbmcgYSBzdHlsZSB3YXJuaW5nIGFib3V0CisgICAgICAgIG5vdCBj aGVja2luZyBhIGZpbGUuCisKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0 cy9wb3J0L3F0LnB5OgorICAgICAgICAoUXRQb3J0Ll9zZWFyY2hfcGF0aHMpOiBVc2UgcGFzc2Vk IGluIE9TLgorICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvdGVzdF9l eHBlY3RhdGlvbnMucHk6CisgICAgICAgIChUZXN0RXhwZWN0YXRpb25zQ2hlY2tlci5jaGVjayk6 IERvbid0IHdhcm4gaWYgd2UgZG9uJ3Qga25vdyBhYm91dCB0aGUgVGVzdEV4cGVjdGF0aW9ucyBm aWxlLgorICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvdGVzdF9leHBl Y3RhdGlvbnNfdW5pdHRlc3QucHk6CisgICAgICAgIChUZXN0RXhwZWN0YXRpb25zVGVzdENhc2Uu dGVzdF9kZXRlcm1pbmVfcG9ydF9mcm9tX2V4cGVjdGF0aW9uc19wYXRoKTogQWRkZWQgYSB0ZXN0 IGNhc2UgZm9yIHRoZSBPUyBmaXguCisgICAgICAgIEFkZCBhIGNvbW1lbnRlZCBvdXQgdGVzdCBm b3IgdGhlIHZlcnNpb24gY2FzZS4KKwogMjAxMi0xMC0wMiAgRG91Z2xhcyBTdG9ja3dlbGwgIDxk c3RvY2t3ZWxsQGNocm9taXVtLm9yZz4KIAogICAgICAgICBbY2hyb21pdW1dIHJ1bm5pbmcgbGF5 b3V0IHRlc3RzIG9uIG1vdW50YWlubGlvbiBzaG91bGQgdXNlIGNocm9taXVtIFRlc3RFeHBlY3Rh dGlvbnMKZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3Bv cnQvcXQucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L3F0LnB5 CmluZGV4IDJlNzNjNzVhODY3MWM1MThjODE0NjRiMWI1YTM4OTczNjY1YTA2NjIuLjQ2ZWU5NWQz OGViNDMyMmJmNGYwYTY5YjViZWE1ZTQ0OGM5ZjZlYjUgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1Njcmlw dHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvcXQucHkKKysrIGIvVG9vbHMvU2NyaXB0cy93 ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9xdC5weQpAQCAtMTM1LDcgKzEzNSw3IEBAIGNsYXNz IFF0UG9ydChQb3J0KToKICAgICAgICAgICAgIHNlYXJjaF9wYXRocy5hcHBlbmQoJ3F0LTQuOCcp CiAgICAgICAgIGVsaWYgdmVyc2lvbjoKICAgICAgICAgICAgIHNlYXJjaF9wYXRocy5hcHBlbmQo J3F0LTUuMCcpCi0gICAgICAgIHNlYXJjaF9wYXRocy5hcHBlbmQoc2VsZi5wb3J0X25hbWUgKyAn LScgKyBzZWxmLmhvc3QucGxhdGZvcm0ub3NfbmFtZSkKKyAgICAgICAgc2VhcmNoX3BhdGhzLmFw cGVuZChzZWxmLnBvcnRfbmFtZSArICctJyArIHNlbGYub3BlcmF0aW5nX3N5c3RlbSgpKQogICAg ICAgICBzZWFyY2hfcGF0aHMuYXBwZW5kKHNlbGYucG9ydF9uYW1lKQogICAgICAgICByZXR1cm4g c2VhcmNoX3BhdGhzCiAKZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvc3R5bGUv Y2hlY2tlcnMvdGVzdF9leHBlY3RhdGlvbnMucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0 eWxlL2NoZWNrZXJzL3Rlc3RfZXhwZWN0YXRpb25zLnB5CmluZGV4IDQ2NDAzYjdkYjllODZiZmUx ZmFhMzA3YWVlMzE2NGM4ZDhhMWUzMTkuLmFlOGZlYWZkZjQ5NGRmZDc5OGY0YTZlNTU1YjVjZjEw NzkxYzQ1NWQgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tl cnMvdGVzdF9leHBlY3RhdGlvbnMucHkKKysrIGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHls ZS9jaGVja2Vycy90ZXN0X2V4cGVjdGF0aW9ucy5weQpAQCAtOTQsOCArOTQsNiBAQCBjbGFzcyBU ZXN0RXhwZWN0YXRpb25zQ2hlY2tlcihvYmplY3QpOgogICAgICAgICBleHBlY3RhdGlvbnMgPSAn XG4nLmpvaW4obGluZXMpCiAgICAgICAgIGlmIHNlbGYuX3BvcnRfb2JqOgogICAgICAgICAgICAg c2VsZi5jaGVja190ZXN0X2V4cGVjdGF0aW9ucyhleHBlY3RhdGlvbnNfc3RyPWV4cGVjdGF0aW9u cywgdGVzdHM9Tm9uZSkKLSAgICAgICAgZWxzZToKLSAgICAgICAgICAgIHNlbGYuX2hhbmRsZV9z dHlsZV9lcnJvcigxLCAndGVzdC9leHBlY3RhdGlvbnMnLCA1LAotICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICdObyBwb3J0IHVzZXMgcGF0aCAlcyBmb3IgdGVzdF9leHBlY3Rh dGlvbnMnICUgc2VsZi5fZmlsZV9wYXRoKQorCiAgICAgICAgICMgV2FybiB0YWJzIGluIGxpbmVz IGFzIHdlbGwKICAgICAgICAgc2VsZi5jaGVja190YWJzKGxpbmVzKQpkaWZmIC0tZ2l0IGEvVG9v bHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy90ZXN0X2V4cGVjdGF0aW9uc191bml0 dGVzdC5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvdGVzdF9leHBl Y3RhdGlvbnNfdW5pdHRlc3QucHkKaW5kZXggZjEyMzk3Nzg3YzM0NDFlMGI2NTFmMWNmNmNiZTJh NDQ2ZWFhNGRmZC4uMTUxNmRlNzk3YTRkYzg2MjliN2VkYmE0Y2I5NzFjN2E1Y2JiNDc1ZSAxMDA2 NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy90ZXN0X2V4cGVj dGF0aW9uc191bml0dGVzdC5weQorKysgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2No ZWNrZXJzL3Rlc3RfZXhwZWN0YXRpb25zX3VuaXR0ZXN0LnB5CkBAIC04Miw2ICs4MiwxMCBAQCBj bGFzcyBUZXN0RXhwZWN0YXRpb25zVGVzdENhc2UodW5pdHRlc3QuVGVzdENhc2UpOgogICAgICAg ICBzZWxmLl9leHBlY3RfcG9ydF9mb3JfZXhwZWN0YXRpb25zX3BhdGgoJ2VmbCcsICdMYXlvdXRU ZXN0cy9wbGF0Zm9ybS9lZmwvVGVzdEV4cGVjdGF0aW9ucycpCiAgICAgICAgIHNlbGYuX2V4cGVj dF9wb3J0X2Zvcl9leHBlY3RhdGlvbnNfcGF0aCgnZWZsJywgJ0xheW91dFRlc3RzL3BsYXRmb3Jt L2VmbC13azEvVGVzdEV4cGVjdGF0aW9ucycpCiAgICAgICAgIHNlbGYuX2V4cGVjdF9wb3J0X2Zv cl9leHBlY3RhdGlvbnNfcGF0aCgnZWZsJywgJ0xheW91dFRlc3RzL3BsYXRmb3JtL2VmbC13azIv VGVzdEV4cGVjdGF0aW9ucycpCisgICAgICAgIHNlbGYuX2V4cGVjdF9wb3J0X2Zvcl9leHBlY3Rh dGlvbnNfcGF0aCgncXQnLCAnTGF5b3V0VGVzdHMvcGxhdGZvcm0vcXQtd2luL1Rlc3RFeHBlY3Rh dGlvbnMnKQorICAgICAgICAjIEZJWE1FOiBjaGVjay13ZWJraXQtc3R5bGUgZG9lc24ndCBrbm93 IGhvdyB0byBjcmVhdGUgcG9ydCBvYmplY3RzIGZvciBhbGwgUXQgdmVyc2lvbiAoNC44LCA1LjAp IGFuZAorICAgICAgICAjIHdpbGwgb25seSBjaGVjayBmaWxlcyBiYXNlZCBvbiB0aGUgaW5zdGFs bGVkIHZlcnNpb24gb2YgUXQuCisgICAgICAgICNzZWxmLl9leHBlY3RfcG9ydF9mb3JfZXhwZWN0 YXRpb25zX3BhdGgoJ3F0JywgJ0xheW91dFRlc3RzL3BsYXRmb3JtL3F0LTUuMC13azIvVGVzdEV4 cGVjdGF0aW9ucycpCiAKICAgICBkZWYgYXNzZXJ0X2xpbmVzX2xpbnQoc2VsZiwgbGluZXMsIHNo b3VsZF9wYXNzLCBleHBlY3RlZF9vdXRwdXQ9Tm9uZSk6CiAgICAgICAgIHNlbGYuX2Vycm9yX2Nv bGxlY3Rvci5yZXNldF9lcnJvcnMoKQo=