Better isolation from current working directory #453

Open
warsaw opened this Issue Feb 7, 2017 · 6 comments

Comments

Projects
None yet
4 participants

warsaw commented Feb 7, 2017

I hate to be prescriptive in the subject because maybe there's a better way to handle this, but I found adding changedir={envtmpdir} to be an easy solution to the problem. Feel free to re-title this bug as needed.

This may be related to #430 but let me explain what's happening. I've been looking at build failures for the lazr.config package, which also depends on lazr.delegates. Both packages are compatible with Python 2 and Python 3 and the upstream repo is a single bilingual source. Under Python 3, lazr is a proper namespace package, but of course, not under Python 2. The source tree has a file lazr/__init__.py which contains:

# This is a namespace package.
try:
    import pkg_resources
    pkg_resources.declare_namespace(__name__)
except ImportError:
    import pkgutil
    __path__ = pkgutil.extend_path(__path__, __name__)

which I believe is still current best practice. Now, if you pip install lazr.config into a venv, your venv Python can import everything just fine. In fact, if you tox -e py35 --notest -r and then activate the .tox/py35 venv, both packages import just fine. This is true for Python 2 also.

However, when you run tox -e py35 the tests fail because lazr.delegates cannot be imported. This confused me a lot because outside of tox, i.e. using the venvs as described above, everything works fine.

What I think is happening is that because tox by defaults runs out of the {toxinidir} it ends up importing lazr.config from the source tree, which at the top level looks roughly like:

lazr/
lazr/__init__.py
tox.ini

I.e. the package directory lives at the top level (but note that the bug does not go away if you put that inside say a src/ directory).

So now, because the source tree has a lazr/__init__.py file, in Python 3 it ends up not being a namespace package, and thus when it tries to import lazr.delegates from the tox venv, it can't find it. Liberally sprinkling pdb's around shows that indeed tox ends up failing with an ImportError on lazr.delegates, and that sys.modules['lazr'] is not a namespace package. Looking at sys.path at the point of failure, you can see that the cwd appears earlier than say .tox/py35/lib/python3.5/site-packages so the import of lazr.config finds the source tree one instead of the venv one.

Adding

[testenv]
changedir = {envtmpdir}

solves the problem, because when tox runs it will not try to import lazr.config from the source tree, but instead from the venv and of course there, lazr is properly a namespace package.

This kind of explains some failures I've seen in other bilingual packages, but for which I've solved in worse ways (e.g. making them Python 3 only and removing the under-Python-2-needed-namespace-__init__.py-file). By eliminating everything else, I'm interpreting this as a tox bug.

So then the question is, how should tox support bilingual source trees of namespace packages? If the source tree has a concrete <namespace>/__init__.py file to support the Python 2 case, then importing from cwd first will always fail under Python 3. So maybe changedir={envtmpdir} is a better default?

Contributor

The-Compiler commented Feb 7, 2017

That default would break a lot - for starters, commands with relative paths.

warsaw commented Feb 7, 2017

Do you have any other suggestions? Can you confirm whether it's tox itself that's trying to import from the source directory rather than the virtualenv directory?

Contributor

The-Compiler commented Feb 8, 2017

I don't, though I'm not very familiar with Python 2 and/or namespace packages. All I'm saying is that that default would be a major breaking change 😉

@warsaw warsaw changed the title from Make changedir={envtmpdir} the default? to Better isolation from current working directory Feb 8, 2017

warsaw commented Feb 8, 2017

I've been playing around with this a bit more and I've come across a solution that doesn't involve changedir. I'm not sure it's a general enough solution for tox to adopt, but it works for my motivating project, and I relate it here for future reference either way.

One part of the solution involved moving the source code into a subdirectory of the top-level repo directory. E.g. src/lazr. So if you're in the top level directory, import lazr will not work.

Second part is to tell nose (the runner this particular package uses) not to modify sys.path. Third part is to isolate Python from any user site file. So the command ends up being:

[testenv]
commands = python -s -m nose -P lazr

epu commented Feb 10, 2017

@warsaw I had this issue with something I am developing. The first entry in sys.path in the environment under test is always "" , and I didn't see a way to remove it easily. I ended up moving my project's module source to src/module and fixing my setup.py to set package_dir.

I tried setting environment PYTHONPATH, but I don't remember the outcome.

Without hiding the source under development from sys.path, my behave tests would fail some import statements.

Is there a tox hook for modifying sys.path of the testenv?

Owner

obestwalter commented Sep 4, 2017

Is there a tox hook for modifying sys.path of the testenv?

All hookspecs are here: https://github.com/tox-dev/tox/blob/master/tox/hookspecs.py

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