Fix issue 7280 - issue warning after retry

news/7280.bugfix

@@ -0,0 +1,4 @@
+When pip is installing or removing a package and is unable to remove a temporary directory on Windows,
+this is likely due to a Virus Scan operation.  This fix warns the user and continues rather than
+preventing the operation.  pip still retries the removal, so this happens quite rarely in most Windows
+environments.

tests/lib/filesystem.py

@@ -1,9 +1,11 @@
 """Helpers for filesystem-dependent tests.
 """
+import multiprocessing
 import os
 import socket
 import subprocess
 import sys
+import traceback
 from functools import partial
 from itertools import chain
 
@@ -34,6 +36,89 @@ def make_unreadable_file(path):
         subprocess.check_call(args)
 
 
+def external_file_opener(conn):
+    """
+    This external process is run with multiprocessing.
+    It waits for a path from the parent, opens it, and then wait for another
+     message before closing it.
+
+    :param conn: bi-directional pipe
+    :return: nothing
+    """
+    f = None
+    try:
+        # Wait for parent to send path
+        msg = conn.recv()
+        if msg is True:
+            # Do nothing - we have been told to exit without a path or action
+            pass
+        else:
+            path = msg
+            # Open the file
+            try:
+                f = open(path, 'r')
+            except (OSError, IOError):
+                # IOError is OSError post PEP 3151
+                traceback.print_exc(None, sys.stderr)
+
+            # Indicate the file is opened
+            conn.send(True)
+            # Now path is open and we wait for signal to exit
+            conn.recv()
+    finally:
+        if f:
+            f.close()
+        conn.close()
+
+
+class FileOpener(object):
+    """
+    Test class acts as a context manager which can open a file from a
+    subprocess, and hold it open to assure that this does not interfere with
+    pip's operations.
+
+    If a path is passed to the FileOpener, it immediately sends a message to
+    the other process to open that path.  An action of "lock" or "noread" can
+    also be sent to the subprocess, resulting in various additional monkey
+    wrenches now and in the future.
+
+    Opening the path and taking the action can be deferred however, so that
+    the FileOpener may function as a pytest fixture if so desired.
+    """
+    def __init__(self, path=None):
+        self._sent = False
+        self.conn, child_conn = multiprocessing.Pipe()
+        self.child = multiprocessing.Process(
+            target=external_file_opener,
+            args=(child_conn,)
+        )
+        self.child.daemon = True
+        self.child.start()
+        if path:
+            self.send(path)
+
+    def send(self, path):
+        assert self._sent is False
+        self._sent = True
+        self.conn.send(str(path))
+        return self.conn.recv()
+
+    def cleanup(self):
+        # send a message to the child to exit
+        if self.child.is_alive():
+            self.conn.send(True)
+            self.child.join()
+            # Be sure the child is truly done
+            if self.child.is_alive():
+                self.child.terminate()
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.cleanup()
+
+
 def get_filelist(base):
     def join(dirpath, dirnames, filenames):
         relative_dirpath = os.path.relpath(dirpath, base)

tests/lib/test_lib_filesystem.py

@@ -0,0 +1,64 @@
+import errno
+import os
+import shutil
+
+import psutil
+import pytest
+
+from tests.lib.filesystem import FileOpener
+
+skip_unless_windows = pytest.mark.skipif("sys.platform != 'win32'")
+
+
+@pytest.fixture()
+def process():
+    return psutil.Process()
+
+
+def test_file_opener_no_file(process):
+    # FileOpener joins the subprocess even if the parent never sends the path
+    with FileOpener():
+        assert len(process.children()) == 1
+    assert len(process.children()) == 0
+
+
+def test_file_opener_not_found(tmpdir, process):
+    # The FileOpener cleans up the subprocess when the file cannot be opened
+    path = tmpdir.joinpath('foo.txt')
+    with FileOpener(path):
+        assert len(process.children()) == 1
+    assert len(process.children()) == 0
+
+
+def test_file_opener_normal(tmpdir, process):
+    # The FileOpener cleans up the subprocess when the file exists
+    path = tmpdir.joinpath('foo.txt')
+    path.write_text('Hello')
+    with FileOpener(path):
+        assert len(process.children()) == 1
+    assert len(process.children()) == 0
+
+
+@skip_unless_windows
+def test_file_opener_produces_unlink_error(tmpdir, process):
+    # FileOpener forces an error on Windows when we attempt to remove a file
+    # The initial path may be deferred; which must be tested with an error
+    path = tmpdir.joinpath('foo.txt')
+    path.write_text('Hello')
+    with FileOpener() as opener:
+        opener.send(path)
+        with pytest.raises(OSError) as e:
+            os.unlink(path)
+        assert e.value.errno == errno.EACCES
+
+
+@skip_unless_windows
+def test_file_opener_produces_rmtree_error(tmpdir, process):
+    subdir = tmpdir.joinpath('foo')
+    os.mkdir(subdir)
+    path = subdir.joinpath('bar.txt')
+    path.write_text('Hello')
+    with FileOpener(path):
+        with pytest.raises(OSError) as e:
+            shutil.rmtree(subdir)
+        assert e.value.errno == errno.EACCES

tests/unit/test_utils_temp_dir.py

@@ -1,7 +1,10 @@
 import itertools
+import logging
 import os
+import shutil
 import stat
 import tempfile
+import time
 
 import pytest
 
@@ -13,6 +16,7 @@
     global_tempdir_manager,
     tempdir_registry,
 )
+from tests.lib.filesystem import FileOpener
 
 
 # No need to test symlinked directories on Windows
@@ -210,6 +214,37 @@ def test_tempdirectory_asserts_global_tempdir(monkeypatch):
         TempDirectory(globally_managed=True)
 
 
+@pytest.mark.skipif("sys.platform != 'win32'")
+@pytest.mark.xfail()
+def test_temp_dir_warns_if_cannot_clean(caplog):
+    temp_dir = TempDirectory()
+    temp_dir_path = temp_dir.path
+
+    stime = time.time()
+
+    # Capture only at WARNING level and up
+    with caplog.at_level(logging.WARNING, 'pip._internal.utils.temp_dir'):
+        # open a file within the temporary directory in a sub-process
+        with FileOpener() as opener:
+            subpath = os.path.join(temp_dir_path, 'foo.txt')
+            with open(subpath, 'w') as f:
+                f.write('Cannot be deleted')
+            opener.send(subpath)
+            # with the file open, attempt to remove the log directory
+            temp_dir.cleanup()
+
+        # assert that a WARNING was logged about virus scanner
+        assert 'WARNING' in caplog.text
+        assert 'virus scanner' in caplog.text
+
+    # Assure that the cleanup was properly retried
+    duration = time.time() - stime
+    assert duration >= 2.0
+
+    # Clean-up for failed TempDirectory cleanup
+    shutil.rmtree(temp_dir_path, ignore_errors=True)
+
+
 deleted_kind = "deleted"
 not_deleted_kind = "not-deleted"
 

tools/requirements/tests.txt

@@ -2,6 +2,7 @@ cryptography==2.8
 freezegun
 mock
 pretend
+psutil
 pytest==3.8.2
 pytest-cov
 # Prevent installing 7.0 which has install_requires "pytest >= 3.10".