From d78232d91746f0e2191e345e6c17f74d7f4adcca Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Tue, 26 Mar 2024 21:25:45 -0400 Subject: [PATCH] Smarter uninstall (#6029) * Initial take at making uninstall like the inverse of upgrade. * Updates based on testing the uninstall command * Handle pre flag * Add news fragment --- news/6029.feature.rst | 1 + pipenv/cli/command.py | 5 +- pipenv/project.py | 10 ++ pipenv/routines/uninstall.py | 234 ++++++++++++++-------------- pipenv/utils/resolver.py | 4 +- tests/integration/test_uninstall.py | 9 +- 6 files changed, 134 insertions(+), 129 deletions(-) create mode 100644 news/6029.feature.rst diff --git a/news/6029.feature.rst b/news/6029.feature.rst new file mode 100644 index 00000000..08708062 --- /dev/null +++ b/news/6029.feature.rst @@ -0,0 +1 @@ +The ``uninstall`` command now does the inverse of ``upgrade`` which means it no longer invokes a full ``lock`` cycle which was problematic for projects with many dependencies. diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index 57fe0604..e4f7bf51 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -284,15 +284,18 @@ def uninstall(ctx, state, all_dev=False, all=False, **kwargs): """Uninstalls a provided package and removes it from Pipfile.""" from pipenv.routines.uninstall import do_uninstall + pre = state.installstate.pre + retcode = do_uninstall( state.project, packages=state.installstate.packages, editable_packages=state.installstate.editables, python=state.python, system=state.system, - lock=True, + lock=False, all_dev=all_dev, all=all, + pre=pre, pypi_mirror=state.pypi_mirror, categories=state.installstate.categories, ctx=ctx, diff --git a/pipenv/project.py b/pipenv/project.py index 7afa4c08..b9875902 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -1133,6 +1133,16 @@ class Project: return True return False + def reset_category_in_pipfile(self, category): + # Read and append Pipfile. + p = self.parsed_pipfile + if category: + del p[category] + p[category] = {} + self.write_toml(p) + return True + return False + def remove_packages_from_pipfile(self, packages): parsed = self.parsed_pipfile packages = {pep423_name(pkg) for pkg in packages} diff --git a/pipenv/routines/uninstall.py b/pipenv/routines/uninstall.py index c3975ad0..27e65cf7 100644 --- a/pipenv/routines/uninstall.py +++ b/pipenv/routines/uninstall.py @@ -3,22 +3,39 @@ import sys from pipenv import exceptions from pipenv.patched.pip._internal.build_env import get_runnable_pip -from pipenv.patched.pip._vendor.packaging.utils import canonicalize_name from pipenv.routines.lock import do_lock from pipenv.utils.dependencies import ( expansive_install_req_from_line, - get_canonical_names, get_lockfile_section_using_pipfile_category, get_pipfile_category_using_lockfile_section, pep423_name, ) from pipenv.utils.processes import run_command, subprocess_run -from pipenv.utils.project import ensure_project from pipenv.utils.requirements import BAD_PACKAGES +from pipenv.utils.resolver import venv_resolve_deps from pipenv.utils.shell import cmd_list_to_shell, project_python from pipenv.vendor import click +def _uninstall_from_environment(project, package, system=False): + # Execute the uninstall command for the package + click.secho(f"Uninstalling {package}...", fg="green", bold=True) + with project.environment.activated(): + cmd = [ + project_python(project, system=system), + get_runnable_pip(), + "uninstall", + package, + "-y", + ] + c = run_command(cmd, is_verbose=project.s.is_verbose()) + click.secho(c.stdout, fg="cyan") + if c.returncode != 0: + click.echo(f"Error occurred while uninstalling package {package}.") + return False + return True + + def do_uninstall( project, packages=None, @@ -28,147 +45,124 @@ def do_uninstall( lock=False, all_dev=False, all=False, + pre=False, pypi_mirror=None, ctx=None, categories=None, ): - # Automatically use an activated virtualenv. - if project.s.PIPENV_USE_SYSTEM: - system = True - # Ensure that virtualenv is available. - ensure_project(project, python=python, pypi_mirror=pypi_mirror) - # Uninstall all dependencies, if --all was provided. + # Initialization similar to the upgrade function if not any([packages, editable_packages, all_dev, all]): raise exceptions.PipenvUsageError("No package provided!", ctx=ctx) + if not categories: - categories = project.get_package_categories(for_lockfile=True) - editable_pkgs = [] - for p in editable_packages: - if p: - install_req, name = expansive_install_req_from_line(f"-e {p}") - editable_pkgs.append(name) - packages += editable_pkgs - package_names = {p for p in packages if p} - package_map = {canonicalize_name(p): p for p in packages if p} - installed_package_names = project.installed_package_names - if project.lockfile_exists: - project_pkg_names = project.lockfile_package_names - else: - project_pkg_names = project.pipfile_package_names - # Uninstall [dev-packages], if --dev was provided. + categories = ["default"] + + lockfile_content = project.lockfile_content + if all_dev: - if ( - "dev-packages" not in project.parsed_pipfile - and not project_pkg_names["develop"] - ): - click.echo( - click.style( - "No {} to uninstall.".format( - click.style("[dev-packages]", fg="yellow") - ), - bold=True, - ) - ) - return click.secho( click.style( - "Un-installing {}...".format(click.style("[dev-packages]", fg="yellow")), - bold=True, - ) - ) - preserve_packages = set() - dev_packages = set() - for category in project.get_package_categories(for_lockfile=True): - if category == "develop": - dev_packages |= set(project_pkg_names[category]) - else: - preserve_packages |= set(project_pkg_names[category]) - - package_names = dev_packages - preserve_packages - - # Remove known "bad packages" from the list. - bad_pkgs = get_canonical_names(BAD_PACKAGES) - ignored_packages = bad_pkgs & set(package_map) - for ignored_pkg in get_canonical_names(ignored_packages): - if project.s.is_verbose(): - click.echo(f"Ignoring {ignored_pkg}.", err=True) - package_names.discard(package_map[ignored_pkg]) - - used_packages = project_pkg_names["combined"] & installed_package_names - failure = False - if all: - click.echo( - click.style( - "Un-installing all {} and {}...".format( - click.style("[dev-packages]", fg="yellow"), - click.style("[packages]", fg="yellow"), + "Un-installing all {}...".format( + click.style("[dev-packages]", fg="yellow") ), bold=True, ) ) - do_purge(project, bare=False, allow_global=system) - sys.exit(0) + # Uninstall all dev-packages from environment + for package in project.get_pipfile_section("dev-packages"): + _uninstall_from_environment(project, package, system=system) + # Remove the package from the Pipfile + if project.reset_category_in_pipfile(category="dev-packages"): + click.echo("Removed [dev-packages] from Pipfile.") + # Finalize changes to lockfile + lockfile_content["develop"] = {} + lockfile_content.update({"_meta": project.get_lockfile_meta()}) + project.write_lockfile(lockfile_content) - selected_pkg_map = {canonicalize_name(p): p for p in package_names} - packages_to_remove = [ - package_name - for normalized, package_name in selected_pkg_map.items() - if normalized in (used_packages - bad_pkgs) - ] - lockfile = project.get_or_create_lockfile(categories=categories) + if all: + click.secho( + click.style( + "Un-installing all {}...".format(click.style("[packages]", fg="yellow")), + bold=True, + ) + ) + # Uninstall all dev-packages from environment + for package in project.get_pipfile_section("packages"): + _uninstall_from_environment(project, package, system=system) + # Remove the package from the Pipfile + if project.reset_category_in_pipfile(category="packages"): + click.echo("Removed [packages] from Pipfile.") + + # Finalize changes to lockfile + lockfile_content["default"] = {} + lockfile_content.update({"_meta": project.get_lockfile_meta()}) + project.write_lockfile(lockfile_content) + + package_args = list(packages) + [f"-e {pkg}" for pkg in editable_packages] + + # Determine packages and their dependencies for removal for category in categories: - category = get_lockfile_section_using_pipfile_category(category) - for normalized_name, package_name in selected_pkg_map.items(): - if normalized_name in project.lockfile_content[category]: - click.echo( - "{} {} {} {}".format( - click.style("Removing", fg="cyan"), - click.style(package_name, fg="green"), - click.style("from", fg="cyan"), - click.style("Pipfile.lock...", fg="white"), - ) - ) - if normalized_name in lockfile[category]: - del lockfile[category][normalized_name] - lockfile.write() + category = get_lockfile_section_using_pipfile_category( + category + ) # In case they passed pipfile category + pipfile_category = get_pipfile_category_using_lockfile_section(category) - pipfile_category = get_pipfile_category_using_lockfile_section(category) + for package in package_args[:]: + install_req, _ = expansive_install_req_from_line(package, expand_env=True) + name, normalized_name, pipfile_entry = project.generate_package_pipfile_entry( + install_req, package, category=pipfile_category + ) + + # Remove the package from the Pipfile if project.remove_package_from_pipfile( - package_name, category=pipfile_category + normalized_name, category=pipfile_category ): - click.secho( - f"Removed {package_name} from Pipfile category {pipfile_category}", - fg="green", - ) + click.echo(f"Removed {normalized_name} from Pipfile.") - for normalized_name, package_name in selected_pkg_map.items(): - still_remains = False - for category in project.get_package_categories(): - if project.get_package_name_in_pipfile(normalized_name, category=category): - still_remains = True - if not still_remains: - # Uninstall the package. - if package_name in packages_to_remove: - click.secho( - f"Uninstalling {click.style(package_name)}...", - fg="green", - bold=True, - ) - with project.environment.activated(): - cmd = [ - project_python(project, system=system), - get_runnable_pip(), - "uninstall", - package_name, - "-y", + # Rebuild the dependencies for resolution from the updated Pipfile + updated_packages = project.get_pipfile_section(pipfile_category) + + # Resolve dependencies with the package removed + resolved_lock_data = venv_resolve_deps( + updated_packages, + which=project._which, + project=project, + lockfile={}, + category=pipfile_category, + pre=pre, + allow_global=system, + pypi_mirror=pypi_mirror, + ) + + # Determine which dependencies are no longer needed + try: + current_lock_data = lockfile_content[category] + if current_lock_data: + deps_to_remove = [ + dep for dep in current_lock_data if dep not in resolved_lock_data ] - c = run_command(cmd, is_verbose=project.s.is_verbose()) - click.secho(c.stdout, fg="cyan") - if c.returncode != 0: - failure = True + # Remove unnecessary dependencies from Pipfile and lockfile + for dep in deps_to_remove: + if ( + category in lockfile_content + and dep in lockfile_content[category] + ): + del lockfile_content[category][dep] + except KeyError: + pass # No lockfile data for this category + + # Finalize changes to lockfile + lockfile_content.update({"_meta": project.get_lockfile_meta()}) + project.write_lockfile(lockfile_content) + + # Perform uninstallation of packages and dependencies + failure = False + for package in package_args: + _uninstall_from_environment(project, package, system=system) if lock: do_lock(project, system=system, pypi_mirror=pypi_mirror) + sys.exit(int(failure)) diff --git a/pipenv/utils/resolver.py b/pipenv/utils/resolver.py index a8c313e5..59f36009 100644 --- a/pipenv/utils/resolver.py +++ b/pipenv/utils/resolver.py @@ -771,10 +771,10 @@ def venv_resolve_deps( if not deps: if not project.pipfile_exists: - return None + return {} deps = project.parsed_pipfile.get(category, {}) if not deps: - return None + return {} if not pipfile: pipfile = getattr(project, category, {}) diff --git a/tests/integration/test_uninstall.py b/tests/integration/test_uninstall.py index 017a7120..e8a6bf53 100644 --- a/tests/integration/test_uninstall.py +++ b/tests/integration/test_uninstall.py @@ -90,11 +90,8 @@ def test_uninstall_all_local_files(pipenv_instance_private_pypi, testsroot): c = p.pipenv(f"install {file_uri}") assert c.returncode == 0 c = p.pipenv("uninstall --all") - assert c.returncode == 0 - assert "tablib" in c.stdout - # Uninstall --all is not supposed to remove things from the pipfile - # Note that it didn't before, but that instead local filenames showed as hashes - assert "tablib" in p.pipfile["packages"] + assert "tablib" not in p.pipfile["packages"] + assert "tablib" not in p.lockfile["default"] @pytest.mark.install @@ -218,7 +215,7 @@ def test_uninstall_category_with_shared_requirement(pipenv_instance_pypi): c = p.pipenv("install") assert c.returncode == 0 - c = p.pipenv("uninstall six --categories packages") + c = p.pipenv("uninstall six --categories default") assert c.returncode == 0 assert "six" in p.lockfile["prereq"]