From cf1336d11ae38c4a3bca087af42c7bf06715e0d3 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sat, 8 Jun 2024 21:00:10 -0700 Subject: [PATCH] Fix `pex3 lock export` handling of exotic reqs. (#2423) 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 --- pex/cli/commands/lock.py | 95 ++++++++-- tests/integration/cli/commands/test_export.py | 162 ++++++++++++++++-- 2 files changed, 223 insertions(+), 34 deletions(-) diff --git a/pex/cli/commands/lock.py b/pex/cli/commands/lock.py index bacbc53dc..ad8302471 100644 --- a/pex/cli/commands/lock.py +++ b/pex/cli/commands/lock.py @@ -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 @@ -85,6 +92,7 @@ class Value(Enum.Value): pass PIP = Value("pip") + PIP_NO_HASHES = Value("pip-no-hashes") PEP_665 = Value("pep-665") @@ -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( @@ -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() @@ -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): diff --git a/tests/integration/cli/commands/test_export.py b/tests/integration/cli/commands/test_export.py index c7c58aa4e..49515c33d 100644 --- a/tests/integration/cli/commands/test_export.py +++ b/tests/integration/cli/commands/test_export.py @@ -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: @@ -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 @@ -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 ( @@ -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")) ) @@ -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 " @@ -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