diff --git a/news/8827.bugfix.rst b/news/8827.bugfix.rst new file mode 100644 index 000000000..608cd3d5c --- /dev/null +++ b/news/8827.bugfix.rst @@ -0,0 +1,2 @@ +Avoid polluting the destination directory by resolution artifacts +when the new resolver is used for ``pip download`` or ``pip wheel``. diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 0757e34f6..03cc52f69 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -203,7 +203,6 @@ class RequirementCommand(IndexGroupCommand): finder, # type: PackageFinder use_user_site, # type: bool download_dir=None, # type: str - wheel_download_dir=None, # type: str ): # type: (...) -> RequirementPreparer """ @@ -229,7 +228,6 @@ class RequirementCommand(IndexGroupCommand): build_dir=temp_build_dir_path, src_dir=options.src_dir, download_dir=download_dir, - wheel_download_dir=wheel_download_dir, build_isolation=options.build_isolation, req_tracker=req_tracker, session=session, diff --git a/src/pip/_internal/commands/download.py b/src/pip/_internal/commands/download.py index 0861d9e67..2f151e049 100644 --- a/src/pip/_internal/commands/download.py +++ b/src/pip/_internal/commands/download.py @@ -87,7 +87,6 @@ class DownloadCommand(RequirementCommand): cmdoptions.check_dist_restriction(options) options.download_dir = normalize_path(options.download_dir) - ensure_dir(options.download_dir) session = self.get_default_session(options) @@ -138,6 +137,7 @@ class DownloadCommand(RequirementCommand): for req in requirement_set.requirements.values(): if not req.editable and req.satisfied_by is None: assert req.name is not None + preparer.save_linked_requirement(req) downloaded.append(req.name) if downloaded: write_output('Successfully downloaded %s', ' '.join(downloaded)) diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index 0f718566b..8f5783c35 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -21,6 +21,7 @@ if MYPY_CHECK_RUNNING: from optparse import Values from typing import List + from pip._internal.req.req_install import InstallRequirement logger = logging.getLogger(__name__) @@ -137,7 +138,7 @@ class WheelCommand(RequirementCommand): req_tracker=req_tracker, session=session, finder=finder, - wheel_download_dir=options.wheel_dir, + download_dir=options.wheel_dir, use_user_site=False, ) @@ -156,10 +157,12 @@ class WheelCommand(RequirementCommand): reqs, check_supported_wheels=True ) - reqs_to_build = [ - r for r in requirement_set.requirements.values() - if should_build_for_wheel_command(r) - ] + reqs_to_build = [] # type: List[InstallRequirement] + for req in requirement_set.requirements.values(): + if req.is_wheel: + preparer.save_linked_requirement(req) + elif should_build_for_wheel_command(req): + reqs_to_build.append(req) # build wheels build_successes, build_failures = build( diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 3060bafa1..de017504a 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -300,7 +300,6 @@ class RequirementPreparer(object): build_dir, # type: str download_dir, # type: Optional[str] src_dir, # type: str - wheel_download_dir, # type: Optional[str] build_isolation, # type: bool req_tracker, # type: RequirementTracker session, # type: PipSession @@ -325,16 +324,6 @@ class RequirementPreparer(object): # not saved, and are deleted immediately after unpacking. self.download_dir = download_dir - # Where still-packed .whl files should be written to. If None, they are - # written to the download_dir parameter. Separate to download_dir to - # permit only keeping wheel archives for pip wheel. - self.wheel_download_dir = wheel_download_dir - - # NOTE - # download_dir and wheel_download_dir overlap semantically and may - # be combined if we're willing to have non-wheel archives present in - # the wheelhouse output by 'pip wheel'. - # Is build isolation allowed? self.build_isolation = build_isolation @@ -353,20 +342,6 @@ class RequirementPreparer(object): # Previous "header" printed for a link-based InstallRequirement self._previous_requirement_header = ("", "") - @property - def _download_should_save(self): - # type: () -> bool - if not self.download_dir: - return False - - if os.path.exists(self.download_dir): - return True - - logger.critical('Could not find download directory') - raise InstallationError( - "Could not find or access download directory '{}'" - .format(self.download_dir)) - def _log_preparing_link(self, req): # type: (InstallRequirement) -> None """Provide context for the requirement being prepared.""" @@ -385,15 +360,8 @@ class RequirementPreparer(object): with indent_log(): logger.info("Using cached %s", req.link.filename) - def _get_download_dir(self, link): - # type: (Link) -> Optional[str] - if link.is_wheel and self.wheel_download_dir: - # Download wheels to a dedicated dir when doing `pip wheel`. - return self.wheel_download_dir - return self.download_dir - - def _ensure_link_req_src_dir(self, req, download_dir, parallel_builds): - # type: (InstallRequirement, Optional[str], bool) -> None + def _ensure_link_req_src_dir(self, req, parallel_builds): + # type: (InstallRequirement, bool) -> None """Ensure source_dir of a linked InstallRequirement.""" # Since source_dir is only set for editable requirements. if req.link.is_wheel: @@ -494,10 +462,9 @@ class RequirementPreparer(object): # Check if the relevant file is already available # in the download directory file_path = None - download_dir = self._get_download_dir(req.link) - if download_dir is not None and link.is_wheel: + if self.download_dir is not None and link.is_wheel: hashes = self._get_linked_req_hashes(req) - file_path = _check_download_dir(req.link, download_dir, hashes) + file_path = _check_download_dir(req.link, self.download_dir, hashes) if file_path is not None: # The file is already available, so mark it as downloaded @@ -528,15 +495,14 @@ class RequirementPreparer(object): # type: (InstallRequirement, bool) -> Distribution assert req.link link = req.link - download_dir = self._get_download_dir(link) - self._ensure_link_req_src_dir(req, download_dir, parallel_builds) + self._ensure_link_req_src_dir(req, parallel_builds) hashes = self._get_linked_req_hashes(req) if link.url not in self._downloaded: try: local_file = unpack_url( link, req.source_dir, self._download, - download_dir, hashes, + self.download_dir, hashes, ) except NetworkConnectionError as exc: raise InstallationError( @@ -557,23 +523,34 @@ class RequirementPreparer(object): dist = _get_prepared_distribution( req, self.req_tracker, self.finder, self.build_isolation, ) - - if download_dir: - if link.is_existing_dir(): - logger.info('Link is a directory, ignoring download_dir') - elif local_file: - download_location = os.path.join(download_dir, link.filename) - if not os.path.exists(download_location): - shutil.copy(local_file.path, download_location) - download_path = display_path(download_location) - logger.info('Saved %s', download_path) - - if self._download_should_save: - # Make a .zip of the source_dir we already created. - if link.is_vcs: - req.archive(self.download_dir) return dist + def save_linked_requirement(self, req): + # type: (InstallRequirement) -> None + assert self.download_dir is not None + assert req.link is not None + link = req.link + if link.is_vcs: + # Make a .zip of the source_dir we already created. + req.archive(self.download_dir) + return + + if link.is_existing_dir(): + logger.debug( + 'Not copying link to destination directory ' + 'since it is a directory: %s', link, + ) + return + if req.local_file_path is None: + # No distribution was downloaded for this requirement. + return + + download_location = os.path.join(self.download_dir, link.filename) + if not os.path.exists(download_location): + shutil.copy(req.local_file_path, download_location) + download_path = display_path(download_location) + logger.info('Saved %s', download_path) + def prepare_editable_requirement( self, req, # type: InstallRequirement @@ -593,14 +570,13 @@ class RequirementPreparer(object): 'hash.'.format(req) ) req.ensure_has_source_dir(self.src_dir) - req.update_editable(not self._download_should_save) + req.update_editable(self.download_dir is None) dist = _get_prepared_distribution( req, self.req_tracker, self.finder, self.build_isolation, ) - if self._download_should_save: - req.archive(self.download_dir) + req.archive(self.download_dir) req.check_if_exists(self.use_user_site) return dist diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 42999a59f..8ce299503 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -700,12 +700,14 @@ class InstallRequirement(object): return self.name + '/' + name def archive(self, build_dir): - # type: (str) -> None + # type: (Optional[str]) -> None """Saves archive to provided build_dir. Used for saving downloaded VCS requirements as part of `pip download`. """ assert self.source_dir + if build_dir is None: + return create_archive = True archive_name = '{}-{}.zip'.format(self.name, self.metadata["version"]) diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 730a26a88..083d2c2c6 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -82,7 +82,6 @@ class TestRequirementSet(object): build_dir=os.path.join(self.tempdir, 'build'), src_dir=os.path.join(self.tempdir, 'src'), download_dir=None, - wheel_download_dir=None, build_isolation=True, req_tracker=tracker, session=session,