From 0bd624275dd86b971ba32a16f070c04febbe2c7d Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 12 May 2020 05:24:28 +0530 Subject: [PATCH] Revert "build in place" This reverts commit 88e6e6bc5c877bfb15cce6f622dfbf9221c7b479. --- docs/html/reference/pip_install.rst | 15 +++----- src/pip/_internal/operations/prepare.py | 47 +++++++++++++------------ tests/unit/test_operations_prepare.py | 39 ++++++++++++++++++-- 3 files changed, 66 insertions(+), 35 deletions(-) diff --git a/docs/html/reference/pip_install.rst b/docs/html/reference/pip_install.rst index ec5889974..294e969f3 100644 --- a/docs/html/reference/pip_install.rst +++ b/docs/html/reference/pip_install.rst @@ -732,18 +732,11 @@ You can install local projects by specifying the project path to pip:: $ pip install path/to/SomeProject -pip treats this directory like an unpacked source archive, and directly -attempts installation. +During regular installation, pip will copy the entire project directory to a +temporary location and install from there. The exception is that pip will +exclude .tox and .nox directories present in the top level of the project from +being copied. -Prior to pip 20.1, pip copied the entire project directory to a temporary -location and attempted installation from that directory. This approach was the -cause of several performance issues, as well as various issues arising when the -project directory depends on its parent directory (such as the presence of a -VCS directory). The main user visible effect of this change is that secondary -build artifacts, if any, would be created in the local directory, whereas -earlier they were created in a temporary copy of the directory and then -deleted. This notably includes the ``build`` and ``.egg-info`` directories in -the case of the setuptools backend. .. _`editable-installs`: diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 3817323bd..1fcbb775e 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -27,7 +27,12 @@ from pip._internal.exceptions import ( 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.misc import display_path, hide_url, path_to_display +from pip._internal.utils.misc import ( + display_path, + hide_url, + path_to_display, + rmtree, +) from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.typing import MYPY_CHECK_RUNNING from pip._internal.utils.unpacking import unpack_file @@ -234,9 +239,11 @@ def unpack_url( unpack_vcs_link(link, location) return None - # If it's a url to a local directory, we build in-place. - # There is nothing to be done here. + # If it's a url to a local directory if link.is_existing_dir(): + if os.path.isdir(location): + rmtree(location) + _copy_source_tree(link.file_path, location) return None # file urls @@ -408,25 +415,21 @@ class RequirementPreparer(object): with indent_log(): # Since source_dir is only set for editable requirements. assert req.source_dir is None - if link.is_existing_dir(): - # Build local directories in place. - req.source_dir = link.file_path - else: - 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. - # FIXME: this won't upgrade when there's an existing - # package unpacked in `req.source_dir` - if os.path.exists(os.path.join(req.source_dir, 'setup.py')): - raise PreviousBuildDirError( - "pip can't proceed with requirements '{}' due to a" - " pre-existing build directory ({}). This is " - "likely due to a previous installation that failed" - ". pip is being responsible and not assuming it " - "can delete this. Please delete it and try again." - .format(req, req.source_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. + # FIXME: this won't upgrade when there's an existing + # package unpacked in `req.source_dir` + if os.path.exists(os.path.join(req.source_dir, 'setup.py')): + raise PreviousBuildDirError( + "pip can't proceed with requirements '{}' due to a" + " pre-existing build directory ({}). This is " + "likely due to a previous installation that failed" + ". pip is being responsible and not assuming it " + "can delete this. Please delete it and try again." + .format(req, req.source_dir) + ) # Now that we have the real link, we can tell what kind of # requirements we have and raise some more informative errors diff --git a/tests/unit/test_operations_prepare.py b/tests/unit/test_operations_prepare.py index 3df542918..0158eed51 100644 --- a/tests/unit/test_operations_prepare.py +++ b/tests/unit/test_operations_prepare.py @@ -214,5 +214,40 @@ class Test_unpack_url(object): unpack_url(dist_url, self.build_dir, downloader=self.no_downloader, download_dir=self.download_dir) - # test that nothing was copied to build_dir since we build in place - assert not os.path.exists(os.path.join(self.build_dir, 'fspkg')) + assert os.path.isdir(os.path.join(self.build_dir, 'fspkg')) + + +@pytest.mark.parametrize('exclude_dir', [ + '.nox', + '.tox' +]) +def test_unpack_url_excludes_expected_dirs(tmpdir, exclude_dir): + src_dir = tmpdir / 'src' + dst_dir = tmpdir / 'dst' + src_included_file = src_dir.joinpath('file.txt') + src_excluded_dir = src_dir.joinpath(exclude_dir) + src_excluded_file = src_dir.joinpath(exclude_dir, 'file.txt') + src_included_dir = src_dir.joinpath('subdir', exclude_dir) + + # set up source directory + src_excluded_dir.mkdir(parents=True) + src_included_dir.mkdir(parents=True) + src_included_file.touch() + src_excluded_file.touch() + + dst_included_file = dst_dir.joinpath('file.txt') + dst_excluded_dir = dst_dir.joinpath(exclude_dir) + dst_excluded_file = dst_dir.joinpath(exclude_dir, 'file.txt') + dst_included_dir = dst_dir.joinpath('subdir', exclude_dir) + + src_link = Link(path_to_url(src_dir)) + unpack_url( + src_link, + dst_dir, + Mock(side_effect=AssertionError), + download_dir=None + ) + assert not os.path.isdir(dst_excluded_dir) + assert not os.path.isfile(dst_excluded_file) + assert os.path.isfile(dst_included_file) + assert os.path.isdir(dst_included_dir)