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

gh-99370: calculate zip path from prefix when in a venv #99371

Merged
merged 1 commit into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions Lib/test/test_getpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,39 @@ def test_venv_changed_name_posix(self):
actual = getpath(ns, expected)
self.assertEqual(expected, actual)

def test_venv_non_installed_zip_path_posix(self):
"Test a venv created from non-installed python has correct zip path."""
ns = MockPosixNamespace(
argv0="/venv/bin/python",
PREFIX="/usr",
ENV_PATH="/venv/bin:/usr/bin",
)
ns.add_known_xfile("/path/to/non-installed/bin/python")
ns.add_known_xfile("/venv/bin/python")
ns.add_known_link("/venv/bin/python",
"/path/to/non-installed/bin/python")
ns.add_known_file("/path/to/non-installed/lib/python9.8/os.py")
ns.add_known_dir("/path/to/non-installed/lib/python9.8/lib-dynload")
ns.add_known_file("/venv/pyvenv.cfg", [
r"home = /path/to/non-installed"
])
expected = dict(
executable="/venv/bin/python",
prefix="/path/to/non-installed",
exec_prefix="/path/to/non-installed",
base_executable="/path/to/non-installed/bin/python",
base_prefix="/path/to/non-installed",
base_exec_prefix="/path/to/non-installed",
module_search_paths_set=1,
module_search_paths=[
"/path/to/non-installed/lib/python98.zip",
"/path/to/non-installed/lib/python9.8",
"/path/to/non-installed/lib/python9.8/lib-dynload",
],
)
actual = getpath(ns, expected)
self.assertEqual(expected, actual)

def test_venv_changed_name_copy_posix(self):
"Test a venv --copies layout on *nix that lacks a distributed 'python'"
ns = MockPosixNamespace(
Expand Down
63 changes: 63 additions & 0 deletions Lib/test/test_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,69 @@ def test_pathsep_error(self):
self.assertRaises(ValueError, venv.create, bad_itempath)
self.assertRaises(ValueError, venv.create, pathlib.Path(bad_itempath))

@unittest.skipIf(os.name == 'nt', 'not relevant on Windows')
@requireVenvCreate
def test_zippath_from_non_installed_posix(self):
"""
Test that when create venv from non-installed python, the zip path
value is as expected.
"""
rmtree(self.env_dir)
# First try to create a non-installed python. It's not a real full
# functional non-installed python, but enough for this test.
non_installed_dir = os.path.realpath(tempfile.mkdtemp())
try:
Copy link
Member

Choose a reason for hiding this comment

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

Rather than an enormous try block, you can use self.addCleanup(rmtree, non_installed_dir)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Do I need to wait for the buildbots test to finish before pushing the change?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, let's get the failures fixed and clean up afterwards :)

bindir = os.path.join(non_installed_dir, self.bindir)
os.mkdir(bindir)
shutil.copy2(sys.executable, bindir)
libdir = os.path.join(non_installed_dir, *self.lib)
os.makedirs(libdir)
landmark = os.path.join(libdir, "os.py")
stdlib_zip = "python%d%d.zip" % sys.version_info[:2]
zip_landmark = os.path.join(non_installed_dir,
self.lib[0],
stdlib_zip)
additional_pythonpath_for_non_installed = []
# Copy stdlib files to the non-installed python so venv can
# correctly calculate the prefix.
for eachpath in sys.path:
if eachpath.endswith(".zip"):
if os.path.isfile(eachpath):
shutil.copyfile(
eachpath,
os.path.join(non_installed_dir, self.lib[0]))
elif os.path.isfile(os.path.join(eachpath, "os.py")):
for name in os.listdir(eachpath):
if name == "site-packages":
continue
fn = os.path.join(eachpath, name)
if os.path.isfile(fn):
shutil.copy(fn, libdir)
elif os.path.isdir(fn):
shutil.copytree(fn, os.path.join(libdir, name))
else:
additional_pythonpath_for_non_installed.append(
eachpath)
cmd = [os.path.join(non_installed_dir, self.bindir, self.exe),
"-m",
"venv",
"--without-pip",
self.env_dir]
# Our fake non-installed python is not fully functional because
# it cannot find the extensions. Set PYTHONPATH so it can run the
# venv module correctly.
pythonpath = os.pathsep.join(
additional_pythonpath_for_non_installed)
subprocess.check_call(cmd, env={"PYTHONPATH": pythonpath})
envpy = os.path.join(self.env_dir, self.bindir, self.exe)
# Now check the venv created from the non-installed python has
# correct zip path in pythonpath.
cmd = [envpy, '-S', '-c', 'import sys; print(sys.path)']
out, err = check_output(cmd)
self.assertTrue(zip_landmark.encode() in out)
finally:
rmtree(non_installed_dir)

@requireVenvCreate
class EnsurePipTest(BaseTest):
"""Test venv module installation of pip."""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix zip path for venv created from a non-installed python on POSIX
platforms.
3 changes: 1 addition & 2 deletions Modules/getpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -679,9 +679,8 @@ def search_up(prefix, *landmarks, test=isfile):
else:
library_dir = executable_dir
pythonpath.append(joinpath(library_dir, ZIP_LANDMARK))
elif build_prefix or venv_prefix:
elif build_prefix:
# QUIRK: POSIX uses the default prefix when in the build directory
# or a venv
pythonpath.append(joinpath(PREFIX, ZIP_LANDMARK))
else:
pythonpath.append(joinpath(prefix, ZIP_LANDMARK))
Expand Down