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 12, 2020
1 parent 8cca170 commit 88e6e6b
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 64 deletions.
9 changes: 7 additions & 2 deletions docs/html/reference/pip_install.rst
Original file line number Diff line number Diff line change
Expand Up @@ -728,8 +728,13 @@ You can install local projects by specifying the project path to pip::

$ pip install path/to/SomeProject

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.
Until version 20.0, pip did copy the entire project directory to a temporary
location and installed from there. This approach was the cause of several
performance and correctness issues. As of version 20.1 pip installs from the
local project directory. Depending on the build backend used by the project,
this may generate secondary build artifacts in the project directory, such as
the ``.egg-info`` and ``build`` directories in the case of the setuptools
backend.


.. _`editable-installs`:
Expand Down
9 changes: 9 additions & 0 deletions news/7555.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
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 its 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 ``build`` and
``.egg-info`` directories 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, we build in-place.
# There is nothing to be done here.
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 88e6e6b

Please sign in to comment.