Skip to content

Commit

Permalink
use .metadata distribution info when possible
Browse files Browse the repository at this point in the history
When performing `install --dry-run` and PEP 658 .metadata files are
available to guide the resolve, do not download the associated wheels.

Rather use the distribution information directly from the .metadata
files when reporting the results on the CLI and in the --report file.

fixed important type error, remove unused method

add very lengthy comment

fix lots of things. move metadata tests to use install --report

add the first-ever legitimate test for the prior --dry-run functionality

add test for new --dry-run functionality

describe the new --dry-run behavior
  • Loading branch information
Jonathan Helmus authored and cosmicexplorer committed Jul 28, 2023
1 parent d064174 commit 720b0c2
Show file tree
Hide file tree
Showing 10 changed files with 662 additions and 449 deletions.
1 change: 1 addition & 0 deletions news/11512.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid downloading wheels when performing a ``--dry-run`` install when .metadata files are used.
4 changes: 3 additions & 1 deletion src/pip/_internal/commands/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ def run(self, options: Values, args: List[str]) -> int:
preparer.save_linked_requirement(req)
downloaded.append(req.name)

preparer.prepare_linked_requirements_more(requirement_set.requirements.values())
preparer.finalize_linked_requirements(
requirement_set.requirements.values(), hydrate_virtual_reqs=True
)
requirement_set.warn_legacy_versions_and_specifiers()

if downloaded:
Expand Down
7 changes: 6 additions & 1 deletion src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ def add_options(self) -> None:
help=(
"Don't actually install anything, just print what would be. "
"Can be used in combination with --ignore-installed "
"to 'resolve' the requirements."
"to 'resolve' the requirements. If PEP 648 or fast-deps metadata is "
"available, --dry-run also avoid downloading the dependency at all."
),
)
self.cmd_opts.add_option(
Expand Down Expand Up @@ -377,6 +378,10 @@ def run(self, options: Values, args: List[str]) -> int:
requirement_set = resolver.resolve(
reqs, check_supported_wheels=not options.target_dir
)
preparer.finalize_linked_requirements(
requirement_set.requirements.values(),
hydrate_virtual_reqs=not options.dry_run,
)

if options.json_report_file:
report = InstallationReport(requirement_set.requirements_to_install)
Expand Down
4 changes: 3 additions & 1 deletion src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ def run(self, options: Values, args: List[str]) -> int:
elif should_build_for_wheel_command(req):
reqs_to_build.append(req)

preparer.prepare_linked_requirements_more(requirement_set.requirements.values())
preparer.finalize_linked_requirements(
requirement_set.requirements.values(), hydrate_virtual_reqs=True
)
requirement_set.warn_legacy_versions_and_specifiers()

# build wheels
Expand Down
75 changes: 59 additions & 16 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,19 +480,7 @@ def _complete_partial_requirements(
logger.debug("Downloading link %s to %s", link, filepath)
req = links_to_fully_download[link]
req.local_file_path = filepath
# TODO: This needs fixing for sdists
# This is an emergency fix for #11847, which reports that
# distributions get downloaded twice when metadata is loaded
# from a PEP 658 standalone metadata file. Setting _downloaded
# fixes this for wheels, but breaks the sdist case (tests
# test_download_metadata). As PyPI is currently only serving
# metadata for wheels, this is not an immediate issue.
# Fixing the problem properly looks like it will require a
# complete refactoring of the `prepare_linked_requirements_more`
# logic, and I haven't a clue where to start on that, so for now
# I have fixed the issue *just* for wheels.
if req.is_wheel:
self._downloaded[req.link.url] = filepath
self._downloaded[req.link.url] = filepath

# This step is necessary to ensure all lazy wheels are processed
# successfully by the 'download', 'wheel', and 'install' commands.
Expand Down Expand Up @@ -531,16 +519,67 @@ def prepare_linked_requirement(
# The file is not available, attempt to fetch only metadata
metadata_dist = self._fetch_metadata_only(req)
if metadata_dist is not None:
# These reqs now have the dependency information from the downloaded
# metadata, without having downloaded the actual dist at all.
req.dist_from_metadata = metadata_dist
req.needs_more_preparation = True
return metadata_dist

# None of the optimizations worked, fully prepare the requirement
return self._prepare_linked_requirement(req, parallel_builds)

def prepare_linked_requirements_more(
self, reqs: Iterable[InstallRequirement], parallel_builds: bool = False
def _ensure_download_info(self, reqs: Iterable[InstallRequirement]) -> None:
"""
`pip install --report` extracts the download info from each requirement for its
JSON output, so we need to make sure every requirement has this before finishing
the resolve. But .download_info will only be populated by the point this method
is called for requirements already found in the wheel cache, so we need to
synthesize it for uncached results. Luckily, a DirectUrl can be parsed directly
from a url without any other context. However, this also means the download info
will only contain a hash if the link itself declares the hash.
"""
for req in reqs:
# If download_info is set, we got it from the wheel cache.
if req.download_info is None:
req.download_info = direct_url_from_link(req.link, req.source_dir)

def _force_fully_prepared(self, reqs: Iterable[InstallRequirement]) -> None:
"""
The legacy resolver seems to prepare requirements differently that can leave
them half-done in certain code paths. I'm not quite sure how it's doing things,
but at least we can do this to make sure they do things right.
"""
for req in reqs:
req.prepared = True
req.needs_more_preparation = False

def finalize_linked_requirements(
self,
reqs: Iterable[InstallRequirement],
hydrate_virtual_reqs: bool,
parallel_builds: bool = False,
) -> None:
"""Prepare linked requirements more, if needed."""
"""Prepare linked requirements more, if needed.
Neighboring .metadata files as per PEP 658 or lazy wheels via fast-deps will be
preferred to extract metadata from any concrete requirement (one that has been
mapped to a Link) without downloading the underlying wheel or sdist. When ``pip
install --dry-run`` is called, we want to avoid ever downloading the underlying
dist, but we still need to provide all of the results that pip commands expect
from the typical resolve process.
Those expectations vary, but one distinction lies in whether the command needs
an actual physical dist somewhere on the filesystem, or just the metadata about
it from the resolver (as in ``pip install --report``). If the command requires
actual physical filesystem locations for the resolved dists, it must call this
method with ``hydrate_virtual_reqs=True`` to fully download anything
that remains.
"""
if not hydrate_virtual_reqs:
self._ensure_download_info(reqs)
self._force_fully_prepared(reqs)
return

reqs = [req for req in reqs if req.needs_more_preparation]
for req in reqs:
# Determine if any of these requirements were already downloaded.
Expand All @@ -549,6 +588,8 @@ def prepare_linked_requirements_more(
file_path = _check_download_dir(req.link, self.download_dir, hashes)
if file_path is not None:
self._downloaded[req.link.url] = file_path
# This is a wheel, so we know there's nothing more we need to do to
# prepare it.
req.needs_more_preparation = False

# Prepare requirements we found were already downloaded for some
Expand All @@ -566,6 +607,8 @@ def prepare_linked_requirements_more(
partially_downloaded_reqs,
parallel_builds=parallel_builds,
)
# NB: Must call this method before returning!
self._force_fully_prepared(reqs)

def _prepare_linked_requirement(
self, req: InstallRequirement, parallel_builds: bool
Expand Down
9 changes: 8 additions & 1 deletion src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ def __init__(
# This requirement needs more preparation before it can be built
self.needs_more_preparation = False

# Distribution from the .metadata file referenced by the PEP 658
# data-dist-info-metadata attribute.
self.dist_from_metadata: Optional[BaseDistribution] = None

def __str__(self) -> str:
if self.req:
s = str(self.req)
Expand Down Expand Up @@ -230,7 +234,7 @@ def name(self) -> Optional[str]:
return None
return self.req.name

@functools.lru_cache() # use cached_property in python 3.8+
@functools.lru_cache(maxsize=None) # TODO: use cached_property in python 3.8+
def supports_pyproject_editable(self) -> bool:
if not self.use_pep517:
return False
Expand Down Expand Up @@ -583,6 +587,7 @@ def prepare_metadata(self) -> None:

@property
def metadata(self) -> Any:
# TODO: use cached_property in python 3.8+
if not hasattr(self, "_metadata"):
self._metadata = self.get_dist().metadata

Expand All @@ -595,6 +600,8 @@ def get_dist(self) -> BaseDistribution:
return get_wheel_distribution(
FilesystemWheel(self.local_file_path), canonicalize_name(self.name)
)
elif self.dist_from_metadata:
return self.dist_from_metadata
raise AssertionError(
f"InstallRequirement {self} has no metadata directory and no wheel: "
f"can't make a distribution."
Expand Down
5 changes: 0 additions & 5 deletions src/pip/_internal/resolution/resolvelib/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,6 @@ def resolve(

req_set.add_named_requirement(ireq)

reqs = req_set.all_requirements
self.factory.preparer.prepare_linked_requirements_more(reqs)
for req in reqs:
req.prepared = True
req.needs_more_preparation = False
return req_set

def get_installation_order(
Expand Down
Loading

0 comments on commit 720b0c2

Please sign in to comment.