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

problem packaging markdown in App using fbs/PyInstaller #1067

Open
jralden opened this issue Nov 15, 2020 · 3 comments
Open

problem packaging markdown in App using fbs/PyInstaller #1067

jralden opened this issue Nov 15, 2020 · 3 comments
Labels
more-info-needed More information needs to be provided. someday-maybe Approved low priority request.

Comments

@jralden
Copy link

jralden commented Nov 15, 2020

I have a question. I'm using markdown in a PyQt5 application, and am trying to package it for distribution using fbs/PyInstaller. The program crashes when the packaged result is executed with an error in line 30 of the htmlparser:

27    # Import a copy of the html.parser lib as `htmlparser` so we can monkeypatch it.
28    # Users can still do `from html import parser` and get the default behavior.
29    spec = importlib.util.find_spec('html.parser')
30    htmlparser = importlib.util.module_from_spec(spec)
31    spec.loader.exec_module(htmlparser)
32    sys.modules['htmlparser'] = htmlparser

The error reported is:

  File "lib\site-packages\markdown\htmlparser.py", line 30, in <module>
  File "<frozen importlib._bootstrap>", line 568, in module_from_spec
AttributeError: 'NoneType' object has no attribute 'loader'

Clearly it's not a bug, since it works correctly in the unpackaged code. I was wondering whether you're aware of any successful attempts to package markdown. Also, fbs is only approved for Python 3.6, although it is reported to work for 3.7. Is there anything about the code above that depends on > 3.6? I should also mention that the app works correctly when packaged if I remove the use of markdown.

@waylan
Copy link
Member

waylan commented Nov 15, 2020

I was wondering whether you're aware of any successful attempts to package markdown.

Not recently (Calibre did for many years although I don't believe they do anymore). And the related code is new as of version 3.3. If you are sure you (or any of your dependencies) are not using html.parser anywhere else in your application, the above code could be replaced with from html import parser as htmlparser.

In any event, based on the traceback, the error appears to occur because importlib.util.module_from_spec is returning None. Checking the docs for that, I see that it internally uses spec.loader.create_module and the documentation for that states:

This method may return None, indicating that default module creation semantics should take place.

Presumably we need to check for None being returned by importlib.util.module_from_spec and act accordingly. Not sure what that is though. @mitya57 you proposed this addition. Any ideas?

@waylan waylan added the needs-decision A decision needs to be made regarding request. label Nov 15, 2020
@mitya57
Copy link
Collaborator

mitya57 commented Nov 16, 2020

@mitya57 you proposed this addition. Any ideas?

Was it really me? Anyway, it seems that the problem is indeed caused by our tricks with sys.modules. Looks like this is an inevitable thing to do if one needs to make a private patched copy of a module: https://stackoverflow.com/q/11170949.

So either we declare that Python-Markdown is not compatible with PyInstaller (or some workaround specific for PyInstaller is found), or we actually bundle a copy of html.parser instead of monkey-patching it.

We also can copy the goahead method of HTMLParser to our subclass and modify it to use our regular expressions, but I'm afraid that would be too fragile (can break with new Python versions).

@waylan
Copy link
Member

waylan commented Nov 16, 2020

I did a little more digging. I was able to artificially generate the same error:

>>> spec = importlib.util.find_spec('unknown')
>>> print(spec)
None
>>> unknown = importlib.util.module_from_spec(spec)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 553, in module_from_spec
AttributeError: 'NoneType' object has no attribute 'loader'

Of course, there is no module named unknown, but it demonstrates the issue. find_spec returns None if it fails to find anything. Passing None to module_from_spec would obviously raise an error.

What I missed yesterday is that find_spec only raises an error if the package parameter is invalid. It does not raise an error if the name parameter is not found, but returns None instead.

My guess is that PyInstaller is using something different than the standard finders because they don't work in that environment. After all, the standard finders work on file paths and PyInstaller builds a single file executable. However, as we are calling the lower level API, presumably PyInstaller's overrides are ignored. As this is an officially supported Python library, I would consider this to be a bug in PyInstaller. Maybe they document some special way to call find_spec, but I didn't find anything in a quick search.

@jralden I see the following option: You can take this upstream to fbs/PyInstaller. Maybe they provide a fix on their end, or maybe they suggest a patch to our code which we would be willing to accept. However, this isn't a high priority for us so we're not going to spend any more time trying to work out a fix ourselves.

@waylan waylan added more-info-needed More information needs to be provided. someday-maybe Approved low priority request. and removed needs-decision A decision needs to be made regarding request. labels Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-info-needed More information needs to be provided. someday-maybe Approved low priority request.
Projects
None yet
Development

No branches or pull requests

3 participants