Let a user resolve shadow a tool lockfile of the same name

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

@@ -259,6 +259,10 @@ class PythonSyntheticLockfileTargetsRequest(SyntheticTargetsRequest):
     path: str = SyntheticTargetsRequest.SINGLE_REQUEST_FOR_ALL_TARGETS
 
 
+def synthetic_lockfile_target_name(resolve: str) -> str:
+    return f"_{resolve}_lockfile"
+
+
 @rule
 async def python_lockfile_synthetic_targets(
     request: PythonSyntheticLockfileTargetsRequest,
@@ -277,7 +281,9 @@ async def python_lockfile_synthetic_targets(
             (
                 os.path.join(spec_path, "BUILD.python-lockfiles"),
                 tuple(
-                    TargetAdaptor("_lockfiles", name=name, sources=[lockfile])
+                    TargetAdaptor(
+                        "_lockfiles", name=synthetic_lockfile_target_name(name), sources=[lockfile]
+                    )
                     for _, lockfile, name in lockfiles
                 ),
             )

src/python/pants/backend/python/macros/common_requirements_rule.py

@@ -10,6 +10,7 @@
 
 from packaging.utils import canonicalize_name as canonicalize_project_name
 
+from pants.backend.python.goals.lockfile import synthetic_lockfile_target_name
 from pants.backend.python.macros.common_fields import (
     ModuleMappingField,
     TypeStubsModuleMappingField,
@@ -83,7 +84,7 @@ async def _generate_requirements(
     if lockfile:
         lockfile_address = Address(
             os.path.dirname(lockfile),
-            target_name=resolve,
+            target_name=synthetic_lockfile_target_name(resolve),
         )
         target_adaptor = await Get(
             TargetAdaptor,
@@ -93,7 +94,7 @@ async def _generate_requirements(
             ),
         )
         if target_adaptor.type_alias == "_lockfiles":
-            req_deps.append(f"{lockfile}:{resolve}")
+            req_deps.append(f"{lockfile}:{synthetic_lockfile_target_name(resolve)}")
         else:
             logger.warning(
                 softwrap(

src/python/pants/backend/python/macros/pipenv_requirements_test.py

@@ -99,7 +99,7 @@ def test_pipfile_lockfile_dependency(rule_runner: RuleRunner) -> None:
     rule_runner.set_options(["--python-enable-resolves"])
     file_addr = Address("", target_name="reqs", relative_file_path="Pipfile.lock")
     lock_addr = Address(
-        "3rdparty/python", target_name="python-default", relative_file_path="default.lock"
+        "3rdparty/python", target_name="_python-default_lockfile", relative_file_path="default.lock"
     )
     assert_pipenv_requirements(
         rule_runner,

src/python/pants/backend/python/macros/poetry_requirements_test.py

@@ -560,7 +560,7 @@ def test_lockfile_dependency(rule_runner: RuleRunner) -> None:
     rule_runner.set_options(["--python-enable-resolves"])
     file_addr = Address("", target_name="reqs", relative_file_path="pyproject.toml")
     lock_addr = Address(
-        "3rdparty/python", target_name="python-default", relative_file_path="default.lock"
+        "3rdparty/python", target_name="_python-default_lockfile", relative_file_path="default.lock"
     )
     assert_poetry_requirements(
         rule_runner,

src/python/pants/backend/python/macros/python_requirements_test.py

@@ -199,7 +199,7 @@ def test_lockfile_dependency(rule_runner: RuleRunner) -> None:
     rule_runner.set_options(["--python-enable-resolves"])
     reqs_addr = Address("", target_name="reqs", relative_file_path="requirements.txt")
     lock_addr = Address(
-        "3rdparty/python", target_name="python-default", relative_file_path="default.lock"
+        "3rdparty/python", target_name="_python-default_lockfile", relative_file_path="default.lock"
     )
     assert_python_requirements(
         rule_runner,

src/python/pants/core/goals/generate_lockfiles.py

@@ -272,9 +272,9 @@ def output_changed(self, title: str, reqs: ChangedPackages) -> Iterator[str]:
 
     def get_bump_attrs(self, prev: PackageVersion, curr: PackageVersion) -> dict[str, str]:
         for key, attrs in self._BUMPS:
-            if key and getattr(prev, key, None) != getattr(curr, key, None):
-                break
-        return attrs
+            if key is None or getattr(prev, key, None) != getattr(curr, key, None):
+                return attrs
+        return {}  # Should never happen, but let's be safe.
 
 
 DEFAULT_TOOL_LOCKFILE = "<default>"
@@ -314,77 +314,30 @@ class _ResolveProviderType(Enum):
 @dataclass(frozen=True, order=True)
 class _ResolveProvider:
     option_name: str
-    type_: _ResolveProviderType
 
 
 class AmbiguousResolveNamesError(Exception):
     def __init__(self, ambiguous_name: str, providers: set[_ResolveProvider]) -> None:
-        tool_providers = []
-        user_providers = []
-        for provider in sorted(providers):
-            if provider.type_ == _ResolveProviderType.TOOL:
-                tool_providers.append(provider.option_name)
-            else:
-                user_providers.append(provider.option_name)
-
-        if tool_providers:
-            if not user_providers:
-                raise AssertionError(
-                    softwrap(
-                        f"""
-                        {len(tool_providers)} tools have the same options_scope: {ambiguous_name}.
-                        If you're writing a plugin, rename your `GenerateToolLockfileSentinel`s so
-                        that there is no ambiguity. Otherwise, please open a bug at
-                        https://github.com/pantsbuild/pants/issues/new.
-                        """
-                    )
-                )
-            if len(user_providers) == 1:
-                msg = softwrap(
-                    f"""
-                    A resolve name from the option `{user_providers[0]}` collides with the
-                    name of a tool resolve: {ambiguous_name}
-
-                    To fix, please update `{user_providers[0]}` to use a different resolve name.
-                    """
-                )
-            else:
-                msg = softwrap(
-                    f"""
-                    Multiple options define the resolve name `{ambiguous_name}`, but it is
-                    already claimed by a tool: {user_providers}
-
-                    To fix, please update these options so that none of them use
-                    `{ambiguous_name}`.
-                    """
-                )
-        else:
-            assert len(user_providers) > 1
-            msg = softwrap(
-                f"""
-                The same resolve name `{ambiguous_name}` is used by multiple options, which
-                causes ambiguity: {user_providers}
+        msg = softwrap(
+            f"""
+            The same resolve name `{ambiguous_name}` is used by multiple options, which
+            causes ambiguity: {providers}
 
-                To fix, please update these options so that `{ambiguous_name}` is not used more
-                than once.
-                """
-            )
+            To fix, please update these options so that `{ambiguous_name}` is not used more
+            than once.
+            """
+        )
         super().__init__(msg)
 
 
 def _check_ambiguous_resolve_names(
     all_known_user_resolve_names: Iterable[KnownUserResolveNames],
-    all_tool_sentinels: Iterable[type[GenerateToolLockfileSentinel]],
 ) -> None:
     resolve_name_to_providers = defaultdict(set)
-    for sentinel in all_tool_sentinels:
-        resolve_name_to_providers[sentinel.resolve_name].add(
-            _ResolveProvider(sentinel.resolve_name, _ResolveProviderType.TOOL)
-        )
     for known_user_resolve_names in all_known_user_resolve_names:
         for resolve_name in known_user_resolve_names.names:
             resolve_name_to_providers[resolve_name].add(
-                _ResolveProvider(known_user_resolve_names.option_name, _ResolveProviderType.USER)
+                _ResolveProvider(known_user_resolve_names.option_name)
             )
 
     for resolve_name, providers in resolve_name_to_providers.items():
@@ -401,11 +354,21 @@ def determine_resolves_to_generate(
 
     Return a tuple of `(user_resolves, tool_lockfile_sentinels)`.
     """
-    _check_ambiguous_resolve_names(all_known_user_resolve_names, all_tool_sentinels)
+    # Let user resolve names silently shadow tools with the same name.
+    # This is necessary since we now support installing a tool from a named resolve,
+    # and it's not reasonable to ban the name of the tool as the resolve name, when it
+    # is the most obvious choice for that...
+    # This is likely only an issue if you were going to, e.g., have a named resolve called flake8
+    # but not use it as the resolve for the flake8 tool, which seems pretty unlikely.
+    all_known_user_resolve_name_strs = set(
+        itertools.chain.from_iterable(akurn.names for akurn in all_known_user_resolve_names)
+    )
+    all_tool_sentinels = [
+        ts for ts in all_tool_sentinels if ts.resolve_name not in all_known_user_resolve_name_strs
+    ]
 
-    resolve_names_to_sentinels = {
-        sentinel.resolve_name: sentinel for sentinel in all_tool_sentinels
-    }
+    # Resolve names must be globally unique, so check for ambiguity across backends.
+    _check_ambiguous_resolve_names(all_known_user_resolve_names)
 
     if not requested_resolve_names:
         return [
@@ -423,9 +386,9 @@ def determine_resolves_to_generate(
             )
 
     specified_sentinels = []
-    for resolve, sentinel in resolve_names_to_sentinels.items():
-        if resolve in requested_resolve_names:
-            requested_resolve_names.discard(resolve)
+    for sentinel in all_tool_sentinels:
+        if sentinel.resolve_name in requested_resolve_names:
+            requested_resolve_names.discard(sentinel.resolve_name)
             specified_sentinels.append(sentinel)
 
     if requested_resolve_names:
@@ -436,7 +399,7 @@ def determine_resolves_to_generate(
                     known_resolve_names.names
                     for known_resolve_names in all_known_user_resolve_names
                 ),
-                *resolve_names_to_sentinels.keys(),
+                *(sentinel.resolve_name for sentinel in all_tool_sentinels),
             },
             description_of_origin="the option `--generate-lockfiles-resolve`",
         )

src/python/pants/core/goals/generate_lockfiles_test.py

@@ -101,22 +101,23 @@ def assert_chosen(
     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(GenerateToolLockfileSentinel):
         resolve_name = "ambiguous"
 
-    with pytest.raises(AmbiguousResolveNamesError):
-        determine_resolves_to_generate(
-            [
-                KnownUserResolveNames(
-                    ("ambiguous",),
-                    "[lang].resolves",
-                    requested_resolve_names_cls=Lang1Requested,
-                )
-            ],
-            [AmbiguousTool],
-            set(),
-        )
+    # Let a user resolve shadow a tool resolve with the same name.
+    assert determine_resolves_to_generate(
+        [
+            KnownUserResolveNames(
+                ("ambiguous",),
+                "[lang].resolves",
+                requested_resolve_names_cls=Lang1Requested,
+            )
+        ],
+        [AmbiguousTool],
+        set(),
+    ) == ([Lang1Requested(["ambiguous"])], [])
+
+    # Error if same resolve name used for multiple user lockfiles.
     with pytest.raises(AmbiguousResolveNamesError):
         determine_resolves_to_generate(
             [

src/python/pants/engine/internals/synthetic_targets_test.py

@@ -309,10 +309,10 @@ def test_target_name_collision_issue_17343(rule_runner: RuleRunner) -> None:
             "src/issues/17343/BUILD": softwrap(
                 """
                 python_requirements(
-                  name="python-default",
+                  name="_python-default_lockfile",
                   overrides={
                     "humbug": {
-                      "dependencies": ["python-default#setuptools"],
+                      "dependencies": ["_python-default_lockfile#setuptools"],
                     },
                   },
                 )
@@ -330,7 +330,9 @@ def test_target_name_collision_issue_17343(rule_runner: RuleRunner) -> None:
 
     tgt = assert_target(
         rule_runner,
-        Address("src/issues/17343", target_name="python-default", generated_name="setuptools"),
+        Address(
+            "src/issues/17343", target_name="_python-default_lockfile", generated_name="setuptools"
+        ),
         alias="python_requirement",
     )