Skip to content

Commit

Permalink
Fix pex3 lock export handling of exotic reqs. (#2423)
Browse files Browse the repository at this point in the history 
Although Pex supports locking VCS requirements and local project
requirements, it does so with a be-spoke system for fingerprinting
each; as such, the `--hash`es emitted when exporting lock files
containing these types of requirements are not actually useable in
practice. Continue to support exporting this class of lock file, but
warn of the potential problems and offer a new `--format pip-no-hash`
mode for the daring. In addition, change the requirements output for
this class of lock file to match the input requirement for best
fidelity when actually attempting to use the resulting exported
requirement file without `--hash`es.

Fixes #2416
  • Loading branch information
@jsirois
jsirois authored Jun 9, 2024
1 parent 1811be0 commit cf1336d
Show file tree
Hide file tree
Showing 2 changed files with 223 additions and 34 deletions.
95 changes: 77 additions & 18 deletions pex/cli/commands/lock.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,14 @@
from pex.resolve.config import finalize as finalize_resolve_config
from pex.resolve.configured_resolver import ConfiguredResolver
from pex.resolve.lock_resolver import resolve_from_lock
from pex.resolve.locked_resolve import LockConfiguration, LockStyle, Resolved, TargetSystem
from pex.resolve.locked_resolve import (
LocalProjectArtifact,
LockConfiguration,
LockStyle,
Resolved,
TargetSystem,
VCSArtifact,
)
from pex.resolve.lockfile import json_codec
from pex.resolve.lockfile.create import create
from pex.resolve.lockfile.model import Lockfile
Expand Down Expand Up @@ -85,6 +92,7 @@ class Value(Enum.Value):
pass

PIP = Value("pip")
PIP_NO_HASHES = Value("pip-no-hashes")
PEP_665 = Value("pep-665")


Expand Down Expand Up @@ -512,8 +520,8 @@ def _add_export_arguments(
choices=ExportFormat.values(),
type=ExportFormat.for_value,
help=(
"The format to export the lock to. Currently only the {pip!r} requirements file "
"format using `--hash` is supported.".format(pip=ExportFormat.PIP)
"The format to export the lock to. Currently only the Pip requirements file "
"formats (using `--hash` or bare) are supported."
),
)
export_parser.add_argument(
Expand Down Expand Up @@ -895,9 +903,11 @@ def dump_with_terminating_newline(out):

def _export(self, requirement_configuration=RequirementConfiguration()):
# type: (RequirementConfiguration) -> Result
if self.options.format != ExportFormat.PIP:
supported_formats = ExportFormat.PIP, ExportFormat.PIP_NO_HASHES
if self.options.format not in supported_formats:
return Error(
"Only the {pip!r} lock format is supported currently.".format(pip=ExportFormat.PIP)
"Only the Pip lock formats are supported currently. "
"Choose one of: {choices}".format(choices=" or ".join(map(str, supported_formats)))
)

lockfile_path, lock_file = self._load_lockfile()
Expand Down Expand Up @@ -939,31 +949,80 @@ def _export(self, requirement_configuration=RequirementConfiguration()):
else:
resolved = subset_result.subsets[0].resolved

requirement_by_pin = {} # type: Dict[Pin, str]
fingerprints_by_pin = OrderedDict() # type: OrderedDict[Pin, List[Fingerprint]]
warnings = [] # type: List[str]

def add_warning(
type_, # type: str
requirement, # type: str
):
# type: (...) -> str
warnings.append("{type} {requirement!r}".format(type=type_, requirement=requirement))
return requirement

for downloaded_artifact in resolved.downloadable_artifacts:
if isinstance(downloaded_artifact.artifact, LocalProjectArtifact):
requirement_by_pin[downloaded_artifact.pin] = add_warning(
"local project requirement",
requirement="{project_name} @ file://{directory}".format(
project_name=downloaded_artifact.pin.project_name,
directory=downloaded_artifact.artifact.directory,
),
)
elif isinstance(downloaded_artifact.artifact, VCSArtifact):
requirement_by_pin[downloaded_artifact.pin] = add_warning(
"VCS requirement",
requirement=downloaded_artifact.artifact.as_unparsed_requirement(
downloaded_artifact.pin.project_name
),
)
else:
requirement_by_pin[downloaded_artifact.pin] = "{project_name}=={version}".format(
project_name=downloaded_artifact.pin.project_name,
version=downloaded_artifact.pin.version.raw,
)
fingerprints_by_pin.setdefault(downloaded_artifact.pin, []).append(
downloaded_artifact.artifact.fingerprint
)

if self.options.format is ExportFormat.PIP and warnings:
print(
"The requirements exported from {lockfile} include the following requirements\n"
"that tools likely won't support --hash for:\n"
"{warnings}\n"
"\n"
"If you can accept a lack of hash checking you can specify "
"`--format pip-no-hashes`.\n".format(
lockfile=lockfile_path,
warnings="\n".join(
"+ {warning}".format(warning=warning) for warning in warnings
),
),
file=sys.stderr,
)
with self.output(self.options) as output:
pins = fingerprints_by_pin.keys() # type: Iterable[Pin]
if self.options.sort_by == ExportSortBy.PROJECT_NAME:
pins = sorted(pins, key=attrgetter("project_name.normalized"))
for pin in pins:
fingerprints = fingerprints_by_pin[pin]
output.write(
"{project_name}=={version} \\\n"
" {hashes}\n".format(
project_name=pin.project_name,
version=pin.version.raw,
hashes=" \\\n ".join(
"--hash={algorithm}:{hash}".format(
algorithm=fingerprint.algorithm, hash=fingerprint.hash
)
for fingerprint in fingerprints
),
requirement = requirement_by_pin[pin]
if self.options.format is ExportFormat.PIP_NO_HASHES:
print(requirement, file=output)
else:
fingerprints = fingerprints_by_pin[pin]
output.write(
"{requirement} \\\n"
" {hashes}\n".format(
requirement=requirement,
hashes=" \\\n ".join(
"--hash={algorithm}:{hash}".format(
algorithm=fingerprint.algorithm, hash=fingerprint.hash
)
for fingerprint in fingerprints
),
)
)
)
return Ok()

def _export_subset(self):
Expand Down
162 changes: 146 additions & 16 deletions tests/integration/cli/commands/test_export.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,35 @@

import json
import os.path
import re
import sys
from textwrap import dedent

import pytest

from pex.dist_metadata import Requirement
from pex.pep_440 import Version
from pex.pep_503 import ProjectName
from pex.pip.version import PipVersion
from pex.resolve.locked_resolve import Artifact, LockedRequirement, LockedResolve, LockStyle
from pex.resolve.locked_resolve import (
Artifact,
LocalProjectArtifact,
LockedRequirement,
LockedResolve,
LockStyle,
VCSArtifact,
)
from pex.resolve.lockfile import json_codec
from pex.resolve.lockfile.model import Lockfile
from pex.resolve.resolved_requirement import Fingerprint, Pin
from pex.resolve.resolved_requirement import ArtifactURL, Fingerprint, Pin
from pex.resolve.resolver_configuration import ResolverVersion
from pex.sorted_tuple import SortedTuple
from pex.typing import TYPE_CHECKING
from pex.version import __version__
from testing.cli import run_pex3

if TYPE_CHECKING:
from typing import Any, Text
from typing import Any, Iterable, Optional, Text

import attr # vendor:skip
else:
Expand Down Expand Up @@ -79,15 +90,17 @@
def export(
tmpdir, # type: Any
lockfile, # type: Lockfile
*export_args # type: str
lockfile_path=None, # type: Optional[str]
export_args=(), # type: Iterable[str]
expected_error_re=None, # type: Optional[str]
):
# type: (...) -> Text
lock = os.path.join(str(tmpdir), "lock.json")
lock = lockfile_path or os.path.join(str(tmpdir), "lock.json")
with open(lock, "w") as fp:
json.dump(json_codec.as_json_data(lockfile), fp, sort_keys=True, indent=2)

result = run_pex3("lock", "export", lock, *export_args)
result.assert_success()
result = run_pex3(*(("lock", "export", lock) + tuple(export_args)))
result.assert_success(expected_error_re=expected_error_re)
return result.output


Expand Down Expand Up @@ -219,7 +232,7 @@ def test_export_sort_by(tmpdir):
--hash=sha256:spamspam
"""
)
== export(tmpdir, ansicolors_plus_attrs, "--sort-by=specificity")
== export(tmpdir, ansicolors_plus_attrs, export_args=("--sort-by", "specificity"))
)

assert (
Expand All @@ -233,7 +246,7 @@ def test_export_sort_by(tmpdir):
--hash=sha256:spameggs
"""
)
== export(tmpdir, ansicolors_plus_attrs, "--sort-by=project-name")
== export(tmpdir, ansicolors_plus_attrs, export_args=("--sort-by", "project-name"))
)


Expand Down Expand Up @@ -279,12 +292,14 @@ def test_export_respects_target(tmpdir):
) == export(
tmpdir,
ansicolors_plus_pywin32,
"--complete-platform",
json.dumps(
{
"marker_environment": {"sys_platform": "win32"},
"compatible_tags": ["cp39-cp39-win32", "py3-none-any"],
}
export_args=(
"--complete-platform",
json.dumps(
{
"marker_environment": {"sys_platform": "win32"},
"compatible_tags": ["cp39-cp39-win32", "py3-none-any"],
}
),
),
), (
"A win32 foreign target should get both ansicolors cross-platform artifacts as well as "
Expand All @@ -299,5 +314,120 @@ def test_export_respects_target(tmpdir):
--hash=sha1:ef567890
"""
)
== export(tmpdir, ansicolors_plus_pywin32, "--python", sys.executable)
== export(tmpdir, ansicolors_plus_pywin32, export_args=("--python", sys.executable))
), "The local interpreter doesn't support Windows; so we should just get ansicolors artifacts."


@pytest.fixture
def ansicolors_plus_vcs_plus_local_project(pex_project_dir):
# type: (str) -> Lockfile
return attr.evolve(
UNIVERSAL_ANSICOLORS,
requirements=SortedTuple(
[
Requirement.parse("ansicolors"),
Requirement.parse("cowsay"),
Requirement.parse("pex=={version}".format(version=__version__)),
],
key=str,
),
locked_resolves=SortedTuple(
[
attr.evolve(
UNIVERSAL_ANSICOLORS.locked_resolves[0],
locked_requirements=SortedTuple(
list(UNIVERSAL_ANSICOLORS.locked_resolves[0].locked_requirements)
+ [
LockedRequirement(
pin=Pin(ProjectName("cowsay"), Version("6.1")),
artifact=VCSArtifact.from_artifact_url(
artifact_url=ArtifactURL.parse(
"git+https://github.com/VaasuDevanS/cowsay-python@3db622ce"
),
fingerprint=Fingerprint(algorithm="sha256", hash="moo"),
),
),
LockedRequirement(
pin=Pin(ProjectName("pex"), Version(__version__)),
artifact=LocalProjectArtifact(
url=ArtifactURL.parse(
"file://{pex_project_dir}".format(
pex_project_dir=pex_project_dir
)
),
fingerprint=Fingerprint(algorithm="sha256", hash="pex"),
verified=False,
directory=pex_project_dir,
),
),
]
),
)
]
),
local_project_requirement_mapping={
pex_project_dir: Requirement.parse("pex=={version}".format(version=__version__))
},
)


def test_export_vcs_and_local_project_requirements_issue_2416(
tmpdir, # type: Any
ansicolors_plus_vcs_plus_local_project, # type: Lockfile
pex_project_dir, # type: str
):
# type: (...) -> None

lockfile_path = os.path.join(str(tmpdir), "lock.json")
expected_error_msg = dedent(
"""\
The requirements exported from {lockfile} include the following requirements
that tools likely won't support --hash for:
+ VCS requirement 'cowsay @ git+https://github.com/VaasuDevanS/cowsay-python@3db622ce'
+ local project requirement 'pex @ file://{pex_project_dir}'
If you can accept a lack of hash checking you can specify `--format pip-no-hashes`.
"""
).format(lockfile=lockfile_path, pex_project_dir=pex_project_dir)
exported = export(
tmpdir,
ansicolors_plus_vcs_plus_local_project,
lockfile_path=lockfile_path,
expected_error_re=re.escape(expected_error_msg),
)
assert (
dedent(
"""\
ansicolors==1.1.8 \\
--hash=md5:abcd1234 \\
--hash=sha1:ef567890
cowsay @ git+https://github.com/VaasuDevanS/cowsay-python@3db622ce \\
--hash=sha256:moo
pex @ file://{pex_project_dir} \\
--hash=sha256:pex
"""
).format(pex_project_dir=pex_project_dir)
== exported
), exported


def test_export_no_hashes(
tmpdir, # type: Any
ansicolors_plus_vcs_plus_local_project, # type: Lockfile
pex_project_dir, # type: str
):
# type: (...) -> None

exported = export(
tmpdir, ansicolors_plus_vcs_plus_local_project, export_args=("--format", "pip-no-hashes")
)
assert (
dedent(
"""\
ansicolors==1.1.8
cowsay @ git+https://github.com/VaasuDevanS/cowsay-python@3db622ce
pex @ file://{pex_project_dir}
"""
).format(pex_project_dir=pex_project_dir)
== exported
), exported

0 comments on commit cf1336d

Please sign in to comment.