RESOLVED FIXED 203213
Python 3: 2to3 script may not be in a user's path 
https://bugs.webkit.org/show_bug.cgi?id=203213
Summary Python 3: 2to3 script may not be in a user's path
Jonathan Bedard
Reported 2019-10-21 14:52:12 PDT
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.
Attachments
Patch (2.70 KB, patch)
2019-10-21 15:03 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jonathan Bedard
Comment 1 2019-10-21 15:03:34 PDT
Created attachment 381464 [details] Patch
dewei_zhu
Comment 2 2019-10-22 18:29:28 PDT
Comment on attachment 381464 [details] 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.
Jonathan Bedard
Comment 3 2019-10-23 07:59:57 PDT
Comment on attachment 381464 [details] 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.
WebKit Commit Bot
Comment 4 2019-10-23 08:43:03 PDT
Comment on attachment 381464 [details] Patch Clearing flags on attachment: 381464 Committed r251478: <https://trac.webkit.org/changeset/251478>
WebKit Commit Bot
Comment 5 2019-10-23 08:43:05 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 6 2019-10-23 08:44:13 PDT
<rdar://problem/56540189>
dewei_zhu
Comment 7 2019-10-23 09:58:09 PDT
Comment on attachment 381464 [details] 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.
Note You need to log in before you can comment on or make changes to this bug.