[internal] Refactor Python lockfile validation (#14330)

Prework for adding validation of user lockfiles. This has four main changes:

1. Move inner helper function for tool lockfiles to a dedicated function
2. DRY Pex rule for metadata, including by making metadata parsing lazy. Now `pex.py` does not deal with `--invalid-lockfile-behavior` at all.
3. Simplify integration test for lockfile validation. Now that `pex.py` does not handle `--invalid-lockfile-behavior` at all, we don't need to test every permutation of it - we have unit tests in `pex_requirements.py` for that.
4. Some simplifications for `pex_requirements_test.py`.

[ci skip-rust]

src/python/pants/backend/python/util_rules/pex.py

@@ -18,7 +18,7 @@
 from pkg_resources import Requirement
 
 from pants.backend.python.subsystems.repos import PythonRepos
-from pants.backend.python.subsystems.setup import InvalidLockfileBehavior, PythonSetup
+from pants.backend.python.subsystems.setup import PythonSetup
 from pants.backend.python.target_types import MainSpecification, PexLayout
 from pants.backend.python.target_types import PexPlatformsField as PythonPlatformsField
 from pants.backend.python.util_rules import pex_cli
@@ -38,7 +38,7 @@
 from pants.backend.python.util_rules.pex_requirements import (
     ToolCustomLockfile,
     ToolDefaultLockfile,
-    validate_metadata,
+    maybe_validate_metadata,
 )
 from pants.engine.collection import Collection, DeduplicatedCollection
 from pants.engine.engine_aware import EngineAwareParameter
@@ -341,57 +341,45 @@ async def build_pex(
     requirements_file_digest = EMPTY_DIGEST
     requirement_count: int
 
-    # TODO(#12314): Capture the resolve name for multiple user lockfiles.
-    resolve_name = (
-        request.requirements.options_scope_name
-        if isinstance(request.requirements, (ToolDefaultLockfile, ToolCustomLockfile))
-        else None
-    )
-
-    if isinstance(request.requirements, Lockfile):
-        is_monolithic_resolve = True
-        argv.extend(["--requirement", request.requirements.file_path])
-        argv.append("--no-transitive")
-        globs = PathGlobs(
-            [request.requirements.file_path],
-            glob_match_error_behavior=GlobMatchErrorBehavior.error,
-            description_of_origin=request.requirements.file_path_description_of_origin,
+    if isinstance(request.requirements, (Lockfile, LockfileContent)):
+        # TODO(#12314): Capture the resolve name for multiple user lockfiles.
+        resolve_name = (
+            request.requirements.options_scope_name
+            if isinstance(request.requirements, (ToolDefaultLockfile, ToolCustomLockfile))
+            else None
         )
-        requirements_file_digest = await Get(Digest, PathGlobs, globs)
-        requirements_file_digest_contents = await Get(
-            DigestContents, Digest, requirements_file_digest
-        )
-        requirement_count = len(requirements_file_digest_contents[0].content.decode().splitlines())
-        if python_setup.invalid_lockfile_behavior in {
-            InvalidLockfileBehavior.warn,
-            InvalidLockfileBehavior.error,
-        }:
-            metadata = PythonLockfileMetadata.from_lockfile(
-                requirements_file_digest_contents[0].content,
-                request.requirements.file_path,
-                resolve_name,
-            )
-            validate_metadata(
-                metadata, request.interpreter_constraints, request.requirements, python_setup
+
+        if isinstance(request.requirements, Lockfile):
+            lock_path = request.requirements.file_path
+            requirements_file_digest = await Get(
+                Digest,
+                PathGlobs(
+                    [lock_path],
+                    glob_match_error_behavior=GlobMatchErrorBehavior.error,
+                    description_of_origin=request.requirements.file_path_description_of_origin,
+                ),
             )
+            _digest_contents = await Get(DigestContents, Digest, requirements_file_digest)
+            lock_bytes = _digest_contents[0].content
+
+            def parse_metadata() -> PythonLockfileMetadata:
+                return PythonLockfileMetadata.from_lockfile(lock_bytes, lock_path, resolve_name)
+
+        else:
+            _fc = request.requirements.file_content
+            lock_path, lock_bytes = (_fc.path, _fc.content)
+            requirements_file_digest = await Get(Digest, CreateDigest([_fc]))
+
+            def parse_metadata() -> PythonLockfileMetadata:
+                return PythonLockfileMetadata.from_lockfile(lock_bytes, resolve_name=resolve_name)
 
-    elif isinstance(request.requirements, LockfileContent):
         is_monolithic_resolve = True
-        file_content = request.requirements.file_content
-        requirement_count = len(file_content.content.decode().splitlines())
-        argv.extend(["--requirement", file_content.path])
-        argv.append("--no-transitive")
-        if python_setup.invalid_lockfile_behavior in {
-            InvalidLockfileBehavior.warn,
-            InvalidLockfileBehavior.error,
-        }:
-            metadata = PythonLockfileMetadata.from_lockfile(
-                file_content.content, resolve_name=resolve_name
-            )
-            validate_metadata(
-                metadata, request.interpreter_constraints, request.requirements, python_setup
-            )
-        requirements_file_digest = await Get(Digest, CreateDigest([file_content]))
+        argv.extend(["--requirement", lock_path, "--no-transitive"])
+        requirement_count = len(lock_bytes.decode().splitlines())
+        maybe_validate_metadata(
+            parse_metadata, request.interpreter_constraints, request.requirements, python_setup  # type: ignore[arg-type]
+        )
+
     else:
         assert isinstance(request.requirements, PexRequirements)
         is_monolithic_resolve = request.requirements.is_all_constraints_resolve

src/python/pants/backend/python/util_rules/pex_requirements.py

@@ -5,7 +5,7 @@
 
 import logging
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Iterable, Iterator
+from typing import TYPE_CHECKING, Callable, Iterable, Iterator
 
 from pants.backend.python.pip_requirement import PipRequirement
 from pants.backend.python.subsystems.setup import InvalidLockfileBehavior, PythonSetup
@@ -15,7 +15,7 @@
     InvalidPythonLockfileReason,
     PythonLockfileMetadata,
 )
-from pants.core.util_rules.lockfile_metadata import InvalidLockfileError
+from pants.core.util_rules.lockfile_metadata import InvalidLockfileError, LockfileMetadataValidation
 from pants.engine.fs import FileContent
 from pants.util.docutil import doc_url
 from pants.util.meta import frozen_after_init
@@ -109,12 +109,14 @@ def __bool__(self) -> bool:
         return bool(self.req_strings)
 
 
-def validate_metadata(
-    metadata: PythonLockfileMetadata,
+def maybe_validate_metadata(
+    parse_metadata: Callable[[], PythonLockfileMetadata],
     interpreter_constraints: InterpreterConstraints,
     requirements: (Lockfile | LockfileContent),
     python_setup: PythonSetup,
 ) -> None:
+    if python_setup.invalid_lockfile_behavior == InvalidLockfileBehavior.ignore:
+        return
 
     # TODO(#12314): Improve this message: `Requirement.parse` raises `InvalidRequirement`, which
     # doesn't have mypy stubs at the moment; it may be hard to catch this exception and typecheck.
@@ -124,103 +126,93 @@ def validate_metadata(
         else None
     )
 
+    metadata = parse_metadata()
     validation = metadata.is_valid_for(
         requirements.lockfile_hex_digest,
         interpreter_constraints,
         python_setup.interpreter_universe,
         req_strings,
     )
-
     if validation:
         return
 
-    def tool_message_parts(
-        requirements: (ToolCustomLockfile | ToolDefaultLockfile),
-    ) -> Iterator[str]:
-
-        tool_name = requirements.options_scope_name
-        uses_source_plugins = requirements.uses_source_plugins
-        uses_project_interpreter_constraints = requirements.uses_project_interpreter_constraints
-
-        yield "You are using "
+    message = (
+        "".join(
+            _invalid_tool_lockfile_error(
+                validation,
+                requirements,
+                actual_interpreter_constraints=interpreter_constraints,
+                lockfile_interpreter_constraints=metadata.valid_for_interpreter_constraints,
+            )
+        ).strip()
+        if isinstance(requirements, (ToolCustomLockfile, ToolDefaultLockfile))
+        else str(validation.failure_reasons)
+    )
 
-        if isinstance(requirements, ToolDefaultLockfile):
-            yield "the `<default>` lockfile provided by Pants "
-        elif isinstance(requirements, ToolCustomLockfile):
-            yield f"the lockfile at {requirements.file_path} "
+    if python_setup.invalid_lockfile_behavior == InvalidLockfileBehavior.error:
+        raise InvalidLockfileError(message)
+    logger.warning("%s", message)
+
+
+def _invalid_tool_lockfile_error(
+    validation: LockfileMetadataValidation,
+    requirements: ToolCustomLockfile | ToolDefaultLockfile,
+    *,
+    actual_interpreter_constraints: InterpreterConstraints,
+    lockfile_interpreter_constraints: InterpreterConstraints,
+) -> Iterator[str]:
+    tool_name = requirements.options_scope_name
+
+    yield "You are using "
+    yield "the `<default>` lockfile provided by Pants " if isinstance(
+        requirements, ToolDefaultLockfile
+    ) else f"the lockfile at {requirements.file_path} "
+    yield (
+        f"to install the tool `{tool_name}`, but it is not compatible with your "
+        "configuration: "
+        "\n\n"
+    )
 
+    if any(
+        i
+        in (
+            InvalidPythonLockfileReason.INVALIDATION_DIGEST_MISMATCH,
+            InvalidPythonLockfileReason.REQUIREMENTS_MISMATCH,
+        )
+        for i in validation.failure_reasons
+    ):
+        # TODO(12314): Add message showing _which_ requirements diverged.
         yield (
-            f"to install the tool `{tool_name}`, but it is not compatible with your "
-            "configuration: "
-            "\n\n"
+            "- You have set different requirements than those used to generate the lockfile. "
+            f"You can fix this by not setting `[{tool_name}].version`, "
         )
+        if requirements.uses_source_plugins:
+            yield f"`[{tool_name}].source_plugins`, "
+        yield f"and `[{tool_name}].extra_requirements`, or by using a new custom lockfile.\n"
 
-        if any(
-            i == InvalidPythonLockfileReason.INVALIDATION_DIGEST_MISMATCH
-            or i == InvalidPythonLockfileReason.REQUIREMENTS_MISMATCH
-            for i in validation.failure_reasons
-        ):
-            # TODO(12314): Add message showing _which_ requirements diverged.
-
-            yield (
-                "- You have set different requirements than those used to generate the lockfile. "
-                f"You can fix this by not setting `[{tool_name}].version`, "
-            )
-
-            if uses_source_plugins:
-                yield f"`[{tool_name}].source_plugins`, "
-
-            yield (
-                f"and `[{tool_name}].extra_requirements`, or by using a new "
-                "custom lockfile."
-                "\n"
-            )
-
-        if (
-            InvalidPythonLockfileReason.INTERPRETER_CONSTRAINTS_MISMATCH
-            in validation.failure_reasons
-        ):
-            yield (
-                f"- You have set interpreter constraints (`{interpreter_constraints}`) that "
-                "are not compatible with those used to generate the lockfile "
-                f"(`{metadata.valid_for_interpreter_constraints}`). "
-            )
-            if not uses_project_interpreter_constraints:
-                yield (
-                    f"You can fix this by not setting `[{tool_name}].interpreter_constraints`, "
-                    "or by using a new custom lockfile. "
-                )
-            else:
-                yield (
-                    f"`{tool_name}` determines its interpreter constraints based on your code's own "
-                    "constraints. To fix this error, you can either change your code's constraints "
-                    f"(see {doc_url('python-interpreter-compatibility')}) or by generating a new "
-                    "custom lockfile. "
-                )
-            yield "\n"
-
+    if InvalidPythonLockfileReason.INTERPRETER_CONSTRAINTS_MISMATCH in validation.failure_reasons:
+        yield (
+            f"- You have set interpreter constraints (`{actual_interpreter_constraints}`) that "
+            "are not compatible with those used to generate the lockfile "
+            f"(`{lockfile_interpreter_constraints}`). "
+        )
+        yield (
+            f"You can fix this by not setting `[{tool_name}].interpreter_constraints`, "
+            "or by using a new custom lockfile. "
+        ) if not requirements.uses_project_interpreter_constraints else (
+            f"`{tool_name}` determines its interpreter constraints based on your code's own "
+            "constraints. To fix this error, you can either change your code's constraints "
+            f"(see {doc_url('python-interpreter-compatibility')}) or generat a new "
+            "custom lockfile. "
+        )
         yield "\n"
 
-        if not isinstance(requirements, ToolCustomLockfile):
-            yield (
-                "To generate a custom lockfile based on your current configuration, set "
-                f"`[{tool_name}].lockfile` to where you want to create the lockfile, then run "
-                f"`./pants generate-lockfiles --resolve={tool_name}`. "
-            )
-        else:
-            yield (
-                "To regenerate your lockfile based on your current configuration, run "
-                f"`./pants generate-lockfiles --resolve={tool_name}`. "
-            )
-
-    message: str
-    if isinstance(requirements, (ToolCustomLockfile, ToolDefaultLockfile)):
-        message = "".join(tool_message_parts(requirements)).strip()
-    else:
-        # TODO(12314): Improve this message
-        raise InvalidLockfileError(f"{validation.failure_reasons}")
-
-    if python_setup.invalid_lockfile_behavior == InvalidLockfileBehavior.error:
-        raise ValueError(message)
-    else:
-        logger.warning("%s", message)
+    yield "\n"
+    yield (
+        "To regenerate your lockfile based on your current configuration, run "
+        f"`./pants generate-lockfiles --resolve={tool_name}`. "
+    ) if isinstance(requirements, ToolCustomLockfile) else (
+        "To generate a custom lockfile based on your current configuration, set "
+        f"`[{tool_name}].lockfile` to where you want to create the lockfile, then run "
+        f"`./pants generate-lockfiles --resolve={tool_name}`. "
+    )