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.
Created attachment 381464 [details] Patch
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.
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.
Comment on attachment 381464 [details] Patch Clearing flags on attachment: 381464 Committed r251478: <https://trac.webkit.org/changeset/251478>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56540189>
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.