203213 2019-10-21 14:52:12 -0700 Python 3: 2to3 script may not be in a user's path 2019-10-23 09:58:09 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=202462 InRadar P2 Normal --- 1 jbedard jbedard aakash_jain commit-queue dewei_zhu ews-watchlist glenn slewis webkit-bug-importer oldest_to_newest 1582059 0 jbedard 2019-10-21 14:52:12 -0700 In <https://trac.webkit.org/changeset/251112/webkit>, I assumed users would have 2to3 on their path. This was a bad assumption (I have a machine which doesn't, as an example). It's also unnecessary because 2to3 is part of the Python 3 standard library, we can leverage that and call it directly. 1582065 1 381464 jbedard 2019-10-21 15:03:34 -0700 Created attachment 381464 Patch 1582698 2 381464 dewei_zhu 2019-10-22 18:29:28 -0700 Comment on attachment 381464 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381464&action=review > Tools/Scripts/webkitpy/thirdparty/__init__.py:-221 > - self._executive.run_command(['2to3', '-w', self._fs.join(_AUTOINSTALLED_DIR, 'bs4')]) It may be cleaner if we invoke self._executive.run_command([os.path.join(os.path.dirname(lib2to3), "__main__.py"), '-w', self._fs.join(_AUTOINSTALLED_DIR, 'bs4')]) This way, we don't have to temporarily change sys.stdout and use mulitiprocessing module. 1582806 3 381464 jbedard 2019-10-23 07:59:57 -0700 Comment on attachment 381464 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381464&action=review >> Tools/Scripts/webkitpy/thirdparty/__init__.py:-221 >> - self._executive.run_command(['2to3', '-w', self._fs.join(_AUTOINSTALLED_DIR, 'bs4')]) > > It may be cleaner if we invoke self._executive.run_command([os.path.join(os.path.dirname(lib2to3), "__main__.py"), '-w', self._fs.join(_AUTOINSTALLED_DIR, 'bs4')]) > This way, we don't have to temporarily change sys.stdout and use mulitiprocessing module. Then we'd have to figure out where 2to3 is installed. I disliked this executive method in the first place, multiprocessing is a bit dirtier in this function, but I feel like it's cleaner for the caller. 1582821 4 381464 commit-queue 2019-10-23 08:43:03 -0700 Comment on attachment 381464 Patch Clearing flags on attachment: 381464 Committed r251478: <https://trac.webkit.org/changeset/251478> 1582822 5 commit-queue 2019-10-23 08:43:05 -0700 All reviewed patches have been landed. Closing bug. 1582825 6 webkit-bug-importer 2019-10-23 08:44:13 -0700 <rdar://problem/56540189> 1582853 7 381464 dewei_zhu 2019-10-23 09:58:09 -0700 Comment on attachment 381464 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381464&action=review >>> Tools/Scripts/webkitpy/thirdparty/__init__.py:-221 >>> - self._executive.run_command(['2to3', '-w', self._fs.join(_AUTOINSTALLED_DIR, 'bs4')]) >> >> It may be cleaner if we invoke self._executive.run_command([os.path.join(os.path.dirname(lib2to3), "__main__.py"), '-w', self._fs.join(_AUTOINSTALLED_DIR, 'bs4')]) >> This way, we don't have to temporarily change sys.stdout and use mulitiprocessing module. > > Then we'd have to figure out where 2to3 is installed. > > I disliked this executive method in the first place, multiprocessing is a bit dirtier in this function, but I feel like it's cleaner for the caller. We don't directly call the `2to3`, we still rely on `lib2to3` module as your initial change. Instead of calling binary '2to3' we invoke '__main__.py' file under root of lib2to3 module. 381464 2019-10-21 15:03:34 -0700 2019-10-23 08:43:03 -0700 Patch patch.patch text/plain 2761 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI1MTM4NSkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE1IEBACisyMDE5LTEwLTIxICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICBQeXRob24gMzogMnRvMyBzY3JpcHQgbWF5IG5vdCBiZSBpbiBhIHVzZXIn cyBwYXRoCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0y MDMyMTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IFNjcmlwdHMvd2Via2l0cHkvdGhpcmRwYXJ0eS9fX2luaXRfXy5weToKKyAgICAgICAgKEF1dG9p bnN0YWxsSW1wb3J0SG9vay5fX2luaXRfXyk6IFJlbW92ZSBleGVjdXRpdmUgZGVwZW5kZW5jaWVz LgorICAgICAgICAoQXV0b2luc3RhbGxJbXBvcnRIb29rLl9pbnN0YWxsX2JlYXV0aWZ1bHNvdXAp OiBVc2UgbXVsdGlwcm9jZXNzIGJlY2F1c2UgMnRvMyBzZXRzCisgICAgICAgIFNvbWUgdW5kZXNp cmFibGUgZ2xvYmFsIGxvZ2dpbmcgc3RhdGUuCisKIDIwMTktMTAtMjEgIEFsZXggQ2hyaXN0ZW5z ZW4gIDxhY2hyaXN0ZW5zZW5Ad2Via2l0Lm9yZz4KIAogICAgICAgICBTZXJ2aWNlV29ya2VyIHRl c3RzIHNob3VsZCB1c2UgVENQU2VydmVyIGluc3RlYWQgb2YgV0tVUkxTY2hlbWVIYW5kbGVyCklu ZGV4OiBUb29scy9TY3JpcHRzL3dlYmtpdHB5L3RoaXJkcGFydHkvX19pbml0X18ucHkKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gVG9vbHMvU2NyaXB0cy93ZWJraXRweS90aGlyZHBhcnR5L19faW5pdF9fLnB5CShy ZXZpc2lvbiAyNTEzODQpCisrKyBUb29scy9TY3JpcHRzL3dlYmtpdHB5L3RoaXJkcGFydHkvX19p bml0X18ucHkJKHdvcmtpbmcgY29weSkKQEAgLTQyLDcgKzQyLDYgQEAgZnJvbSBjb2xsZWN0aW9u cyBpbXBvcnQgbmFtZWR0dXBsZQogZnJvbSBkaXN0dXRpbHMgaW1wb3J0IHNwYXduCiBmcm9tIHdl YmtpdHB5LmNvbW1vbi5zeXN0ZW0uYXV0b2luc3RhbGwgaW1wb3J0IEF1dG9JbnN0YWxsZXIKIGZy b20gd2Via2l0cHkuY29tbW9uLnN5c3RlbS5maWxlc3lzdGVtIGltcG9ydCBGaWxlU3lzdGVtCi1m cm9tIHdlYmtpdHB5LmNvbW1vbi5zeXN0ZW0uZXhlY3V0aXZlIGltcG9ydCBFeGVjdXRpdmUKIAog X1RISVJEUEFSVFlfRElSID0gb3MucGF0aC5kaXJuYW1lKF9fZmlsZV9fKQogX0FVVE9JTlNUQUxM RURfRElSID0gb3MucGF0aC5qb2luKF9USElSRFBBUlRZX0RJUiwgImF1dG9pbnN0YWxsZWQiKQpA QCAtNzgsOSArNzcsOCBAQCBpZiBub3QgZnMuZXhpc3RzKHJlYWRtZV9wYXRoKToKIAogCiBjbGFz cyBBdXRvaW5zdGFsbEltcG9ydEhvb2sob2JqZWN0KToKLSAgICBkZWYgX19pbml0X18oc2VsZiwg ZmlsZXN5c3RlbT1Ob25lLCBleGVjdXRpdmU9Tm9uZSk6CisgICAgZGVmIF9faW5pdF9fKHNlbGYs IGZpbGVzeXN0ZW09Tm9uZSk6CiAgICAgICAgIHNlbGYuX2ZzID0gZmlsZXN5c3RlbSBvciBGaWxl U3lzdGVtKCkKLSAgICAgICAgc2VsZi5fZXhlY3V0aXZlID0gZXhlY3V0aXZlIG9yIEV4ZWN1dGl2 ZSgpCiAKICAgICBkZWYgX2Vuc3VyZV9hdXRvaW5zdGFsbGVkX2Rpcl9pc19pbl9zeXNfcGF0aChz ZWxmKToKICAgICAgICAgIyBTb21lIHBhY2thZ2VzIHJlcXVpcmUgdGhhdCB0aGUgYXJlIGJlaW5n IHB1dCBzb21ld2hlcmUgdW5kZXIgYSBkaXJlY3RvcnkgaW4gc3lzLnBhdGguCkBAIC0yMTgsNyAr MjE2LDE2IEBAIGNsYXNzIEF1dG9pbnN0YWxsSW1wb3J0SG9vayhvYmplY3QpOgogICAgICAgICBk aWRfZG93bmxvYWRfYnM0ID0gc2VsZi5faW5zdGFsbCgiaHR0cHM6Ly9maWxlcy5weXRob25ob3N0 ZWQub3JnL3BhY2thZ2VzLzg2L2NkLzQ5NWM2OGYwNTM2ZGNkMjVmMDE2ZTAwNjczMWJhN2JlNzJl MDcyMjgwMzA1ZWM1MjU5MDAxMmMxZTZmMi9iZWF1dGlmdWxzb3VwNC00LjguMS50YXIuZ3oiLAog ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAiYmVhdXRpZnVsc291cDQt NC44LjEvYnM0IikKICAgICAgICAgaWYgZGlkX2Rvd25sb2FkX2JzNDoKLSAgICAgICAgICAgIHNl bGYuX2V4ZWN1dGl2ZS5ydW5fY29tbWFuZChbJzJ0bzMnLCAnLXcnLCBzZWxmLl9mcy5qb2luKF9B VVRPSU5TVEFMTEVEX0RJUiwgJ2JzNCcpXSkKKyAgICAgICAgICAgIGZyb20gbXVsdGlwcm9jZXNz aW5nIGltcG9ydCBQcm9jZXNzCisgICAgICAgICAgICBmcm9tIGxpYjJ0bzMubWFpbiBpbXBvcnQg bWFpbgorCisgICAgICAgICAgICB0cnk6CisgICAgICAgICAgICAgICAgc3lzLnN0ZG91dCA9IG9w ZW4ob3MuZGV2bnVsbCwgJ3cnKQorICAgICAgICAgICAgICAgIHByb2Nlc3MgPSBQcm9jZXNzKHRh cmdldD1tYWluLCBhcmdzPSgnbGliMnRvMy5maXhlcycsIFsnLXcnLCBzZWxmLl9mcy5qb2luKF9B VVRPSU5TVEFMTEVEX0RJUiwgJ2JzNCcpXSkpCisgICAgICAgICAgICAgICAgcHJvY2Vzcy5zdGFy dCgpCisgICAgICAgICAgICAgICAgcHJvY2Vzcy5qb2luKCkKKyAgICAgICAgICAgIGZpbmFsbHk6 CisgICAgICAgICAgICAgICAgc3lzLnN0ZG91dCA9IHN5cy5fX3N0ZG91dF9fCiAKIAogICAgIGRl ZiBfaW5zdGFsbF9weWxpbnQoc2VsZik6Cg==