manage.py adds to sys.path and forcibly imports the parent dir of the settings module

[Jorge Vargas <jorge.vargas@…>]

I have tested this with django 1.2.5

Due to django's custom handling of apps and using it's own backport of importlib it is leaving some cruft behind in the module name. Although it's doing the right job as cleaning everything else.

The ofending code is in django/core/management/init.py The last couple of lines from

def setup_environ(settings_mod, original_settings_path=None):

     ............

    # Import the project module. We add the parent directory to PYTHONPATH to
    # avoid some of the path errors new users can have.
    sys.path.append(os.path.join(project_directory, os.pardir))
    project_module = import_module(project_name)
    sys.path.pop()

I have created a vanilla project with some print/trace statements that show the problem.The code is here ​https://bitbucket.org/elpargo/django_import_cruft_test if you run that you will see the problem.

(test_4148)elpargo@elpargo-linux:~/venvs/test_4148/src/import_cruft_test$ python manage.py runserver
pre setting 1
********************************************************************************
in settings.py
********************************************************************************
post settings 2
********************************************************************************
pre execute_manager 3
********************************************************************************
in settings.py
import_cruft_test.sys None
import_cruft_test <module 'import_cruft_test' from '/home/elpargo/venvs/test_4148/src/import_cruft_test/../import_cruft_test/__init__.pyc'>
import_cruft_test.settings <module 'import_cruft_test.settings' from '/home/elpargo/venvs/test_4148/src/import_cruft_test/../import_cruft_test/settings.pyc'>
********************************************************************************
pre setting 1
********************************************************************************
in settings.py
********************************************************************************
post settings 2
********************************************************************************
pre execute_manager 3
********************************************************************************
in settings.py
import_cruft_test.sys None
import_cruft_test <module 'import_cruft_test' from '/home/elpargo/venvs/test_4148/src/import_cruft_test/../import_cruft_test/__init__.pyc'>
import_cruft_test.settings <module 'import_cruft_test.settings' from '/home/elpargo/venvs/test_4148/src/import_cruft_test/../import_cruft_test/settings.pyc'>
********************************************************************************
Validating models...
0 errors found

Django version 1.2.5, using settings 'import_cruft_test.settings'
Development server is running at http://127.0.0.1:8000/
Quit the server with CONTROL-C.

The attached patch seems to fix this, my colleague ran the 2.x test suite against it and everything passes. I try to run it against trunk (1.3.0 pre-beta) however I think there is something wrong with my setup as I get 13 failures (with or without the patch) In either case We don't think it breaks anything however there is a change this is not tested extensibly.

Why is all of this relevant? The django-hudson extension looks into sys.modules in order to generate the coverage report. Which means that in hudson UI you get some ugly paths like venv/src/your_app/../your_app/path/to/your/code

[Russell Keith-Magee]

Closing as a duplicate of #15064 -- It isn't strictly the same bug, but the root cause is the same: django-admin shouldn't be fiddling with sys.path like this.

[Jorge Vargas <jorge.vargas@…>]

I don't think it's the same bug. Please note my problem is not limited to runserver. I used that as an example. The actual command we run is django manage.py hudson from the django-hudson app.

As you point out it's the same cause. IMO this bug is the main bug and #15064 is a consequence of this.

You said the patch needs improvement but do not point to how. I don't know this code much but removing the sys.path modifications probably means a mayor rewrite of the way django imports apps. And this code (according to SVN history) hasn't been touch significantly in over two years.

[Łukasz Rekucki]

1) Don't assign tickets to people without their permission.

2) This problem has two causes: one is sys.path mangling done by Django; second is a poor report generator in ​coverage.py (this is the module that actually creates the Cobertura report - not django-hudson). First is being tracked by #15064, second is out of scope for Django.

Personally, I tweaked the way reports are generated in my fork of django-hudson to enjoy pretty names relative to my codebase: ​https://github.com/lqc/django-hudson/blob/unittest2/django_hudson/plugins/coverage.py#L173

[Jorge Vargas <jorge.vargas@…>]

Replying to lrekucki:

1) Don't assign tickets to people without their permission.

Just making the notification work, as he was the one that assigned the status.

2) This problem has two causes: one is sys.path mangling done by Django; second is a poor report generator in ​coverage.py (this is the module that actually creates the Cobertura report - not django-hudson). First is being tracked by #15064, second is out of scope for Django.

Again I believe #15064 is a subset of this, as this ticket explains the problem better. As for coverage, I don't agree, it's ok that coverage.py generates full path reports based on the module. But as you said that's out of scope.

Personally, I tweaked the way reports are generated in my fork of django-hudson to enjoy pretty names relative to my codebase: ​https://github.com/lqc/django-hudson/blob/unittest2/django_hudson/plugins/coverage.py#L173

Thanks for the pointer. I'll take a look, although the problem still is django messing up sys.path.

[Carl Meyer]

Reopening - this was closed as duplicate of #15064, but the fix for #15064 didn't actually deal with the core problem, the sys.path mangling done by setup_environ. This ticket is the clearest explanation of that problem that I can find, and thus should remain open.

To be clear, this ticket will be fixed when Django is no longer mucking with sys.path, not just if the mucking is done with an absolute path; thus the attached patch is not adequate.

[Carl Meyer]

Proposed patch: ​https://github.com/carljm/django/compare/master...t15372

Marking "needs tests" tentatively - all current tests pass, and the code changes here are almost entirely to the project template, which is currently untested and would need quite a bit of new test harness work to make testable. But I may look into what tests could reasonably be added there.

[Carl Meyer]

Added the patch as an attachment for those who prefer that; also created a pull request at ​https://github.com/django/django/pull/62, to allow for inline comments on the patch.

[Carl Meyer]

Much more detailed breakdown of the problem and proposed solution at ​http://groups.google.com/group/django-developers/browse_frm/thread/44b70a37ff73298b

[Carl Meyer]

deprecate setup_environ and fix default project layout and manage.py

[Carl Meyer]

In [16964]:

Fixed #15372 -- Switched to a startproject default layout that allows us to avoid sys.path hacks.

[Jorge Vargas <jorge.vargas@…>]

This looks like a great solution, Thanks Carl!

[Tim Graham <timograham@…>]

In 80f4ecc6:

Refs #15372 -- Removed obsolete docs about manage.py setting sys.path.

[Tim Graham <timograham@…>]

In d2939489:

[2.2.x] Refs #15372 -- Removed obsolete docs about manage.py setting sys.path.

Backport of 80f4ecc64751d5cf13a7233aa520209fee4757c8 from master