[internal] --generate-lockfiles-resolve does not determine PythonLockfileRequest for unspecified tools

src/python/pants/backend/awslambda/python/lambdex.py

@@ -25,8 +25,8 @@ class Lambdex(PythonToolBase):
     default_lockfile_url = git_url(default_lockfile_path)
 
 
-class LambdexLockfileSentinel:
-    pass
+class LambdexLockfileSentinel(PythonToolLockfileSentinel):
+    options_scope = Lambdex.options_scope
 
 
 @rule

src/python/pants/backend/codegen/protobuf/python/python_protobuf_subsystem.py

@@ -74,8 +74,8 @@ class PythonProtobufMypyPlugin(PythonToolRequirementsBase):
     default_lockfile_url = git_url(default_lockfile_path)
 
 
-class MypyProtobufLockfileSentinel:
-    pass
+class MypyProtobufLockfileSentinel(PythonToolLockfileSentinel):
+    options_scope = PythonProtobufMypyPlugin.options_scope
 
 
 @rule

src/python/pants/backend/python/goals/coverage_py.py

@@ -216,8 +216,8 @@ def global_report(self) -> bool:
         return cast(bool, self.options.global_report)
 
 
-class CoveragePyLockfileSentinel:
-    pass
+class CoveragePyLockfileSentinel(PythonToolLockfileSentinel):
+    options_scope = CoverageSubsystem.options_scope
 
 
 @rule

src/python/pants/backend/python/goals/lockfile.py

@@ -6,7 +6,7 @@
 import logging
 from dataclasses import dataclass
 from pathlib import PurePath
-from typing import Iterable, Sequence, cast
+from typing import ClassVar, Iterable, Sequence, cast
 
 from pants.backend.python.subsystems.poetry import (
     POETRY_LAUNCHER,
@@ -45,7 +45,7 @@
 
 @union
 class PythonToolLockfileSentinel:
-    pass
+    options_scope: ClassVar[str]
 
 
 class GenerateLockfilesSubsystem(GoalSubsystem):
@@ -128,6 +128,13 @@ def from_tool(
         rather than the option `--interpreter-constraints`, you must pass the arg
         `interpreter_constraints`.
         """
+        if not subsystem.uses_lockfile:
+            return cls(
+                FrozenOrderedSet(),
+                InterpreterConstraints(),
+                resolve_name=subsystem.options_scope,
+                lockfile_dest=subsystem.lockfile,
+            )
         return cls(
             requirements=FrozenOrderedSet((*subsystem.all_requirements, *extra_requirements)),
             interpreter_constraints=(
@@ -239,20 +246,21 @@ async def generate_lockfiles_goal(
     union_membership: UnionMembership,
     generate_lockfiles_subsystem: GenerateLockfilesSubsystem,
 ) -> GenerateLockfilesGoal:
-    # TODO(#12314): this factoring requires that `PythonLockfileRequest`s need to be calculated
-    #  for tools even if the user does not want to generate a lockfile for that tool. That code
-    #  can be quite slow when it requires computing interpreter constraints.
-    all_requests = await MultiGet(
+    specified_tool_requests = await MultiGet(
         Get(PythonLockfileRequest, PythonToolLockfileSentinel, sentinel())
-        for sentinel in union_membership.get(PythonToolLockfileSentinel)
+        for sentinel in determine_tool_sentinels_to_generate(
+            union_membership[PythonToolLockfileSentinel],
+            generate_lockfiles_subsystem.resolve_names,
+        )
     )
-
     results = await MultiGet(
         Get(PythonLockfile, PythonLockfileRequest, req)
-        for req in determine_resolves_to_generate(
-            all_requests, generate_lockfiles_subsystem.resolve_names
+        for req in filter_tool_lockfile_requests(
+            specified_tool_requests,
+            resolve_specified=bool(generate_lockfiles_subsystem.resolve_names),
         )
     )
+
     merged_digest = await Get(Digest, MergeDigests(res.digest for res in results))
     workspace.write_digest(merged_digest)
     for result in results:
@@ -265,38 +273,22 @@ class UnrecognizedResolveNamesError(Exception):
     pass
 
 
-def determine_resolves_to_generate(
-    all_tool_lockfile_requests: Sequence[PythonLockfileRequest],
+def determine_tool_sentinels_to_generate(
+    all_tool_sentinels: Iterable[type[PythonToolLockfileSentinel]],
     requested_resolve_names: Sequence[str],
-) -> list[PythonLockfileRequest]:
+) -> list[type[PythonToolLockfileSentinel]]:
     if not requested_resolve_names:
-        return [
-            req
-            for req in all_tool_lockfile_requests
-            if req.lockfile_dest not in (NO_TOOL_LOCKFILE, DEFAULT_TOOL_LOCKFILE)
-        ]
-
-    resolve_names_to_requests = {
-        request.resolve_name: request for request in all_tool_lockfile_requests
-    }
+        return list(all_tool_sentinels)
 
-    specified_requests = []
+    resolve_names_to_sentinels = {
+        sentinel.options_scope: sentinel for sentinel in all_tool_sentinels
+    }
+    specified_sentinels = []
     unrecognized_resolve_names = []
     for resolve_name in requested_resolve_names:
-        request = resolve_names_to_requests.get(resolve_name)
-        if request:
-            if request.lockfile_dest in (NO_TOOL_LOCKFILE, DEFAULT_TOOL_LOCKFILE):
-                logger.warning(
-                    f"You requested to generate a lockfile for {request.resolve_name} because "
-                    "you included it in `--generate-lockfiles-resolve`, but "
-                    f"`[{request.resolve_name}].lockfile` is set to `{request.lockfile_dest}` "
-                    "so a lockfile will not be generated.\n\n"
-                    f"If you would like to generate a lockfile for {request.resolve_name}, please "
-                    f"set `[{request.resolve_name}].lockfile` to the path where it should be "
-                    "generated and run again."
-                )
-            else:
-                specified_requests.append(request)
+        sentinel = resolve_names_to_sentinels.get(resolve_name)
+        if sentinel:
+            specified_sentinels.append(sentinel)
         else:
             unrecognized_resolve_names.append(resolve_name)
 
@@ -311,10 +303,33 @@ def determine_resolves_to_generate(
         raise UnrecognizedResolveNamesError(
             f"Unrecognized resolve {name_description} from the option "
             f"`--generate-lockfiles-resolve`: {unrecognized_str}\n\n"
-            f"All valid resolve names: {sorted(resolve_names_to_requests.keys())}"
+            f"All valid resolve names: {sorted(resolve_names_to_sentinels.keys())}"
         )
 
-    return specified_requests
+    return specified_sentinels
+
+
+def filter_tool_lockfile_requests(
+    specified_requests: Sequence[PythonLockfileRequest], *, resolve_specified: bool
+) -> list[PythonLockfileRequest]:
+    result = []
+    for req in specified_requests:
+        if req.lockfile_dest not in (NO_TOOL_LOCKFILE, DEFAULT_TOOL_LOCKFILE):
+            result.append(req)
+            continue
+        if resolve_specified:
+            resolve = req.resolve_name
+            logger.warning(
+                f"You requested to generate a lockfile for {resolve} because "
+                "you included it in `--generate-lockfiles-resolve`, but "
+                f"`[{resolve}].lockfile` is set to `{req.lockfile_dest}` "
+                "so a lockfile will not be generated.\n\n"
+                f"If you would like to generate a lockfile for {resolve}, please "
+                f"set `[{resolve}].lockfile` to the path where it should be "
+                "generated and run again."
+            )
+
+    return result
 
 
 def rules():

src/python/pants/backend/python/goals/lockfile_test.py

@@ -7,48 +7,42 @@
 
 from pants.backend.python.goals.lockfile import (
     PythonLockfileRequest,
+    PythonToolLockfileSentinel,
     UnrecognizedResolveNamesError,
-    determine_resolves_to_generate,
+    determine_tool_sentinels_to_generate,
+    filter_tool_lockfile_requests,
 )
 from pants.backend.python.subsystems.python_tool_base import DEFAULT_TOOL_LOCKFILE, NO_TOOL_LOCKFILE
 from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
 from pants.util.ordered_set import FrozenOrderedSet
 
 
-def test_determine_resolves_to_generate(caplog) -> None:
-    def create_request(name: str, lockfile_dest: str | None = None) -> PythonLockfileRequest:
-        return PythonLockfileRequest(
-            FrozenOrderedSet(),
-            InterpreterConstraints(),
-            resolve_name=name,
-            lockfile_dest=lockfile_dest or f"{name}.txt",
-        )
+def test_determine_tool_sentinels_to_generate() -> None:
+    class Tool1(PythonToolLockfileSentinel):
+        options_scope = "tool1"
 
-    tool1 = create_request("tool1")
-    tool2 = create_request("tool2")
-    tool3 = create_request("tool3")
-    disabled_tool = create_request("none", lockfile_dest=NO_TOOL_LOCKFILE)
-    default_tool = create_request("default", lockfile_dest=DEFAULT_TOOL_LOCKFILE)
+    class Tool2(PythonToolLockfileSentinel):
+        options_scope = "tool2"
 
-    def assert_chosen(requested: list[str], expected: list[PythonLockfileRequest]) -> None:
-        assert (
-            determine_resolves_to_generate(
-                [tool1, tool2, tool3, disabled_tool, default_tool], requested
-            )
-            == expected
-        )
+    class Tool3(PythonToolLockfileSentinel):
+        options_scope = "tool3"
 
-    assert_chosen([tool2.resolve_name], [tool2])
-    assert_chosen([tool1.resolve_name, tool3.resolve_name], [tool1, tool3])
+    def assert_chosen(
+        requested: list[str], expected: list[type[PythonToolLockfileSentinel]]
+    ) -> None:
+        assert determine_tool_sentinels_to_generate([Tool1, Tool2, Tool3], requested) == expected
 
-    # If none are specifically requested, return all valid.
-    assert_chosen([], [tool1, tool2, tool3])
+    assert_chosen([Tool2.options_scope], [Tool2])
+    assert_chosen([Tool1.options_scope, Tool3.options_scope], [Tool1, Tool3])
+
+    # If none are specifically requested, return all.
+    assert_chosen([], [Tool1, Tool2, Tool3])
 
     with pytest.raises(UnrecognizedResolveNamesError) as exc:
         assert_chosen(["fake"], [])
     assert (
         "Unrecognized resolve name from the option `--generate-lockfiles-resolve`: fake\n\n"
-        "All valid resolve names: ['default', 'none', 'tool1', 'tool2', 'tool3']"
+        "All valid resolve names: ['tool1', 'tool2', 'tool3']"
     ) in str(exc.value)
 
     with pytest.raises(UnrecognizedResolveNamesError) as exc:
@@ -58,18 +52,56 @@ def assert_chosen(requested: list[str], expected: list[PythonLockfileRequest]) -
         "['fake1', 'fake2']"
     ) in str(exc.value)
 
-    # Warn if requested tool is set to disabled or default.
-    assert_chosen([disabled_tool.resolve_name], [])
-    assert len(caplog.records) == 1
-    assert (
-        f"`[{disabled_tool.resolve_name}].lockfile` is set to `{NO_TOOL_LOCKFILE}`" in caplog.text
+
+def test_filter_tool_lockfile_requests(caplog) -> None:
+    def create_request(name: str, lockfile_dest: str | None = None) -> PythonLockfileRequest:
+        return PythonLockfileRequest(
+            FrozenOrderedSet(),
+            InterpreterConstraints(),
+            resolve_name=name,
+            lockfile_dest=lockfile_dest or f"{name}.txt",
+        )
+
+    tool1 = create_request("tool1")
+    tool2 = create_request("tool2")
+    disabled_tool = create_request("none", lockfile_dest=NO_TOOL_LOCKFILE)
+    default_tool = create_request("default", lockfile_dest=DEFAULT_TOOL_LOCKFILE)
+
+    def assert_filtered(
+        extra_request: PythonLockfileRequest | None,
+        *,
+        resolve_specified: bool,
+        expected_log: str | None,
+    ) -> None:
+        requests = [tool1, tool2]
+        if extra_request:
+            requests.append(extra_request)
+        assert filter_tool_lockfile_requests(requests, resolve_specified=resolve_specified) == [
+            tool1,
+            tool2,
+        ]
+        if expected_log:
+            assert len(caplog.records) == 1
+            assert expected_log in caplog.text
+        else:
+            assert not caplog.records
+        caplog.clear()
+
+    assert_filtered(None, resolve_specified=False, expected_log=None)
+    assert_filtered(None, resolve_specified=True, expected_log=None)
+
+    assert_filtered(disabled_tool, resolve_specified=False, expected_log=None)
+    assert_filtered(
+        disabled_tool,
+        resolve_specified=True,
+        expected_log=f"`[{disabled_tool.resolve_name}].lockfile` is set to `{NO_TOOL_LOCKFILE}`",
     )
-    caplog.clear()
 
-    assert_chosen([default_tool.resolve_name], [])
-    assert len(caplog.records) == 1
-    assert (
-        f"`[{default_tool.resolve_name}].lockfile` is set to `{DEFAULT_TOOL_LOCKFILE}`"
-        in caplog.text
+    assert_filtered(default_tool, resolve_specified=False, expected_log=None)
+    assert_filtered(
+        default_tool,
+        resolve_specified=True,
+        expected_log=(
+            f"`[{default_tool.resolve_name}].lockfile` is set to `{DEFAULT_TOOL_LOCKFILE}`"
+        ),
     )
-    caplog.clear()

src/python/pants/backend/python/lint/bandit/subsystem.py

@@ -107,8 +107,8 @@ def config_request(self) -> ConfigFilesRequest:
         )
 
 
-class BanditLockfileSentinel:
-    pass
+class BanditLockfileSentinel(PythonToolLockfileSentinel):
+    options_scope = Bandit.options_scope
 
 
 @rule(
@@ -121,6 +121,9 @@ class BanditLockfileSentinel:
 async def setup_bandit_lockfile(
     _: BanditLockfileSentinel, bandit: Bandit, python_setup: PythonSetup
 ) -> PythonLockfileRequest:
+    if not bandit.uses_lockfile:
+        return PythonLockfileRequest.from_tool(bandit)
+
     # While Bandit will run in partitions, we need a single lockfile that works with every
     # partition.
     #

src/python/pants/backend/python/lint/bandit/subsystem_test.py

@@ -27,7 +27,8 @@ def test_setup_lockfile_interpreter_constraints() -> None:
 
     global_constraint = "==3.9.*"
     rule_runner.set_options(
-        [], env={"PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS": f"['{global_constraint}']"}
+        ["--bandit-lockfile=lockfile.txt"],
+        env={"PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS": f"['{global_constraint}']"},
     )
 
     def assert_ics(build_file: str, expected: list[str]) -> None:

src/python/pants/backend/python/lint/black/subsystem.py

@@ -106,8 +106,8 @@ def config_request(self, dirs: Iterable[str]) -> ConfigFilesRequest:
         )
 
 
-class BlackLockfileSentinel:
-    pass
+class BlackLockfileSentinel(PythonToolLockfileSentinel):
+    options_scope = Black.options_scope
 
 
 @rule(
@@ -117,6 +117,9 @@ class BlackLockfileSentinel:
 async def setup_black_lockfile(
     _: BlackLockfileSentinel, black: Black, python_setup: PythonSetup
 ) -> PythonLockfileRequest:
+    if not black.uses_lockfile:
+        return PythonLockfileRequest.from_tool(black)
+
     constraints = black.interpreter_constraints
     if black.options.is_default("interpreter_constraints"):
         all_build_targets = await Get(UnexpandedTargets, AddressSpecs([DescendantAddresses("")]))

src/python/pants/backend/python/lint/black/subsystem_test.py

@@ -27,7 +27,8 @@ def test_setup_lockfile_interpreter_constraints() -> None:
 
     global_constraint = "==3.9.*"
     rule_runner.set_options(
-        [], env={"PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS": f"['{global_constraint}']"}
+        ["--black-lockfile=lockfile.txt"],
+        env={"PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS": f"['{global_constraint}']"},
     )
 
     def assert_ics(build_file: str, expected: list[str]) -> None:

src/python/pants/backend/python/lint/docformatter/subsystem.py

@@ -58,8 +58,8 @@ def args(self) -> Tuple[str, ...]:
         return tuple(self.options.args)
 
 
-class DocformatterLockfileSentinel:
-    pass
+class DocformatterLockfileSentinel(PythonToolLockfileSentinel):
+    options_scope = Docformatter.options_scope
 
 
 @rule