Skip to content

Commit

Permalink
Deprecate BuildConfiguration.subsystems in favor of optionables.
Browse files Browse the repository at this point in the history
  • Loading branch information
stuhood committed Jan 2, 2019
1 parent 4e2ee57 commit d03d989
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 24 deletions.
50 changes: 35 additions & 15 deletions src/python/pants/build_graph/build_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
from builtins import object, str
from collections import Iterable, namedtuple

from pants.base.deprecated import deprecated
from pants.base.parse_context import ParseContext
from pants.build_graph.addressable import AddressableCallProxy
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.build_graph.target_addressable import TargetAddressable
from pants.option.optionable import Optionable
from pants.subsystem.subsystem import Subsystem
from pants.util.memo import memoized_method

Expand All @@ -32,7 +34,7 @@ def __init__(self):
self._target_macro_factory_by_alias = {}
self._exposed_object_by_alias = {}
self._exposed_context_aware_object_factory_by_alias = {}
self._subsystems = set()
self._optionables = set()
self._rules = []

def registered_aliases(self):
Expand Down Expand Up @@ -82,15 +84,15 @@ def _register_target_alias(self, alias, target_type):
logger.debug('Target alias {} has already been registered. Overwriting!'.format(alias))

self._target_by_alias[alias] = target_type
self._subsystems.update(target_type.subsystems())
self.register_optionables(target_type.subsystems())

def _register_target_macro_factory_alias(self, alias, target_macro_factory):
if alias in self._target_macro_factory_by_alias:
logger.debug('TargetMacro alias {} has already been registered. Overwriting!'.format(alias))

self._target_macro_factory_by_alias[alias] = target_macro_factory
for target_type in target_macro_factory.target_types:
self._subsystems.update(target_type.subsystems())
self.register_optionables(target_type.subsystems())

def _register_exposed_object(self, alias, obj):
if alias in self._exposed_object_by_alias:
Expand All @@ -99,7 +101,7 @@ 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())
self.register_optionables(obj.subsystems())

def _register_exposed_context_aware_object_factory(self, alias, context_aware_object_factory):
if alias in self._exposed_context_aware_object_factory_by_alias:
Expand All @@ -108,28 +110,46 @@ def _register_exposed_context_aware_object_factory(self, alias, context_aware_ob

self._exposed_context_aware_object_factory_by_alias[alias] = context_aware_object_factory

@deprecated('1.15.0.dev1', hint_message='Use self.register_optionables().')
def register_subsystems(self, subsystems):
return self.register_optionables(subsystems)

def register_optionables(self, optionables):
"""Registers the given subsystem types.
:param subsystems: The subsystem types to register.
:type subsystems: :class:`collections.Iterable` containing
:class:`pants.subsystem.subsystem.Subsystem` subclasses.
:param optionables: The Optionable types to register.
:type optionables: :class:`collections.Iterable` containing
:class:`pants.option.optionable.Optionable` subclasses.
"""
if not isinstance(subsystems, Iterable):
raise TypeError('The subsystems must be an iterable, given {}'.format(subsystems))
invalid_subsystems = [s for s in subsystems if not Subsystem.is_subsystem_type(s)]
if invalid_subsystems:
raise TypeError('The following items from the given subsystems are not Subsystem '
'subclasses:\n\t{}'.format('\n\t'.join(str(i) for i in invalid_subsystems)))
if not isinstance(optionables, Iterable):
raise TypeError('The optionables must be an iterable, given {}'.format(optionables))
optionables = tuple(optionables)
if not optionables:
return

invalid_optionables = [s
for s in optionables
if not isinstance(s, type) or not issubclass(s, Optionable)]
if invalid_optionables:
raise TypeError('The following items from the given optionables are not Optionable '
'subclasses:\n\t{}'.format('\n\t'.join(str(i) for i in invalid_optionables)))

self._optionables.update(optionables)

self._subsystems.update(subsystems)
def optionables(self):
"""Returns the registered Optionable types.
:rtype set
"""
return self._optionables

@deprecated('1.15.0.dev1', hint_message='Use self.optionables().')
def subsystems(self):
"""Returns the registered Subsystem types.
:rtype set
"""
return self._subsystems
return {o for o in self._optionables if issubclass(o, Subsystem)}

def register_rules(self, rules):
"""Registers the given rules.
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/init/extension_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def load_plugins(build_configuration, plugins, working_set):

if 'global_subsystems' in entries:
subsystems = entries['global_subsystems'].load()()
build_configuration.register_subsystems(subsystems)
build_configuration.register_optionables(subsystems)

loaded[dist.as_requirement().key] = dist

Expand Down Expand Up @@ -141,7 +141,7 @@ def invoke_entrypoint(name):

subsystems = invoke_entrypoint('global_subsystems')
if subsystems:
build_configuration.register_subsystems(subsystems)
build_configuration.register_optionables(subsystems)

rules = invoke_entrypoint('rules')
if rules:
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/init/options_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def _construct_options(options_bootstrapper, build_configuration):
top_level_optionables = (
{GlobalOptionsRegistrar} |
GlobalSubsystems.get() |
build_configuration.subsystems() |
build_configuration.optionables() |
set(Goal.get_optionables())
)

Expand Down
2 changes: 1 addition & 1 deletion tests/python/pants_test/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ def context(self, for_task_types=None, for_subsystems=None, options=None,
if subsystem.options_scope is None:
raise TaskError('You must set a scope on your subsystem type before using it in tests.')

optionables = {SourceRootConfig} | self._build_configuration.subsystems() | for_subsystems
optionables = {SourceRootConfig} | self._build_configuration.optionables() | for_subsystems

for_task_types = for_task_types or ()
for task_type in for_task_types:
Expand Down
8 changes: 4 additions & 4 deletions tests/python/pants_test/init/test_extension_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def assert_empty_aliases(self):
self.assertEqual(0, len(registered_aliases.target_macro_factories))
self.assertEqual(0, len(registered_aliases.objects))
self.assertEqual(0, len(registered_aliases.context_aware_object_factories))
self.assertEqual(self.build_configuration.subsystems(), set())
self.assertEqual(self.build_configuration.optionables(), set())
self.assertEqual(0, len(self.build_configuration.rules()))

def test_load_valid_empty(self):
Expand All @@ -164,7 +164,7 @@ def test_load_valid_partial_aliases(self):
self.assertEqual(DummyTarget, registered_aliases.target_types['bob'])
self.assertEqual(DummyObject1, registered_aliases.objects['obj1'])
self.assertEqual(DummyObject2, registered_aliases.objects['obj2'])
self.assertEqual(self.build_configuration.subsystems(), {DummySubsystem1, DummySubsystem2})
self.assertEqual(self.build_configuration.optionables(), {DummySubsystem1, DummySubsystem2})

def test_load_valid_partial_goals(self):
def register_goals():
Expand Down Expand Up @@ -309,14 +309,14 @@ def reg_alias():
self.assertEqual(DummyTarget, registered_aliases.target_types['pluginalias'])
self.assertEqual(DummyObject1, registered_aliases.objects['FROMPLUGIN1'])
self.assertEqual(DummyObject2, registered_aliases.objects['FROMPLUGIN2'])
self.assertEqual(self.build_configuration.subsystems(), {DummySubsystem1, DummySubsystem2})
self.assertEqual(self.build_configuration.optionables(), {DummySubsystem1, DummySubsystem2})

def test_subsystems(self):
def global_subsystems():
return {DummySubsystem1, DummySubsystem2}
with self.create_register(global_subsystems=global_subsystems) as backend_package:
load_backend(self.build_configuration, backend_package)
self.assertEqual(self.build_configuration.subsystems(),
self.assertEqual(self.build_configuration.optionables(),
{DummySubsystem1, DummySubsystem2})

def test_rules(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/python/pants_test/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ def context(self, for_task_types=None, for_subsystems=None, options=None,
if subsystem.options_scope is None:
raise TaskError('You must set a scope on your subsystem type before using it in tests.')

optionables = {SourceRootConfig} | self._build_configuration.subsystems() | for_subsystems
optionables = {SourceRootConfig} | self._build_configuration.optionables() | for_subsystems

for_task_types = for_task_types or ()
for task_type in for_task_types:
Expand Down

0 comments on commit d03d989

Please sign in to comment.