Skip to content

Commit

Permalink
build in place
Browse files Browse the repository at this point in the history
  • Loading branch information
sbidoul committed Apr 10, 2020
1 parent 8cca170 commit 9d5566d
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 62 deletions.
10 changes: 10 additions & 0 deletions news/7555.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Building of local directories is now done in place. Previously
pip did copy the local directory tree to a temporary location before
building. That approach had a number of drawbacks, among which performance
issues, as well as various issues arising when the python project directory
depends on it's parent directory (such as the presence of a VCS directory).
The user visible effect of this change is that secondary build artifacts,
if any, may therefore be created in the local directory, whereas before they
were created in a temporary copy of the directory and then deleted.
This notably includes the ``.egg-info`` directory in the case of the
setuptools build backend.
47 changes: 22 additions & 25 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,7 @@
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,
rmtree,
)
from pip._internal.utils.misc import display_path, hide_url, path_to_display
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
Expand Down Expand Up @@ -239,11 +234,9 @@ def unpack_url(
unpack_vcs_link(link, location)
return None

# If it's a url to a local directory
# If it's a url to a local directory,
# build in place so there is nothing to unpack.
if link.is_existing_dir():
if os.path.isdir(location):
rmtree(location)
_copy_source_tree(link.file_path, location)
return None

# file urls
Expand Down Expand Up @@ -415,21 +408,25 @@ def prepare_linked_requirement(
with indent_log():
# Since source_dir is only set for editable requirements.
assert req.source_dir is None
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)
)
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)
)

# Now that we have the real link, we can tell what kind of
# requirements we have and raise some more informative errors
Expand Down
39 changes: 2 additions & 37 deletions tests/unit/test_operations_prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,40 +214,5 @@ def test_unpack_url_thats_a_dir(self, tmpdir, data):
unpack_url(dist_url, self.build_dir,
downloader=self.no_downloader,
download_dir=self.download_dir)
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)
# 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'))

0 comments on commit 9d5566d

Please sign in to comment.