Skip to content

Commit

Permalink
Eliminate most rule registration boilerplate.
Browse files Browse the repository at this point in the history
Introduce a register_rules function that can find and register all
@rules and the SubsystemRules they depend on. This can be used to
both reduce boilerplate and also make rule development less surprising.
Using register_rules it now becomes rarer to edit rules and encounter
errors due to forgetting to register rules for the various introduced
subsystems and @rule functions.

Also streamline the Rule interface and the `def rules()` semantics;
now all rules are Rules save for UnionRule which can be dealt away
with later to leave users only exposed to registering Rules.

[ci skip-rust-tests]
  • Loading branch information
jsirois committed Jul 27, 2020
1 parent 605a039 commit 3d5d8d9
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 74 deletions.
11 changes: 3 additions & 8 deletions src/python/pants/build_graph/build_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import typing
from collections.abc import Iterable
from dataclasses import dataclass, field
from typing import Any, Callable, Dict, Type, Union
from typing import Any, Dict, Type, Union

from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.engine.rules import Rule, RuleIndex
Expand All @@ -23,7 +23,7 @@ class BuildConfiguration:

registered_aliases: BuildFileAliases
optionables: FrozenOrderedSet[Optionable]
rules: FrozenOrderedSet[Union[Rule, Callable]]
rules: FrozenOrderedSet[Rule]
union_rules: FrozenOrderedSet[UnionRule]
target_types: FrozenOrderedSet[Type[Target]]

Expand Down Expand Up @@ -133,12 +133,7 @@ def register_rules(self, rules):
self._rules.update(rules)
self._union_rules.update(union_rules)
self.register_optionables(
{
do
for rule in rules
for do in rule.dependency_optionables
if rule.dependency_optionables
}
rule.output_type for rule in self._rules if issubclass(rule.output_type, Optionable)
)

# NB: We expect the parameter to be Iterable[Type[Target]], but we can't be confident in this
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/engine/internals/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from pants.option.global_options import ExecutionOptions
from pants.util.contextutil import temporary_file_path
from pants.util.logging import LogLevel
from pants.util.ordered_set import FrozenOrderedSet
from pants.util.strutil import pluralize

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -89,7 +90,7 @@ def __init__(
local_store_dir: str,
local_execution_root_dir: str,
named_caches_dir: str,
rules: Tuple[Rule, ...],
rules: FrozenOrderedSet[Rule],
union_membership: UnionMembership,
execution_options: ExecutionOptions,
include_trace_on_error: bool = True,
Expand Down
89 changes: 44 additions & 45 deletions src/python/pants/engine/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,25 @@
from abc import ABC, abstractmethod
from dataclasses import dataclass
from enum import Enum
from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, Type, Union, get_type_hints
from types import FrameType, ModuleType
from typing import (
Any,
Callable,
Dict,
Iterable,
List,
Mapping,
Optional,
Tuple,
Type,
Union,
get_type_hints,
)

from pants.engine.goal import Goal
from pants.engine.selectors import GetConstraints
from pants.engine.unions import UnionRule
from pants.option.optionable import Optionable, OptionableFactory
from pants.option.optionable import OptionableFactory
from pants.util.collections import assert_single_element
from pants.util.logging import LogLevel
from pants.util.memo import memoized
Expand Down Expand Up @@ -235,9 +248,6 @@ def resolve_type(name):

gets = FrozenOrderedSet(rule_visitor.gets)

# Register dependencies for @goal_rule/Goal.
dependency_rules = (SubsystemRule(return_type.subsystem_cls),) if is_goal_cls else None

# Set our own custom `__line_number__` dunder so that the engine may visualize the line number.
func.__line_number__ = func.__code__.co_firstlineno

Expand All @@ -249,7 +259,6 @@ def resolve_type(name):
canonical_name=canonical_name,
desc=desc,
level=level,
dependency_rules=dependency_rules,
cacheable=cacheable,
)

Expand Down Expand Up @@ -413,21 +422,36 @@ class Rule(ABC):
def output_type(self):
"""An output `type` for the rule."""

@property
@abstractmethod
def dependency_rules(self):
"""A tuple of @rules that are known to be necessary to run this rule.

Note that installing @rules as flat lists is generally preferable, as Rules already
implicitly form a loosely coupled RuleGraph: this facility exists only to assist with
boilerplate removal.
"""
def register_rules(*namespaces: Union[ModuleType, Mapping[str, Any]]) -> Iterable[Rule]:
"""Registers all @rules in the given namespaces.
@property
@abstractmethod
def dependency_optionables(self):
"""A tuple of Optionable classes that are known to be necessary to run this rule."""
return ()
If no namespaces are given, registers all the @rules in the caller's module namespace.
"""
if not namespaces:
currentframe = inspect.currentframe()
assert isinstance(currentframe, FrameType)
caller_frame = currentframe.f_back
caller_module = inspect.getmodule(caller_frame)
assert isinstance(caller_module, ModuleType)
namespaces = (caller_module,)

def iter_rules():
for namespace in namespaces:
mapping = namespace.__dict__ if isinstance(namespace, ModuleType) else namespace
for name, item in mapping.items():
if not callable(item):
continue
rule = getattr(item, "rule", None)
if isinstance(rule, TaskRule):
for input in rule.input_selectors:
if issubclass(input, OptionableFactory):
yield SubsystemRule(input)
if issubclass(rule.output_type, Goal):
yield SubsystemRule(rule.output_type.subsystem_cls)
yield rule

return list(iter_rules())


@frozen_after_init
Expand All @@ -443,8 +467,6 @@ class TaskRule(Rule):
input_selectors: Tuple[Type, ...]
input_gets: Tuple[GetConstraints, ...]
func: Callable
_dependency_rules: Tuple["TaskRule", ...]
_dependency_optionables: Tuple[Type[Optionable], ...]
cacheable: bool
canonical_name: str
desc: Optional[str]
Expand All @@ -459,43 +481,30 @@ def __init__(
canonical_name: str,
desc: Optional[str] = None,
level: LogLevel = LogLevel.DEBUG,
dependency_rules: Optional[Iterable["TaskRule"]] = None,
dependency_optionables: Optional[Iterable[Type[Optionable]]] = None,
cacheable: bool = True,
):
self._output_type = output_type
self.input_selectors = tuple(input_selectors)
self.input_gets = tuple(input_gets)
self.func = func # type: ignore[assignment] # cannot assign to a method
self._dependency_rules = tuple(dependency_rules or ())
self._dependency_optionables = tuple(dependency_optionables or ())
self.cacheable = cacheable
self.canonical_name = canonical_name
self.desc = desc
self.level = level

def __str__(self):
return "(name={}, {}, {!r}, {}, gets={}, opts={})".format(
return "(name={}, {}, {!r}, {}, gets={})".format(
getattr(self, "name", "<not defined>"),
self.output_type.__name__,
self.input_selectors,
self.func.__name__,
self.input_gets,
self.dependency_optionables,
)

@property
def output_type(self):
return self._output_type

@property
def dependency_rules(self):
return self._dependency_rules

@property
def dependency_optionables(self):
return self._dependency_optionables


@frozen_after_init
@dataclass(unsafe_hash=True)
Expand All @@ -515,14 +524,6 @@ def __init__(self, output_type: Type) -> None:
def output_type(self):
return self._output_type

@property
def dependency_rules(self):
return tuple()

@property
def dependency_optionables(self):
return tuple()


@dataclass(frozen=True)
class RuleIndex:
Expand All @@ -547,8 +548,6 @@ def add_rule(rule: Rule) -> None:
if output_type not in serializable_rules:
serializable_rules[output_type] = OrderedSet()
serializable_rules[output_type].add(rule)
for dep_rule in rule.dependency_rules:
add_rule(dep_rule)

for entry in rule_entries:
if isinstance(entry, Rule):
Expand Down
36 changes: 17 additions & 19 deletions src/python/pants/init/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from pants.engine.internals.scheduler import Scheduler, SchedulerSession
from pants.engine.internals.target_adaptor import TargetAdaptor
from pants.engine.platform import create_platform_rules
from pants.engine.rules import RootRule, rule
from pants.engine.rules import RootRule, register_rules, rule
from pants.engine.selectors import Params
from pants.engine.target import RegisteredTargetTypes
from pants.engine.unions import UnionMembership
Expand All @@ -33,6 +33,7 @@
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.scm.subsystems.changed import rules as changed_rules
from pants.subsystem.subsystem import Subsystem
from pants.util.ordered_set import FrozenOrderedSet

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -282,24 +283,21 @@ def build_root_singleton() -> BuildRoot:
return cast(BuildRoot, BuildRoot.instance)

# Create a Scheduler containing graph and filesystem rules, with no installed goals.
rules = (
RootRule(Console),
glob_match_error_behavior_singleton,
build_configuration_singleton,
symbol_table_singleton,
registered_target_types_singleton,
union_membership_singleton,
build_root_singleton,
*fs.rules(),
*interactive_process.rules(),
*graph.rules(),
*uuid.rules(),
*options_parsing.rules(),
*process.rules(),
*create_platform_rules(),
*create_graph_rules(address_mapper),
*changed_rules(),
*rules,
rules = FrozenOrderedSet(
(
*register_rules(locals()),
RootRule(Console),
*fs.rules(),
*interactive_process.rules(),
*graph.rules(),
*uuid.rules(),
*options_parsing.rules(),
*process.rules(),
*create_platform_rules(),
*create_graph_rules(address_mapper),
*changed_rules(),
*rules,
)
)

goal_map = EngineInitializer._make_goal_map_from_rules(rules)
Expand Down
1 change: 0 additions & 1 deletion src/python/pants/option/optionable.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ def signature(cls):
func=partial_construct_optionable,
input_gets=(GetConstraints(product_type=ScopedOptions, subject_declared_type=Scope),),
canonical_name=name,
dependency_optionables=(cls.optionable_cls,),
)


Expand Down

0 comments on commit 3d5d8d9

Please sign in to comment.