be more careful with PYTHONPATH when invoking flask

[mwhudson]

The Ubuntu package of Flask-Migrate failed in a rather strange way because
the PYTHONPATH that setuptools sets when running the python 2.7 tests via
"python2.7 setup.py test" was causing the /usr/bin/flask to look in various
2.7-specific locations for modules and failing. This patch overrides the
PYTHONPATH setuptools sets to just '..'.

[miguelgrinberg]

I don't think it is a good idea to override PYTHONPATH. Basically this change prevents the caller to ever set a custom path. Sounds like you are patching this side because it is easier, when the problem is somewhere else.

Why is the PYTHONPATH wrong in this specific case?

[mwhudson]

I agree this is a hack.

The problem is a total mess though. Here's what's happening in excruciating detail:

1. /usr/bin/flask is a python3 program (it starts #!/usr/bin/python3)
2. as part of building the flask-migrate source package, it runs "python2.7 setup.py test"
3. setuptools (only since pypa/setuptools@5e4eea7, so actually fairly recently, I didn't realise the behaviour was new) puts the location of all required distributions onto PYTHONPATH while running the tests. In the context of a package build, this is /usr/lib/python2.7/dist-packages
4. when your tests invoke /usr/bin/flask, this means that even though it is a python 3 program, it looks for things in /usr/lib/python2.7/dist-packages before looking in /usr/lib/python3/dist-packages.
5. this actually works surprisingly well, but is obviously not guaranteed to (and importing jinja2 when /usr/bin/python3 is 3.6 is a particular case where this fails).

Basically everything is horrible (I realise now that my patch probably means that the flask-migrate code is not really tested under python 2.7 at all in these test cases now -- maybe running sys.executable -m flask instead of just 'flask' would work better...

[miguelgrinberg]

Why is the flask tool configured to run on Python3? When I install Flask on a Python 2.7 virtualenv I get the shebang line set to 2.7.

[mwhudson]

On a given system, the flask binary on the global PATH is going to be one or the other. If it was 2.7 we'd just have the problem when running the Python 3 tests.

[mwhudson]

I've pushed what looks like a better fix to me (insanely repetitive, but the idea is clear and it works). Out of time for this week now, sorry.

[mwhudson]

I was just reminded of this PR and I've just pushed a new version. Any updates from your side?

[miguelgrinberg]

As I mentioned before, overriding the PYTHONPATH is guaranteed to affect users that need it set in a specific way. Packages are not supposed to rely on this variable, this is for end users to set. As I said, the problem is not in this package, if Ubuntu used virtual environments to run unit tests, or allowed you to set the PYTHONPATH before running the tests, then we wouldn't be talking about this.

[miguelgrinberg]

This issue will be automatically closed due to being inactive for more than six months. Please reopen if you need more assistance.