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

Cannot read Python modules with importlib.resources using PyOxidizer #207

Closed
layday opened this issue Nov 22, 2019 · 8 comments
Closed

Cannot read Python modules with importlib.resources using PyOxidizer #207

layday opened this issue Nov 22, 2019 · 8 comments
Labels
bug Something isn't working

Comments

@layday
Copy link

layday commented Nov 22, 2019

CPython:

Python 3.7.5 (default, Oct 24 2019, 06:30:32)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.9.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: !ls -1 instawow
__init__.py
__main__.py
__pycache__
_import_wrapper.py
_version.py
cli.py
config.py
exceptions.py
manager.py
matchers.py
migrations
models.py
prompts.py
resolvers.py
utils.py
wa_templates
wa_updater.py

In [2]: from importlib.resources import contents, read_binary

In [3]: read_binary('instawow', '__main__.py')
Out[3]: b"from instawow.cli import main\n\n\nif __name__ == '__main__':\n    main()\n"

In [4]: list(contents('instawow'))
Out[4]:
['migrations',
 'config.py',
 'models.py',
 '_version.py',
 'resolvers.py',
 'wa_updater.py',
 '__init__.py',
 '__pycache__',
 'prompts.py',
 'cli.py',
 'utils.py',
 'exceptions.py',
 'matchers.py',
 '_import_wrapper.py',
 '__main__.py',
 'manager.py',
 'wa_templates']

PyOxidizer:

>>> from importlib.resources import contents, read_binary
>>> read_binary('instawow', '__main__.py')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "importlib.resources", line 154, in read_binary
  File "importlib.resources", line 91, in open_binary
FileNotFoundError: resource not found
>>> contents('instawow')
['wa_templates/COPYING.WeakAuras-Companion', 'wa_templates/data.lua', 'wa_templates/init.lua', 'wa_templates/WeakAurasCompanion.toc']

PyOxidizer also handles is_resource differently:

>>> from importlib.resources import is_resource
>>> is_resource('instawow', 'foo')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "importlib.resources", line 227, in is_resource
FileNotFoundError: resource not found

Compare with the default reader:

In [5]: from importlib.resources import is_resource

In [6]: is_resource('instawow', 'foo')
Out[6]: False
@indygreg indygreg added the bug Something isn't working label Nov 24, 2019
@indygreg
Copy link
Owner

The importlib.resources API is sufficiently undefined that bugs like this are expected at this time. In its current form, the implementation of importlib.resources in CPython is such that it is a simple wrapping to filesystem APIs. Perhaps my biggest issue with things is the inconsistent handling of directory separators in resources. See https://bugs.python.org/issue36128.

>>> read_binary('instawow', '__main__.py')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "importlib.resources", line 154, in read_binary
  File "importlib.resources", line 91, in open_binary
FileNotFoundError: resource not found

The reason this fails in PyOxidizer is because PyOxidizer currently flags each embedded entity as just a single type. e.g. a thing is a source module, a bytecode module, or a resource entry. It can't be multiple. We should probably expose embedded module sources and bytecode as resources as well to maintain compatibility with CPython. This would address this particular problem.

But maintaining parity for listing contents is something I'd prefer to hold off implementing until the Python maintainers give a canonical answer as to how all of this is supposed to work.

@layday
Copy link
Author

layday commented Jan 1, 2020

Whether relative paths become a blessed alternative to recurse into subdirectories I think is unlikely to affect the output of contents because that would be a breaking change. The way importlib.resources works right now, you can recurse into subpackages by piping the output of contents into is_resource. If an entry exists and it is not a resource, then it must be a directory; and if it is a directory and it does not contain an __init__.py, then it is not a package (for the purposes of importlib.resources), its contents will be empty and it can be safely discarded. This assumption does not carry over to PyOxidizer - not least because directories are not listed among the contents. This will not work with PyOxidizer:

def list_resources(package: str) -> Iterable[Tuple[str, str]]:
    "Recursively list package resources."
    from importlib.resources import contents as list_contents, is_resource

    resources = True
    subpackages = False

    contents: DefaultDict[bool, List[str]] = bucketise(list_contents(package), 
                                                       partial(is_resource, package))
    for resource in contents[resources]:
        yield (package, resource)
    for subpackage in contents[subpackages]:
        yield from list_resources(package + '.' + subpackage)

But should it?

@layday
Copy link
Author

layday commented Mar 10, 2020

FYI importlib_resources version 1.1.0 has added a new API which "supersedes all of the previous functionality as it provides a more general-purpose access to a package’s resources". See here.

@indygreg
Copy link
Owner

I sat down to implement the new API today. Unfortunately, it involved a lot of head scratching and I failed to make meaningful progress. I'll have to wait for the Python developers to address my feedback on the new API design.

https://gitlab.com/python-devs/importlib_resources/-/issues/90

@indygreg
Copy link
Owner

indygreg commented Apr 5, 2020

I've made a lot of improvements to resources handling in the current development release. Notable changes include:

  • Resource names are no longer munged incorrectly. e.g. a file foo/bar/resource.txt used to be munged to foo:bar.resource.txt if foo.bar wasn't a package name. Now, the resource name will be preserved as foo:bar/resource.txt.
  • Support for loading resources from the filesystem. We can now mark resources as loadable from the filesystem instead of memory. When installing resources this way, filesystem paths should be preserved and compatible with Python's default filesystem-based resource reader.
  • Documentation of where ResourceReader is incompatible with Python's and why this is.

Unless there are some bugs lingering, I'm quite confident that the new code/features is good enough to handle most cases where it was failing before. Please open new issues tracking specific feature requests or bugs.

@indygreg indygreg closed this as completed Apr 5, 2020
@layday
Copy link
Author

layday commented Apr 10, 2020

I might be missing something, but I don't see how I might be able to access Python source files through importlib.resources still.

@indygreg
Copy link
Owner

No, PyOxidizer's importer doesn't yet support accessing source and bytecode data via the resources API.

This issue was tracking a few different things and I got side-tracked by all the various correctness issues around path resolution that I forgot to address the initial feature request of exposing source and bytecode data.

Could someone please file a new issue requesting those features? It might be worthwhile to file separate issues for source from bytecode, as the latter is a bit more scope to implement, since PyOxidizer currently doesn't embed the 16 byte .pyc header in memory. So if you requested bytecode data, behavior would be different than with the filesystem importer.

@layday
Copy link
Author

layday commented Apr 10, 2020

#237

xmunoz added a commit to Cronenbrogues/cronenbroguelike that referenced this issue Apr 6, 2021
This is the only way to get this game to build with pyoxidizer and as a
python module.

See: indygreg/PyOxidizer#207
xmunoz added a commit to Cronenbrogues/cronenbroguelike that referenced this issue Apr 6, 2021
This is the only way to get this game to build with pyoxidizer and as a
python module.

See: indygreg/PyOxidizer#207
xmunoz added a commit to Cronenbrogues/cronenbroguelike that referenced this issue Apr 6, 2021
This is the only way to get this game to build with pyoxidizer and as a
python module.

See: indygreg/PyOxidizer#207
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants