136477 2014-09-03 02:01:45 -0700 [GTK] run-webkit-test check for jhBuild is not correct 2014-09-10 00:11:58 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Minor --- 1 fvallee webkit-unassigned pnormand oldest_to_newest 1032867 0 fvallee 2014-09-03 02:01:45 -0700 Tools/Scripts/webkitpy/port/gtk.py is checking if tests must be run within jhBuild wrapper. If webkit/WebKitBuild/Dependencies exists, tests are run using jhBuild wrapper - it works fine using default configuration, however it has 2 issues: 1) WebKitBuild/Dependencies exists even if jhBuild is not used, because it is needed for fonts (as you can see in getFontsPath() method from http://trac.webkit.org/browser/trunk/Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp) So a build using default WebKitBuild folder but NOT using jhBuild will be using jhBuild wrapper anyway (and jhBuild checkout during tests mess up everything). 2) build folder (default is webkit/WebKitBuild) can be overridden using WEBKIT_OUTPUTDIR env variable. In that case jhBuild is never detected and regression tests will fail (tests are using system libs instead of jhBuild) 1032869 1 237551 fvallee 2014-09-03 02:29:56 -0700 Created attachment 237551 Patch 1032872 2 pnormand 2014-09-03 03:06:30 -0700 (In reply to comment #0) > Tools/Scripts/webkitpy/port/gtk.py is checking if tests must be run within jhBuild wrapper. > If webkit/WebKitBuild/Dependencies exists, tests are run using jhBuild wrapper - it works fine using default configuration, however it has 2 issues: > > 1) WebKitBuild/Dependencies exists even if jhBuild is not used, because it is needed for fonts (as you can see in getFontsPath() method from http://trac.webkit.org/browser/trunk/Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp) > So a build using default WebKitBuild folder but NOT using jhBuild will be using jhBuild wrapper anyway (and jhBuild checkout during tests mess up everything). > But if you set WEBKIT_OUTPUTDIR and don't have Dependencies/ that function will look for fonts in WEBKIT_OUTPUTDIR, or am I misunderstanding that code? 1032886 3 fvallee 2014-09-03 05:14:49 -0700 You are 100% right. Fonts don't need to be in WebKitBuild/Dependencies anymore, so please forget 1). (sorry for that. Got that issue long time ago because DumpRenderTree was printing error message "Could not locate test fonts at $WEBKIT_TOP_LEVEL/WebKitBuild/Dependencies/Root/webkitgtk-test-fonts", but WebKitTestRunner works fine w/ fonts directly in $WEBKIT_TOP_LEVEL/WebKitBuild or in $WEBKIT_OUTPUTDIR). I will upload a new patch (first was not working anyway) for review with the WEBKIT_OUTPUTDIR check in gtk.py. 1032887 4 fvallee 2014-09-03 05:28:41 -0700 bug can be closed. Issue has been fixed already http://trac.webkit.org/changeset/172830 (os.path.exists(self.path_from_webkit_base('WebKitBuild', 'Dependencies')): has been replaced by self._should_use_jhbuild()) 1034236 5 237551 pnormand 2014-09-10 00:11:58 -0700 Comment on attachment 237551 Patch Removing patch from review queue 237551 2014-09-03 02:29:56 -0700 2014-09-10 00:11:57 -0700 Patch bug-136477-20140903112950.patch text/plain 2794 fvallee ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCA3NzRm OWY5NjFjOGU3ZWI1MzNlN2NjZjMxZDc5OWZkMmY2YTliOTI2Li5iY2JlMzM1ZjIyNGY5M2FiNWM5 YjljNmRhNGI4YzVjNjQ3NTZjNzk3IDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIv VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTQtMDktMDMgIEZhYmllbiBWYWxs w6llICA8ZnZhbGxlZUBjb25uZWN0ZWQtbGFicy5jb20+CisKKyAgICAgICAgW0dUS10gcnVuLXdl YmtpdC10ZXN0IGNoZWNrIGZvciBqaEJ1aWxkIGlzIG5vdCBjb3JyZWN0CisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzY0NzcKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDaGVjayBXRUJLSVRfT1VUUFVURElS IGVudiB2YXJpYWJsZSB0byBmaW5kIFdlYktpdEJ1aWxkCisgICAgICAgIGZvbGRlciBhbmQgY2hl Y2sgamhCdWlsZCBjaGVja291dCBkaXIgdG8gZGV0ZWN0IGlmIGpoQnVpbGQKKyAgICAgICAgaXMg dXNlZCBvciBub3QKKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0ZGlycy5wbToKKyAgICAgICAg KGpoYnVpbGRXcmFwcGVyUHJlZml4SWZOZWVkZWQpOgorICAgICAgICAqIFNjcmlwdHMvd2Via2l0 cHkvcG9ydC9ndGsucHk6CisgICAgICAgIChHdGtQb3J0Ll9faW5pdF9fKToKKwogMjAxNC0wOC0x OSAgQWtvcyBLaXNzICA8YWtpc3NAaW5mLnUtc3plZ2VkLmh1PgogCiAgICAgICAgIExldCB0aGUg Y2FsbGVyIGRlZmluZSBKU0NfdGltZW91dCBmb3IgcnVuLWphdmFzY3JpcHRjb3JlLXRlc3RzCmRp ZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdGRpcnMucG0gYi9Ub29scy9TY3JpcHRzL3dl YmtpdGRpcnMucG0KaW5kZXggNTEyMTdjMjJjMWY3M2JkOGJlOGRiY2M2OTlhYmE5NDU4ZjFlMmUz OS4uYjI4ZDdiMmU0NDFlODFkNjUxMTNmMjZiY2YyYTQxYzcyMzQ4ZGY1OCAxMDA3NTUKLS0tIGEv VG9vbHMvU2NyaXB0cy93ZWJraXRkaXJzLnBtCisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0ZGly cy5wbQpAQCAtMTgxMSw3ICsxODExLDEwIEBAIHN1YiBpc0NhY2hlZEFyZ3VtZW50ZmlsZU91dE9m RGF0ZSgkQCkKIAogc3ViIGpoYnVpbGRXcmFwcGVyUHJlZml4SWZOZWVkZWQoKQogewotICAgIGlm ICgtZSBnZXRKaGJ1aWxkUGF0aCgpKSB7CisgICAgIyBjaGVjayBpZiBqaEJ1aWxkIGNoZWNrb3V0 IGRpciBleGlzdHMKKyAgICBteSBAamhidWlsZFBhdGggPSBnZXRKaGJ1aWxkUGF0aCgpOworICAg IHB1c2goQGpoYnVpbGRQYXRoLCAiU291cmNlIik7CisgICAgaWYgKC1lIEBqaGJ1aWxkUGF0aCkg ewogICAgICAgICBteSBAcHJlZml4ID0gKEZpbGU6OlNwZWMtPmNhdGZpbGUoc291cmNlRGlyKCks ICJUb29scyIsICJqaGJ1aWxkIiwgImpoYnVpbGQtd3JhcHBlciIpKTsKICAgICAgICAgaWYgKGlz RWZsKCkpIHsKICAgICAgICAgICAgIHB1c2goQHByZWZpeCwgIi0tZWZsIik7CmRpZmYgLS1naXQg YS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3BvcnQvZ3RrLnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJr aXRweS9wb3J0L2d0ay5weQppbmRleCBlODJkYjkwZWZhMjY2MmEwZGFkM2NlZmE0MzIwYzk3YTRi Mjc4YjdiLi44MjY1MzNiNGY4ODc0ODQ4NTNiNmFjMTJjYWViOGVhMTAzNDkxYzg4IDEwMDY0NAot LS0gYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3BvcnQvZ3RrLnB5CisrKyBiL1Rvb2xzL1Njcmlw dHMvd2Via2l0cHkvcG9ydC9ndGsucHkKQEAgLTUzLDggKzUzLDExIEBAIGNsYXNzIEd0a1BvcnQo UG9ydCk6CiAgICAgICAgICAgICBzZWxmLl9sZWFrZGV0ZWN0b3IgPSBMZWFrRGV0ZWN0b3JWYWxn cmluZChzZWxmLl9leGVjdXRpdmUsIHNlbGYuX2ZpbGVzeXN0ZW0sIHNlbGYucmVzdWx0c19kaXJl Y3RvcnkoKSkKICAgICAgICAgICAgIGlmIG5vdCBzZWxmLmdldF9vcHRpb24oIndyYXBwZXIiKToK ICAgICAgICAgICAgICAgICByYWlzZSBWYWx1ZUVycm9yKCd1c2UgLS13cmFwcGVyPVwidmFsZ3Jp bmRcIiBmb3IgbWVtb3J5IGxlYWsgZGV0ZWN0aW9uIG9uIEdUSycpCi0KLSAgICAgICAgaWYgb3Mu cGF0aC5leGlzdHMoc2VsZi5wYXRoX2Zyb21fd2Via2l0X2Jhc2UoJ1dlYktpdEJ1aWxkJywgJ0Rl cGVuZGVuY2llcycpKToKKyAgICAgICAgaWYgJ1dFQktJVF9PVVRQVVRESVInIGluIG9zLmVudmly b246CisgICAgICAgICAgICBqaEJ1aWxkQ2hlY2tvdXRSb290ID0gb3MucGF0aC5hYnNwYXRoKG9z LnBhdGguam9pbihvcy5lbnZpcm9uWydXRUJLSVRfT1VUUFVURElSJ10sICdEZXBlbmRlbmNpZXMn LCAnU291cmNlJykpCisgICAgICAgIGVsc2U6CisgICAgICAgICAgICBqaEJ1aWxkQ2hlY2tvdXRS b290ID0gc2VsZi5wYXRoX2Zyb21fd2Via2l0X2Jhc2UoJ1dlYktpdEJ1aWxkJywgJ0RlcGVuZGVu Y2llcycsICdTb3VyY2UnKQorICAgICAgICBpZiBvcy5wYXRoLmV4aXN0cyhqaEJ1aWxkQ2hlY2tv dXRSb290KToKICAgICAgICAgICAgIHNlbGYuX2poYnVpbGRfd3JhcHBlciA9IFtzZWxmLnBhdGhf ZnJvbV93ZWJraXRfYmFzZSgnVG9vbHMnLCAnamhidWlsZCcsICdqaGJ1aWxkLXdyYXBwZXInKSwg Jy0tZ3RrJywgJ3J1biddCiAgICAgICAgICAgICBpZiBzZWxmLmdldF9vcHRpb24oJ3dyYXBwZXIn KToKICAgICAgICAgICAgICAgICBzZWxmLnNldF9vcHRpb24oJ3dyYXBwZXInLCAnICcuam9pbihz ZWxmLl9qaGJ1aWxkX3dyYXBwZXIpICsgJyAnICsgc2VsZi5nZXRfb3B0aW9uKCd3cmFwcGVyJykp Cg==