From 8c319aeb5d0413dfa04d1b538186fb1c1607e785 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Tue, 31 Aug 2021 17:37:52 -0700 Subject: [PATCH] [internal] Add `[python-setup].experimental_resolves_to_lockfiles` and hook up to `./pants generate-lockfiles` (#12703) This option does not currently do anything..but it will be where users define their "named resolve": ```toml [python-setup] resolves_to_lockfiles = { default = "default_lock.txt", data-science = "ds_lock.txt" } ``` Then, users can set `resolve="data-science"` on targets. They can generate the lockfile with `./pants generate-lockfiles --resolve='data-science'`. -- This PR is prework to add the option and wire it up to `./pants generate-lockfiles`, including checking that the named resolve is not ambiguous with a tool's resolve name (its option scope). [ci skip-rust] [ci skip-build-wheels] --- .../pants/backend/python/goals/lockfile.py | 102 +++++++++++++----- .../backend/python/goals/lockfile_test.py | 63 ++++++++--- src/python/pants/python/python_setup.py | 17 +++ 3 files changed, 142 insertions(+), 40 deletions(-) diff --git a/src/python/pants/backend/python/goals/lockfile.py b/src/python/pants/backend/python/goals/lockfile.py index d729b2f2aac..70441df8c63 100644 --- a/src/python/pants/backend/python/goals/lockfile.py +++ b/src/python/pants/backend/python/goals/lockfile.py @@ -37,6 +37,7 @@ from pants.engine.process import ProcessCacheScope, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule, rule from pants.engine.unions import UnionMembership, union +from pants.python.python_setup import PythonSetup from pants.util.logging import LogLevel from pants.util.ordered_set import FrozenOrderedSet @@ -63,13 +64,17 @@ def register_options(cls, register) -> None: advanced=False, help=( "Only generate lockfiles for the specified resolve(s).\n\n" - "For now, resolves are the options scope for each Python tool that supports " - "lockfiles, such as `black`, `pytest`, and `mypy-protobuf`. For example, you can " - "run `./pants generate-lockfiles --resolve=black --resolve=pytest` to only " - "generate the lockfile for those two tools.\n\n" + "Resolves are the logical names for the different lockfiles used in your project. " + "For your own code's dependencies, these come from the option " + "`[python-setup].experimental_resolves_to_lockfiles`. For tool lockfiles, resolve " + "names are the options scope for that tool such as `black`, `pytest`, and " + "`mypy-protobuf`.\n\n" + "For example, you can run `./pants generate-lockfiles --resolve=black " + "--resolve=pytest --resolve=data-science` to only generate lockfiles for those " + "two tools and your resolve named `data-science`.\n\n" "If you specify an invalid resolve name, like 'fake', Pants will output all " "possible values.\n\n" - "If not specified, will generate for all resolves." + "If not specified, Pants will generate lockfiles for all resolves." ), ) register( @@ -245,13 +250,16 @@ async def generate_lockfiles_goal( workspace: Workspace, union_membership: UnionMembership, generate_lockfiles_subsystem: GenerateLockfilesSubsystem, + python_setup: PythonSetup, ) -> GenerateLockfilesGoal: + _specified_user_lockfiles, specified_tool_sentinels = determine_resolves_to_generate( + python_setup.resolves_to_lockfiles.keys(), + union_membership[PythonToolLockfileSentinel], + generate_lockfiles_subsystem.resolve_names, + ) specified_tool_requests = await MultiGet( Get(PythonLockfileRequest, PythonToolLockfileSentinel, sentinel()) - for sentinel in determine_tool_sentinels_to_generate( - union_membership[PythonToolLockfileSentinel], - generate_lockfiles_subsystem.resolve_names, - ) + for sentinel in specified_tool_sentinels ) results = await MultiGet( Get(PythonLockfile, PythonLockfileRequest, req) @@ -269,44 +277,88 @@ async def generate_lockfiles_goal( return GenerateLockfilesGoal(exit_code=0) +class AmbiguousResolveNamesError(Exception): + def __init__(self, ambiguous_names: list[str]) -> None: + if len(ambiguous_names) == 1: + first_paragraph = ( + "A resolve name from the option " + "`[python-setup].experimental_resolves_to_lockfiles` collides with the name of a " + f"tool resolve: {ambiguous_names[0]}" + ) + else: + first_paragraph = ( + "Some resolve names from the option " + "`[python-setup].experimental_resolves_to_lockfiles` collide with the names of " + f"tool resolves: {sorted(ambiguous_names)}" + ) + super().__init__( + f"{first_paragraph}\n\n" + "To fix, please update `[python-setup].experimental_resolves_to_lockfiles` to use " + "different resolve names." + ) + + class UnrecognizedResolveNamesError(Exception): - pass + def __init__( + self, unrecognized_resolve_names: list[str], all_valid_names: Iterable[str] + ) -> None: + # TODO(#12314): maybe implement "Did you mean?" + if len(unrecognized_resolve_names) == 1: + unrecognized_str = unrecognized_resolve_names[0] + name_description = "name" + else: + unrecognized_str = str(sorted(unrecognized_resolve_names)) + name_description = "names" + super().__init__( + f"Unrecognized resolve {name_description} from the option " + f"`--generate-lockfiles-resolve`: {unrecognized_str}\n\n" + f"All valid resolve names: {sorted(all_valid_names)}" + ) -def determine_tool_sentinels_to_generate( +def determine_resolves_to_generate( + all_user_resolves: Iterable[str], all_tool_sentinels: Iterable[type[PythonToolLockfileSentinel]], requested_resolve_names: Sequence[str], -) -> list[type[PythonToolLockfileSentinel]]: - if not requested_resolve_names: - return list(all_tool_sentinels) +) -> tuple[list[str], list[type[PythonToolLockfileSentinel]]]: + """Apply the `--resolve` option to determine which resolves are specified. + Return a tuple of `(user_resolves, tool_lockfile_sentinels)`. + """ resolve_names_to_sentinels = { sentinel.options_scope: sentinel for sentinel in all_tool_sentinels } + + ambiguous_resolve_names = [ + resolve_name + for resolve_name in all_user_resolves + if resolve_name in resolve_names_to_sentinels + ] + if ambiguous_resolve_names: + raise AmbiguousResolveNamesError(ambiguous_resolve_names) + + if not requested_resolve_names: + return list(all_user_resolves), list(all_tool_sentinels) + + specified_user_resolves = [] specified_sentinels = [] unrecognized_resolve_names = [] for resolve_name in requested_resolve_names: sentinel = resolve_names_to_sentinels.get(resolve_name) if sentinel: specified_sentinels.append(sentinel) + elif resolve_name in all_user_resolves: + specified_user_resolves.append(resolve_name) else: unrecognized_resolve_names.append(resolve_name) if unrecognized_resolve_names: - # TODO(#12314): maybe implement "Did you mean?" - if len(unrecognized_resolve_names) == 1: - unrecognized_str = unrecognized_resolve_names[0] - name_description = "name" - else: - unrecognized_str = str(sorted(unrecognized_resolve_names)) - name_description = "names" 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_sentinels.keys())}" + unrecognized_resolve_names, + {*all_user_resolves, *resolve_names_to_sentinels.keys()}, ) - return specified_sentinels + return specified_user_resolves, specified_sentinels def filter_tool_lockfile_requests( diff --git a/src/python/pants/backend/python/goals/lockfile_test.py b/src/python/pants/backend/python/goals/lockfile_test.py index c99e33ce3bf..8f6c335e4c6 100644 --- a/src/python/pants/backend/python/goals/lockfile_test.py +++ b/src/python/pants/backend/python/goals/lockfile_test.py @@ -6,10 +6,11 @@ import pytest from pants.backend.python.goals.lockfile import ( + AmbiguousResolveNamesError, PythonLockfileRequest, PythonToolLockfileSentinel, UnrecognizedResolveNamesError, - determine_tool_sentinels_to_generate, + determine_resolves_to_generate, filter_tool_lockfile_requests, ) from pants.backend.python.subsystems.python_tool_base import DEFAULT_TOOL_LOCKFILE, NO_TOOL_LOCKFILE @@ -27,29 +28,61 @@ class Tool2(PythonToolLockfileSentinel): class Tool3(PythonToolLockfileSentinel): options_scope = "tool3" + all_user_resolves = ["u1", "u2", "u3"] + def assert_chosen( - requested: list[str], expected: list[type[PythonToolLockfileSentinel]] + requested: list[str], + expected_user_resolves: list[str], + expected_tools: list[type[PythonToolLockfileSentinel]], ) -> None: - assert determine_tool_sentinels_to_generate([Tool1, Tool2, Tool3], requested) == expected + user_resolves, tools = determine_resolves_to_generate( + all_user_resolves, [Tool1, Tool2, Tool3], requested + ) + assert user_resolves == expected_user_resolves + assert tools == expected_tools - assert_chosen([Tool2.options_scope], [Tool2]) - assert_chosen([Tool1.options_scope, Tool3.options_scope], [Tool1, Tool3]) + assert_chosen( + [Tool2.options_scope, "u2"], expected_user_resolves=["u2"], expected_tools=[Tool2] + ) + assert_chosen( + [Tool1.options_scope, Tool3.options_scope], + expected_user_resolves=[], + expected_tools=[Tool1, Tool3], + ) # If none are specifically requested, return all. - assert_chosen([], [Tool1, Tool2, Tool3]) + assert_chosen( + [], expected_user_resolves=["u1", "u2", "u3"], expected_tools=[Tool1, Tool2, Tool3] + ) + + with pytest.raises(UnrecognizedResolveNamesError): + assert_chosen(["fake"], expected_user_resolves=[], expected_tools=[]) + + # Error if same resolve name used for tool lockfiles and user lockfiles. + class AmbiguousTool(PythonToolLockfileSentinel): + options_scope = "ambiguous" + + with pytest.raises(AmbiguousResolveNamesError): + determine_resolves_to_generate( + {"ambiguous": "lockfile.txt"}, [AmbiguousTool], ["ambiguous"] + ) - 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: ['tool1', 'tool2', 'tool3']" - ) in str(exc.value) +@pytest.mark.parametrize( + "unrecognized,bad_entry_str,name_str", + ( + (["fake"], "fake", "name"), + (["fake1", "fake2"], "['fake1', 'fake2']", "names"), + ), +) +def test_unrecognized_resolve_names_error( + unrecognized: list[str], bad_entry_str: str, name_str: str +) -> None: with pytest.raises(UnrecognizedResolveNamesError) as exc: - assert_chosen(["fake1", "fake2"], []) + raise UnrecognizedResolveNamesError(unrecognized, ["valid1", "valid2", "valid3"]) assert ( - "Unrecognized resolve names from the option `--generate-lockfiles-resolve`: " - "['fake1', 'fake2']" + f"Unrecognized resolve {name_str} from the option `--generate-lockfiles-resolve`: " + f"{bad_entry_str}\n\nAll valid resolve names: ['valid1', 'valid2', 'valid3']" ) in str(exc.value) diff --git a/src/python/pants/python/python_setup.py b/src/python/pants/python/python_setup.py index 5f52454a4a6..bfdafdb37b7 100644 --- a/src/python/pants/python/python_setup.py +++ b/src/python/pants/python/python_setup.py @@ -127,6 +127,19 @@ def register_options(cls, register): "Mutually exclusive with `[python-setup].requirement_constraints`." ), ) + register( + "--experimental-resolves-to-lockfiles", + advanced=True, + type=dict, + help=( + "A mapping of logical names to lockfile paths used in your project, e.g. " + "`{ default = '3rdparty/default_lockfile.txt', py2 = '3rdparty/py2.txt' }`.\n\n" + "To generate a lockfile, run `./pants generate-lockfiles --resolve=` or " + "`./pants generate-lockfiles` to generate for all resolves (including tool " + "lockfiles).\n\n" + "This is highly experimental and will likely change." + ), + ) register( "--invalid-lockfile-behavior", advanced=True, @@ -220,6 +233,10 @@ def requirement_constraints(self) -> str | None: def lockfile(self) -> str | None: return cast("str | None", self.options.experimental_lockfile) + @property + def resolves_to_lockfiles(self) -> dict[str, str]: + return cast("dict[str, str]", self.options.experimental_resolves_to_lockfiles) + @property def invalid_lockfile_behavior(self) -> InvalidLockfileBehavior: return cast(InvalidLockfileBehavior, self.options.invalid_lockfile_behavior)