From b2e59c1714e71f88881f4c8fe1097d91d0a4df16 Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Thu, 23 Nov 2023 17:14:30 +0800 Subject: [PATCH 1/3] fix: inplace update Signed-off-by: Frost Ming --- news/2423.bugfix.md | 1 + src/pdm/installers/installers.py | 9 ++++---- src/pdm/installers/manager.py | 23 ++++++++++++++++--- src/pdm/installers/synchronizers.py | 3 +-- src/pdm/installers/uninstallers.py | 35 ++++++++++++++++------------- src/pdm/pytest.py | 27 +++++++++++++--------- 6 files changed, 62 insertions(+), 36 deletions(-) create mode 100644 news/2423.bugfix.md diff --git a/news/2423.bugfix.md b/news/2423.bugfix.md new file mode 100644 index 0000000000..eb2d8e249b --- /dev/null +++ b/news/2423.bugfix.md @@ -0,0 +1 @@ +Updating package now overwrites the old files instead of removing before installing. diff --git a/src/pdm/installers/installers.py b/src/pdm/installers/installers.py index 6bb97fa365..a43727de1b 100644 --- a/src/pdm/installers/installers.py +++ b/src/pdm/installers/installers.py @@ -168,7 +168,7 @@ def _symlink_files(symlink_to: str) -> Iterator[tuple[Scheme, RecordEntry]]: return super().finalize_installation(scheme, record_file_path, records) -def install_wheel(wheel: str, environment: BaseEnvironment, direct_url: dict[str, Any] | None = None) -> None: +def install_wheel(wheel: str, environment: BaseEnvironment, direct_url: dict[str, Any] | None = None) -> str: """Install a normal wheel file into the environment.""" additional_metadata = None if direct_url is not None: @@ -178,12 +178,10 @@ def install_wheel(wheel: str, environment: BaseEnvironment, direct_url: dict[str interpreter=str(environment.interpreter.executable), script_kind=_get_kind(environment), ) - _install_wheel(wheel=wheel, destination=destination, additional_metadata=additional_metadata) + return _install_wheel(wheel=wheel, destination=destination, additional_metadata=additional_metadata) -def install_wheel_with_cache( - wheel: str, environment: BaseEnvironment, direct_url: dict[str, Any] | None = None -) -> None: +def install_wheel_with_cache(wheel: str, environment: BaseEnvironment, direct_url: dict[str, Any] | None = None) -> str: """Only create .pth files referring to the cached package. If the cache doesn't exist, create one. """ @@ -245,6 +243,7 @@ def skip_files(source: WheelFile, element: WheelContentElement) -> bool: additional_metadata=additional_metadata, ) package_cache.add_referrer(dist_info_dir) + return dist_info_dir def _install_wheel( diff --git a/src/pdm/installers/manager.py b/src/pdm/installers/manager.py index d8333f5ab9..2cd29c3965 100644 --- a/src/pdm/installers/manager.py +++ b/src/pdm/installers/manager.py @@ -3,12 +3,12 @@ from typing import TYPE_CHECKING from pdm import termui +from pdm.compat import Distribution from pdm.exceptions import UninstallError from pdm.installers.installers import install_wheel, install_wheel_with_cache from pdm.installers.uninstallers import BaseRemovePaths, StashedRemovePaths if TYPE_CHECKING: - from pdm.compat import Distribution from pdm.environments import BaseEnvironment from pdm.models.candidates import Candidate @@ -23,14 +23,16 @@ def __init__(self, environment: BaseEnvironment, *, use_install_cache: bool = Fa self.environment = environment self.use_install_cache = use_install_cache - def install(self, candidate: Candidate) -> None: + def install(self, candidate: Candidate) -> Distribution: + """Install a candidate into the environment, return the distribution""" if self.use_install_cache and candidate.req.is_named and candidate.name not in self.NO_CACHE_PACKAGES: # Only cache wheels from PyPI installer = install_wheel_with_cache else: installer = install_wheel prepared = candidate.prepare(self.environment) - installer(str(prepared.build()), self.environment, prepared.direct_url()) + dist_info = installer(str(prepared.build()), self.environment, prepared.direct_url()) + return Distribution.at(dist_info) def get_paths_to_remove(self, dist: Distribution) -> BaseRemovePaths: """Get the path collection to be removed from the disk""" @@ -48,3 +50,18 @@ def uninstall(self, dist: Distribution) -> None: termui.logger.info("Error occurred during uninstallation, roll back the changes now.") remove_path.rollback() raise UninstallError(e) from e + + def overwrite(self, dist: Distribution, candidate: Candidate) -> None: + """An in-place update to overwrite the distribution with a new candidate""" + paths_to_remove = self.get_paths_to_remove(dist) + termui.logger.info("Overwriting distribution %s", dist.metadata["Name"]) + installed = self.install(candidate) + installed_paths = self.get_paths_to_remove(installed) + # Remove the paths that are in the new distribution + paths_to_remove.difference_update(installed_paths) + try: + paths_to_remove.remove() + paths_to_remove.commit() + except OSError as e: + termui.logger.info("Error occurred during overwriting, roll back the changes now.") + raise UninstallError(e) from e diff --git a/src/pdm/installers/synchronizers.py b/src/pdm/installers/synchronizers.py index 23739e6428..82d38e9d6a 100644 --- a/src/pdm/installers/synchronizers.py +++ b/src/pdm/installers/synchronizers.py @@ -304,8 +304,7 @@ def update_candidate(self, key: str, progress: Progress) -> tuple[Distribution, ) can.prepare(self.environment, RichProgressReporter(progress, job)) try: - self.manager.uninstall(dist) - self.manager.install(can) + self.manager.overwrite(dist, can) except Exception: progress.live.console.print( f" [error]{termui.Emoji.FAIL}[/] Update [req]{key}[/] " diff --git a/src/pdm/installers/uninstallers.py b/src/pdm/installers/uninstallers.py index 1652ca55ea..bc3ef37601 100644 --- a/src/pdm/installers/uninstallers.py +++ b/src/pdm/installers/uninstallers.py @@ -6,7 +6,7 @@ import shutil from pathlib import Path from tempfile import TemporaryDirectory -from typing import TYPE_CHECKING, Iterable, TypeVar, cast +from typing import TYPE_CHECKING, Iterable, NewType, TypeVar, cast from pdm import termui from pdm.exceptions import UninstallError @@ -18,6 +18,7 @@ from pdm.environments import BaseEnvironment _T = TypeVar("_T", bound="BaseRemovePaths") +NormalizedPath = NewType("NormalizedPath", str) def renames(old: str, new: str) -> None: @@ -37,26 +38,26 @@ def renames(old: str, new: str) -> None: pass -def compress_for_rename(paths: Iterable[str]) -> set[str]: +def compress_for_rename(paths: Iterable[NormalizedPath]) -> set[NormalizedPath]: """Returns a set containing the paths that need to be renamed. This set may include directories when the original sequence of paths included every file on disk. """ - case_map = {os.path.normcase(p): p for p in paths if os.path.exists(p)} + case_map = {NormalizedPath(os.path.normcase(p)): p for p in paths if os.path.exists(p)} remaining = set(case_map) - unchecked = sorted({os.path.split(p)[0] for p in case_map.values()}, key=len) - wildcards: set[str] = set() + unchecked = sorted({NormalizedPath(os.path.split(p)[0]) for p in case_map.values()}, key=len) + wildcards: set[NormalizedPath] = set() - def norm_join(*a: str) -> str: - return os.path.normcase(os.path.join(*a)) + def norm_join(*a: str) -> NormalizedPath: + return NormalizedPath(os.path.normcase(os.path.join(*a))) for root in unchecked: if any(os.path.normcase(root).startswith(w) for w in wildcards): # This directory has already been handled. continue - all_files: set[str] = set() + all_files: set[NormalizedPath] = set() for dirname, subdirs, files in os.walk(root): all_files.update(norm_join(root, dirname, f) for f in files) for d in subdirs: @@ -69,10 +70,10 @@ def norm_join(*a: str) -> str: # for the directory. if not (all_files - remaining): remaining.difference_update(all_files) - wildcards.add(root + os.sep) + wildcards.add(NormalizedPath(root + os.sep)) collected = set(map(case_map.__getitem__, remaining)) | wildcards - shortened: set[str] = set() + shortened: set[NormalizedPath] = set() # Filter out any paths that are sub paths of another path in the path collection. for path in sorted(collected, key=len): if not any(is_path_relative_to(path, p) for p in shortened): @@ -91,13 +92,13 @@ def _script_names(script_name: str, is_gui: bool) -> Iterable[str]: yield script_name + "-script.py" -def _cache_file_from_source(py_file: str) -> Iterable[str]: +def _cache_file_from_source(py_file: NormalizedPath) -> Iterable[NormalizedPath]: py2_cache = py_file[:-3] + ".pyc" if os.path.isfile(py2_cache): - yield py2_cache + yield NormalizedPath(py2_cache) parent, base = os.path.split(py_file) cache_dir = os.path.join(parent, "__pycache__") - yield from glob.glob(os.path.join(cache_dir, base[:-3] + ".*.pyc")) + yield from map(NormalizedPath, glob.glob(os.path.join(cache_dir, base[:-3] + ".*.pyc"))) def _get_file_root(path: str, base: str) -> str | None: @@ -116,10 +117,14 @@ class BaseRemovePaths(abc.ABC): def __init__(self, dist: Distribution, environment: BaseEnvironment) -> None: self.dist = dist self.environment = environment - self._paths: set[str] = set() + self._paths: set[NormalizedPath] = set() self._pth_entries: set[str] = set() self.refer_to: str | None = None + def difference_update(self, other: BaseRemovePaths) -> None: + self._paths.difference_update(other._paths) + self._pth_entries.difference_update(other._pth_entries) + @abc.abstractmethod def remove(self) -> None: """Remove the files""" @@ -192,7 +197,7 @@ def add_pth(self, line: str) -> None: self._pth_entries.add(line) def add_path(self, path: str) -> None: - normalized_path = os.path.normcase(os.path.expanduser(os.path.abspath(path))) + normalized_path = NormalizedPath(os.path.normcase(os.path.expanduser(os.path.abspath(path)))) self._paths.add(normalized_path) if path.endswith(".py"): self._paths.update(_cache_file_from_source(normalized_path)) diff --git a/src/pdm/pytest.py b/src/pdm/pytest.py index d061b60046..9e78078b0d 100644 --- a/src/pdm/pytest.py +++ b/src/pdm/pytest.py @@ -452,22 +452,27 @@ def working_set(mocker: MockerFixture, repository: TestRepository) -> MockWorkin Returns: a mock working set """ + from pdm.installers import InstallManager + rv = MockWorkingSet() mocker.patch.object(BaseEnvironment, "get_working_set", return_value=rv) - def install(candidate: Candidate) -> None: - key = normalize_name(candidate.name or "") - dist = Distribution(key, cast(str, candidate.version), candidate.req.editable) - dist.dependencies = repository.get_raw_dependencies(candidate) - rv.add_distribution(dist) + class MockInstallManager(InstallManager): + def install(self, candidate: Candidate) -> Distribution: # type: ignore[override] + key = normalize_name(candidate.name or "") + dist = Distribution(key, cast(str, candidate.version), candidate.req.editable) + dist.dependencies = repository.get_raw_dependencies(candidate) + rv.add_distribution(dist) + return dist + + def uninstall(self, dist: Distribution) -> None: # type: ignore[override] + del rv[dist.name] - def uninstall(dist: Distribution) -> None: - del rv[dist.name] + def overwrite(self, dist: Distribution, candidate: Candidate) -> None: # type: ignore[override] + self.uninstall(dist) + self.install(candidate) - install_manager = mocker.MagicMock() - install_manager.install.side_effect = install - install_manager.uninstall.side_effect = uninstall - mocker.patch("pdm.installers.Synchronizer.get_manager", return_value=install_manager) + mocker.patch.object(Core, "install_manager_class", MockInstallManager) return rv From 2b10d56d9a89b201602709a60ec6765ae9d1c794 Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Thu, 23 Nov 2023 17:53:23 +0800 Subject: [PATCH 2/3] fix: file removal Signed-off-by: Frost Ming --- src/pdm/installers/uninstallers.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/pdm/installers/uninstallers.py b/src/pdm/installers/uninstallers.py index bc3ef37601..fe051ae75a 100644 --- a/src/pdm/installers/uninstallers.py +++ b/src/pdm/installers/uninstallers.py @@ -111,6 +111,12 @@ def _get_file_root(path: str, base: str) -> str | None: return os.path.normcase(os.path.join(base, root)) +def _get_all_parents(path: NormalizedPath) -> Iterable[NormalizedPath]: + while (parent := NormalizedPath(os.path.split(path)[0])) != path: + yield path + path = parent + + class BaseRemovePaths(abc.ABC): """A collection of paths and/or pth entries to remove""" @@ -122,8 +128,12 @@ def __init__(self, dist: Distribution, environment: BaseEnvironment) -> None: self.refer_to: str | None = None def difference_update(self, other: BaseRemovePaths) -> None: - self._paths.difference_update(other._paths) self._pth_entries.difference_update(other._pth_entries) + for p in other._paths: + # if other_p is a file, remove all parent dirs of it + self._paths.difference_update(_get_all_parents(p)) + # other_p is a symlink dir, remove all files under it + self._paths.difference_update({p2 for p2 in self._paths if p2.startswith(p + os.sep)}) @abc.abstractmethod def remove(self) -> None: From 919e46492adfcb33ad9aca5216aa53d95a38b6a1 Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Thu, 23 Nov 2023 17:57:04 +0800 Subject: [PATCH 3/3] fix python 37 Signed-off-by: Frost Ming --- src/pdm/installers/uninstallers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pdm/installers/uninstallers.py b/src/pdm/installers/uninstallers.py index fe051ae75a..8c7061508f 100644 --- a/src/pdm/installers/uninstallers.py +++ b/src/pdm/installers/uninstallers.py @@ -112,8 +112,11 @@ def _get_file_root(path: str, base: str) -> str | None: def _get_all_parents(path: NormalizedPath) -> Iterable[NormalizedPath]: - while (parent := NormalizedPath(os.path.split(path)[0])) != path: + while True: yield path + parent = NormalizedPath(os.path.split(path)[0]) + if parent == path: + break path = parent