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

Fix several ResourceWarnings and DeprecationWarnings #3677

Merged
merged 6 commits into from Sep 9, 2018

Conversation

Projects
None yet
2 participants
@BoboTiG
Contributor

BoboTiG commented Aug 8, 2018

Fixes Python 3 warnings:

utils/misc.py:224: DeprecationWarning: 'U' mode is deprecated
  f = open(filename, 'rU', encoding='utf-8')

I fixed the whole module in several places to be consistent.


Also fixes a lot of resource leaks:

utils/misc.py:150: ResourceWarning: unclosed file <_io.BufferedReader name='/.../loader/pyimod03_importers.pyc'>
  (open(obj_fnm, 'rb').read()[:4] != BYTECODE_MAGIC)

archive/writers.py:86: ResourceWarning: unclosed file <_io.BufferedReader name='.../project/PYZ-00.pyz'>
  self.add(toc_entry)  # The guts of the archive.

Actually, there were numbers of those 2 specific warnings, more another one not really spotted:

Exception ignored in: <_io.FileIO name='.../struct.pyo' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='.../struct.pyo'>
Exception ignored in: <_io.FileIO name='PyInstaller/loader/pyimod01_os_path.pyc' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='PyInstaller/loader/pyimod01_os_path.pyc'>
Exception ignored in: <_io.FileIO name='PyInstaller/loader/pyimod02_archive.pyc' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='PyInstaller/loader/pyimod02_archive.pyc'>
Exception ignored in: <_io.FileIO name='.../struct.pyo' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='.../struct.pyo'>
Exception ignored in: <_io.FileIO name='PyInstaller/loader/pyimod01_os_path.pyc' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='PyInstaller/loader/pyimod01_os_path.pyc'>
Exception ignored in: <_io.FileIO name='PyInstaller/loader/pyimod02_archive.pyc' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='PyInstaller/loader/pyimod02_archive.pyc'>
Exception ignored in: <_io.FileIO name='PyInstaller/loader/pyimod03_importers.pyc' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='PyInstaller/loader/pyimod03_importers.pyc'>
Exception ignored in: <_io.FileIO name='/tmp/pytest-15/test_egg_unzipped_onedir_0/build/test_egg_unzipped/PYZ-00.pyz' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.BufferedReader name='/tmp/pytest-15/test_egg_unzipped_onedir_0/build/test_egg_unzipped/PYZ-00.pyz'>

I changed almost every file handling to use the with context manager.

@BoboTiG BoboTiG changed the title from Fix several ResourceWarnings to Fix several ResourceWarnings and DeprecationWarnings Aug 8, 2018

@htgoebel htgoebel self-assigned this Aug 25, 2018

@htgoebel

Thank you very much for this fix. May I ask you to

  • In the 1. commit: split the changes to modulegraph into a commit of its own. This eases pushing the change upstream. The commit-message should refer to module graph.
  • Change the commit-message of the 2. commit to refer to modulegraph.
  • Split the 3. commit into
    • one commit actually fixing the ResourceWarnings
    • one commit replacing open/close by with.

Thank you very much.

For reasoning why I'm so picky about commits please have a look at the commit message guide.

When updating a pull-request, you can simply (force) push the updated branch to github again. This will automatically update the pull-request (which follows the branch, not the commit). So you do not need to close the pull-request and open a new one. This also has the benefit that the discussion history is kept. For detailed instructions please read Updating a Pull-Request in the manual.

Please also take the chance and rebase your branch on the current develop head. Thanks.

@@ -95,6 +95,7 @@
# module as io.open(). The Python 2 open() built-in is commonly regarded as
# unsafe in regards to character encodings and hence inferior to io.open().
open_file = open if is_py3 else io.open
read_mode = 'r' if is_py3 else 'rU'

This comment has been minimized.

@htgoebel

htgoebel Aug 28, 2018

Member

What dou you think about naming this text_read_mode to clearly state this is for text only?

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-resources-warnings branch from c871b15 to ab230b1 Aug 28, 2018

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Aug 28, 2018

All changes applied :)

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Sep 6, 2018

@htgoebel Do you think we can make this PR merged for 3.4? 🤞

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-resources-warnings branch from 5bb0689 to 79cec7a Sep 6, 2018

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Sep 6, 2018

Rebased on develop and added the news fragment.

@htgoebel

This comment has been minimized.

Member

htgoebel commented Sep 9, 2018

Thanks you very much for this update. It's really nice :-)

Yes, I am about to integrate this into 3.4. Curiously I get a merge-conflict, while github says there is none.

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Sep 9, 2018

@htgoebel htgoebel force-pushed the BoboTiG:fix-resources-warnings branch from 79cec7a to 36ce3de Sep 9, 2018

@htgoebel htgoebel force-pushed the BoboTiG:fix-resources-warnings branch from 36ce3de to 18ef90c Sep 9, 2018

@htgoebel htgoebel merged commit 5dc9f3e into pyinstaller:develop Sep 9, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@BoboTiG BoboTiG deleted the BoboTiG:fix-resources-warnings branch Sep 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment