From 83af37a2d153d41a71cc463742d4e0f9e558cc51 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 1 Nov 2018 16:20:45 -0400 Subject: [PATCH] Fix uninstallation if comments are present Signed-off-by: Dan Ryan --- pipenv/core.py | 65 ++++++++++++++++++---------------- pipenv/project.py | 36 +++++++++++++++---- tests/integration/test_sync.py | 2 +- 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index 727de0e8..8c8d7e39 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1165,8 +1165,8 @@ def do_purge(bare=False, downloads=False, allow_global=False): if environments.is_verbose(): click.echo("$ {0}".format(command)) c = delegator.run(command) - if c.return_code != 0 or c.return_code == 0: - raise exceptions.UninstallError(installed, command, c.out + c.err, 1) + if c.return_code != 0: + raise exceptions.UninstallError(installed, command, c.out + c.err, c.return_code) if not bare: click.echo(crayons.blue(c.out)) click.echo(crayons.green("Environment now purged and fresh!")) @@ -2110,48 +2110,53 @@ def do_uninstall( del package_names[package_names.index( canonicalize_name(bad_package) )] - used_packages = (develop | default) & installed_package_names + used_packages = develop | default & installed_package_names failure = False packages_to_remove = set() if all_dev: packages_to_remove |= develop & installed_package_names - package_names = set([canonicalize_name(pkg_name) for pkg_name in package_names]) - packages_to_remove = package_names & used_packages - for package_name in packages_to_remove: + package_names = set([pkg_name for pkg_name in package_names]) + packages_to_remove = [ + pkg_name for pkg_name in packages + if canonicalize_name(pkg_name) in used_packages + ] + for package_name in package_names: click.echo( crayons.white( fix_utf8("Uninstalling {0}…".format(repr(package_name))), bold=True ) ) # Uninstall the package. - cmd = "{0} uninstall {1} -y".format( - escape_grouped_arguments(which_pip()), package_name - ) - if environments.is_verbose(): - click.echo("$ {0}".format(cmd)) - c = delegator.run(cmd) - click.echo(crayons.blue(c.out)) - if c.return_code != 0: - failure = True - else: - if pipfile_remove: - in_packages = project.get_package_name_in_pipfile(package_name, dev=False) - in_dev_packages = project.get_package_name_in_pipfile( - package_name, dev=True - ) - if not in_dev_packages and not in_packages: - click.echo( - "No package {0} to remove from Pipfile.".format( - crayons.green(package_name) - ) + if package_name in packages_to_remove: + cmd = "{0} uninstall {1} -y".format( + escape_grouped_arguments(which_pip()), package_name ) - continue - + if environments.is_verbose(): + click.echo("$ {0}".format(cmd)) + c = delegator.run(cmd) + click.echo(crayons.blue(c.out)) + if c.return_code != 0: + failure = True + if not failure and pipfile_remove: + in_packages = project.get_package_name_in_pipfile(package_name, dev=False) + in_dev_packages = project.get_package_name_in_pipfile( + package_name, dev=True + ) + if not in_dev_packages and not in_packages: click.echo( - fix_utf8("Removing {0} from Pipfile…".format(crayons.green(package_name))) + "No package {0} to remove from Pipfile.".format( + crayons.green(package_name) + ) ) - # Remove package from both packages and dev-packages. + continue + + click.echo( + fix_utf8("Removing {0} from Pipfile…".format(crayons.green(package_name))) + ) + # Remove package from both packages and dev-packages. + if in_dev_packages: project.remove_package_from_pipfile(package_name, dev=True) + if in_packages: project.remove_package_from_pipfile(package_name, dev=False) if lock: do_lock(system=system, keep_outdated=keep_outdated, pypi_mirror=pypi_mirror) diff --git a/pipenv/project.py b/pipenv/project.py index 9f441fb5..74debc24 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -916,26 +916,48 @@ class Project(object): # Read and append Pipfile. name = self.get_package_name_in_pipfile(package_name, dev) key = "dev-packages" if dev else "packages" - p = self._pipfile - if name: - del p.pipfile[key][name] + p = self.parsed_pipfile + lines = [l for l in p[key].serialized().splitlines()] + if not any(line.startswith("#") for line in lines) and name: + del p[key][name] + self.write_toml(p) + else: + p = self._pipfile + del p[key][name] p.write() def remove_packages_from_pipfile(self, packages): p = self._pipfile + parsed = self.parsed_pipfile packages = [pep423_name(pkg) for pkg in packages] deleted_pkgs = [] + has_comments_as_lines = False for section in ("dev-packages", "packages"): pipfile_section = self.parsed_pipfile.get(section, {}) + lines = [l for l in p[section].serialized().splitlines()] pipfile_packages = [ pkg_name for pkg_name in pipfile_section.keys() if pep423_name(pkg_name) in packages ] - for pkg in pipfile_packages: - deleted_pkgs.append(pkg) - del p.pipfile[section][pkg] + # The normal toml parser can't handle deleting packages with preceding newlines + is_dev = section == "dev-packages" + if any(line.startswith("#") for line in lines): + has_comments_as_lines = True + for pkg in pipfile_packages: + pkg_name = self.get_package_name_in_pipfile(pkg, dev=is_dev) + deleted_pkgs.append(pkg) + del p.pipfile[section][pkg_name] + # However the alternative parser can't handle inline comment preservation + else: + for pkg in pipfile_packages: + pkg_name = self.get_package_name_in_pipfile(pkg, dev=is_dev) + deleted_pkgs.append(pkg) + del parsed[section][pkg_name] if deleted_pkgs: - p.write() + if has_comments_as_lines: + p.write() + else: + self.write_toml(parsed) def add_package_to_pipfile(self, package, dev=False): from .vendor.requirementslib import Requirement diff --git a/tests/integration/test_sync.py b/tests/integration/test_sync.py index c50e259b..2ef06ddc 100644 --- a/tests/integration/test_sync.py +++ b/tests/integration/test_sync.py @@ -15,7 +15,7 @@ def test_sync_error_without_lockfile(PipenvInstance, pypi): c = p.pipenv('sync') assert c.return_code != 0 - assert 'Pipfile.lock is missing!' in c.err + assert 'Pipfile.lock not found!' in c.err @pytest.mark.sync