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

Improved handling of module __path__ attribute for namespace packages, fixes #1321 #1402

Merged
merged 6 commits into from Sep 16, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.d/1402.change.rst
@@ -0,0 +1,2 @@
Fixed a bug with namespace packages under python-3.6 when one package in
current directory hides another which is installed.
11 changes: 6 additions & 5 deletions pkg_resources/__init__.py
Expand Up @@ -2135,12 +2135,13 @@ def position_in_sys_path(path):
parts = path_parts[:-module_parts]
return safe_sys_path_index(_normalize_cached(os.sep.join(parts)))

if not isinstance(orig_path, list):
# Is this behavior useful when module.__path__ is not a list?
return
new_path = sorted(orig_path, key=position_in_sys_path)
new_path = [_normalize_cached(p) for p in new_path]

orig_path.sort(key=position_in_sys_path)
module.__path__[:] = [_normalize_cached(p) for p in orig_path]
if isinstance(module.__path__, list):
module.__path__[:] = new_path
else:
module.__path__ = new_path


def declare_namespace(packageName):
Expand Down
31 changes: 31 additions & 0 deletions setuptools/tests/test_namespaces.py
Expand Up @@ -109,3 +109,34 @@ def test_namespace_package_installed_and_cwd(self, tmpdir):
]
with test.test.paths_on_pythonpath([str(target)]):
subprocess.check_call(pkg_resources_imp, cwd=str(pkg_A))

@pytest.mark.skipif(bool(os.environ.get("APPVEYOR")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the bool here.

Also, can you make this xfail? This test should pass on Appveyor, so it would be good to know if it changes from xfail to xpass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that in appveyor run for this pull request 3 of 4 namespace tests passed when running under python-2.7.15 and all 4 passed when running under python-3.6.6. Should I remove xfail for them?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daa The rest of the repo is not super consistent about xfail vs skip. Going forward we're trying to make a clear distinction, and at some point I'll go through and try and clean it up so that we can turn on strict xfail (e.g. if an xfail test succeeds, that's a failure).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, I misunderstood. Yeah, I think you should change these so that you don't introduce any new XPASSes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I removed xfail marks for namespace tests that pass on AppVeyor.

reason="https://github.com/pypa/setuptools/issues/851")
def test_packages_in_the_sampe_namespace_installed_and_cwd(self, tmpdir):
"""
Installing one namespace package and also have another in the same
namespace in the current working directory, both of them must be
importable.
"""
pkg_A = namespaces.build_namespace_package(tmpdir, 'myns.pkgA')
pkg_B = namespaces.build_namespace_package(tmpdir, 'myns.pkgB')
target = tmpdir / 'packages'
# use pip to install to the target directory
install_cmd = [
sys.executable,
'-m',
'pip.__main__',
'install',
str(pkg_A),
'-t', str(target),
]
subprocess.check_call(install_cmd)
namespaces.make_site_dir(target)

# ensure that all packages import and pkg_resources imports
pkg_resources_imp = [
sys.executable,
'-c', 'import pkg_resources; import myns.pkgA; import myns.pkgB',
]
with test.test.paths_on_pythonpath([str(target)]):
subprocess.check_call(pkg_resources_imp, cwd=str(pkg_B))