Repository: pypa/pip
Pull request: Make uninstall less verbose
Closes #4229



Problem statement:
"pip uninstall" is too noisy
* Pip version: 9.0.1
* Python version: 3.6.0
* Operating System: Arch Linux

### Description:

`pip uninstall` is too noisy.

It should list the package(s) and its directory/location, but instead it lists
all the files in there:

```
% pip uninstall pip
Uninstalling pip-9.0.1:
  /home/user/.local/bin/pip
  /home/user/.local/bin/pip3
  /home/user/.local/bin/pip3.5
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/DESCRIPTION.rst
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/INSTALLER
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/METADATA
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/RECORD
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/WHEEL
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/entry_points.txt
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/metadata.json
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/top_level.txt
  /home/user/.local/lib/python3.5/site-packages/pip/…

  [almost 500 extra lines]

  /home/user/.local/lib/python3.5/site-packages/pip/vcs/bazaar.py
  /home/user/.local/lib/python3.5/site-packages/pip/vcs/git.py
  /home/user/.local/lib/python3.5/site-packages/pip/vcs/mercurial.py
  /home/user/.local/lib/python3.5/site-packages/pip/vcs/subversion.py
  /home/user/.local/lib/python3.5/site-packages/pip/wheel.py
Proceed (y/n)? y
  Successfully uninstalled pip-9.0.1
You are using pip version 8.1.1, however version 9.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
```

Hints from issue discussion:
Agreed. `pip` should probably list just the directories and scripts it would remove, instead of *all* the files.
Hello ! 

What do you think about this display ? 

```
$ pip uninstall hypothesis
Uninstalling hypothesis-3.5.0:
Proceed (y/n)? y
  Successfully uninstalled hypothesis-3.5.0
```

And we can get `verbose` version with `-v` (`--verbose`)
@AwesomeTurtle That's too terse. I'd suggest we need to at least list the number of files we'll be removing, and the top-level directories/files. Something like

```
$ pip uninstall pip
Uninstalling pip-9.0.1 (500 files):
  /home/user/.local/bin/pip
  /home/user/.local/bin/pip3
  /home/user/.local/bin/pip3.5
  /home/user/.local/lib/python3.5/site-packages/pip-9.0.1.dist-info/*
  /home/user/.local/lib/python3.5/site-packages/pip/*
Proceed (y/n)? y
  Successfully uninstalled pip-9.0.1
```

As you say, ```--verbose``` can give the full list of files.
I have this display

```
# pip uninstall pip
Uninstalling pip-9.1.0.dev0 (503 files):
  /usr/local/bin/pip
  /usr/local/bin/pip3
  /usr/local/bin/pip3.6
  /usr/local/lib/python3.6/site-packages/pip-9.1.0.dev0-py3.6.egg-info/*
  /usr/local/lib/python3.6/site-packages/pip/*
Proceed (y/n)?
```

Example with Django: 

```
# pip uninstall django
Uninstalling Django-1.10.5 (4036 files):
  /usr/local/bin/__pycache__/django-admin.cpython-36.pyc
  /usr/local/bin/django-admin
  /usr/local/bin/django-admin.py
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/DESCRIPTION.rst/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/INSTALLER/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/LICENSE.txt/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/METADATA/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/RECORD/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/WHEEL/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/entry_points.txt/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/metadata.json/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/top_level.txt/*
  /usr/local/lib/python3.6/site-packages/Django/*
Proceed (y/n)?
```





Ooops, there is a bug with Django ! 
```
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/DESCRIPTION.rst/*
  /usr/local/lib/python3.6/site-packages/Django-1.10.5.dist-info/INSTALLER/*
```

Huh? That wasn't what I was suggesting. There should be just one entry for the ```dist-info``` directory.

Of course, this is all just proposal at this point. Until someone creates a PR, we're just speculating.
I suggest only to show root directory of package:

>$ pip uninstall pip
>Uninstalling pip-9.0.1 (500 files):
>  /home/user/.local/bin/pip/*
>Proceed (y/n)? y
> Successfully uninstalled pip-9.0.1

By this message you know number of files and package directory which is being deleted


@ibakirov There isn't just one root directory involved, in general. There's a ```.dist-info``` directory as well, and in theory could be arbitrary locations in the wheel (for example ```setuptools``` installs both a ```setuptools``` directory and a ```pkg_resources.py```, and some wrapper executables in ```.../bin``` as well as its ```.dist-info``` directory). Hence my suggestion of all top-level directories or files, plus the total number of files

Commit message: :art:

Patch to review:
diff --git a/news/4493.feature b/news/4493.feature
new file mode 100644
index 00000000000..78654bf836b
--- /dev/null
+++ b/news/4493.feature
@@ -0,0 +1 @@
+Make uninstall command less verbose by default
diff --git a/pip/commands/uninstall.py b/pip/commands/uninstall.py
index 130bec9582e..d1bbc534560 100644
--- a/pip/commands/uninstall.py
+++ b/pip/commands/uninstall.py
@@ -64,5 +64,7 @@ def run(self, options, args):
                     '"pip help %(name)s")' % dict(name=self.name)
                 )
             for req in reqs_to_uninstall.values():
-                req.uninstall(auto_confirm=options.yes)
+                req.uninstall(
+                    auto_confirm=options.yes, verbose=options.verbose != 0
+                )
                 req.uninstalled_pathset.commit()
diff --git a/pip/req/req_install.py b/pip/req/req_install.py
index 6c7fe4f7514..a788d520c94 100644
--- a/pip/req/req_install.py
+++ b/pip/req/req_install.py
@@ -623,7 +623,7 @@ def update_editable(self, obtain=True):
                 'Unexpected version control type (in %s): %s'
                 % (self.link, vc_type))
 
-    def uninstall(self, auto_confirm=False):
+    def uninstall(self, auto_confirm=False, verbose=False):
         """
         Uninstall the distribution currently satisfying this requirement.
 
@@ -643,7 +643,7 @@ def uninstall(self, auto_confirm=False):
         dist = self.satisfied_by or self.conflicts_with
 
         self.uninstalled_pathset = UninstallPathSet.from_dist(dist)
-        self.uninstalled_pathset.remove(auto_confirm)
+        self.uninstalled_pathset.remove(auto_confirm, verbose)
 
     def archive(self, build_dir):
         assert self.source_dir
diff --git a/pip/req/req_uninstall.py b/pip/req/req_uninstall.py
index 39809fe0b75..4d5d5beeb71 100644
--- a/pip/req/req_uninstall.py
+++ b/pip/req/req_uninstall.py
@@ -131,12 +131,16 @@ def compact(self, paths):
         necessary to contain all paths in the set. If /a/path/ and
         /a/path/to/a/file.txt are both in the set, leave only the
         shorter path."""
+
+        sep = os.path.sep
         short_paths = set()
         for path in sorted(paths, key=len):
-            if not any([
-                    (path.startswith(shortpath) and
-                     path[len(shortpath.rstrip(os.path.sep))] == os.path.sep)
-                    for shortpath in short_paths]):
+            should_add = any(
+                path.startswith(shortpath.rstrip("*")) and
+                path[len(shortpath.rstrip("*").rstrip(sep))] == sep
+                for shortpath in short_paths
+            )
+            if not should_add:
                 short_paths.add(path)
         return short_paths
 
@@ -144,47 +148,61 @@ def _stash(self, path):
         return os.path.join(
             self.save_dir, os.path.splitdrive(path)[1].lstrip(os.path.sep))
 
-    def remove(self, auto_confirm=False):
+    def _display_neatly(self, paths, verbose):
+        if not verbose:
+            new_paths = []
+            for path in paths:
+                if path.endswith("__init__.py"):
+                    new_paths.append(path[:-12] + os.path.sep + "*")
+                elif ".dist-info" in path:
+                    new_paths.append(os.path.dirname(path) + os.path.sep + "*")
+                else:
+                    new_paths.append(path)
+            paths = new_paths
+
+        for path in sorted(self.compact(paths)):
+            logger.info(path)
+
+    def remove(self, auto_confirm=False, verbose=False):
         """Remove paths in ``self.paths`` with confirmation (unless
         ``auto_confirm`` is True)."""
+
         if not self.paths:
             logger.info(
                 "Can't uninstall '%s'. No files were found to uninstall.",
                 self.dist.project_name,
             )
             return
-        logger.info(
-            'Uninstalling %s-%s:',
-            self.dist.project_name, self.dist.version
+
+        dist_name_version = (
+            self.dist.project_name + "-" + self.dist.version
         )
+        logger.info('Uninstalling %s:', dist_name_version)
 
         with indent_log():
-            paths = sorted(self.compact(self.paths))
-
             if auto_confirm:
                 response = 'y'
             else:
-                for path in paths:
-                    logger.info(path)
+                self._display_neatly(self.paths, verbose)
                 response = ask('Proceed (y/n)? ', ('y', 'n'))
+
             if self._refuse:
                 logger.info('Not removing or modifying (outside of prefix):')
-                for path in self.compact(self._refuse):
-                    logger.info(path)
+                self._display_neatly(self._refuse, verbose)
+
             if response == 'y':
                 self.save_dir = tempfile.mkdtemp(suffix='-uninstall',
                                                  prefix='pip-')
-                for path in paths:
+
+                for path in sorted(self.compact(self.paths)):
                     new_path = self._stash(path)
                     logger.debug('Removing file or directory %s', path)
                     self._moved_paths.append(path)
                     renames(path, new_path)
                 for pth in self.pth.values():
                     pth.remove()
-                logger.info(
-                    'Successfully uninstalled %s-%s',
-                    self.dist.project_name, self.dist.version
-                )
+
+                logger.info('Successfully uninstalled %s', dist_name_version)
 
     def rollback(self):
         """Rollback the changes previously made by remove()."""


Code review comments:
diff_hunk: @@ -131,60 +131,78 @@ def compact(self, paths):
         necessary to contain all paths in the set. If /a/path/ and
         /a/path/to/a/file.txt are both in the set, leave only the
         shorter path."""
+
+        sep = os.path.sep
         short_paths = set()
         for path in sorted(paths, key=len):
-            if not any([
-                    (path.startswith(shortpath) and
-                     path[len(shortpath.rstrip(os.path.sep))] == os.path.sep)
-                    for shortpath in short_paths]):
+            should_add = any(
+                path.startswith(shortpath.rstrip("*")) and
+                path[len(shortpath.rstrip("*").rstrip(sep))] == sep
+                for shortpath in short_paths
+            )
+            if not should_add:
                 short_paths.add(path)
         return short_paths
 
     def _stash(self, path):
         return os.path.join(
             self.save_dir, os.path.splitdrive(path)[1].lstrip(os.path.sep))
 
-    def remove(self, auto_confirm=False):
+    def _display_neatly(self, paths, verbose):
+        if not verbose:
+            new_paths = []
+            for path in paths:
+                if path.endswith("__init__.py"):
+                    new_paths.append(path[:-12] + os.path.sep + "*")
path: pip/req/req_uninstall.py
text: @user1:
os.path.dirname would be cleaner :o

@author:
Indeed.