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

Broken namespace packages corner-case? (py3.5) #900

Open
IwanVosloo opened this issue Dec 27, 2016 · 6 comments
Open

Broken namespace packages corner-case? (py3.5) #900

IwanVosloo opened this issue Dec 27, 2016 · 6 comments
Labels
Needs Triage Issues that need to be evaluated for severity and status.

Comments

@IwanVosloo
Copy link

IwanVosloo commented Dec 27, 2016

I have hit a complicated corner case on py3.5 regarding namespace packages on code that used to work before. I suspect it is a bug in a recent setuptools, but I may be wrong... tox is also involved, but I doubt it is at fault.

I have reproduced the whole thing in IwanVosloo/setuptools-namespaces

The gist of it is: code in one module in a namespace package cannot import code from another module in a different setuptools distribution that shares the same namespace package.

I have the following directory structure:

 ├── makeit.sh
 ├── nspace-three
 │   ├── nspace
 │   │   ├── __init__.py
 │   │   └── three
 │   │       ├── __init__.py
 │   │       └── morestuff.py
 │   └── setup.py
 └── nspace-two
     ├── MANIFEST.in
     ├── nspace
     │   ├── __init__.py
     │   └── two
     │       ├── __init__.py
     │       └── stuff.py
     ├── setup.py
     ├── tmp
     └── tox.ini
 

nspace-two and nspace-three are different Distributions sharing a namespace package (nspace), and nspace-two depends on nspace-three.

The module nspace-two/nspace/two/stuff.py starts off with an "import pkg_resources" [1] before it imports something from nspace.three.

If you run tox, nspace-two/tox.ini runs, as a test:
python -c 'from nspace.three.morestuff import *' (and this always works)
and then:
python -c 'from nspace.two.stuff import *' (which breaks on py3.5 but works on 2.7)

If I comment out the import in [1] above - it also works.

If I instruct tox to changedir away from nspace-two directory before doing those imports - it also works.

python: 3.5.2
virtualenv: 15.1.0
pip: 9.0.1
setuptools: 32.3.0

@IwanVosloo
Copy link
Author

Hi there. I have now managed to reproduce this without involving tox at all.

I updated my repository reproducing the bug in IwanVosloo/setuptools-namespaces.

@jaraco
Copy link
Member

jaraco commented Jan 5, 2017

Thanks for the repro repo and for eliminating tox. I've forked your repo as https://github.com/jaraco/setuptools-issue-900 and further distilled the issue in master. I'm setting PYTHONPATH=nspace-two to cause the same effect as having it as the CWD, and doing all of the work in the current directory. In the sans-virtualenv branch, I've attempted to replicate the issue without virtualenv, but in that mode, removing the import pkg_resources line has no effect - it fails every time. So it's a better test because it removes a dependency and variable, but since the issue isn't corrected by the removal of the import, I worry it's not accurately representing the issue.

In any case, I don't yet have a good understanding of the issue, but I'm getting closer and will give it another look another time.

@IwanVosloo
Copy link
Author

IwanVosloo commented Jan 5, 2017

Thanks @jaraco

We have wrestled with the issue a bit more and have learnt some more about it.

Firstly, this is a strange (yet probably common) scenario: we are running code from the current directory of the source code of a package, but that package is installed (via pip) AS WELL. This seems to confuse setuptools somewhat, because it picks up ./nspace as well as the installed site-packages version of nspace. (./nspace only contains two, whereas the version in site-packages contains three as well.)

Secondly, we realised that this is related to setuptools' handling of PEP420 implicit namespaces.

According to PEP420, namespace packages should not have __init__.py files at all. We're on Python 3.5 with this issue, thus, according to PEP420, our namespace packages should not have __init__.py files. This example had __init__.py files that contained calls to .declare_namespace(). If we remove the relevant __init__.py files, the problem goes away.

What causes the issue is:

There are two versions of the nspace package:
./nspace (which is part of the source code of nspace-two)
and
....site-packages/nspace

When importing nspace.three, somehow the ./nspace is found first, and inside it it finds an init.py. According to PEP420 then, if a package has an __init__.py in it the package is treated as a normal package - NOT a namespace package.

I am not sure how one would handle this scenario. Our code base runs on Python 2.7 as well and I'm not sure if one still needs __init__.py files in that case, since PEP420 is Python >= 3.3. Perhaps a dependency on new enough setuptools would solve it?

On the other hand, the situation where you run, eg "python ./setup.py" in the source code tree of a package which also happens to be installed in site-packages is kindof weird. Yet, I think it often happens. For example, when people run tests against installed packages: python ./setup.py test.

@IwanVosloo
Copy link
Author

IwanVosloo commented Jan 5, 2017

BTW, once one removes the __init__.py files of namespace packges, you run into #97

@tlandschoff-scale
Copy link
Contributor

Thank you for the analysis, we were running into the same problem. Workaround was to change current directory before executing the tests. This has one downside though: One can only test after installation, which is non ideal during development (on CI we test the installed package).

Workaround for that: pip install -e the package during development and run the tests from another folder.

@daa
Copy link
Contributor

daa commented Sep 17, 2018

@IwanVosloo @tlandschoff-scale May you please check if you have this problem with latest setuptools? Recently my pull request fixing similar issue was merged, I hope it should help with this issue too.

@pganssle pganssle added the Needs Triage Issues that need to be evaluated for severity and status. label Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

No branches or pull requests

5 participants