Skip to content

Commit

Permalink
Handle file:// URL deps in distributions. (#2041)
Browse files Browse the repository at this point in the history
This has been a very long standing bug. The root issue is that
`pip download`, although it processes these deps and fails if not
present on the local filesystem, only handles resolving recursive
dependencies and it does not "download" the local distribution from its
referenced path on the file system to the `pip download` download dir.
As such, the normal Pex post-processing is foiled for these deps.

The fix provided here is in common infra used by both normal Pex builds
and `--lock` builds and uses a recursive post-processing step that
gathers all such dependencies from the downloaded distributions and
injects them into the normal Pex post-processing chain.

Fixes #2038
  • Loading branch information
jsirois authored Jan 18, 2023
1 parent f433ad6 commit 8fdb5d7
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 6 deletions.
1 change: 0 additions & 1 deletion pex/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import os
import re
import sys
from contextlib import contextmanager

from pex import attrs, dist_metadata, pex_warnings
Expand Down
77 changes: 72 additions & 5 deletions pex/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from pex.atomic_directory import AtomicDirectory, atomic_directory
from pex.auth import PasswordEntry
from pex.common import safe_mkdir, safe_mkdtemp
from pex.dist_metadata import Distribution, Requirement
from pex.compatibility import unquote, urlparse
from pex.dist_metadata import DistMetadata, Distribution, Requirement
from pex.fingerprinted_distribution import FingerprintedDistribution
from pex.jobs import Raise, SpawnedJob, execute_parallel
from pex.network_configuration import NetworkConfiguration
Expand Down Expand Up @@ -45,7 +46,17 @@
from pex.variables import ENV

if TYPE_CHECKING:
from typing import DefaultDict, Iterable, Iterator, List, Mapping, Optional, Sequence, Tuple
from typing import (
DefaultDict,
Iterable,
Iterator,
List,
Mapping,
Optional,
Sequence,
Set,
Tuple,
)

import attr # vendor:skip

Expand Down Expand Up @@ -657,6 +668,54 @@ def _spawn_install(
)
return SpawnedJob.wait(job=install_job, result=install_result)

def _resolve_direct_file_deps(
self,
install_requests, # type: Iterable[InstallRequest]
max_parallel_jobs=None, # type: Optional[int]
analyzed=None, # type: Optional[Set[ProjectName]]
):
# type: (...) -> Iterable[InstallRequest]

already_analyzed = analyzed or set() # type: Set[ProjectName]

to_install = OrderedSet() # type: OrderedSet[InstallRequest]
to_build = OrderedSet() # type: OrderedSet[BuildRequest]
for install_request in install_requests:
metadata = DistMetadata.load(install_request.wheel_path)
for requirement in metadata.requires_dists:
if requirement.project_name in already_analyzed:
continue
if not requirement.url:
continue
urlinfo = urlparse.urlparse(requirement.url)
if urlinfo.scheme != "file":
continue
dist_path = unquote(urlinfo.path).rstrip()
if not os.path.exists(dist_path):
raise Unsatisfiable(
"The {wheel} wheel has a dependency on {url} which does not exist on this "
"machine.".format(wheel=install_request.wheel_file, url=requirement.url)
)
if dist_path.endswith(".whl"):
to_install.add(InstallRequest.create(install_request.target, dist_path))
else:
to_build.add(BuildRequest.create(install_request.target, dist_path))
already_analyzed.add(metadata.project_name)

all_install_requests = OrderedSet(install_requests)
if to_build:
build_results = self._wheel_builder.build_wheels(
build_requests=to_build, max_parallel_jobs=max_parallel_jobs
)
to_install.update(itertools.chain.from_iterable(build_results.values()))
if to_install:
all_install_requests.update(
self._resolve_direct_file_deps(
to_install, max_parallel_jobs=max_parallel_jobs, analyzed=already_analyzed
)
)
return all_install_requests

def install_distributions(
self,
ignore_errors=False, # type: bool
Expand All @@ -680,7 +739,15 @@ def install_distributions(
)
to_install.extend(itertools.chain.from_iterable(build_results.values()))

# 2. All requirements are now in wheel form: calculate any missing direct requirement
# 2. (Recursively) post-process all wheels with file:// URL direct references. During the
# download phase, Pip considers these dependencies satisfied and does not download them
# or transfer them to the download directory (although it does download their
# non file:// URL dependencies); it just leaves them where they lay on the file system.
all_install_requests = self._resolve_direct_file_deps(
to_install, max_parallel_jobs=max_parallel_jobs
)

# 3. All requirements are now in wheel form: calculate any missing direct requirement
# project names from the wheel names.
with TRACER.timed(
"Calculating project names for direct requirements:"
Expand Down Expand Up @@ -724,14 +791,14 @@ def iter_direct_requirements():
direct_requirement
)

# 3. Install wheels in individual chroots.
# 4. Install wheels in individual chroots.

# Dedup by wheel name; e.g.: only install universal wheels once even though they'll get
# downloaded / built for each interpreter or platform.
install_requests_by_wheel_file = (
OrderedDict()
) # type: OrderedDict[str, List[InstallRequest]]
for install_request in to_install:
for install_request in all_install_requests:
install_requests_by_wheel_file.setdefault(install_request.wheel_file, []).append(
install_request
)
Expand Down
104 changes: 104 additions & 0 deletions tests/integration/test_issue_2038.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import print_function

import glob
import os.path
import subprocess
import sys
from textwrap import dedent

import pytest

from pex.common import touch
from pex.testing import run_pex_command
from pex.typing import TYPE_CHECKING

if TYPE_CHECKING:
from typing import Any


@pytest.mark.skipif(
sys.version_info[:2] < (3, 7),
reason="This test needs to run Poetry which requires at least Python 3.7",
)
def test_wheel_file_url_dep(tmpdir):
# type: (Any) -> None

poetry = os.path.join(str(tmpdir), "poetry.pex")
run_pex_command(args=["poetry==1.3.2", "-c", "poetry", "-o", poetry]).assert_success()

corelibrary = os.path.join(str(tmpdir), "corelibrary")
touch(os.path.join(corelibrary, "README.md"))
with open(os.path.join(corelibrary, "corelibrary.py"), "w") as fp:
print("TWO = 2", file=fp)
subprocess.check_call(args=[poetry, "init", "--no-interaction"], cwd=corelibrary)

anotherlibrary = os.path.join(str(tmpdir), "anotherlibrary")
touch(os.path.join(anotherlibrary, "README.md"))
with open(os.path.join(anotherlibrary, "anotherlibrary.py"), "w") as fp:
fp.write(
dedent(
"""\
from corelibrary import TWO
MEANING_OF_LIFE = TWO * 21
"""
)
)
subprocess.check_call(
args=[poetry, "init", "--no-interaction", "--dependency", "../corelibrary"],
cwd=anotherlibrary,
)

mylibrary = os.path.join(str(tmpdir), "mylibrary")
touch(os.path.join(mylibrary, "README.md"))
with open(os.path.join(mylibrary, "mylibrary.py"), "w") as fp:
fp.write(
dedent(
"""\
from anotherlibrary import MEANING_OF_LIFE
def deep_thought():
print(f"MEANING_OF_LIFE = {MEANING_OF_LIFE}")
"""
)
)
subprocess.check_call(
args=[poetry, "init", "--no-interaction", "--dependency", "../anotherlibrary"],
cwd=mylibrary,
)

subprocess.check_call(args=[poetry, "build", "-f", "wheel"], cwd=mylibrary)
wheels = glob.glob(os.path.join(mylibrary, "dist", "*.whl"))
assert len(wheels) == 1
wheel = wheels[0]

pex_root = os.path.join(str(tmpdir), "pex_root")
testing_pex = os.path.join(str(tmpdir), "testing.pex")
run_pex_command(
args=[
"--pex-root",
pex_root,
"--runtime-pex-root",
pex_root,
"--resolver-version",
"pip-2020-resolver",
# N.B.: Modern Pip is needed to handle Poetry relative path deps. Older Pip does
# building off in a tmp dir and that breaks relative path references.
"--pip-version",
"22.3",
wheel,
"-e",
"mylibrary:deep_thought",
"-o",
testing_pex,
]
).assert_success()
assert (
"MEANING_OF_LIFE = 42"
== subprocess.check_output(args=[testing_pex]).decode("utf-8").strip()
)

0 comments on commit 8fdb5d7

Please sign in to comment.