Handle temp dir cleanup errors gracefully

Instead of blowing up in the user's face, gracefully carry on with a
warning that the operation failed.

news/7567.feature

@@ -0,0 +1 @@
+Handle temporary directory cleanup errors more gracefully.

src/pip/_internal/utils/misc.py

@@ -6,6 +6,7 @@
 
 import contextlib
 import errno
+import functools
 import getpass
 import hashlib
 import io
@@ -133,13 +134,13 @@ def get_prog():
 
 # Retry every half second for up to 3 seconds
 @retry(stop_max_delay=3000, wait_fixed=500)
-def rmtree(dir, ignore_errors=False):
-    # type: (Text, bool) -> None
-    shutil.rmtree(dir, ignore_errors=ignore_errors,
-                  onerror=rmtree_errorhandler)
+def rmtree(dir, ignore_errors=False, onerror=None):
+    # type: (Text, bool, Optional[Callable[[Any, str, Any], Any]]) -> None
+    error_handler = functools.partial(rmtree_errorhandler, onerror=onerror)
+    shutil.rmtree(dir, ignore_errors=ignore_errors, onerror=error_handler)
 
 
-def rmtree_errorhandler(func, path, exc_info):
+def rmtree_errorhandler(func, path, exc_info, onerror=None):
     """On Windows, the files in .svn are read-only, so when rmtree() tries to
     remove them, an exception is thrown.  We catch that here, remove the
     read-only attribute, and hopefully continue without problems."""
@@ -152,9 +153,13 @@ def rmtree_errorhandler(func, path, exc_info):
     if has_attr_readonly:
         # convert to read/write
         os.chmod(path, stat.S_IWRITE)
-        # use the original function to repeat the operation
-        func(path)
+        # use the original function to repeat the operation, but skip this
+        # read-only check
+        func(path, onerror=onerror)
         return
+
+    if onerror:
+        onerror(func, path, exc_info)
     else:
         raise
 

src/pip/_internal/utils/temp_dir.py

@@ -188,6 +188,10 @@ def _create(self, kind):
         logger.debug("Created temporary directory: %s", path)
         return path
 
+    def _handle_cleanup_error(self, func, path, excinfo):
+        # type: (Any, str, Any) -> Any
+        logger.warning("Failed to clean up %s (%s)", path, excinfo[1])
+
     def cleanup(self):
         # type: () -> None
         """Remove the temporary directory created and reset state
@@ -196,7 +200,7 @@ def cleanup(self):
         if os.path.exists(self._path):
             # Make sure to pass unicode on Python 2 to make the contents also
             # use unicode, ensuring non-ASCII names and can be represented.
-            rmtree(ensure_text(self._path))
+            rmtree(ensure_text(self._path), onerror=self._handle_cleanup_error)
 
 
 class AdjacentTempDirectory(TempDirectory):

tests/unit/test_utils.py

@@ -357,7 +357,7 @@ def test_rmtree_errorhandler_readonly_directory(tmpdir):
     # Make sure mock_func is called with the given path
     mock_func = Mock()
     rmtree_errorhandler(mock_func, path, None)
-    mock_func.assert_called_with(path)
+    mock_func.assert_called_with(path, onerror=None)
 
     # Make sure the path is now writable
     assert os.stat(path).st_mode & stat.S_IWRITE

tests/unit/test_utils_temp_dir.py

@@ -1,4 +1,5 @@
 import itertools
+import logging
 import os
 import stat
 import tempfile
@@ -63,6 +64,35 @@ def readonly_file(*args):
         readonly_file(tmp_dir.path, "subfolder", "readonly-file")
 
 
+@pytest.mark.skipif("sys.platform != 'win32'")
+def test_undeletable_files_warning_on_windows(caplog):
+    caplog.set_level(logging.WARNING)
+
+    def create_file(*args):
+        fpath = os.path.join(*args)
+        ensure_dir(os.path.dirname(fpath))
+        with open(fpath, "w") as f:
+            f.write("Holla!")
+
+    open_f = None
+    try:
+        with TempDirectory() as tmp_dir:
+            with open(os.path.join(tmp_dir.path, "normal-file"), "w") as f:
+                f.write("normal")
+
+            # Intentionally leave this file open so the temp directory cleanup
+            # triggers a warning on Windows.
+            open_path = os.path.join(tmp_dir.path, "open-file")
+            open_f = open(open_path, "w")
+            open_f.write("open")
+            open_f.flush()
+
+        assert "Failed to clean up {}".format(open_path) in caplog.text
+    finally:
+        if open_f:
+            open_f.close()
+
+
 def test_path_access_after_context_raises():
     with TempDirectory() as tmp_dir:
         path = tmp_dir.path