Make python-setup resolve_all_constraints a bool.

pants.toml

@@ -72,7 +72,6 @@ root_patterns = [
 
 [python-setup]
 requirement_constraints = "3rdparty/python/constraints.txt"
-resolve_all_constraints = "nondeployables"
 interpreter_constraints = [">=3.7,<3.10"]
 
 [black]

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

@@ -42,7 +42,7 @@
     TransitiveTargets,
     TransitiveTargetsRequest,
 )
-from pants.python.python_setup import PythonSetup, ResolveAllConstraintsOption
+from pants.python.python_setup import PythonSetup
 from pants.util.logging import LogLevel
 from pants.util.meta import frozen_after_init
 from pants.util.ordered_set import FrozenOrderedSet
@@ -274,10 +274,7 @@ async def pex_from_targets(
                 f"requirements: {', '.join(unconstrained_projects)}"
             )
 
-        if python_setup.resolve_all_constraints == ResolveAllConstraintsOption.ALWAYS or (
-            python_setup.resolve_all_constraints == ResolveAllConstraintsOption.NONDEPLOYABLES
-            and request.internal_only
-        ):
+        if python_setup.resolve_all_constraints:
             if unconstrained_projects:
                 logger.warning(
                     "Ignoring `[python_setup].resolve_all_constraints` option because constraints "
@@ -308,13 +305,12 @@ async def pex_from_targets(
                     ),
                 )
     elif (
-        python_setup.resolve_all_constraints != ResolveAllConstraintsOption.NEVER
+        python_setup.resolve_all_constraints
         and python_setup.resolve_all_constraints_was_set_explicitly()
     ):
         raise ValueError(
-            "[python-setup].resolve_all_constraints is set to "
-            f"{python_setup.resolve_all_constraints.value}, so "
-            "either [python-setup].requirement_constraints or "
+            "[python-setup].resolve_all_constraints is enabled, so either "
+            "[python-setup].requirement_constraints or "
             "[python-setup].requirement_constraints_target must also be provided."
         )
 

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

@@ -1,13 +1,15 @@
 # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from __future__ import annotations
+
 import json
 import subprocess
 import sys
 from dataclasses import dataclass
 from pathlib import Path, PurePath
 from textwrap import dedent
-from typing import List, Optional, cast
+from typing import List, cast
 
 import pytest
 from _pytest.tmpdir import TempPathFactory
@@ -18,7 +20,6 @@
 from pants.backend.python.util_rules.pex_from_targets import PexFromTargetsRequest
 from pants.build_graph.address import Address
 from pants.engine.internals.scheduler import ExecutionError
-from pants.python.python_setup import ResolveAllConstraintsOption
 from pants.testutil.rule_runner import QueryRule, RuleRunner
 from pants.util.contextutil import pushd
 
@@ -173,8 +174,8 @@ def test_constraints_validation(tmp_path_factory: TempPathFactory, rule_runner:
     )
 
     def get_pex_request(
-        constraints_file: Optional[str],
-        resolve_all: Optional[ResolveAllConstraintsOption],
+        constraints_file: str | None,
+        resolve_all_constraints: bool | None,
         *,
         direct_deps_only: bool = False,
     ) -> PexRequest:
@@ -185,28 +186,28 @@ def get_pex_request(
             internal_only=True,
             direct_deps_only=direct_deps_only,
         )
-        if resolve_all:
-            args.append(f"--python-setup-resolve-all-constraints={resolve_all.value}")
+        if resolve_all_constraints is not None:
+            args.append(f"--python-setup-resolve-all-constraints={resolve_all_constraints!r}")
         if constraints_file:
             args.append(f"--python-setup-requirement-constraints={constraints_file}")
         args.append("--python-repos-indexes=[]")
         args.append(f"--python-repos-repos={find_links}")
         rule_runner.set_options(args, env_inherit={"PATH"})
         return rule_runner.request(PexRequest, [request])
 
-    pex_req1 = get_pex_request("constraints1.txt", ResolveAllConstraintsOption.NEVER)
+    pex_req1 = get_pex_request("constraints1.txt", resolve_all_constraints=False)
     assert pex_req1.requirements == PexRequirements(
         ["foo-bar>=0.1.2", "bar==5.5.5", "baz", url_req]
     )
     assert pex_req1.repository_pex is None
 
     pex_req1_direct = get_pex_request(
-        "constraints1.txt", ResolveAllConstraintsOption.NEVER, direct_deps_only=True
+        "constraints1.txt", resolve_all_constraints=False, direct_deps_only=True
     )
     assert pex_req1_direct.requirements == PexRequirements(["baz", url_req])
     assert pex_req1_direct.repository_pex is None
 
-    pex_req2 = get_pex_request("constraints1.txt", ResolveAllConstraintsOption.ALWAYS)
+    pex_req2 = get_pex_request("constraints1.txt", resolve_all_constraints=True)
     assert pex_req2.requirements == PexRequirements(
         ["foo-bar>=0.1.2", "bar==5.5.5", "baz", url_req]
     )
@@ -217,20 +218,20 @@ def get_pex_request(
     )
 
     pex_req2_direct = get_pex_request(
-        "constraints1.txt", ResolveAllConstraintsOption.ALWAYS, direct_deps_only=True
+        "constraints1.txt", resolve_all_constraints=True, direct_deps_only=True
     )
     assert pex_req2_direct.requirements == PexRequirements(["baz", url_req])
     assert pex_req2_direct.repository_pex == repository_pex
 
     with pytest.raises(ExecutionError) as err:
-        get_pex_request(None, ResolveAllConstraintsOption.ALWAYS)
+        get_pex_request(None, resolve_all_constraints=True)
     assert len(err.value.wrapped_exceptions) == 1
     assert isinstance(err.value.wrapped_exceptions[0], ValueError)
     assert (
-        "[python-setup].resolve_all_constraints is set to always, so "
+        "[python-setup].resolve_all_constraints is enabled, so "
         "either [python-setup].requirement_constraints or "
         "[python-setup].requirement_constraints_target must also be provided."
     ) in str(err.value)
 
     # Shouldn't error, as we don't explicitly set --resolve-all-constraints.
-    get_pex_request(None, None)
+    get_pex_request(None, resolve_all_constraints=None)

src/python/pants/help/help_info_extracter.py

@@ -433,7 +433,7 @@ def get_option_help_info(self, args, kwargs):
                 scoped_arg = arg
             scoped_cmd_line_args.append(scoped_arg)
 
-            if kwargs.get("type") == bool:
+            if Parser.is_bool(kwargs):
                 if is_short_arg:
                     display_args.append(scoped_arg)
                 else:

src/python/pants/option/parser.py

@@ -9,6 +9,7 @@
 import os
 import re
 import traceback
+import typing
 from collections import defaultdict
 from dataclasses import dataclass
 from enum import Enum
@@ -79,7 +80,19 @@ class Parser:
     """
 
     @staticmethod
-    def _ensure_bool(val: bool | str) -> bool:
+    def is_bool(kwargs: Mapping[str, Any]) -> bool:
+        type_arg = kwargs.get("type")
+        if type_arg is None:
+            return False
+        if type_arg is bool:
+            return True
+        try:
+            return typing.get_type_hints(type_arg).get("return") is bool
+        except TypeError:
+            return False
+
+    @staticmethod
+    def ensure_bool(val: bool | str) -> bool:
         if isinstance(val, bool):
             return val
         if isinstance(val, str):
@@ -95,7 +108,7 @@ def _ensure_bool(val: bool | str) -> bool:
     def _invert(cls, s: bool | str | None) -> bool | None:
         if s is None:
             return None
-        b = cls._ensure_bool(s)
+        b = cls.ensure_bool(s)
         return not b
 
     @classmethod
@@ -235,7 +248,7 @@ def parse_args(self, parse_args_request: ParseArgsRequest) -> OptionValueContain
             # specified as a command-line flag, so we don't spam users with deprecated option values
             # specified in config, which isn't something they control.
             implicit_value = kwargs.get("implicit_value")
-            if implicit_value is None and kwargs.get("type") == bool:
+            if implicit_value is None and self.is_bool(kwargs):
                 implicit_value = True  # Allows --foo to mean --foo=true.
 
             flag_vals: list[int | float | bool | str] = []
@@ -253,7 +266,7 @@ def add_flag_val(v: int | float | bool | str | None) -> None:
             for arg in args:
                 # If the user specified --no-foo on the cmd line, treat it as if the user specified
                 # --foo, but with the inverse value.
-                if kwargs.get("type") == bool:
+                if self.is_bool(kwargs):
                     inverse_arg = self._inverse_arg(arg)
                     if inverse_arg in flag_value_map:
                         flag_value_map[arg] = [self._invert(v) for v in flag_value_map[inverse_arg]]
@@ -386,13 +399,13 @@ def register(self, *args, **kwargs) -> None:
             dest = self.parse_dest(*args, **kwargs)
             self._check_deprecated(dest, kwargs, print_warning=False)
 
-        if kwargs.get("type") == bool:
+        if self.is_bool(kwargs):
             default = kwargs.get("default")
             if default is None:
                 # Unless a tri-state bool is explicitly opted into with the `UnsetBool` default value,
                 # boolean options always have an implicit boolean-typed default. We make that default
                 # explicit here.
-                kwargs["default"] = not self._ensure_bool(kwargs.get("implicit_value", True))
+                kwargs["default"] = not self.ensure_bool(kwargs.get("implicit_value", True))
             elif default is UnsetBool:
                 kwargs["default"] = None
 
@@ -560,7 +573,7 @@ def to_value_type(self, val_str, type_arg, member_type, dest):
         if val_str is None:
             return None
         if type_arg == bool:
-            return self._ensure_bool(val_str)
+            return self.ensure_bool(val_str)
         try:
             if type_arg == list:
                 return ListValueComponent.create(val_str, member_type=member_type)

src/python/pants/python/python_setup.py

@@ -13,8 +13,11 @@
 from pex.variables import Variables
 
 from pants.base.build_environment import get_buildroot
+from pants.base.deprecated import deprecated_conditional
 from pants.engine.environment import Environment
 from pants.option.custom_types import file_option, target_option
+from pants.option.errors import BooleanConversionError
+from pants.option.parser import Parser
 from pants.option.subsystem import Subsystem
 from pants.util.memo import memoized_method
 
@@ -38,6 +41,21 @@ class ResolveAllConstraintsOption(Enum):
     # Always use the entire constraints file.
     ALWAYS = "always"
 
+    @classmethod
+    def parse(cls, value: bool | str) -> bool:
+        try:
+            return Parser.ensure_bool(value)
+        except BooleanConversionError:
+            enum_value = cls(value)
+            bool_value = enum_value is not cls.NEVER
+            deprecated_conditional(
+                lambda: True,
+                removal_version="2.6.0.dev0",
+                entity_description="python-setup resolve_all_constraints non boolean values",
+                hint_message=f"Instead of {enum_value.value!r} use {bool_value!r}.",
+            )
+            return bool_value
+
 
 class PythonSetup(Subsystem):
     options_scope = "python-setup"
@@ -90,19 +108,18 @@ def register_options(cls, register):
         register(
             "--resolve-all-constraints",
             advanced=True,
-            default=ResolveAllConstraintsOption.NONDEPLOYABLES,
-            type=ResolveAllConstraintsOption,
+            default=True,
+            choices=(*(raco.value for raco in ResolveAllConstraintsOption), True, False),
+            type=ResolveAllConstraintsOption.parse,
             help=(
-                "If set, and the requirements of the code being operated on are a subset of the "
-                "constraints file, then the entire constraints file will be used instead of the "
-                "subset. If unset, or any requirement of the code being operated on is not in the "
-                "constraints file, each subset will be independently resolved as needed, which is "
-                "more correct - work is only invalidated if a requirement it actually depends on "
-                "changes - but also a lot slower, due to the extra resolving. "
-                "\n\n* `never` will always use proper subsets, regardless of the goal being "
-                "run.\n* `nondeployables` will use proper subsets for `./pants package`, but "
-                "otherwise attempt to use a single resolve.\n* `always` will always attempt to use "
-                "a single resolve."
+                "If enabled, when resolving requirements, Pants will first resolve your entire "
+                "constraints file as a single global resolve. Then, if the code uses a subset of "
+                "your constraints file, Pants will extract the relevant requirements from that "
+                "global resolve so that only what's actually needed gets used. If disabled, Pants "
+                "will not use a global resolve and will resolve each subset of your requirements "
+                "independently."
+                "\n\nUsually this option should be enabled because it can result in far fewer "
+                "resolves."
                 "\n\nRequires [python-setup].requirement_constraints to be set."
             ),
         )
@@ -162,8 +179,8 @@ def requirement_constraints_target(self) -> str | None:
         return cast("str | None", self.options.requirement_constraints_target)
 
     @property
-    def resolve_all_constraints(self) -> ResolveAllConstraintsOption:
-        return cast(ResolveAllConstraintsOption, self.options.resolve_all_constraints)
+    def resolve_all_constraints(self) -> bool:
+        return cast(bool, self.options.resolve_all_constraints)
 
     def resolve_all_constraints_was_set_explicitly(self) -> bool:
         return not self.options.is_default("resolve_all_constraints")