Make GoalSubsystem subclass Subsystem.

[ci skip-rust]

[ci skip-build-wheels]

src/python/pants/engine/goal.py

@@ -8,16 +8,14 @@
 
 from typing_extensions import final
 
-from pants.option.option_value_container import OptionValueContainer
-from pants.option.optionable import Optionable
-from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin
+from pants.subsystem.subsystem import Subsystem
 from pants.util.meta import classproperty
 
 if TYPE_CHECKING:
     from pants.engine.console import Console
 
 
-class GoalSubsystem(SubsystemClientMixin, Optionable):
+class GoalSubsystem(Subsystem):
     """The Subsystem used by `Goal`s to register the external API, meaning the goal name, the help
     message, and any options.
 
@@ -49,19 +47,6 @@ def name(cls):
     def options_scope(cls) -> str:  # type: ignore[override]
         return cast(str, cls.name)
 
-    @final
-    def __init__(self, scope: str, scoped_options: OptionValueContainer) -> None:
-        # NB: This constructor is shaped to meet the contract of `Optionable(Factory).signature`.
-        super().__init__()
-        self._scope = scope
-        self._scoped_options = scoped_options
-
-    @final
-    @property
-    def options(self) -> OptionValueContainer:
-        """Returns the option values."""
-        return self._scoped_options
-
 
 @dataclass(frozen=True)
 class Goal:

src/python/pants/option/options_test.py

@@ -1739,7 +1739,7 @@ class DummyOptionable1(Optionable):
                 global_scope(),
                 DummyOptionable1.get_scope_info(),
                 # A deprecated, scoped dependency on `DummyOptionable1`. This
-                # imitates the construction of SubsystemClientMixin.known_scope_infos.
+                # imitates the construction of Subsystem.known_scope_infos.
                 ScopeInfo(
                     DummyOptionable1.subscope("sub"),
                     DummyOptionable1,

src/python/pants/subsystem/subsystem.py

@@ -9,8 +9,9 @@
 from pants.option.option_value_container import OptionValueContainer
 from pants.option.optionable import Optionable
 from pants.option.options import Options
-from pants.option.scope import ScopeInfo
-from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin, SubsystemDependency
+from pants.option.scope import ScopeInfo, GLOBAL_SCOPE
+from pants.subsystem.subsystem_client_mixin import SubsystemDependency
+from pants.util.ordered_set import OrderedSet
 
 logger = logging.getLogger(__name__)
 
@@ -22,7 +23,7 @@ class SubsystemError(Exception):
 _S = TypeVar("_S", bound="Subsystem")
 
 
-class Subsystem(SubsystemClientMixin, Optionable):
+class Subsystem(Optionable):
     """A separable piece of functionality that may be reused across multiple tasks or other code.
 
     Subsystems encapsulate the configuration and initialization of things like JVMs,
@@ -40,7 +41,7 @@ class Subsystem(SubsystemClientMixin, Optionable):
     For example, the global artifact cache options would be in scope `cache`, but the
     compile.java task can override those options in scope `cache.compile.java`.
 
-    Subsystems may depend on other subsystems, and therefore mix in SubsystemClientMixin.
+    Subsystems may depend on other subsystems.
 
     :API: public
     """
@@ -60,7 +61,7 @@ def scoped(cls, optionable, removal_version=None, removal_hint=None):
         :param removal_version: An optional deprecation version for this scoped Subsystem dependency.
         :param removal_hint: An optional hint to accompany a deprecation removal_version.
 
-        Return value is suitable for use in SubsystemClientMixin.subsystem_dependencies().
+        Return value is suitable for use in Subsystem.subsystem_dependencies().
         """
         return SubsystemDependency(cls, optionable.options_scope, removal_version, removal_hint)
 
@@ -143,8 +144,6 @@ def reset(cls, reset_options: bool = True) -> None:
     def __init__(self, scope: str, scoped_options: OptionValueContainer) -> None:
         """Note: A subsystem has no access to options in scopes other than its own.
 
-    TODO: We'd like that to be true of Tasks some day. Subsystems will help with that.
-
     Code should call scoped_instance() or global_instance() to get a subsystem instance.
     It should not invoke this constructor directly.
 
@@ -228,3 +227,122 @@ def get_streaming_workunit_callbacks(subsystem_names: Iterable[str]) -> List[Cal
                 continue
 
         return callables
+
+    @classmethod
+    def subsystem_dependencies(cls):
+        """The subsystems this object uses.
+
+        Override to specify your subsystem dependencies. Always add them to your superclass's value.
+
+        Note: Do not call this directly to retrieve dependencies. See subsystem_dependencies_iter().
+
+        :return: A tuple of SubsystemDependency instances.
+                 In the common case where you're an optionable and you want to get an instance scoped
+                 to you, call subsystem_cls.scoped(cls) to get an appropriate SubsystemDependency.
+                 As a convenience, you may also provide just a subsystem_cls, which is shorthand for
+                 SubsystemDependency(subsystem_cls, GLOBAL SCOPE) and indicates that we want to use
+                 the global instance of that subsystem.
+        """
+        return tuple()
+
+    @classmethod
+    def subsystem_dependencies_iter(cls):
+        """Iterate over the direct subsystem dependencies of this Optionable."""
+        for dep in cls.subsystem_dependencies():
+            if isinstance(dep, SubsystemDependency):
+                yield dep
+            else:
+                yield SubsystemDependency(
+                    dep, GLOBAL_SCOPE, removal_version=None, removal_hint=None
+                )
+
+    @classmethod
+    def subsystem_closure_iter(cls):
+        """Iterate over the transitive closure of subsystem dependencies of this Optionable.
+
+        :rtype: :class:`collections.Iterator` of :class:`SubsystemDependency`
+        :raises: :class:`pants.subsystem.subsystem_client_mixin.Subsystem.CycleException`
+                 if a dependency cycle is detected.
+        """
+        seen = set()
+        dep_path = OrderedSet()
+
+        def iter_subsystem_closure(subsystem_cls):
+            if subsystem_cls in dep_path:
+                raise cls.CycleException(list(dep_path) + [subsystem_cls])
+            dep_path.add(subsystem_cls)
+
+            for dep in subsystem_cls.subsystem_dependencies_iter():
+                if dep not in seen:
+                    seen.add(dep)
+                    yield dep
+                    for d in iter_subsystem_closure(dep.subsystem_cls):
+                        yield d
+
+            dep_path.remove(subsystem_cls)
+
+        for dep in iter_subsystem_closure(cls):
+            yield dep
+
+    class CycleException(Exception):
+        """Thrown when a circular subsystem dependency is detected."""
+
+        def __init__(self, cycle):
+            message = "Cycle detected:\n\t{}".format(
+                " ->\n\t".join(
+                    "{} scope: {}".format(optionable_cls, optionable_cls.options_scope)
+                    for optionable_cls in cycle
+                )
+            )
+            super().__init__(message)
+
+    @classmethod
+    def known_scope_infos(cls):
+        """Yield ScopeInfo for all known scopes for this optionable, in no particular order.
+
+        :rtype: set of :class:`pants.option.scope.ScopeInfo`
+        :raises: :class:`pants.subsystem.subsystem_client_mixin.Subsystem.CycleException`
+                 if a dependency cycle is detected.
+        """
+        known_scope_infos = set()
+        optionables_path = (
+            OrderedSet()
+        )  # To check for cycles at the Optionable level, ignoring scope.
+
+        def collect_scope_infos(optionable_cls, scoped_to, removal_version=None, removal_hint=None):
+            if optionable_cls in optionables_path:
+                raise cls.CycleException(list(optionables_path) + [optionable_cls])
+            optionables_path.add(optionable_cls)
+
+            scope = (
+                optionable_cls.options_scope
+                if scoped_to == GLOBAL_SCOPE
+                else optionable_cls.subscope(scoped_to)
+            )
+            scope_info = ScopeInfo(
+                scope, optionable_cls, removal_version=removal_version, removal_hint=removal_hint,
+            )
+
+            if scope_info not in known_scope_infos:
+                known_scope_infos.add(scope_info)
+                for dep in scope_info.optionable_cls.subsystem_dependencies_iter():
+                    # A subsystem always exists at its global scope (for the purpose of options
+                    # registration and specification), even if in practice we only use it scoped to
+                    # some other scope.
+                    #
+                    # NB: We do not apply deprecations to this implicit global copy of the scope, because if
+                    # the intention was to deprecate the entire scope, that could be accomplished by
+                    # deprecating all options in the scope.
+                    collect_scope_infos(dep.subsystem_cls, GLOBAL_SCOPE)
+                    if not dep.is_global():
+                        collect_scope_infos(
+                            dep.subsystem_cls,
+                            scope,
+                            removal_version=dep.removal_version,
+                            removal_hint=dep.removal_hint,
+                        )
+
+            optionables_path.remove(scope_info.optionable_cls)
+
+        collect_scope_infos(cls, GLOBAL_SCOPE)
+        return known_scope_infos

src/python/pants/subsystem/subsystem_test.py

@@ -6,7 +6,6 @@
 from pants.option.optionable import Optionable
 from pants.option.scope import ScopeInfo
 from pants.subsystem.subsystem import Subsystem
-from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin
 from pants.testutil.test_base import TestBase
 
 
@@ -146,7 +145,7 @@ def subsystem_dependencies(cls):
                 return (SubsystemB,)
 
         for root in SubsystemA, SubsystemB, SubsystemC:
-            with self.assertRaises(SubsystemClientMixin.CycleException):
+            with self.assertRaises(Subsystem.CycleException):
                 root.known_scope_infos()
 
     def test_scoping_complex(self) -> None:
@@ -299,7 +298,7 @@ class SubsystemB(Subsystem):
             def subsystem_dependencies(cls):
                 return (SubsystemA,)
 
-        with self.assertRaises(SubsystemClientMixin.CycleException):
+        with self.assertRaises(Subsystem.CycleException):
             list(SubsystemB.subsystem_closure_iter())
 
     def test_get_streaming_workunit_callbacks(self) -> None: