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

PackageLoader fails on namespace packages #1097

Closed
iomintz opened this issue Nov 1, 2019 · 7 comments · Fixed by #1113
Closed

PackageLoader fails on namespace packages #1097

iomintz opened this issue Nov 1, 2019 · 7 comments · Fixed by #1113
Milestone

Comments

@iomintz
Copy link

@iomintz iomintz commented Nov 1, 2019

Expected Behavior

In the attached jinja_namespace_bug.zip, app.py prints "Hi"

Actual Behavior

A traceback occurs. This is due to python3.7 namespace packages having their __file__ attribute set to None.

Reproduction steps

Instead of running the commands below, you can also extract the attached zip file and run app.py.

mkdir -p foo/bar
mkdir foo/bar/templates
echo 'x = 1' > foo/bar/baz.py
echo 'Hi' > foo/bar/templates/test.html
cat << 'EOF' > app.py
#!/usr/bin/env python3

import jinja2
jinja_env = jinja2.Environment(loader=jinja2.PackageLoader('foo.bar', 'templates'))
print(jinja_env.get_template('test.html').render())
EOF
chmod +x app.py
./app.py

Full Traceback

Traceback (most recent call last):
  File "./app.py", line 5, in <module>
    jinja_env = jinja2.Environment(loader=jinja2.PackageLoader('foo.bar', 'templates'))
  File "/home/io/code/language/python/3/snippets/.venv/lib/python3.7/site-packages/jinja2/loaders.py", line 224, in __init__
    provider = get_provider(package_name)
  File "/home/io/code/language/python/3/snippets/.venv/lib/python3.7/site-packages/pkg_resources/__init__.py", line 364, in get_provider
    return _find_adapter(_provider_factories, loader)(module)
  File "/home/io/code/language/python/3/snippets/.venv/lib/python3.7/site-packages/pkg_resources/__init__.py", line 1392, in __init__
    self.module_path = os.path.dirname(getattr(module, '__file__', ''))
  File "/usr/lib/python3.7/posixpath.py", line 156, in dirname
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType

Your Environment

  • Python version: 3.7.4
  • Jinja version: 2.10.3
@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Nov 1, 2019

The loader has been reworked, and will be updated in 2.11. Does this fail on master?

@iomintz

This comment has been minimized.

Copy link
Author

@iomintz iomintz commented Nov 1, 2019

Yes

➤ ./app.py
Traceback (most recent call last):
  File "./app.py", line 4, in <module>
    jinja_env = jinja2.Environment(loader=jinja2.PackageLoader('foo.bar', 'templates'))
  File "/home/io/code/language/python/3/snippets/.venv/lib/python3.7/site-packages/jinja2/loaders.py", line 254, in __init__
    os.path.dirname(self._loader.get_filename(package_name)), package_path
AttributeError: 'NoneType' object has no attribute 'get_filename'
@iomintz

This comment has been minimized.

Copy link
Author

@iomintz iomintz commented Nov 1, 2019

As a workaround, __path__ can be used instead for namespace packages. However, __path__ is an iterable of strings, and I'm not sure if always using the first element is the right approach.

@iomintz

This comment has been minimized.

Copy link
Author

@iomintz iomintz commented Nov 1, 2019

For now I'm doing

import foo.bar.templates
jinja_env = jinja2.Environment(loader=jinja2.FileSystemLoader(next(iter(foo.bar.templates.__path__))))
@kevin-brown

This comment has been minimized.

Copy link
Collaborator

@kevin-brown kevin-brown commented Nov 14, 2019

https://docs.python.org/3/library/pkgutil.html#pkgutil.get_loader

This would seem to suggest that foo.bar can't be properly imported. This appears to be correct, per the spec, for namespace packages.

Unlike in coveragepy, where __file___ is being used on the module to get the path. PackageLoader is getting the internal loader and using that to get the file path to the module itself. Since we can't depend on package.templates being a proper Python module, we're then joining the package module with templates instead of checking for the path to the package.templates module.

@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Dec 4, 2019

Ignore the current release, as pkg_resources doesn't support loading resources from namespace packages.

pkgutil.get_loader() returns None if a namespace package hasn't been imported yet. Trying to import the given package is a good fix regardless, as it gives users a clearer error for typos or missing packages.

Python's _NamespaceLoader doesn't support get_filename. We could look through loader._path to find the first one with the templates folder, but that assumes the package isn't treating the templates folder itself as a namespace package, which would mean we need to adapt PackageLoader to scan multiple paths.

Namespace package contributors don't have to be directories, they can also be zip files, just like regular packages. Unlike normal zip packages though, there's no loader API for namespace zips, so we now have to manage opening, scanning, and loading files from the zip file ourselves.

I'm not enthusiastic about developing and maintaining all this, it seems out of scope for Jinja at this point. We could support a limited version of this by assuming a single location and skipping zip files, but ultimately Python needs to improve their loader or importlib.resources APIs to make loading package resources more consistent.

@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Dec 5, 2019

OK, since your example isn't really using namespace packages for their intended use (multiple sources contributing to one package) and is instead just taking advantage of omitting __init__.py, I'm sort of on the fence. Really, you should just add __init__.py and be done with it. But adding limited support for one non-zip directory wasn't really an issue, so I'll support that.

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

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.