From c7cb3cd81d73739fd8179526b6a2e114a6f9e208 Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:04:38 -0500
Subject: [PATCH 1/9] Calculate autodelete_unpacked earlier in
 prepare_linked_requirement

We want to use this value to determine whether a globally-managed
source_dir should delegate choosing deletion to the global tempdir
manager, so it needs to be above our call to
InstallRequirement.ensure_has_source_dir.
---
 src/pip/_internal/operations/prepare.py | 37 +++++++++++++------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py
index 7d3aa69495b..3038cb5503d 100644
--- a/src/pip/_internal/operations/prepare.py
+++ b/src/pip/_internal/operations/prepare.py
@@ -416,6 +416,25 @@ def prepare_linked_requirement(
         else:
             logger.info('Collecting %s', req.req or req)
 
+        download_dir = self.download_dir
+        if link.is_wheel and self.wheel_download_dir:
+            # when doing 'pip wheel` we download wheels to a
+            # dedicated dir.
+            download_dir = self.wheel_download_dir
+
+        if link.is_wheel:
+            if download_dir:
+                # When downloading, we only unpack wheels to get
+                # metadata.
+                autodelete_unpacked = True
+            else:
+                # When installing a wheel, we use the unpacked
+                # wheel.
+                autodelete_unpacked = False
+        else:
+            # We always delete unpacked sdists after pip runs.
+            autodelete_unpacked = True
+
         with indent_log():
             # @@ if filesystem packages are not marked
             # editable in a req, a non deterministic error
@@ -471,12 +490,6 @@ def prepare_linked_requirement(
                 # showing the user what the hash should be.
                 hashes = MissingHashes()
 
-            download_dir = self.download_dir
-            if link.is_wheel and self.wheel_download_dir:
-                # when doing 'pip wheel` we download wheels to a
-                # dedicated dir.
-                download_dir = self.wheel_download_dir
-
             try:
                 local_file = unpack_url(
                     link, req.source_dir, self.downloader, download_dir,
@@ -498,18 +511,6 @@ def prepare_linked_requirement(
             if local_file:
                 req.local_file_path = local_file.path
 
-            if link.is_wheel:
-                if download_dir:
-                    # When downloading, we only unpack wheels to get
-                    # metadata.
-                    autodelete_unpacked = True
-                else:
-                    # When installing a wheel, we use the unpacked
-                    # wheel.
-                    autodelete_unpacked = False
-            else:
-                # We always delete unpacked sdists after pip runs.
-                autodelete_unpacked = True
             if autodelete_unpacked:
                 write_delete_marker_file(req.source_dir)
 

From 470c39990e71f16cb75fb72c68b675564ea873d0 Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:13:23 -0500
Subject: [PATCH 2/9] Wrap InstallRequirement.ensure_build_location in
 TempDirectory

Since we explicitly disable deletion this is a no-op, but we'll
parameterize the deletion soon.
---
 src/pip/_internal/req/req_install.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py
index 6088bacc1af..4c58ac0167c 100644
--- a/src/pip/_internal/req/req_install.py
+++ b/src/pip/_internal/req/req_install.py
@@ -372,7 +372,13 @@ def ensure_build_location(self, build_dir):
         if not os.path.exists(build_dir):
             logger.debug('Creating directory %s', build_dir)
             os.makedirs(build_dir)
-        return os.path.join(build_dir, name)
+        actual_build_dir = os.path.join(build_dir, name)
+        return TempDirectory(
+            path=actual_build_dir,
+            delete=False,
+            kind=tempdir_kinds.REQ_BUILD,
+            globally_managed=True,
+        ).path
 
     def _set_requirement(self):
         # type: () -> None

From 8197a4bbc553258e0b78bb8506520b047b5b1fa9 Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:26:11 -0500
Subject: [PATCH 3/9] Do not write delete marker file to track source_dir
 delete preference

Previously we were writing a delete marker file which is checked in
InstallRequirement.remove_temporary_source which is only invoked if the
user did not pass --no-clean (and a PreviousBuildDirError was not
raised). Since our TempDirectory machinery now respects these conditions
we can just wrap our source directory in that instead of using this
ad-hoc mechanism for tracking our delete preference.

This will let us clean up a lot of dead code that only existed for this
use case.
---
 src/pip/_internal/operations/prepare.py |  6 +-----
 src/pip/_internal/req/req_install.py    | 17 +++++++++++------
 tests/unit/test_req_install.py          |  4 +++-
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py
index 3038cb5503d..f32517e50c7 100644
--- a/src/pip/_internal/operations/prepare.py
+++ b/src/pip/_internal/operations/prepare.py
@@ -28,7 +28,6 @@
 from pip._internal.utils.filesystem import copy2_fixed
 from pip._internal.utils.hashes import MissingHashes
 from pip._internal.utils.logging import indent_log
-from pip._internal.utils.marker_files import write_delete_marker_file
 from pip._internal.utils.misc import (
     ask_path_exists,
     backup_dir,
@@ -442,7 +441,7 @@ def prepare_linked_requirement(
             # build directory
             # Since source_dir is only set for editable requirements.
             assert req.source_dir is None
-            req.ensure_has_source_dir(self.build_dir)
+            req.ensure_has_source_dir(self.build_dir, autodelete_unpacked)
             # If a checkout exists, it's unwise to keep going.  version
             # inconsistencies are logged later, but do not fail the
             # installation.
@@ -511,9 +510,6 @@ def prepare_linked_requirement(
             if local_file:
                 req.local_file_path = local_file.path
 
-            if autodelete_unpacked:
-                write_delete_marker_file(req.source_dir)
-
             abstract_dist = _get_prepared_distribution(
                 req, self.req_tracker, self.finder, self.build_isolation,
             )
diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py
index 4c58ac0167c..b8aadb5cd0a 100644
--- a/src/pip/_internal/req/req_install.py
+++ b/src/pip/_internal/req/req_install.py
@@ -348,8 +348,8 @@ def from_path(self):
                 s += '->' + comes_from
         return s
 
-    def ensure_build_location(self, build_dir):
-        # type: (str) -> str
+    def ensure_build_location(self, build_dir, autodelete):
+        # type: (str, bool) -> str
         assert build_dir is not None
         if self._temp_build_dir is not None:
             assert self._temp_build_dir.path
@@ -373,9 +373,12 @@ def ensure_build_location(self, build_dir):
             logger.debug('Creating directory %s', build_dir)
             os.makedirs(build_dir)
         actual_build_dir = os.path.join(build_dir, name)
+        # `None` indicates that we respect the globally-configured deletion
+        # settings, which is what we actually want when auto-deleting.
+        delete_arg = None if autodelete else False
         return TempDirectory(
             path=actual_build_dir,
-            delete=False,
+            delete=delete_arg,
             kind=tempdir_kinds.REQ_BUILD,
             globally_managed=True,
         ).path
@@ -605,8 +608,8 @@ def assert_source_matches_version(self):
             )
 
     # For both source distributions and editables
-    def ensure_has_source_dir(self, parent_dir):
-        # type: (str) -> None
+    def ensure_has_source_dir(self, parent_dir, autodelete=False):
+        # type: (str, bool) -> None
         """Ensure that a source_dir is set.
 
         This will create a temporary build dir if the name of the requirement
@@ -617,7 +620,9 @@ def ensure_has_source_dir(self, parent_dir):
         :return: self.source_dir
         """
         if self.source_dir is None:
-            self.source_dir = self.ensure_build_location(parent_dir)
+            self.source_dir = self.ensure_build_location(
+                parent_dir, autodelete
+            )
 
     # For editable installations
     def update_editable(self, obtain=True):
diff --git a/tests/unit/test_req_install.py b/tests/unit/test_req_install.py
index a8eae8249bb..c3482d5360a 100644
--- a/tests/unit/test_req_install.py
+++ b/tests/unit/test_req_install.py
@@ -20,7 +20,9 @@ def test_tmp_build_directory(self):
         # Make sure we're handling it correctly with real path.
         requirement = InstallRequirement(None, None)
         tmp_dir = tempfile.mkdtemp('-build', 'pip-')
-        tmp_build_dir = requirement.ensure_build_location(tmp_dir)
+        tmp_build_dir = requirement.ensure_build_location(
+            tmp_dir, autodelete=False
+        )
         assert (
             os.path.dirname(tmp_build_dir) ==
             os.path.realpath(os.path.dirname(tmp_dir))

From e6cc9e9351d66a3dc27dc2f0e31167d88162d762 Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:30:05 -0500
Subject: [PATCH 4/9] Do not remove source directory in
 cleanup_temporary_source

Since nothing in our code writes the delete marker file, this block will
never execute.
---
 src/pip/_internal/req/req_install.py | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py
index b8aadb5cd0a..3bf879ce5f0 100644
--- a/src/pip/_internal/req/req_install.py
+++ b/src/pip/_internal/req/req_install.py
@@ -33,10 +33,7 @@
 from pip._internal.utils.deprecation import deprecated
 from pip._internal.utils.hashes import Hashes
 from pip._internal.utils.logging import indent_log
-from pip._internal.utils.marker_files import (
-    PIP_DELETE_MARKER_FILENAME,
-    has_delete_marker_file,
-)
+from pip._internal.utils.marker_files import PIP_DELETE_MARKER_FILENAME
 from pip._internal.utils.misc import (
     ask_path_exists,
     backup_dir,
@@ -46,7 +43,6 @@
     get_installed_version,
     hide_url,
     redact_auth_from_url,
-    rmtree,
 )
 from pip._internal.utils.packaging import get_metadata
 from pip._internal.utils.temp_dir import TempDirectory, tempdir_kinds
@@ -423,11 +419,6 @@ def warn_on_mismatching_name(self):
 
     def remove_temporary_source(self):
         # type: () -> None
-        """Remove the source files from this requirement, if they are marked
-        for deletion"""
-        if self.source_dir and has_delete_marker_file(self.source_dir):
-            logger.debug('Removing source in %s', self.source_dir)
-            rmtree(self.source_dir)
         self.source_dir = None
         self._temp_build_dir = None
 

From 98cd1937278c49269e5bc71896f7d8a958e2a004 Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:30:59 -0500
Subject: [PATCH 5/9] Remove delete_marker_file writing in tests

Nothing checks for this file, so no need to write it.
---
 tests/functional/test_install_cleanup.py | 2 --
 tests/functional/test_wheel.py           | 2 --
 2 files changed, 4 deletions(-)

diff --git a/tests/functional/test_install_cleanup.py b/tests/functional/test_install_cleanup.py
index 8810402f416..b07c809079b 100644
--- a/tests/functional/test_install_cleanup.py
+++ b/tests/functional/test_install_cleanup.py
@@ -4,7 +4,6 @@
 import pytest
 
 from pip._internal.cli.status_codes import PREVIOUS_BUILD_DIR_ERROR
-from pip._internal.utils.marker_files import write_delete_marker_file
 from tests.lib import need_mercurial, windows_workaround_7667
 from tests.lib.local_repos import local_checkout
 
@@ -126,7 +125,6 @@ def test_cleanup_prevented_upon_build_dir_exception(script, data):
     build = script.venv_path / 'build'
     build_simple = build / 'simple'
     os.makedirs(build_simple)
-    write_delete_marker_file(build_simple)
     build_simple.joinpath("setup.py").write_text("#")
     result = script.pip(
         'install', '-f', data.find_links, '--no-index', 'simple',
diff --git a/tests/functional/test_wheel.py b/tests/functional/test_wheel.py
index 792f5fe6c10..181ca05845b 100644
--- a/tests/functional/test_wheel.py
+++ b/tests/functional/test_wheel.py
@@ -6,7 +6,6 @@
 import pytest
 
 from pip._internal.cli.status_codes import ERROR, PREVIOUS_BUILD_DIR_ERROR
-from pip._internal.utils.marker_files import write_delete_marker_file
 from tests.lib import pyversion
 
 
@@ -225,7 +224,6 @@ def test_pip_wheel_fail_cause_of_previous_build_dir(script, data):
     # Given that I have a previous build dir of the `simple` package
     build = script.venv_path / 'build' / 'simple'
     os.makedirs(build)
-    write_delete_marker_file(script.venv_path / 'build' / 'simple')
     build.joinpath('setup.py').write_text('#')
 
     # When I call pip trying to install things again

From efe663d476132f2f1668079dd647469d44a8032b Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:32:37 -0500
Subject: [PATCH 6/9] Remove unused utils.marker_files

---
 src/pip/_internal/req/req_install.py    |  3 ---
 src/pip/_internal/utils/marker_files.py | 25 -------------------------
 2 files changed, 28 deletions(-)
 delete mode 100644 src/pip/_internal/utils/marker_files.py

diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py
index 3bf879ce5f0..dc2a6cd94a2 100644
--- a/src/pip/_internal/req/req_install.py
+++ b/src/pip/_internal/req/req_install.py
@@ -33,7 +33,6 @@
 from pip._internal.utils.deprecation import deprecated
 from pip._internal.utils.hashes import Hashes
 from pip._internal.utils.logging import indent_log
-from pip._internal.utils.marker_files import PIP_DELETE_MARKER_FILENAME
 from pip._internal.utils.misc import (
     ask_path_exists,
     backup_dir,
@@ -757,8 +756,6 @@ def archive(self, build_dir):
                     zipdir.external_attr = 0x1ED << 16  # 0o755
                     zip_output.writestr(zipdir, '')
                 for filename in filenames:
-                    if filename == PIP_DELETE_MARKER_FILENAME:
-                        continue
                     file_arcname = self._get_archive_name(
                         filename, parentdir=dirpath, rootdir=dir,
                     )
diff --git a/src/pip/_internal/utils/marker_files.py b/src/pip/_internal/utils/marker_files.py
deleted file mode 100644
index 42ea8140508..00000000000
--- a/src/pip/_internal/utils/marker_files.py
+++ /dev/null
@@ -1,25 +0,0 @@
-import os.path
-
-DELETE_MARKER_MESSAGE = '''\
-This file is placed here by pip to indicate the source was put
-here by pip.
-
-Once this package is successfully installed this source code will be
-deleted (unless you remove this file).
-'''
-PIP_DELETE_MARKER_FILENAME = 'pip-delete-this-directory.txt'
-
-
-def has_delete_marker_file(directory):
-    # type: (str) -> bool
-    return os.path.exists(os.path.join(directory, PIP_DELETE_MARKER_FILENAME))
-
-
-def write_delete_marker_file(directory):
-    # type: (str) -> None
-    """
-    Write the pip delete marker file into this directory.
-    """
-    filepath = os.path.join(directory, PIP_DELETE_MARKER_FILENAME)
-    with open(filepath, 'w') as marker_fp:
-        marker_fp.write(DELETE_MARKER_MESSAGE)

From 5cca8f10b304a5a7f3a96dfd66937615324cf826 Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:34:29 -0500
Subject: [PATCH 7/9] Remove InstallRequirement.remove_temporary_source

Since all directories are now globally-managed, we don't need to be
concerned with resetting the member values.

This will also let us remove several responsibilities from
RequirementSet, which will make integrating the new resolver easier.
---
 src/pip/_internal/req/req_install.py | 5 -----
 src/pip/_internal/req/req_set.py     | 7 +------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py
index dc2a6cd94a2..b23b2268a54 100644
--- a/src/pip/_internal/req/req_install.py
+++ b/src/pip/_internal/req/req_install.py
@@ -416,11 +416,6 @@ def warn_on_mismatching_name(self):
         )
         self.req = Requirement(metadata_name)
 
-    def remove_temporary_source(self):
-        # type: () -> None
-        self.source_dir = None
-        self._temp_build_dir = None
-
     def check_if_exists(self, use_user_site):
         # type: (bool) -> None
         """Find an installed distribution that satisfies or conflicts
diff --git a/src/pip/_internal/req/req_set.py b/src/pip/_internal/req/req_set.py
index 1312622b87b..fc1f3ec4927 100644
--- a/src/pip/_internal/req/req_set.py
+++ b/src/pip/_internal/req/req_set.py
@@ -11,7 +11,6 @@
 from pip._internal import pep425tags
 from pip._internal.exceptions import InstallationError
 from pip._internal.models.wheel import Wheel
-from pip._internal.utils.logging import indent_log
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
 if MYPY_CHECK_RUNNING:
@@ -202,8 +201,4 @@ def get_requirement(self, name):
 
     def cleanup_files(self):
         # type: () -> None
-        """Clean up files, remove builds."""
-        logger.debug('Cleaning up...')
-        with indent_log():
-            for req in self.reqs_to_cleanup:
-                req.remove_temporary_source()
+        pass

From 4a93045be187f77b4de7c17d03045fd7ec7f341d Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:37:14 -0500
Subject: [PATCH 8/9] Remove no-op RequirementSet.cleanup_files

---
 src/pip/_internal/commands/download.py | 4 ----
 src/pip/_internal/commands/install.py  | 4 ----
 src/pip/_internal/commands/wheel.py    | 3 ---
 src/pip/_internal/req/req_set.py       | 4 ----
 4 files changed, 15 deletions(-)

diff --git a/src/pip/_internal/commands/download.py b/src/pip/_internal/commands/download.py
index d1c59990a83..9c82c419646 100644
--- a/src/pip/_internal/commands/download.py
+++ b/src/pip/_internal/commands/download.py
@@ -140,8 +140,4 @@ def run(self, options, args):
             if downloaded:
                 write_output('Successfully downloaded %s', downloaded)
 
-            # Clean up
-            if not options.no_clean:
-                requirement_set.cleanup_files()
-
         return requirement_set
diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py
index ce0aa9d86fa..95c90499be0 100644
--- a/src/pip/_internal/commands/install.py
+++ b/src/pip/_internal/commands/install.py
@@ -443,10 +443,6 @@ def run(self, options, args):
             except PreviousBuildDirError:
                 options.no_clean = True
                 raise
-            finally:
-                # Clean up
-                if not options.no_clean:
-                    requirement_set.cleanup_files()
 
         if options.target_dir:
             self._handle_target_dir(
diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py
index 35ba2d9c268..8c03c6b82e4 100644
--- a/src/pip/_internal/commands/wheel.py
+++ b/src/pip/_internal/commands/wheel.py
@@ -191,6 +191,3 @@ def run(self, options, args):
             except PreviousBuildDirError:
                 options.no_clean = True
                 raise
-            finally:
-                if not options.no_clean:
-                    requirement_set.cleanup_files()
diff --git a/src/pip/_internal/req/req_set.py b/src/pip/_internal/req/req_set.py
index fc1f3ec4927..15d10007349 100644
--- a/src/pip/_internal/req/req_set.py
+++ b/src/pip/_internal/req/req_set.py
@@ -198,7 +198,3 @@ def get_requirement(self, name):
             return self.requirements[project_name]
 
         raise KeyError("No project with the name %r" % name)
-
-    def cleanup_files(self):
-        # type: () -> None
-        pass

From 85ab574dc1e61ad6ec42ab34643a7de48b08a3b8 Mon Sep 17 00:00:00 2001
From: Chris Hunt <chrahunt@gmail.com>
Date: Tue, 4 Feb 2020 22:38:47 -0500
Subject: [PATCH 9/9] Remove unused RequirementSet.reqs_to_cleanup

---
 src/pip/_internal/legacy_resolve.py | 3 ---
 src/pip/_internal/req/req_set.py    | 2 --
 2 files changed, 5 deletions(-)

diff --git a/src/pip/_internal/legacy_resolve.py b/src/pip/_internal/legacy_resolve.py
index ca269121b60..d05a4c063a0 100644
--- a/src/pip/_internal/legacy_resolve.py
+++ b/src/pip/_internal/legacy_resolve.py
@@ -327,9 +327,6 @@ def _resolve_one(
 
         req_to_install.prepared = True
 
-        # register tmp src for cleanup in case something goes wrong
-        requirement_set.reqs_to_cleanup.append(req_to_install)
-
         abstract_dist = self._get_abstract_dist_for(req_to_install)
 
         # Parse and return dependencies
diff --git a/src/pip/_internal/req/req_set.py b/src/pip/_internal/req/req_set.py
index 15d10007349..df5fbee08dc 100644
--- a/src/pip/_internal/req/req_set.py
+++ b/src/pip/_internal/req/req_set.py
@@ -33,7 +33,6 @@ def __init__(self, check_supported_wheels=True):
 
         self.unnamed_requirements = []  # type: List[InstallRequirement]
         self.successfully_downloaded = []  # type: List[InstallRequirement]
-        self.reqs_to_cleanup = []  # type: List[InstallRequirement]
 
     def __str__(self):
         # type: () -> str
@@ -161,7 +160,6 @@ def add_requirement(
             )
         )
         if does_not_satisfy_constraint:
-            self.reqs_to_cleanup.append(install_req)
             raise InstallationError(
                 "Could not satisfy constraints for '{}': "
                 "installation from path or url cannot be "