Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace py dependency with termcolor. #575

Merged
merged 3 commits into from
Jan 31, 2023
Merged

Conversation

bwoodsend
Copy link
Contributor

Only the colored terminal printing feature of the borderline deprecated py library is used so swap it out for termcolor which is slimmer. Doing so also fixes the pains that py's submodule deprecations sculdugery have been causing PyInstaller users (e.g. #245, #447, plus a new regression specific to pyshark 0.5.2 leading to a ModuleNotFoundError: No module named 'py._io').

Normally I'd raise an issue first to see what you think about the idea but it looked like the easiest way to measure feasibility of this change was just to try it and see.

@KimiNewt
Copy link
Owner

This is great. If you have the time, this is also a good chance to change strings to f-strings

Only the colored terminal printing feature of the borderline deprecated py
library is used so swap it out for termcolor which is slimmer. Doing so also
fixes the pains that py's submodule deprecations sculdugery have been causing
PyInstaller users (e.g. #245, #447, plus a new regression specific to pyshark
0.5.2 leading to a `ModuleNotFoundError: No module named 'py._io'`).
@bwoodsend
Copy link
Contributor Author

This is great. If you have the time, this is also a good chance to change strings to f-strings

Done.

@miaotony
Copy link
Contributor

I also found this bug when running my program packaged with pyinstaller which the version of pyshark was 0.5.3.

图片

...
  File "PyInstaller\loader\pyimod02_importers.py", line 493, in exec_module
  File "pyshark\__init__.py", line 13, in <module>
  File "PyInstaller\loader\pyimod02_importers.py", line 493, in exec_module
  File "pyshark\capture\live_capture.py", line 7, in <module>
  File "PyInstaller\loader\pyimod02_importers.py", line 493, in exec_module
  File "pyshark\capture\capture.py", line 11, in <module>
  File "PyInstaller\loader\pyimod02_importers.py", line 493, in exec_module
  File "pyshark\packet\packet.py", line 8, in <module>
  File "PyInstaller\loader\pyimod02_importers.py", line 493, in exec_module
  File "pyshark\packet\layers\base.py", line 11, in <module>
  File "pyshark\packet\layers\base.py", line 61, in BaseLayer
  File "py\_vendored_packages\apipkg\__init__.py", line 157, in __makeattr
  File "py\_vendored_packages\apipkg\__init__.py", line 75, in importobj
ModuleNotFoundError: No module named 'py._io'

Could you please fix the bug by merging this PR ASAP?

cc @KimiNewt

@bwoodsend
Copy link
Contributor Author

I've reluctantly put a fragile temporary fix (pyinstaller/pyinstaller-hooks-contrib#477) into PyInstaller since this PR hasn't moved in the last 6 weeks. If you pip install "pyinstaller-hooks-contrib>=v2022.9" then run your next pyinstaller build adding the --clean flag (or just delete the build directory) then it should work.

And come on @KimiNewt! I didn't refactor all those f-strings for fun you know/

@miaotony
Copy link
Contributor

I've reluctantly put a fragile temporary fix (pyinstaller/pyinstaller-hooks-contrib#477) into PyInstaller since this PR hasn't moved in the last 6 weeks. If you pip install "pyinstaller-hooks-contrib>=v2022.9" then run your next pyinstaller build adding the --clean flag (or just delete the build directory) then it should work.

And come on @KimiNewt! I didn't refactor all those f-strings for fun you know/

Wow, thank you very much!

Copy link

@XChikuX XChikuX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as this was tested. Looks good to me.

@KimiNewt KimiNewt merged commit ec6e38e into KimiNewt:master Jan 31, 2023
@bwoodsend bwoodsend deleted the remove-py branch January 31, 2023 18:10
@vihangm
Copy link

vihangm commented Mar 30, 2023

@KimiNewt can we get a new release with this included so folks can upgrade pyshark and close out CVE-2022-42969?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants