Skip to content

Commit

Permalink
During seeding properly uninstall present versions of the wheels (#2186)
Browse files Browse the repository at this point in the history
* During seeding remove dist-info for present versions of the wheels

An existing dist-info may contain entrypoints that may interfere with
normal functioning of the redeployed seeded wheel if there is a version
mismatch

fixes #2185

* Remove package directories from dist-info top_level packages
Remove other recorded files from RECORD
Remove dist-info itself

* Do not resolve paths prior to removal for symlink mode
In the test ensure the directories are compared as sets and not lists
Add setuptools downgrade to ensure proper cleanup of the existing version

* PR Feedback

Signed-off-by: Bernát Gábor <[email protected]>

Co-authored-by: Bernát Gábor <[email protected]>
  • Loading branch information
arcivanov and gaborbernat authored Sep 24, 2021
1 parent 3b9d954 commit e13ea2a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 8 deletions.
3 changes: 3 additions & 0 deletions docs/changelog/2185.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed a bug where while creating a venv on top of an existing one, without cleaning, when seeded
wheel version mismatch occurred, multiple ``.dist-info`` directories may be present, confounding entrypoint
discovery - by :user:`arcivanov`
37 changes: 32 additions & 5 deletions src/virtualenv/seed/embed/via_app_data/pip_install/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import re
import zipfile
from abc import ABCMeta, abstractmethod
from itertools import chain
from tempfile import mkdtemp

from distlib.scripts import ScriptMaker, enquote_executable
Expand All @@ -31,14 +32,10 @@ def _sync(self, src, dst):

def install(self, version_info):
self._extracted = True
self._uninstall_previous_version()
# sync image
for filename in self._image_dir.iterdir():
into = self._creator.purelib / filename.name
if into.exists():
if into.is_dir() and not into.is_symlink():
safe_delete(into)
else:
into.unlink()
self._sync(filename, into)
# generate console executables
consoles = set()
Expand Down Expand Up @@ -150,6 +147,36 @@ def _create_console_entry_point(self, name, value, to_folder, version_info):
result.extend(Path(i) for i in new_files)
return result

def _uninstall_previous_version(self):
dist_name = self._dist_info.stem.split("-")[0]
in_folders = chain.from_iterable([i.iterdir() for i in {self._creator.purelib, self._creator.platlib}])
paths = (p for p in in_folders if p.stem.split("-")[0] == dist_name and p.suffix == ".dist-info" and p.is_dir())
existing_dist = next(paths, None)
if existing_dist is not None:
self._uninstall_dist(existing_dist)

@staticmethod
def _uninstall_dist(dist):
dist_base = dist.parent
logging.debug("uninstall existing distribution %s from %s", dist.stem, dist_base)

top_txt = dist / "top_level.txt" # add top level packages at folder level
paths = {dist.parent / i.strip() for i in top_txt.read_text().splitlines()} if top_txt.exists() else set()
paths.add(dist) # add the dist-info folder itself

base_dirs, record = paths.copy(), dist / "RECORD" # collect entries in record that we did not register yet
for name in (i.split(",")[0] for i in record.read_text().splitlines()) if record.exists() else ():
path = dist_base / name
if not any(p in base_dirs for p in path.parents): # only add if not already added as a base dir
paths.add(path)

for path in sorted(paths): # actually remove stuff in a stable order
if path.exists():
if path.is_dir() and not path.is_symlink():
safe_delete(path)
else:
path.unlink()

def clear(self):
if self._image_dir.exists():
safe_delete(self._image_dir)
Expand Down
16 changes: 13 additions & 3 deletions tests/unit/seed/embed/test_bootstrap_link_via_app_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_seed_link_via_app_data(tmp_path, coverage_env, current_fastest, copies)
pip = site_package / "pip"
setuptools = site_package / "setuptools"

files_post_first_create = list(site_package.iterdir())
files_post_first_create = set(site_package.iterdir())
assert pip in files_post_first_create
assert setuptools in files_post_first_create
for pip_exe in [
Expand Down Expand Up @@ -82,15 +82,25 @@ def test_seed_link_via_app_data(tmp_path, coverage_env, current_fastest, copies)
assert not process.returncode
assert site_package.exists()

files_post_first_uninstall = list(site_package.iterdir())
files_post_first_uninstall = set(site_package.iterdir())
assert pip in files_post_first_uninstall
assert setuptools not in files_post_first_uninstall

# install a different setuptools to test that virtualenv removes this before installing new
version = "setuptools<{}".format(bundle_ver["setuptools"].split("-")[1])
install_cmd = [str(result.creator.script("pip")), "--verbose", "--disable-pip-version-check", "install", version]
process = Popen(install_cmd)
process.communicate()
assert not process.returncode
assert site_package.exists()
files_post_downgrade = set(site_package.iterdir())
assert setuptools in files_post_downgrade

# check we can run it again and will work - checks both overwrite and reuse cache
result = cli_run(create_cmd)
coverage_env()
assert result
files_post_second_create = list(site_package.iterdir())
files_post_second_create = set(site_package.iterdir())
assert files_post_first_create == files_post_second_create

# Windows does not allow removing a executable while running it, so when uninstalling pip we need to do it via
Expand Down

0 comments on commit e13ea2a

Please sign in to comment.