From 88bfa5567f154432cd21f7e5a31a5515f7dd30db Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Wed, 11 Mar 2020 09:47:59 +0100 Subject: [PATCH 01/10] Add a --keep-untracked option to the install command. --- poetry/console/commands/install.py | 6 ++++++ poetry/installation/installer.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/poetry/console/commands/install.py b/poetry/console/commands/install.py index fabffec4a24..0daec01fe12 100644 --- a/poetry/console/commands/install.py +++ b/poetry/console/commands/install.py @@ -19,6 +19,11 @@ class InstallCommand(EnvCommand): "Output the operations but do not execute anything " "(implicitly enables --verbose).", ), + option( + "keep-untracked", + None, + "Does not remove packages not present in the lock file.", + ), option( "extras", "E", @@ -58,6 +63,7 @@ def handle(self): installer.extras(extras) installer.dev_mode(not self.option("no-dev")) installer.dry_run(self.option("dry-run")) + installer.keep_untracked(self.option("keep-untracked")) installer.verbose(self.option("verbose")) return_code = installer.run() diff --git a/poetry/installation/installer.py b/poetry/installation/installer.py index 74e8258ffa7..78efa9a01cd 100644 --- a/poetry/installation/installer.py +++ b/poetry/installation/installer.py @@ -39,6 +39,7 @@ def __init__( self._pool = pool self._dry_run = False + self._keep_untracked = False self._update = False self._verbose = False self._write_lock = True @@ -83,6 +84,14 @@ def dry_run(self, dry_run=True): # type: (bool) -> Installer def is_dry_run(self): # type: () -> bool return self._dry_run + def keep_untracked(self, keep_untracked=True): # type: (bool) -> Installer + self._keep_untracked = keep_untracked + + return self + + def is_keep_untracked(self): # type: () -> bool + return self._keep_untracked + def verbose(self, verbose=True): # type: (bool) -> Installer self._verbose = verbose @@ -424,6 +433,17 @@ def _get_operations_from_lock( ops.append(op) + if not self._keep_untracked: + for installed in installed_repo.packages: + is_in_lock_file = False + for locked in locked_repository.packages: + if locked.name == installed.name: + is_in_lock_file = True + break + + if not is_in_lock_file: + ops.append(Uninstall(installed)) + return ops def _filter_operations( From d717e0bcc28796c58fef91c1a730bc707b663f2c Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Wed, 18 Mar 2020 11:00:36 +0100 Subject: [PATCH 02/10] Call the option --remove-untracked instead. --- poetry/console/commands/install.py | 6 ++---- poetry/installation/installer.py | 12 ++++++------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/poetry/console/commands/install.py b/poetry/console/commands/install.py index 0daec01fe12..a50dfdabc85 100644 --- a/poetry/console/commands/install.py +++ b/poetry/console/commands/install.py @@ -20,9 +20,7 @@ class InstallCommand(EnvCommand): "(implicitly enables --verbose).", ), option( - "keep-untracked", - None, - "Does not remove packages not present in the lock file.", + "remove-untracked", None, "Removes packages not present in the lock file.", ), option( "extras", @@ -63,7 +61,7 @@ def handle(self): installer.extras(extras) installer.dev_mode(not self.option("no-dev")) installer.dry_run(self.option("dry-run")) - installer.keep_untracked(self.option("keep-untracked")) + installer.remove_untracked(self.option("remove-untracked")) installer.verbose(self.option("verbose")) return_code = installer.run() diff --git a/poetry/installation/installer.py b/poetry/installation/installer.py index 78efa9a01cd..d74bc2b7fdc 100644 --- a/poetry/installation/installer.py +++ b/poetry/installation/installer.py @@ -39,7 +39,7 @@ def __init__( self._pool = pool self._dry_run = False - self._keep_untracked = False + self._remove_untracked = False self._update = False self._verbose = False self._write_lock = True @@ -84,13 +84,13 @@ def dry_run(self, dry_run=True): # type: (bool) -> Installer def is_dry_run(self): # type: () -> bool return self._dry_run - def keep_untracked(self, keep_untracked=True): # type: (bool) -> Installer - self._keep_untracked = keep_untracked + def remove_untracked(self, remove_untracked=True): # type: (bool) -> Installer + self._remove_untracked = remove_untracked return self - def is_keep_untracked(self): # type: () -> bool - return self._keep_untracked + def is_remove_untracked(self): # type: () -> bool + return self._remove_untracked def verbose(self, verbose=True): # type: (bool) -> Installer self._verbose = verbose @@ -433,7 +433,7 @@ def _get_operations_from_lock( ops.append(op) - if not self._keep_untracked: + if self._remove_untracked: for installed in installed_repo.packages: is_in_lock_file = False for locked in locked_repository.packages: From e7f56519eedbe548a93a6efebaa054443e4972df Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Wed, 18 Mar 2020 11:30:49 +0100 Subject: [PATCH 03/10] Add test and fix Installer. --- poetry/installation/installer.py | 22 ++++++------- tests/installation/test_installer.py | 49 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/poetry/installation/installer.py b/poetry/installation/installer.py index d74bc2b7fdc..0aee406bc1a 100644 --- a/poetry/installation/installer.py +++ b/poetry/installation/installer.py @@ -233,6 +233,17 @@ def _do_install(self, local_repo): ops = solver.solve(use_latest=whitelist) + if self._remove_untracked: + for installed in self._installed_repository.packages: + is_in_lock_file = False + for locked in locked_repository.packages: + if locked.name == installed.name: + is_in_lock_file = True + break + + if not is_in_lock_file: + ops.append(Uninstall(installed)) + # We need to filter operations so that packages # not compatible with the current system, # or optional and not requested, are dropped @@ -433,17 +444,6 @@ def _get_operations_from_lock( ops.append(op) - if self._remove_untracked: - for installed in installed_repo.packages: - is_in_lock_file = False - for locked in locked_repository.packages: - if locked.name == installed.name: - is_in_lock_file = True - break - - if not is_in_lock_file: - ops.append(Uninstall(installed)) - return ops def _filter_operations( diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index c236141be44..b89f500c18b 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -295,6 +295,55 @@ def test_run_install_no_dev(installer, locker, repo, package, installed): assert len(removals) == 1 +def test_run_install_remove_untracked(installer, locker, repo, package, installed): + locker.locked(True) + locker.mock_lock_data( + { + "package": [ + { + "name": "A", + "version": "1.0", + "category": "main", + "optional": False, + "platform": "*", + "python-versions": "*", + "checksum": [], + } + ], + "metadata": { + "python-versions": "*", + "platform": "*", + "content-hash": "123456789", + "hashes": {"A": []}, + }, + } + ) + package_a = get_package("A", "1.0") + package_b = get_package("B", "1.1") + package_c = get_package("C", "1.2") + repo.add_package(package_a) + repo.add_package(package_b) + repo.add_package(package_c) + + installed.add_package(package_a) + installed.add_package(package_b) + installed.add_package(package_c) + + package.add_dependency("A", "~1.0") + + installer.dev_mode(True).remove_untracked(True) + installer.run() + + installs = installer.installer.installs + assert len(installs) == 0 + + updates = installer.installer.updates + assert len(updates) == 0 + + removals = installer.installer.removals + assert len(removals) == 2 + + def test_run_whitelist_add(installer, locker, repo, package): locker.locked(True) locker.mock_lock_data( From 78e8f16d3c7e4e7d293cf79e65c94e9e6d312d48 Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Wed, 18 Mar 2020 17:16:47 +0100 Subject: [PATCH 04/10] Add documentation. --- docs/docs/cli.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/docs/cli.md b/docs/docs/cli.md index 63d63fbc990..4732f1bd3d1 100644 --- a/docs/docs/cli.md +++ b/docs/docs/cli.md @@ -116,6 +116,13 @@ the `--no-dev` option. poetry install --no-dev ``` +If you want to remove old dependencies no longer present in the lock file, use the +`--remove-untracked` option. + +```bash +poetry install --remove-untracked +``` + You can also specify the extras you want installed by passing the `--E|--extras` option (See [Extras](#extras) for more info) From 201044c9dd761cf073d5b0b7a1c8995f5ebc71cc Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Thu, 26 Mar 2020 19:37:44 +0100 Subject: [PATCH 05/10] Make sure to never remove essential packages like pip, setuptools and the root. --- poetry/installation/installer.py | 16 +++++++++------- tests/installation/test_installer.py | 16 ++++++++++------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/poetry/installation/installer.py b/poetry/installation/installer.py index 0aee406bc1a..25ea54c6fa0 100644 --- a/poetry/installation/installer.py +++ b/poetry/installation/installer.py @@ -11,6 +11,7 @@ from poetry.puzzle.operations import Uninstall from poetry.puzzle.operations import Update from poetry.puzzle.operations.operation import Operation +from poetry.puzzle.provider import Provider from poetry.repositories import Pool from poetry.repositories import Repository from poetry.repositories.installed_repository import InstalledRepository @@ -234,14 +235,15 @@ def _do_install(self, local_repo): ops = solver.solve(use_latest=whitelist) if self._remove_untracked: - for installed in self._installed_repository.packages: - is_in_lock_file = False - for locked in locked_repository.packages: - if locked.name == installed.name: - is_in_lock_file = True - break + locked_names = {locked.name for locked in locked_repository.packages} - if not is_in_lock_file: + for installed in self._installed_repository.packages: + if installed.name == self._package.name: + continue + if installed.name in Provider.UNSAFE_PACKAGES: + # Never remove pip, setuptools etc. + continue + if installed.name not in locked_names: ops.append(Uninstall(installed)) # We need to filter operations so that packages diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index b89f500c18b..cb53bc0612e 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -301,7 +301,7 @@ def test_run_install_remove_untracked(installer, locker, repo, package, installe { "package": [ { - "name": "A", + "name": "a", "version": "1.0", "category": "main", "optional": False, @@ -314,20 +314,24 @@ def test_run_install_remove_untracked(installer, locker, repo, package, installe "python-versions": "*", "platform": "*", "content-hash": "123456789", - "hashes": {"A": []}, + "hashes": {"a": []}, }, } ) - package_a = get_package("A", "1.0") - package_b = get_package("B", "1.1") - package_c = get_package("C", "1.2") + package_a = get_package("a", "1.0") + package_b = get_package("b", "1.1") + package_c = get_package("c", "1.2") + package_pip = get_package("pip", "20.0.0") repo.add_package(package_a) repo.add_package(package_b) repo.add_package(package_c) + repo.add_package(package_pip) installed.add_package(package_a) installed.add_package(package_b) installed.add_package(package_c) + installed.add_package(package_pip) # Always required and never removed. + installed.add_package(package) # Root package never removed. package.add_dependency("A", "~1.0") @@ -341,7 +345,7 @@ def test_run_install_remove_untracked(installer, locker, repo, package, installe assert len(updates) == 0 removals = installer.installer.removals - assert len(removals) == 2 + assert set(r.name for r in removals) == {"b", "c"} def test_run_whitelist_add(installer, locker, repo, package): From 601881bedfa76f9a15b670e99b684e148bfec8a1 Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Fri, 1 May 2020 10:10:55 +0200 Subject: [PATCH 06/10] Move logic to the Solver instead. --- poetry/installation/installer.py | 8 +++++++- poetry/puzzle/solver.py | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/poetry/installation/installer.py b/poetry/installation/installer.py index 5576343dcae..442961c0653 100644 --- a/poetry/installation/installer.py +++ b/poetry/installation/installer.py @@ -164,6 +164,7 @@ def _do_install(self, local_repo): self._installed_repository, locked_repository, self._io, + remove_untracked=self._remove_untracked, ) ops = solver.solve(use_latest=self._whitelist) @@ -230,7 +231,12 @@ def _do_install(self, local_repo): whitelist.append(pkg.name) solver = Solver( - root, pool, self._installed_repository, locked_repository, NullIO() + root, + pool, + self._installed_repository, + locked_repository, + NullIO(), + remove_untracked=self._remove_untracked, ) with solver.use_environment(self._env): diff --git a/poetry/puzzle/solver.py b/poetry/puzzle/solver.py index 5ee6f5a485e..2b6e1f03c93 100644 --- a/poetry/puzzle/solver.py +++ b/poetry/puzzle/solver.py @@ -21,7 +21,7 @@ class Solver: - def __init__(self, package, pool, installed, locked, io): + def __init__(self, package, pool, installed, locked, io, remove_untracked=False): self._package = package self._pool = pool self._installed = installed @@ -29,6 +29,7 @@ def __init__(self, package, pool, installed, locked, io): self._io = io self._provider = Provider(self._package, self._pool, self._io) self._overrides = [] + self._remove_untracked = remove_untracked @property def provider(self): # type: () -> Provider @@ -132,6 +133,18 @@ def solve(self, use_latest=None): # type: (...) -> List[Operation] operations.append(op) + if self._remove_untracked: + locked_names = {locked.name for locked in self._locked.packages} + + for installed in self._installed.packages: + if installed.name == self._package.name: + continue + if installed.name in Provider.UNSAFE_PACKAGES: + # Never remove pip, setuptools etc. + continue + if installed.name not in locked_names: + operations.append(Uninstall(installed)) + return sorted( operations, key=lambda o: ( From 40295966b756bebe4261a48f3f2771bb5cde6e40 Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Fri, 1 May 2020 10:21:35 +0200 Subject: [PATCH 07/10] Add a few unit tests for Solver. --- tests/puzzle/test_solver.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index e9d32ba058d..b44144a27ba 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -1851,3 +1851,25 @@ def test_solver_should_not_go_into_an_infinite_loop_on_duplicate_dependencies( {"job": "install", "package": package_a}, ], ) + + +def test_solver_remove_untracked_single(package, pool, installed, locked, io): + solver = Solver(package, pool, installed, locked, io, remove_untracked=True) + package_a = get_package("a", "1.0") + installed.add_package(package_a) + + ops = solver.solve() + + check_solver_result(ops, [{"job": "remove", "package": package_a}]) + + +def test_solver_remove_untracked_keeps_critical_package( + package, pool, installed, locked, io +): + solver = Solver(package, pool, installed, locked, io, remove_untracked=True) + package_pip = get_package("pip", "1.0") + installed.add_package(package_pip) + + ops = solver.solve() + + check_solver_result(ops, []) From 44cd316e126d569505ee9537c768fa6e727d9a33 Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Fri, 1 May 2020 18:40:57 +0200 Subject: [PATCH 08/10] Add type hints to solver.py. Co-authored-by: Steph Samson --- poetry/puzzle/solver.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/poetry/puzzle/solver.py b/poetry/puzzle/solver.py index 2b6e1f03c93..d23acd69f05 100644 --- a/poetry/puzzle/solver.py +++ b/poetry/puzzle/solver.py @@ -21,7 +21,15 @@ class Solver: - def __init__(self, package, pool, installed, locked, io, remove_untracked=False): + def __init__( + self, + package, # type: ProjectPackage + pool, # type: Pool + installed, # type: Repository + locked, # type: Repository + io, # type: ConsoleIO + remove_untracked=False # type: bool + ): self._package = package self._pool = pool self._installed = installed From d9d20271327814180e32a8c7c6cee46e3510847d Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Fri, 1 May 2020 18:43:03 +0200 Subject: [PATCH 09/10] Run black after commit fron Github. --- poetry/puzzle/solver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/poetry/puzzle/solver.py b/poetry/puzzle/solver.py index d23acd69f05..112a4af02c3 100644 --- a/poetry/puzzle/solver.py +++ b/poetry/puzzle/solver.py @@ -28,7 +28,7 @@ def __init__( installed, # type: Repository locked, # type: Repository io, # type: ConsoleIO - remove_untracked=False # type: bool + remove_untracked=False, # type: bool ): self._package = package self._pool = pool From 12c7221b5ea65688df2862b553b9f5fdfbd71013 Mon Sep 17 00:00:00 2001 From: Petter Strandmark Date: Fri, 1 May 2020 18:50:39 +0200 Subject: [PATCH 10/10] Import the identifiers used in type annotations. --- poetry/puzzle/solver.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/poetry/puzzle/solver.py b/poetry/puzzle/solver.py index 112a4af02c3..c68b7c80106 100644 --- a/poetry/puzzle/solver.py +++ b/poetry/puzzle/solver.py @@ -5,10 +5,15 @@ from typing import Dict from typing import List +from clikit.io import ConsoleIO + from poetry.core.packages import Package +from poetry.core.packages.project_package import ProjectPackage from poetry.mixology import resolve_version from poetry.mixology.failure import SolveFailure from poetry.packages import DependencyPackage +from poetry.repositories import Pool +from poetry.repositories import Repository from poetry.utils.env import Env from .exceptions import OverrideNeeded