Opened 14 years ago
Last modified 10 years ago
#14087 new Bug
django.core.management.get_commands only sees commands in the last package of a namespace package
Reported by: | KyleMac | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Johannes Bornhold, David Reynolds, lnielsen@…, bhuztez, alex@…, daevaorn@…, Nicola | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
If using the namespace features of setuptools, then get_commands will only see the commands in the last package. If you have something like the following:
company.app1.management.commands.command1 company.app2.management.commands.command2
then Django will only spot command2. I don't know what the proper fix would be, but I use the following to fill in the gaps:
def find_namespace_commands(): try: from django.conf import settings apps = settings.INSTALLED_APPS except (AttributeError, EnvironmentError, ImportError): apps = [] import imp from django.core.management import _commands, find_commands from django.utils.importlib import import_module for app in apps: try: app_path = import_module(app).__path__ except AttributeError: continue try: imp.find_module('management', app_path) except ImportError: continue mod = import_module('%s.management' % app) if hasattr(mod, '__path__'): for cmd in find_commands(mod.__path__[0]): if _commands is None: _commands = {} if cmd not in _commands: _commands[cmd] = app find_namespace_commands()
Attachments (14)
Change History (44)
comment:1 by , 14 years ago
Cc: | added |
---|
comment:2 by , 14 years ago
by , 14 years ago
Attachment: | patch_14087_manage.diff added |
---|
Solution which tries to stay with old API
comment:3 by , 14 years ago
Has patch: | set |
---|
I just added two patches which seem to fix this problem:
patch_14087_manage.diff
includes some if-statements which shall assure that the old API still will work in case the methods are used elsewhere. I marked two places by a "TODO:" because I think there should be some mechanism added to issue a deprecation warning, so that this could be cleaned up in the future.
patch_14087_manage_trunk.diff
does not include these if-statements
comment:4 by , 14 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
The problem with this is that it will mask a bunch of other problems. Django uses the application name as a namespace, and having two applications with the same name will cause problems. This is something that needs a much higher level workaround; it may be addressed by the #3591 app-refactor that was handled as part of the 2010 GSoC.
comment:5 by , 14 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
Triage Stage: | Unreviewed → Ready for checkin |
This discussion on django-dev points out that the problem is a bit deeper that my original analysis concluded. This github branch has what appears to be an RFC patch.
The patch needs a final review to make sure that there aren't any dragons hidden in changing the module lookup code.
by , 14 years ago
Attachment: | namespaced_managment_commands.diff added |
---|
find_managment_module with support for namespaced packages, with tests.
comment:6 by , 14 years ago
Cc: | added |
---|
comment:7 by , 14 years ago
Cc: | added |
---|
Hi,
I've tested out the namespaced_managment_commands.diff patch and it works for me when my namespace packages are installed in develop mode or as eggs (with either easy_install or a python setup.py install), however if install the package with pip then pip automatically removes the namespacepackage/init.py file and above method no longer works.
Cheers,
Lars
Output from installing my package:
(virtualenv) <package>$ pip install . Unpacking .../<pacakge> Running setup.py egg_info for package from file:///.../<pacakge> no previously-included directories found matching 'docs/build' warning: no files found matching '*.css' under directory 'src' warning: no files found matching '*.js' under directory 'src' Installing collected packages: <pacakge> Running setup.py install for <pacakge> no previously-included directories found matching 'docs/build' warning: no files found matching '*.css' under directory 'src' warning: no files found matching '*.js' under directory 'src' Skipping installation of ../virtualenv/lib/python2.7/site-packages/<pacakge>/__init__.py (namespace package) <------- Installing ...virtualenv/lib/python2.7/site-packages/<pacakge>-0.1.0-py2.7-nspkg.pth Successfully installed <pacakge> Cleaning up...
comment:8 by , 14 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
When I looked at this patch 2 nights ago, I was afraid that there would be a problem (either with eggs, or with the namespace packaging usage), but I hadn't got as far as confirming the problem. Thanks to Lars for finding the case where it all falls apart.
comment:9 by , 14 years ago
Hi,
I did a bit more investigation why the init file is being skipped. The skipping of the init file happens when you use the "--single-version-externally-managed" option during install (e.g. python setup.py install --single-version-externally-managed). Pip automatically adds this option by default and as far as I can read it's also used by system package builders (like e.g. Debian or rpms). What setuptools does with this option is to add a .pth file for each part of a namespace module - e.g.:
site-packages/ <namespace package>_app1_<version>-nspkg.pth <namespace package>_app2_<version>-nspkg.pth ... <namespace package>/app1 <namespace package>/app2
The pth files contains a bit of python which will be loaded by the site module when you start python. This piece of code in the pth file will add <namespace pacakge> as a builtin module and set the modules path variable. So while find_module will not be able to locate the module, the module will already be loaded and available in sys.modules.
I haven't gotten around to fix it yet :-)
Cheers,
Lars
comment:10 by , 13 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:11 by , 13 years ago
Easy pickings: | unset |
---|---|
UI/UX: | unset |
I tried to improve, but could not get it work with zip-archived eggs.
comment:12 by , 13 years ago
Cc: | added |
---|
comment:13 by , 13 years ago
Needs documentation: | set |
---|
with patch zip_egg_fixed.2.diff, django will be able to find all commands in a package, if importer for that package implements interfaces explained below and is not in sys.meta_path.
- importer.iter_modules(prefix='')
importer.iter_modules is not defined in PEP 302, however pkgutil.ImpImporter does implement this method. Django would use this interface to list all modules in the commands package.
- loader.get_filename(fullname)
loader.get_filename is defined in PEP 302. But this patch use it to figure out search path of packages.
filename = loader.get_filename(fullname) path_item = os.path.dirname(filename)
Just assume os.path.dirname would work out the search path.
- loader.is_package(fullname)
This is the same as loader.is_package defined in PEP 302.
by , 13 years ago
Attachment: | zip_egg_fixed.3.diff added |
---|
if management has already been imported and is not a package
comment:15 by , 13 years ago
Needs documentation: | unset |
---|
namespace_package_pth.2.diff will not look up importer in sys.path_importer_cache, since django do not support PEP 302 importer.
should find commands in modules loaded by PEP 302 importer be in another ticket?
by , 13 years ago
Attachment: | namespace_package_pth.3.diff added |
---|
add test from zip_egg_fixed.3.diff
follow-up: 17 comment:16 by , 13 years ago
The zip_egg_fixed patch should also fix #12206.
I can't seem to find a separate bug for the management-commands-in-eggs issue, though I thought there was one. Yes, there should be a separate bug for that (or more broadly, PEP 302 importers). It's confusing to have two patches under development on a single ticket.
comment:17 by , 13 years ago
I thought there was one.
They are #8280 and #13587. and add support PEP 302 importer should also help improve the error message of load_backend, which is mentioned in #8238.
It's confusing to have two patches under development on a single ticket.
the namespace_package_pth patch is cleaned up to address the problem of finding command in namespace packages only, so that it could be applied without further investigation. However, in practice, namespaced packages are often being zip archived. So, we need a general solution for PEP 302 importers.
comment:19 by , 13 years ago
Patch needs improvement: | unset |
---|---|
Version: | 1.2 → SVN |
I found #596 is a better place for patch zip_egg_fixed. I will move it there.
comment:20 by , 13 years ago
Cc: | added |
---|
comment:22 by , 12 years ago
my pull request for the namespace_package_pth patch: https://github.com/django/django/pull/178
comment:23 by , 12 years ago
My patch for Python 1.4.1 : https://code.djangoproject.com/ticket/19048
comment:24 by , 12 years ago
Status: | reopened → new |
---|
comment:25 by , 12 years ago
bhuztez' pull request certainly addresses the problem I am seeing, any way we can move this forward?
comment:27 by , 11 years ago
Cc: | added |
---|
comment:29 by , 11 years ago
Patch needs improvement: | set |
---|
From apollo13 on the pull request:
As I noted on the previous PR (#866) I'd like to see: * Support for PEP 420 * Check other locations in Django for the same issues (app template loaders etc come to mind)
comment:30 by , 10 years ago
Cc: | added |
---|
I just ran into the same trouble. After some research it seems to be a problem which is not so easy to solve as long as one wants to avoid importing the modules. In source:django/trunk/django/core/management/__init__.py@13489#L31 the code is using imp to find the module. I think this is not suitable, because the variable
__path__
of the modules is not respected when traversing the tree.It would be easy to solve if
find_module
would return all found modules. May be that it can be simulated by calling find_module for each element of a list of paths and aggregating the results into a tuple. I will try this out and add my results to this ticket.