Skip to content

Commit

Permalink
New resolver: Avoid polluting dest dir
Browse files Browse the repository at this point in the history
Previously, during dependency resolution for `pip download -d <dir>`
or `pip wheel -w <dir>`, distributions downloaded are always saved
to <dir>, even for those are only used in backtracking and are not
part of the returned requirement set.
  • Loading branch information
McSinyx committed Oct 7, 2020
1 parent 6887b07 commit b28e2c4
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 17 deletions.
2 changes: 2 additions & 0 deletions news/8827.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Avoid polluting the destination directory by resolution artifacts
when the new resolver is used for ``pip download`` or ``pip wheel``.
1 change: 1 addition & 0 deletions src/pip/_internal/commands/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ def run(self, options, args):
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)

This comment has been minimized.

Copy link
@jsirois

jsirois Dec 15, 2020

Contributor

Noting that unlike in the wheel command change below, which tests req.is_wheel which implies req.link, no vetting of req.link is done here (constraint-only reqs will have no link). This leads to the bug filed in #9283:

$ pip --disable-pip-version-check --use-deprecated legacy-resolver download --dest download.dir ansicolors==1.1.8 --constraint <(cat <<EOF
ansicolors==1.1.8
pex==2.1.23
EOF
)
Collecting ansicolors==1.1.8
  File was already downloaded /home/jsirois/dev/pypa/pip/download.dir/ansicolors-1.1.8-py2.py3-none-any.whl
ERROR: Exception:
Traceback (most recent call last):
  File "/home/jsirois/.local/lib/python3.9/site-packages/pip/_internal/cli/base_command.py", line 224, in _main
    status = self.run(options, args)
  File "/home/jsirois/.local/lib/python3.9/site-packages/pip/_internal/cli/req_command.py", line 180, in wrapper
    return func(self, options, args)
  File "/home/jsirois/.local/lib/python3.9/site-packages/pip/_internal/commands/download.py", line 138, in run
    preparer.save_linked_requirement(req)
  File "/home/jsirois/.local/lib/python3.9/site-packages/pip/_internal/operations/prepare.py", line 531, in save_linked_requirement
    assert req.link is not None
AssertionError

This comment has been minimized.

Copy link
@jsirois

jsirois Dec 15, 2020

Contributor

Fix in #9301

downloaded.append(req.name)
if downloaded:
write_output('Successfully downloaded %s', ' '.join(downloaded))
Expand Down
11 changes: 7 additions & 4 deletions src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from optparse import Values
from typing import List

from pip._internal.req.req_install import InstallRequirement

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -156,10 +157,12 @@ def run(self, options, args):
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(
Expand Down
36 changes: 23 additions & 13 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -523,23 +523,33 @@ def _prepare_linked_requirement(self, req, parallel_builds):
dist = _get_prepared_distribution(
req, self.req_tracker, self.finder, self.build_isolation,
)
return dist

if self.download_dir is not None:
if link.is_existing_dir():
logger.info('Link is a directory, ignoring download_dir')
elif local_file:
download_location = os.path.join(
self.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)

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 dist
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,
Expand Down

0 comments on commit b28e2c4

Please sign in to comment.