144360 2015-04-28 16:10:39 -0700 check-webkit-style fails due to system pylint 2015-05-01 17:25:21 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mario mario bfulgham cgarcia commit-queue dbates dino glenn mcatanzaro mrobinson ossy oldest_to_newest 1089779 0 mario 2015-04-28 16:10:39 -0700 Today I hit the following error when trying to run the Tools/Scripts/check-webkit-style script in my Fedora 21 box: $ Tools/Scripts/check-webkit-style -g HEAD Traceback (most recent call last): File "Tools/Scripts/check-webkit-style", line 44, in <module> from webkitpy.style.main import CheckWebKitStyle File "/home/mario/work/WebKit/Tools/Scripts/webkitpy/style/main.py", line 27, in <module> import webkitpy.style.checker as checker File "/home/mario/work/WebKit/Tools/Scripts/webkitpy/style/checker.py", line 51, in <module> from checkers.python import PythonChecker File "/home/mario/work/WebKit/Tools/Scripts/webkitpy/style/checkers/python.py", line 31, in <module> from webkitpy.thirdparty.autoinstalled.pylint import lint File "/home/mario/work/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 52, in <module> from pylint.interfaces import ILinter, IRawChecker, IASTNGChecker ImportError: cannot import name ILinter After some investigation I finally found out that simply removing the pylint module would do the trick: $ sudo dnf remove pylint # If installed from an rpm package or $ sudo pip uninstall pylint # If installed via pip Apparently, the problem is that the thirdparty/autoinstalled/pylint/lint.py file is trying to import the ILinter class from the pylint.interfaces module, which is provided by the thirdparty/autoinstalled/pylint/interfaces.py file as part of pylint 0.25.1. But pylint is also installed in the system under /usr/lib/python2.7, which is picked instead of the one inside thirdparty/autoinstalled... and this other one is a fairly different version (1.3.1 instead of 0.25.1) which, among other things, does not provide an ILinter class, causing the conflict. So, there should be a way to prevent developers from having this type of conflicts, which can be quite confusing, or at least warning them in some way, so that they can have a working environment out of the box. 1089951 1 251942 mario 2015-04-29 03:45:42 -0700 Created attachment 251942 Patch proposal Updating the PYTHONPATH to make sure the third party modules are always found first also 'fixes' the issue, so proposing a small patch that would do just that. Please review, thanks 1089985 2 mario 2015-04-29 07:47:42 -0700 Adding some potential reviewers to CC 1090829 3 251942 dbates 2015-05-01 10:56:30 -0700 Comment on attachment 251942 Patch proposal This doesn't seem like the correct place to put this functionality. Currently we explicitly append the directory thirdparty/autoinstalled to the end of sys.path when installing pylint by <http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/thirdparty/__init__.py?rev=181145#L107> and <http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/thirdparty/__init__.py?rev=181145#L71>. 1090830 4 dbates 2015-05-01 10:59:33 -0700 On another note, how much effort would it be to make our Python code compatible with the latest pylint and then have autoinstall install the latest pylint? 1090885 5 mario 2015-05-01 14:42:27 -0700 (In reply to comment #3) > Comment on attachment 251942 [details] > Patch proposal > > This doesn't seem like the correct place to put this functionality. > Currently we explicitly append the directory thirdparty/autoinstalled to the > end of sys.path when installing pylint by > <http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/thirdparty/ > __init__.py?rev=181145#L107> and > <http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/thirdparty/ > __init__.py?rev=181145#L71>. Ah! I was looking for such a place and could not find it for some reason, thanks for pointing it out! Unfortunately, the problem is precisely that, being at the end of sys.path instead of the beginning, is what's causing the confusion: the checker tries to import ILinter as referenced from third_party/autoinstalled, but then it finds it outside (in the system), which is a very different version not providing that class, and fails. So, I think it has to be at the beginning of sys.path, so that everything we use from inside third_party/autoinstalled will look first in the same place, before falling back to the system paths. For the record, I've reverted my previously proposed patch and applied the following one instead: diff --git a/Tools/Scripts/webkitpy/thirdparty/__init__.py b/Tools/Scripts/webkitpy/thirdparty/__init__.py index 7e2e4f3..c3baf71 100644 --- a/Tools/Scripts/webkitpy/thirdparty/__init__.py +++ b/Tools/Scripts/webkitpy/thirdparty/__init__.py @@ -68,7 +68,7 @@ class AutoinstallImportHook(object): def _ensure_autoinstalled_dir_is_in_sys_path(self): # Some packages require that the are being put somewhere under a directory in sys.path. if not _AUTOINSTALLED_DIR in sys.path: - sys.path.append(_AUTOINSTALLED_DIR) + sys.path.insert(0, _AUTOINSTALLED_DIR) def find_module(self, fullname, _): ...and that WORKS fine again. Do you think that patch could be acceptable? If so, I'll update the previous one and ask for review right away. Thanks (In reply to comment #4) > On another note, how much effort would it be to make our Python code > compatible with the latest pylint and then have autoinstall install the > latest pylint? My understanding of python is relatively basic, and no idea about pylint, so I don't think I can answer this. That's why I focused on getting what we already have to work, but perhaps you're right and, if feasible, that's a better approach. In any case, I think I solution to this issue would be good, because is very confusing when it happens, and I know I'm not alone (at least another person in #webkitgtk mentioned this recently). 1090932 6 252182 mario 2015-05-01 15:50:46 -0700 Created attachment 252182 Patch proposal As I will have to leave in a few minutes, and I'm not sure I will be able to devote time to WebKit next week, I'm attaching a new patch simply prepending the path instead of appending it, in case it's good enough so that it can be integrated. Please review, thanks! 1090993 7 252182 commit-queue 2015-05-01 17:25:16 -0700 Comment on attachment 252182 Patch proposal Clearing flags on attachment: 252182 Committed r183701: <http://trac.webkit.org/changeset/183701> 1090994 8 commit-queue 2015-05-01 17:25:21 -0700 All reviewed patches have been landed. Closing bug. 251942 2015-04-29 03:45:42 -0700 2015-05-01 15:50:39 -0700 Patch proposal bug-144360-20150429114431.patch text/plain 3892 mario U3VidmVyc2lvbiBSZXZpc2lvbjogMTgzNDkzCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggN2JiZDAyMDlkOWQyMGU3YzU3NTg5MGJhOGNlMTJiNWRh NWI5ZWNhZS4uM2FhNzg2ZTYzZjgyZWFiOWE3MjljN2Q3NzZjZmI5YWE3N2I1Mzk3MyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4 IEBACisyMDE1LTA0LTI5ICBNYXJpbyBTYW5jaGV6IFByYWRhICA8bWFyaW9AZW5kbGVzc20uY29t PgorCisgICAgICAgIGNoZWNrLXdlYmtpdC1zdHlsZSBmYWlscyBkdWUgdG8gc3lzdGVtIHB5bGlu dAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ0MzYw CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTWFrZSBz dXJlIHRoYXQgdGhlIGRpcmVjdG9yeSBjb250YWluaW5nIHRoaXJkIHBhcnR5IGF1dG9pbnN0YWxs ZWQgbW9kdWxlcworICAgICAgICBpcyBhdCB0aGUgYmVnaW5uaW5nIG9mIHRoZSBzZWFyY2ggcGF0 aCwgdG8gYXZvaWQgY29uZmxpY3RzIHdpdGggb3RoZXIKKyAgICAgICAgdmVyc2lvbnMgb2YgdGhl IHNhbWUgbW9kdWxlcyBpbnN0YWxsZWQgaW4gdGhlIHN5c3RlbSAoZS5nLiBweWxpbnQpLgorCisg ICAgICAgICogU2NyaXB0cy93ZWJraXRweS9zdHlsZS9wYXRoX2NoZWNrLnB5OiBBZGRlZCwgY2hl Y2tzIHRoZSBjdXJyZW50IHNlYXJjaAorICAgICAgICAgIHBhdGggYW5kIHByZXBlbmRzIHRoZSBv bmUgZm9yIDNyZCBwYXJ0eSBpbnN0YWxsZWQgbW9kdWxlcyBpZiBuZWVkZWQuCisgICAgICAgICog U2NyaXB0cy9jaGVjay13ZWJraXQtc3R5bGU6IEp1c3QgaW1wb3J0IHRoZSBuZXcgbW9kdWxlIGJl Zm9yZSB0aGUgY2hlY2tlci4KKwogMjAxNS0wNC0yOCAgTWFyaW8gU2FuY2hleiBQcmFkYSAgPG1h cmlvQGVuZGxlc3NtLmNvbT4KIAogICAgICAgICBbR1RLXSBDcmFzaCBpbiBXZWJQcm9jZXNzIHdo ZW4gbG9hZGluZyBsYXJnZSBjb250ZW50IHdpdGggY3VzdG9tIFVSSSBzY2hlbWVzCmRpZmYgLS1n aXQgYS9Ub29scy9TY3JpcHRzL2NoZWNrLXdlYmtpdC1zdHlsZSBiL1Rvb2xzL1NjcmlwdHMvY2hl Y2std2Via2l0LXN0eWxlCmluZGV4IDU0Y2EyNzZlYWQxYWU2MzQ5ODliMzFmZmZkMTY1ZWZlNWRh Y2QyZDAuLjA5MzdlMTRkODNmNDkxNTdmOGJkZjMzZjlmMmQ1NDk2MDY1ZjkzYzIgMTAwNzU1Ci0t LSBhL1Rvb2xzL1NjcmlwdHMvY2hlY2std2Via2l0LXN0eWxlCisrKyBiL1Rvb2xzL1NjcmlwdHMv Y2hlY2std2Via2l0LXN0eWxlCkBAIC00MCw2ICs0MCw3IEBAIGZpbmQgaXMgbGVnaXRpbWF0ZWx5 IGEgcHJvYmxlbS4iIiIKIGltcG9ydCBzeXMKIAogaW1wb3J0IHdlYmtpdHB5LmNvbW1vbi52ZXJz aW9uX2NoZWNrCitpbXBvcnQgd2Via2l0cHkuc3R5bGUucGF0aF9jaGVjawogCiBmcm9tIHdlYmtp dHB5LnN0eWxlLm1haW4gaW1wb3J0IENoZWNrV2ViS2l0U3R5bGUKIApkaWZmIC0tZ2l0IGEvVG9v bHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9wYXRoX2NoZWNrLnB5IGIvVG9vbHMvU2NyaXB0cy93 ZWJraXRweS9zdHlsZS9wYXRoX2NoZWNrLnB5Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLjY3YmU5Njg0NGZjZjRmZThh ZDZmMzVjYmZhYjBkYWFiZDEzNWI4YmYKLS0tIC9kZXYvbnVsbAorKysgYi9Ub29scy9TY3JpcHRz L3dlYmtpdHB5L3N0eWxlL3BhdGhfY2hlY2sucHkKQEAgLTAsMCArMSwzOSBAQAorIyBDb3B5cmln aHQgKGMpIDIwMTUgRW5kbGVzcyBNb2JpbGUsIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KKyMK KyMgUmVkaXN0cmlidXRpb24gYW5kIHVzZSBpbiBzb3VyY2UgYW5kIGJpbmFyeSBmb3Jtcywgd2l0 aCBvciB3aXRob3V0CisjIG1vZGlmaWNhdGlvbiwgYXJlIHBlcm1pdHRlZCBwcm92aWRlZCB0aGF0 IHRoZSBmb2xsb3dpbmcgY29uZGl0aW9ucyBhcmUKKyMgbWV0OgorIworIyAgICAgKiBSZWRpc3Ry aWJ1dGlvbnMgb2Ygc291cmNlIGNvZGUgbXVzdCByZXRhaW4gdGhlIGFib3ZlIGNvcHlyaWdodAor IyBub3RpY2UsIHRoaXMgbGlzdCBvZiBjb25kaXRpb25zIGFuZCB0aGUgZm9sbG93aW5nIGRpc2Ns YWltZXIuCisjICAgICAqIFJlZGlzdHJpYnV0aW9ucyBpbiBiaW5hcnkgZm9ybSBtdXN0IHJlcHJv ZHVjZSB0aGUgYWJvdmUKKyMgY29weXJpZ2h0IG5vdGljZSwgdGhpcyBsaXN0IG9mIGNvbmRpdGlv bnMgYW5kIHRoZSBmb2xsb3dpbmcgZGlzY2xhaW1lcgorIyBpbiB0aGUgZG9jdW1lbnRhdGlvbiBh bmQvb3Igb3RoZXIgbWF0ZXJpYWxzIHByb3ZpZGVkIHdpdGggdGhlCisjIGRpc3RyaWJ1dGlvbi4K KyMgICAgICogTmVpdGhlciB0aGUgbmFtZSBvZiBHb29nbGUgSW5jLiBub3IgdGhlIG5hbWVzIG9m IGl0cworIyBjb250cmlidXRvcnMgbWF5IGJlIHVzZWQgdG8gZW5kb3JzZSBvciBwcm9tb3RlIHBy b2R1Y3RzIGRlcml2ZWQgZnJvbQorIyB0aGlzIHNvZnR3YXJlIHdpdGhvdXQgc3BlY2lmaWMgcHJp b3Igd3JpdHRlbiBwZXJtaXNzaW9uLgorIworIyBUSElTIFNPRlRXQVJFIElTIFBST1ZJREVEIEJZ IFRIRSBDT1BZUklHSFQgSE9MREVSUyBBTkQgQ09OVFJJQlVUT1JTCisjICJBUyBJUyIgQU5EIEFO WSBFWFBSRVNTIE9SIElNUExJRUQgV0FSUkFOVElFUywgSU5DTFVESU5HLCBCVVQgTk9UCisjIExJ TUlURUQgVE8sIFRIRSBJTVBMSUVEIFdBUlJBTlRJRVMgT0YgTUVSQ0hBTlRBQklMSVRZIEFORCBG SVRORVNTIEZPUgorIyBBIFBBUlRJQ1VMQVIgUFVSUE9TRSBBUkUgRElTQ0xBSU1FRC4gSU4gTk8g RVZFTlQgU0hBTEwgVEhFIENPUFlSSUdIVAorIyBPV05FUiBPUiBDT05UUklCVVRPUlMgQkUgTElB QkxFIEZPUiBBTlkgRElSRUNULCBJTkRJUkVDVCwgSU5DSURFTlRBTCwKKyMgU1BFQ0lBTCwgRVhF TVBMQVJZLCBPUiBDT05TRVFVRU5USUFMIERBTUFHRVMgKElOQ0xVRElORywgQlVUIE5PVAorIyBM SU1JVEVEIFRPLCBQUk9DVVJFTUVOVCBPRiBTVUJTVElUVVRFIEdPT0RTIE9SIFNFUlZJQ0VTOyBM T1NTIE9GIFVTRSwKKyMgREFUQSwgT1IgUFJPRklUUzsgT1IgQlVTSU5FU1MgSU5URVJSVVBUSU9O KSBIT1dFVkVSIENBVVNFRCBBTkQgT04gQU5ZCisjIFRIRU9SWSBPRiBMSUFCSUxJVFksIFdIRVRI RVIgSU4gQ09OVFJBQ1QsIFNUUklDVCBMSUFCSUxJVFksIE9SIFRPUlQKKyMgKElOQ0xVRElORyBO RUdMSUdFTkNFIE9SIE9USEVSV0lTRSkgQVJJU0lORyBJTiBBTlkgV0FZIE9VVCBPRiBUSEUgVVNF CisjIE9GIFRISVMgU09GVFdBUkUsIEVWRU4gSUYgQURWSVNFRCBPRiBUSEUgUE9TU0lCSUxJVFkg T0YgU1VDSCBEQU1BR0UuCisKK2ltcG9ydCBvcworaW1wb3J0IHN5cworCitzY3JpcHRzX3BhdGgg PSBvcy5wYXRoLmRpcm5hbWUob3MucGF0aC5kaXJuYW1lKG9zLnBhdGguYWJzcGF0aChfX2ZpbGVf XykpKQorYXV0b2luc3RhbGxlZF9tb2R1bGVzX3BhdGggPSBvcy5wYXRoLmpvaW4oc2NyaXB0c19w YXRoLCAidGhpcmRwYXJ0eS9hdXRvaW5zdGFsbGVkIikKKworIyBNYWtlIHN1cmUgdGhlIGF1dG9p bnN0YWxsZWQgbW9kdWxlcyBhcmUgZm91bmQgYmVmb3JlIHN5c3RlbSBtb2R1bGVzLCBzbworIyB0 aGF0IHdlIGF2b2lkIGNvbmZsaWN0cyBiZXR3ZWVuIG1pc21hdGNoaW5nIG1vZHVsZSB2ZXJzaW9u cyAoZS5nLiBweWxpbnQpCisjIFNlZSBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTQ0MzYwIGZvciBtb3JlIGRldGFpbHMuCitpZiBhdXRvaW5zdGFsbGVkX21vZHVsZXNf cGF0aCAhPSBzeXMucGF0aFswXToKKyAgICBzeXMucGF0aC5pbnNlcnQoMCwgYXV0b2luc3RhbGxl ZF9tb2R1bGVzX3BhdGgpCg== 252182 2015-05-01 15:50:46 -0700 2015-05-01 17:25:16 -0700 Patch proposal bug-144360-20150501234934.patch text/plain 1739 mario U3VidmVyc2lvbiBSZXZpc2lvbjogMTgzNDkzCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZDQ3MjdjZGVkN2U5MWIwOGVkNGQ0NjdkMzRhNDcwNDg5 ZjA1NzUwNS4uM2QwZmE1ZGE0YTFiMmM4N2FhNGVkNDIzMjUzNTk3YzQwNWY4ZGU0ZCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3 IEBACisyMDE1LTA1LTAxICBNYXJpbyBTYW5jaGV6IFByYWRhICA8bWFyaW9AZW5kbGVzc20uY29t PgorCisgICAgICAgIGNoZWNrLXdlYmtpdC1zdHlsZSBmYWlscyBkdWUgdG8gc3lzdGVtIHB5bGlu dAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ0MzYw CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTWFrZSBz dXJlIHRoYXQgdGhlIGRpcmVjdG9yeSBjb250YWluaW5nIHRoaXJkIHBhcnR5IGF1dG9pbnN0YWxs ZWQgbW9kdWxlcworICAgICAgICBpcyBhdCB0aGUgYmVnaW5uaW5nIG9mIHRoZSBzZWFyY2ggcGF0 aCwgdG8gYXZvaWQgY29uZmxpY3RzIHdpdGggb3RoZXIKKyAgICAgICAgdmVyc2lvbnMgb2YgdGhl IHNhbWUgbW9kdWxlcyBpbnN0YWxsZWQgaW4gdGhlIHN5c3RlbSAoZS5nLiBweWxpbnQpLgorCisg ICAgICAgICogU2NyaXB0cy93ZWJraXRweS90aGlyZHBhcnR5L19faW5pdF9fLnB5OiBQcmVwZW5k IHRoZSBhdXRvaW5zdGFsbGVkCisgICAgICAgIG1vZHVsZXMgZGlyZWN0b3J5IHRvIHN5cy5wYXRo LCBpbnN0ZWFkIG9mIGFwcGVuZGluZyBpdC4KKwogMjAxNS0wNC0yOCAgQWxleGV5IFByb3NrdXJ5 YWtvdiAgPGFwQGFwcGxlLmNvbT4KIAogICAgICAgICBEaXNhYmxlIEFwcCBOYXAgaW4gV2ViS2l0 VGVzdFJ1bm5lciBVSSBwcm9jZXNzCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5 L3RoaXJkcGFydHkvX19pbml0X18ucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3RoaXJkcGFy dHkvX19pbml0X18ucHkKaW5kZXggN2UyZTRmM2ZmYzk0YmEyY2JiMmYyMjUwMDAyMDAwOWVlNGY3 YjQ4YS4uYzNiYWY3MTFhMWVlMzNiNWE3MjViNDQwYjI2MGIyNzZiZTIyNTNmZSAxMDA2NDQKLS0t IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS90aGlyZHBhcnR5L19faW5pdF9fLnB5CisrKyBiL1Rv b2xzL1NjcmlwdHMvd2Via2l0cHkvdGhpcmRwYXJ0eS9fX2luaXRfXy5weQpAQCAtNjgsNyArNjgs NyBAQCBjbGFzcyBBdXRvaW5zdGFsbEltcG9ydEhvb2sob2JqZWN0KToKICAgICBkZWYgX2Vuc3Vy ZV9hdXRvaW5zdGFsbGVkX2Rpcl9pc19pbl9zeXNfcGF0aChzZWxmKToKICAgICAgICAgIyBTb21l IHBhY2thZ2VzIHJlcXVpcmUgdGhhdCB0aGUgYXJlIGJlaW5nIHB1dCBzb21ld2hlcmUgdW5kZXIg YSBkaXJlY3RvcnkgaW4gc3lzLnBhdGguCiAgICAgICAgIGlmIG5vdCBfQVVUT0lOU1RBTExFRF9E SVIgaW4gc3lzLnBhdGg6Ci0gICAgICAgICAgICBzeXMucGF0aC5hcHBlbmQoX0FVVE9JTlNUQUxM RURfRElSKQorICAgICAgICAgICAgc3lzLnBhdGguaW5zZXJ0KDAsIF9BVVRPSU5TVEFMTEVEX0RJ UikKIAogICAgIGRlZiBmaW5kX21vZHVsZShzZWxmLCBmdWxsbmFtZSwgXyk6CiAgICAgICAgICMg VGhpcyBtZXRob2Qgd2lsbCBydW4gYmVmb3JlIGVhY2ggaW1wb3J0LiBTZWUgaHR0cDovL3d3dy5w eXRob24ub3JnL2Rldi9wZXBzL3BlcC0wMzAyLwo=