From 78ae321dbfbb7f31f0e876aa7daa58ac9f7d52a7 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Fri, 20 Aug 2021 21:27:35 -0400 Subject: [PATCH] Simplify subsystem registration. We don't actually need to support the `subsystems` classmethod on aliases. That was a v1 thing. v2 subsystems have to be produced by a rule, so registering them only as rule return values is sufficient. [ci skip-rust] [ci skip-build-wheels] --- .../pants/build_graph/build_configuration.py | 14 ++------------ .../pants_test/init/test_extension_loader.py | 7 +------ 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index f4c3a4654e3..16b281a2926 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -4,7 +4,6 @@ from __future__ import annotations import logging -import typing from collections import defaultdict from collections.abc import Iterable from dataclasses import dataclass, field @@ -137,9 +136,6 @@ def _register_exposed_object(self, alias, obj): ) self._exposed_object_by_alias[alias] = obj - # obj doesn't implement any common base class, so we have to test for this attr. - if hasattr(obj, "subsystems"): - self.register_subsystems(obj.subsystems()) def _register_exposed_context_aware_object_factory( self, alias, context_aware_object_factory @@ -154,13 +150,7 @@ def _register_exposed_context_aware_object_factory( alias ] = context_aware_object_factory - # NB: We expect the parameter to be Iterable[Type[Subsystem]], but we can't be confident - # in this because we pass whatever people put in their `register.py`s to this function; - # I.e., this is an impure function that reads from the outside world. So, we use the type - # hint `Any` and perform runtime type checking. - # TODO: Is this still true? Do we still support a hook for registering subsystems, - # instead of just having them be discovered as rule params? - def register_subsystems(self, subsystems: typing.Iterable[Type[Subsystem]] | Any): + def register_subsystems(self, subsystems: Iterable[Type[Subsystem]]): """Registers the given subsystem types.""" if not isinstance(subsystems, Iterable): raise TypeError("The subsystems must be an iterable, given {}".format(subsystems)) @@ -202,7 +192,7 @@ def register_rules(self, rules): # this because we pass whatever people put in their `register.py`s to this function; # I.e., this is an impure function that reads from the outside world. So, we use the type # hint `Any` and perform runtime type checking. - def register_target_types(self, target_types: typing.Iterable[Type[Target]] | Any) -> None: + def register_target_types(self, target_types: Iterable[Type[Target]] | Any) -> None: """Registers the given target types.""" if not isinstance(target_types, Iterable): raise TypeError( diff --git a/tests/python/pants_test/init/test_extension_loader.py b/tests/python/pants_test/init/test_extension_loader.py index 961ba46f6f4..932fba37360 100644 --- a/tests/python/pants_test/init/test_extension_loader.py +++ b/tests/python/pants_test/init/test_extension_loader.py @@ -67,10 +67,7 @@ class DummyObject1: class DummyObject2: - # Objects can have a special classmethod `subsystems` to register subsystems. - @classmethod - def subsystems(cls): - return (DummySubsystem,) + pass @dataclass(frozen=True) @@ -158,7 +155,6 @@ def test_load_valid_partial_aliases(self): registered_aliases = build_configuration.registered_aliases self.assertEqual(DummyObject1, registered_aliases.objects["obj1"]) self.assertEqual(DummyObject2, registered_aliases.objects["obj2"]) - self.assertEqual(build_configuration.subsystems, FrozenOrderedSet([DummySubsystem])) def test_load_invalid_entrypoint(self): def build_file_aliases(bad_arg): @@ -280,7 +276,6 @@ def reg_alias(): registered_aliases = build_configuration.registered_aliases self.assertEqual(DummyObject1, registered_aliases.objects["FROMPLUGIN1"]) self.assertEqual(DummyObject2, registered_aliases.objects["FROMPLUGIN2"]) - self.assertEqual(build_configuration.subsystems, FrozenOrderedSet([DummySubsystem])) def test_rules(self): def backend_rules():