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/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/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 7d3aa69495b..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, @@ -416,6 +415,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 @@ -423,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. @@ -471,12 +489,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,21 +510,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) - 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 6088bacc1af..b23b2268a54 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -33,10 +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, - has_delete_marker_file, -) from pip._internal.utils.misc import ( ask_path_exists, backup_dir, @@ -46,7 +42,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 @@ -348,8 +343,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 @@ -372,7 +367,16 @@ 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) + # `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=delete_arg, + kind=tempdir_kinds.REQ_BUILD, + globally_managed=True, + ).path def _set_requirement(self): # type: () -> None @@ -412,16 +416,6 @@ def warn_on_mismatching_name(self): ) self.req = Requirement(metadata_name) - 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 - def check_if_exists(self, use_user_site): # type: (bool) -> None """Find an installed distribution that satisfies or conflicts @@ -599,8 +593,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 @@ -611,7 +605,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): @@ -755,8 +751,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/req/req_set.py b/src/pip/_internal/req/req_set.py index 1312622b87b..df5fbee08dc 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: @@ -34,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 @@ -162,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 " @@ -199,11 +196,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 - """Clean up files, remove builds.""" - logger.debug('Cleaning up...') - with indent_log(): - for req in self.reqs_to_cleanup: - req.remove_temporary_source() 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) 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 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))