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

-r otherExe.exe causes inevitable Python crash due to coding bug #2675

Closed
dgehlhaar opened this Issue Jun 30, 2017 · 10 comments

Comments

Projects
None yet
2 participants
@dgehlhaar

dgehlhaar commented Jun 30, 2017

PyInstaller 3.2.1, Python 3.5.1, Windows 10 Pro.

This one is easy to reproduce. I am trying to copy resources from an executable into my generated PyInstaller executable using "-r otherExe.exe" on the command line. This gives the following crash:

File "C:\python\Lib\site-packages\PyInstaller\utils\win32\winresource.py", line 248, in UpdateResourcesFromDict
[type_], [name], [language])
File "C:\python\Lib\site-packages\PyInstaller\utils\win32\winresource.py", line 186, in UpdateResources
res = GetResources(dstpath, [type_], names, languages)
File "C:\python\Lib\site-packages\PyInstaller\utils\win32\winresource.py", line 172, in GetResources
res = _GetResources(hsrc, types, names, languages)
File "C:\python\Lib\site-packages\PyInstaller\utils\win32\winresource.py", line 113, in _GetResources
types = set(types)
TypeError: unhashable type: 'list'

Note that UpdateResourcesFromDict() wraps "type_" in a list, and then UpdateResources() does it again when calling GetResources(), so the call to set(types) gets a list of lists which cannot be converted to a set. I think one of the "wrap layers" needs to go away.

@dgehlhaar

This comment has been minimized.

dgehlhaar commented Jun 30, 2017

Fix by changing "[_type]" to "_type" in UpdateResourcesFromDict().

@dgehlhaar

This comment has been minimized.

dgehlhaar commented Jun 30, 2017

But then you'll hit another bug: when you load resources from a .exe file they are brought in as bytes. The following code in UpdateResources() will crash because "bytes" doesn't have an "encode" function:

                win32api.UpdateResource(hdst, type_, name,
                                        data.encode('UTF-8'), language)

Fix by testing the type first:

           if type(data) is str:
                win32api.UpdateResource(hdst, type_, name,
                                        data.encode('UTF-8'), language)
            else:
                win32api.UpdateResource(hdst, type_, name,
                                        data, language)

@htgoebel htgoebel added the Windows label Jul 25, 2017

@htgoebel

This comment has been minimized.

Member

htgoebel commented Jul 25, 2017

Usage says:

Add or update a resource to a Windows executable. The RESOURCE is one to four items, FILE[,TYPE[,NAME[,LANGUAGE]]]. FILE can be a data file or an exe/dll. … For exe/dll files, all resources from FILE will be added/updated to the final executable if TYPE, NAME and LANGUAGE are omitted or specified as wildcard *.

Some windows-guy needs to have a look at it.

@dgehlhaar

This comment has been minimized.

dgehlhaar commented Jul 25, 2017

@htgoebel

This comment has been minimized.

Member

htgoebel commented Jul 25, 2017

Please submit a pull-request.

@dgehlhaar

This comment has been minimized.

dgehlhaar commented Jul 25, 2017

@htgoebel

This comment has been minimized.

Member

htgoebel commented Jul 26, 2017

Oh, thanks for being so cooperative! NOT

@dgehlhaar

This comment has been minimized.

dgehlhaar commented Jul 26, 2017

@htgoebel

This comment has been minimized.

Member

htgoebel commented Jul 26, 2017

Free software is a give and take.

  • Yes, you provided a fix, but you are leaving the burden of implement it to me. If I need to spend my time on doing this kind of things, I have less time for e.g. testing.
  • If you are not satisfied with the test-quality, go ahead and provide more test-cases.
  • If you don't like PyInstaller anyway, just don't use it. I don't care, I have no benefit if you are using it.
  • Mind your words!
@dgehlhaar

This comment has been minimized.

dgehlhaar commented Jul 26, 2017

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