Skip to content

Commit 6f4da4c

Browse files
Jonathan Helmuscosmicexplorer
Jonathan Helmus
authored andcommitted
use .metadata distribution info when possible
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
1 parent d064174 commit 6f4da4c

File tree

10 files changed

+664
-449
lines changed

10 files changed

+664
-449
lines changed

news/11512.bugfix.rst

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Avoid downloading wheels when performing a ``--dry-run`` install when .metadata files are used.

src/pip/_internal/commands/download.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ def run(self, options: Values, args: List[str]) -> int:
138138
preparer.save_linked_requirement(req)
139139
downloaded.append(req.name)
140140

141-
preparer.prepare_linked_requirements_more(requirement_set.requirements.values())
141+
preparer.finalize_linked_requirements(
142+
requirement_set.requirements.values(), hydrate_virtual_reqs=True
143+
)
142144
requirement_set.warn_legacy_versions_and_specifiers()
143145

144146
if downloaded:

src/pip/_internal/commands/install.py

+6-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ def add_options(self) -> None:
8484
help=(
8585
"Don't actually install anything, just print what would be. "
8686
"Can be used in combination with --ignore-installed "
87-
"to 'resolve' the requirements."
87+
"to 'resolve' the requirements. If PEP 648 or fast-deps metadata is "
88+
"available, --dry-run also avoid downloading the dependency at all."
8889
),
8990
)
9091
self.cmd_opts.add_option(
@@ -377,6 +378,10 @@ def run(self, options: Values, args: List[str]) -> int:
377378
requirement_set = resolver.resolve(
378379
reqs, check_supported_wheels=not options.target_dir
379380
)
381+
preparer.finalize_linked_requirements(
382+
requirement_set.requirements.values(),
383+
hydrate_virtual_reqs=not options.dry_run,
384+
)
380385

381386
if options.json_report_file:
382387
report = InstallationReport(requirement_set.requirements_to_install)

src/pip/_internal/commands/wheel.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,9 @@ def run(self, options: Values, args: List[str]) -> int:
153153
elif should_build_for_wheel_command(req):
154154
reqs_to_build.append(req)
155155

156-
preparer.prepare_linked_requirements_more(requirement_set.requirements.values())
156+
preparer.finalize_linked_requirements(
157+
requirement_set.requirements.values(), hydrate_virtual_reqs=True
158+
)
157159
requirement_set.warn_legacy_versions_and_specifiers()
158160

159161
# build wheels

src/pip/_internal/operations/prepare.py

+59-16
Original file line numberDiff line numberDiff line change
@@ -480,19 +480,7 @@ def _complete_partial_requirements(
480480
logger.debug("Downloading link %s to %s", link, filepath)
481481
req = links_to_fully_download[link]
482482
req.local_file_path = filepath
483-
# TODO: This needs fixing for sdists
484-
# This is an emergency fix for #11847, which reports that
485-
# distributions get downloaded twice when metadata is loaded
486-
# from a PEP 658 standalone metadata file. Setting _downloaded
487-
# fixes this for wheels, but breaks the sdist case (tests
488-
# test_download_metadata). As PyPI is currently only serving
489-
# metadata for wheels, this is not an immediate issue.
490-
# Fixing the problem properly looks like it will require a
491-
# complete refactoring of the `prepare_linked_requirements_more`
492-
# logic, and I haven't a clue where to start on that, so for now
493-
# I have fixed the issue *just* for wheels.
494-
if req.is_wheel:
495-
self._downloaded[req.link.url] = filepath
483+
self._downloaded[req.link.url] = filepath
496484

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

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

540-
def prepare_linked_requirements_more(
541-
self, reqs: Iterable[InstallRequirement], parallel_builds: bool = False
531+
def _ensure_download_info(self, reqs: Iterable[InstallRequirement]) -> None:
532+
"""
533+
`pip install --report` extracts the download info from each requirement for its
534+
JSON output, so we need to make sure every requirement has this before finishing
535+
the resolve. But .download_info will only be populated by the point this method
536+
is called for requirements already found in the wheel cache, so we need to
537+
synthesize it for uncached results. Luckily, a DirectUrl can be parsed directly
538+
from a url without any other context. However, this also means the download info
539+
will only contain a hash if the link itself declares the hash.
540+
"""
541+
for req in reqs:
542+
# If download_info is set, we got it from the wheel cache.
543+
if req.download_info is None:
544+
req.download_info = direct_url_from_link(req.link, req.source_dir)
545+
546+
def _force_fully_prepared(self, reqs: Iterable[InstallRequirement]) -> None:
547+
"""
548+
The legacy resolver seems to prepare requirements differently that can leave
549+
them half-done in certain code paths. I'm not quite sure how it's doing things,
550+
but at least we can do this to make sure they do things right.
551+
"""
552+
for req in reqs:
553+
req.prepared = True
554+
req.needs_more_preparation = False
555+
556+
def finalize_linked_requirements(
557+
self,
558+
reqs: Iterable[InstallRequirement],
559+
hydrate_virtual_reqs: bool,
560+
parallel_builds: bool = False,
542561
) -> None:
543-
"""Prepare linked requirements more, if needed."""
562+
"""Prepare linked requirements more, if needed.
563+
564+
Neighboring .metadata files as per PEP 658 or lazy wheels via fast-deps will be
565+
preferred to extract metadata from any concrete requirement (one that has been
566+
mapped to a Link) without downloading the underlying wheel or sdist. When ``pip
567+
install --dry-run`` is called, we want to avoid ever downloading the underlying
568+
dist, but we still need to provide all of the results that pip commands expect
569+
from the typical resolve process.
570+
571+
Those expectations vary, but one distinction lies in whether the command needs
572+
an actual physical dist somewhere on the filesystem, or just the metadata about
573+
it from the resolver (as in ``pip install --report``). If the command requires
574+
actual physical filesystem locations for the resolved dists, it must call this
575+
method with ``hydrate_virtual_reqs=True`` to fully download anything
576+
that remains.
577+
"""
578+
if not hydrate_virtual_reqs:
579+
self._ensure_download_info(reqs)
580+
self._force_fully_prepared(reqs)
581+
return
582+
544583
reqs = [req for req in reqs if req.needs_more_preparation]
545584
for req in reqs:
546585
# Determine if any of these requirements were already downloaded.
@@ -549,6 +588,8 @@ def prepare_linked_requirements_more(
549588
file_path = _check_download_dir(req.link, self.download_dir, hashes)
550589
if file_path is not None:
551590
self._downloaded[req.link.url] = file_path
591+
# This is a wheel, so we know there's nothing more we need to do to
592+
# prepare it.
552593
req.needs_more_preparation = False
553594

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

570613
def _prepare_linked_requirement(
571614
self, req: InstallRequirement, parallel_builds: bool

src/pip/_internal/req/req_install.py

+8-1
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,10 @@ def __init__(
183183
# This requirement needs more preparation before it can be built
184184
self.needs_more_preparation = False
185185

186+
# Distribution from the .metadata file referenced by the PEP 658
187+
# data-dist-info-metadata attribute.
188+
self.dist_from_metadata: Optional[BaseDistribution] = None
189+
186190
def __str__(self) -> str:
187191
if self.req:
188192
s = str(self.req)
@@ -230,7 +234,7 @@ def name(self) -> Optional[str]:
230234
return None
231235
return self.req.name
232236

233-
@functools.lru_cache() # use cached_property in python 3.8+
237+
@functools.lru_cache(maxsize=None) # TODO: use cached_property in python 3.8+
234238
def supports_pyproject_editable(self) -> bool:
235239
if not self.use_pep517:
236240
return False
@@ -583,6 +587,7 @@ def prepare_metadata(self) -> None:
583587

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

@@ -595,6 +600,8 @@ def get_dist(self) -> BaseDistribution:
595600
return get_wheel_distribution(
596601
FilesystemWheel(self.local_file_path), canonicalize_name(self.name)
597602
)
603+
elif self.dist_from_metadata:
604+
return self.dist_from_metadata
598605
raise AssertionError(
599606
f"InstallRequirement {self} has no metadata directory and no wheel: "
600607
f"can't make a distribution."

src/pip/_internal/resolution/resolvelib/resolver.py

-5
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,6 @@ def resolve(
157157

158158
req_set.add_named_requirement(ireq)
159159

160-
reqs = req_set.all_requirements
161-
self.factory.preparer.prepare_linked_requirements_more(reqs)
162-
for req in reqs:
163-
req.prepared = True
164-
req.needs_more_preparation = False
165160
return req_set
166161

167162
def get_installation_order(

0 commit comments

Comments
 (0)