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

Update utils.py #3281

Merged
merged 4 commits into from Feb 26, 2018

Conversation

Projects
None yet
2 participants
@shawnluo-528
Contributor

shawnluo-528 commented Feb 7, 2018

To fix the MemoryError issue when pack a big file.
File "c:\python27\lib\site-packages\PyInstaller\building\utils.py", line 333, in cacheDigest
data = open(fnm, "rb").read()
MemoryError

Update utils.py
To fix the MemoryError issue when pack a big file.
  File "c:\python27\lib\site-packages\PyInstaller\building\utils.py", line 333, in cacheDigest
    data = open(fnm, "rb").read()
MemoryError
if not data:
break
hasher.update(data)
return hasher

This comment has been minimized.

@ghost

ghost Feb 8, 2018

def md5(fname):
    hash_md5 = hashlib.md5()
    with open(fname, "rb") as f:
        for chunk in iter(lambda: f.read(4096), b""):
            hash_md5.update(chunk)
    return hash_md5

This comment has been minimized.

@shawnluo-528

shawnluo-528 Feb 8, 2018

Contributor

that's good

Update utils.py
update get_md5 according to comments.
@htgoebel

This comment has been minimized.

Member

htgoebel commented Feb 8, 2018

Thank you for this fix, which is a great small improvement :-) Just two minot changes:

  • Using only 4096 bytes per read seems to be a bit greedy. In PyInstaller/archive/writers.py we use chunks of 16*1024 bytes. I suggest doing the same here.
  • Please put the code directly into cacheDigest(), no into a (sub-) function. md5 is not used elsewhere in PyInstaller.
    Thanks.

@xoviat This use of iter is quite cool. I did not know it yet :-)

@htgoebel

This comment has been minimized.

Member

htgoebel commented Feb 20, 2018

@shawnluo-528 Please update the pull-request as requested. Thanks.

Update utils.py
update according to htgoebel's comments.

@htgoebel htgoebel merged commit 1934b77 into pyinstaller:develop Feb 26, 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

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Mar 7, 2018

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