From 3a8a0252deaa5d01278f67b8a00e36a4085a86c2 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 18 Aug 2024 12:56:29 +0200 Subject: [PATCH 1/9] Allow deleting rules in plugins to override behaviour --- .../pants/build_graph/build_configuration.py | 15 ++++++++++----- src/python/pants/engine/rules.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index 109f0a4af1b..0eb10a901cf 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -14,7 +14,7 @@ from pants.build_graph.build_file_aliases import BuildFileAliases from pants.core.util_rules.environments import EnvironmentsSubsystem from pants.engine.goal import GoalSubsystem -from pants.engine.rules import Rule, RuleIndex +from pants.engine.rules import Rule, RuleIndex, TaskRule from pants.engine.target import Target from pants.engine.unions import UnionRule from pants.option.alias import CliOptions @@ -134,6 +134,7 @@ class Builder: default_factory=lambda: defaultdict(list) ) _rule_to_providers: dict[Rule, list[str]] = field(default_factory=lambda: defaultdict(list)) + _delete_rules: set[str] = field(default_factory=set) _union_rule_to_providers: dict[UnionRule, list[str]] = field( default_factory=lambda: defaultdict(list) ) @@ -189,9 +190,9 @@ def _register_exposed_context_aware_object_factory( "Overwriting!".format(alias) ) - self._exposed_context_aware_object_factory_by_alias[ - alias - ] = context_aware_object_factory + self._exposed_context_aware_object_factory_by_alias[alias] = ( + context_aware_object_factory + ) def register_subsystems( self, plugin_or_backend: str, subsystems: Iterable[type[Subsystem]] @@ -225,6 +226,8 @@ def register_rules(self, plugin_or_backend: str, rules: Iterable[Rule | UnionRul rules_and_queries: tuple[Rule, ...] = (*rule_index.rules, *rule_index.queries) for rule in rules_and_queries: self._rule_to_providers[rule].append(plugin_or_backend) + for delete_rule in rule_index.delete_rules: + self._delete_rules.add(delete_rule.canonical_name) for union_rule in rule_index.union_rules: self._union_rule_to_providers[union_rule].append(plugin_or_backend) self.register_subsystems( @@ -309,7 +312,9 @@ def create(self) -> BuildConfiguration: (k, tuple(v)) for k, v in self._target_type_to_providers.items() ), rule_to_providers=FrozenDict( - (k, tuple(v)) for k, v in self._rule_to_providers.items() + (k, tuple(v)) + for k, v in self._rule_to_providers.items() + if (not isinstance(k, TaskRule) or k.canonical_name not in self._delete_rules) ), union_rule_to_providers=FrozenDict( (k, tuple(v)) for k, v in self._union_rule_to_providers.items() diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index 4e442558d7d..354abb26f40 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -472,6 +472,11 @@ def iter_rules(): return list(iter_rules()) +@dataclass(frozen=True) +class DeleteRule: + canonical_name: str + + @dataclass(frozen=True) class TaskRule: """A Rule that runs a task function when all of its input selectors are satisfied. @@ -521,6 +526,7 @@ class RuleIndex: """Holds a normalized index of Rules used to instantiate Nodes.""" rules: FrozenOrderedSet[TaskRule] + delete_rules: FrozenOrderedSet[DeleteRule] queries: FrozenOrderedSet[QueryRule] union_rules: FrozenOrderedSet[UnionRule] @@ -528,12 +534,15 @@ class RuleIndex: def create(cls, rule_entries: Iterable[Rule | UnionRule]) -> RuleIndex: """Creates a RuleIndex with tasks indexed by their output type.""" rules: OrderedSet[TaskRule] = OrderedSet() + delete_rules: OrderedSet[DeleteRule] = OrderedSet() queries: OrderedSet[QueryRule] = OrderedSet() union_rules: OrderedSet[UnionRule] = OrderedSet() for entry in rule_entries: if isinstance(entry, TaskRule): rules.add(entry) + elif isinstance(entry, DeleteRule): + delete_rules.add(entry) elif isinstance(entry, UnionRule): union_rules.add(entry) elif isinstance(entry, QueryRule): @@ -551,6 +560,7 @@ def create(cls, rule_entries: Iterable[Rule | UnionRule]) -> RuleIndex: return RuleIndex( rules=FrozenOrderedSet(rules), + delete_rules=FrozenOrderedSet(delete_rules), queries=FrozenOrderedSet(queries), union_rules=FrozenOrderedSet(union_rules), ) From 966e81a7551e654d66977ef4e4cd7a83a98a0303 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 18 Aug 2024 12:58:56 +0200 Subject: [PATCH 2/9] pants fix --- src/python/pants/build_graph/build_configuration.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index 0eb10a901cf..b53ffe16738 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -190,9 +190,9 @@ def _register_exposed_context_aware_object_factory( "Overwriting!".format(alias) ) - self._exposed_context_aware_object_factory_by_alias[alias] = ( - context_aware_object_factory - ) + self._exposed_context_aware_object_factory_by_alias[ + alias + ] = context_aware_object_factory def register_subsystems( self, plugin_or_backend: str, subsystems: Iterable[type[Subsystem]] From ec857ec6e32ad2bd74b69c4cf4f257fea8008628 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 18 Aug 2024 13:09:25 +0200 Subject: [PATCH 3/9] Create DeleteRule from rule --- src/python/pants/engine/rules.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index 354abb26f40..7358e1d6089 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -476,6 +476,13 @@ def iter_rules(): class DeleteRule: canonical_name: str + @classmethod + def create(cls, f: Any): + if not hasattr(f, "rule_id"): + raise ValueError(f"`{f.__qualname__}` is not a rule") + + return DeleteRule(canonical_name=f.rule_id) + @dataclass(frozen=True) class TaskRule: From 66da0091c1f19dca5c4518346c9024754b6a0ddf Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 18 Aug 2024 16:48:10 +0200 Subject: [PATCH 4/9] Add docs and notes --- .../the-rules-api/delete-rules-advanced.mdx | 51 +++++++++++++++++++ .../logging-and-dynamic-output.mdx | 2 +- .../the-rules-api/testing-plugins.mdx | 2 +- .../the-rules-api/tips-and-debugging.mdx | 2 +- docs/notes/2.24.x.md | 1 + 5 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 docs/docs/writing-plugins/the-rules-api/delete-rules-advanced.mdx diff --git a/docs/docs/writing-plugins/the-rules-api/delete-rules-advanced.mdx b/docs/docs/writing-plugins/the-rules-api/delete-rules-advanced.mdx new file mode 100644 index 00000000000..05b66d96e27 --- /dev/null +++ b/docs/docs/writing-plugins/the-rules-api/delete-rules-advanced.mdx @@ -0,0 +1,51 @@ +--- + title: Delete rules (advanced) + sidebar_position: 8 +--- + +Delete builtin rules and define your own to override behavior. + +--- + +Sometimes you might want to change the behavior of some existing backend in a way that is not supported by the backend. `DeleteRule` allows you to remove a single rule from the backend, then you can implement your own version of the same rule. + +For example, you might want to change the behavior of the `list` goal, here is how you do it: + + +```python title="pants-plugins/custom_list/register.py" +from pants.backend.project_info.list_targets import List, ListSubsystem +from pants.backend.project_info.list_targets import list_targets as original_rule +from pants.engine.addresses import Addresses +from pants.engine.console import Console +from pants.engine.rules import DeleteRule, collect_rules, goal_rule + + +@goal_rule +async def list_targets(addresses: Addresses, list_subsystem: ListSubsystem, console: Console) -> List: + with list_subsystem.line_oriented(console) as print_stdout: + print_stdout("ha cool") + return List(exit_code=0) + + +def target_types(): + return [] + + +def rules(): + return ( + *collect_rules(), + DeleteRule.create(original_rule), + ) +``` + +```python title="pants.toml" +[GLOBAL] +... +backend_packages = [ + ... + "custom_list", +] +``` + +```python title="pants-plugins/custom_list/__init__.py" +``` diff --git a/docs/docs/writing-plugins/the-rules-api/logging-and-dynamic-output.mdx b/docs/docs/writing-plugins/the-rules-api/logging-and-dynamic-output.mdx index 0a6b735636d..2ac21947206 100644 --- a/docs/docs/writing-plugins/the-rules-api/logging-and-dynamic-output.mdx +++ b/docs/docs/writing-plugins/the-rules-api/logging-and-dynamic-output.mdx @@ -1,6 +1,6 @@ --- title: Logging and dynamic output - sidebar_position: 8 + sidebar_position: 9 --- How to add logging and influence the dynamic UI. diff --git a/docs/docs/writing-plugins/the-rules-api/testing-plugins.mdx b/docs/docs/writing-plugins/the-rules-api/testing-plugins.mdx index f3e7cc2875a..a076262f6ae 100644 --- a/docs/docs/writing-plugins/the-rules-api/testing-plugins.mdx +++ b/docs/docs/writing-plugins/the-rules-api/testing-plugins.mdx @@ -1,6 +1,6 @@ --- title: Testing plugins - sidebar_position: 9 + sidebar_position: 10 --- How to verify your plugin works. diff --git a/docs/docs/writing-plugins/the-rules-api/tips-and-debugging.mdx b/docs/docs/writing-plugins/the-rules-api/tips-and-debugging.mdx index 9644d98594a..55870607a3c 100644 --- a/docs/docs/writing-plugins/the-rules-api/tips-and-debugging.mdx +++ b/docs/docs/writing-plugins/the-rules-api/tips-and-debugging.mdx @@ -1,6 +1,6 @@ --- title: Tips and debugging - sidebar_position: 10 + sidebar_position: 11 --- --- diff --git a/docs/notes/2.24.x.md b/docs/notes/2.24.x.md index d0586dce1cb..c15e9688d5b 100644 --- a/docs/notes/2.24.x.md +++ b/docs/notes/2.24.x.md @@ -44,6 +44,7 @@ Pants will now warn if any errors are encountered while fingerprinting candidate ### Plugin API changes +Plugins may now use `DeleteRule` to delete rules from other backends to override behavior. ## Full Changelog From 97405ff1c511f1a8e4c98d5306b7b222e7733a85 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 18 Aug 2024 18:09:37 +0200 Subject: [PATCH 5/9] Add integration test --- .../pants/build_graph/build_configuration.py | 6 +- .../engine/delete_rule_integration_test.py | 55 +++++++++++++++++++ src/python/pants/engine/rules.py | 2 +- 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 src/python/pants/engine/delete_rule_integration_test.py diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index b53ffe16738..23c7b743a70 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -14,7 +14,7 @@ from pants.build_graph.build_file_aliases import BuildFileAliases from pants.core.util_rules.environments import EnvironmentsSubsystem from pants.engine.goal import GoalSubsystem -from pants.engine.rules import Rule, RuleIndex, TaskRule +from pants.engine.rules import DeleteRule, Rule, RuleIndex, TaskRule from pants.engine.target import Target from pants.engine.unions import UnionRule from pants.option.alias import CliOptions @@ -216,7 +216,9 @@ def register_subsystems( for subsystem in subsystems: self._subsystem_to_providers[subsystem].append(plugin_or_backend) - def register_rules(self, plugin_or_backend: str, rules: Iterable[Rule | UnionRule]): + def register_rules( + self, plugin_or_backend: str, rules: Iterable[Rule | UnionRule | DeleteRule] + ): """Registers the given rules.""" if not isinstance(rules, Iterable): raise TypeError(f"The rules must be an iterable, given {rules!r}") diff --git a/src/python/pants/engine/delete_rule_integration_test.py b/src/python/pants/engine/delete_rule_integration_test.py new file mode 100644 index 00000000000..c4e5a6dd68c --- /dev/null +++ b/src/python/pants/engine/delete_rule_integration_test.py @@ -0,0 +1,55 @@ +# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). +from dataclasses import dataclass + +from pants.engine.rules import DeleteRule, collect_rules, rule +from pants.testutil.rule_runner import QueryRule, RuleRunner + + +@dataclass(frozen=True) +class IntRequest: + pass + + +@rule +async def original_rule(request: IntRequest) -> int: + return 0 + + +@rule +def new_rule(request: IntRequest) -> int: + return 42 + + +def test_delete() -> None: + rule_runner = RuleRunner( + target_types=[], + rules=[ + *collect_rules( + { + "original_rule": original_rule, + } + ), + QueryRule(int, [IntRequest]), + ], + ) + + result = rule_runner.request(int, [IntRequest()]) + assert result == 0 + + rule_runner = RuleRunner( + target_types=[], + rules=[ + *collect_rules( + { + "original_rule": original_rule, + "new_rule": new_rule, + } + ), + DeleteRule.create(original_rule), + QueryRule(int, [IntRequest]), + ], + ) + + result = rule_runner.request(int, [IntRequest()]) + assert result == 42 diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index 7358e1d6089..70547eb934c 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -538,7 +538,7 @@ class RuleIndex: union_rules: FrozenOrderedSet[UnionRule] @classmethod - def create(cls, rule_entries: Iterable[Rule | UnionRule]) -> RuleIndex: + def create(cls, rule_entries: Iterable[Rule | UnionRule | DeleteRule]) -> RuleIndex: """Creates a RuleIndex with tasks indexed by their output type.""" rules: OrderedSet[TaskRule] = OrderedSet() delete_rules: OrderedSet[DeleteRule] = OrderedSet() From 812ecaf9dacd0cdffdb13ce8f15f52201cfd2fa5 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sat, 14 Sep 2024 22:19:03 +0200 Subject: [PATCH 6/9] call-by-name doesn't seem to work --- .../engine/delete_rule_integration_test.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/python/pants/engine/delete_rule_integration_test.py b/src/python/pants/engine/delete_rule_integration_test.py index c4e5a6dd68c..8cfb05b22ba 100644 --- a/src/python/pants/engine/delete_rule_integration_test.py +++ b/src/python/pants/engine/delete_rule_integration_test.py @@ -21,6 +21,16 @@ def new_rule(request: IntRequest) -> int: return 42 +@dataclass(frozen=True) +class WrapperUsingCallByNameRequest: + pass + + +@rule +async def wrapper_using_call_by_name(request: WrapperUsingCallByNameRequest) -> int: + return await original_rule(IntRequest()) + + def test_delete() -> None: rule_runner = RuleRunner( target_types=[], @@ -28,13 +38,14 @@ def test_delete() -> None: *collect_rules( { "original_rule": original_rule, + "wrapper_using_call_by_name": wrapper_using_call_by_name, } ), - QueryRule(int, [IntRequest]), + QueryRule(int, [WrapperUsingCallByNameRequest]), ], ) - result = rule_runner.request(int, [IntRequest()]) + result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) assert result == 0 rule_runner = RuleRunner( @@ -43,13 +54,14 @@ def test_delete() -> None: *collect_rules( { "original_rule": original_rule, + "wrapper_using_call_by_name": wrapper_using_call_by_name, "new_rule": new_rule, } ), DeleteRule.create(original_rule), - QueryRule(int, [IntRequest]), + QueryRule(int, [WrapperUsingCallByNameRequest]), ], ) - result = rule_runner.request(int, [IntRequest()]) + result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) assert result == 42 From 1f624db4e707f36d2a4a603adefda55bb86fd6b1 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sat, 14 Sep 2024 22:26:08 +0200 Subject: [PATCH 7/9] Call by type does work --- .../engine/delete_rule_integration_test.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/python/pants/engine/delete_rule_integration_test.py b/src/python/pants/engine/delete_rule_integration_test.py index 8cfb05b22ba..22ab2d32c06 100644 --- a/src/python/pants/engine/delete_rule_integration_test.py +++ b/src/python/pants/engine/delete_rule_integration_test.py @@ -4,6 +4,7 @@ from pants.engine.rules import DeleteRule, collect_rules, rule from pants.testutil.rule_runner import QueryRule, RuleRunner +from pants.engine.rules import Get @dataclass(frozen=True) @@ -21,6 +22,16 @@ def new_rule(request: IntRequest) -> int: return 42 +@dataclass(frozen=True) +class WrapperUsingCallByTypeRequest: + pass + + +@rule +async def wrapper_using_call_by_type(request: WrapperUsingCallByTypeRequest) -> int: + return await Get(int, IntRequest()) + + @dataclass(frozen=True) class WrapperUsingCallByNameRequest: pass @@ -38,13 +49,17 @@ def test_delete() -> None: *collect_rules( { "original_rule": original_rule, + "wrapper_using_call_by_type": wrapper_using_call_by_type, "wrapper_using_call_by_name": wrapper_using_call_by_name, } ), + QueryRule(int, [WrapperUsingCallByTypeRequest]), QueryRule(int, [WrapperUsingCallByNameRequest]), ], ) + result = rule_runner.request(int, [WrapperUsingCallByTypeRequest()]) + assert result == 0 result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) assert result == 0 @@ -54,14 +69,18 @@ def test_delete() -> None: *collect_rules( { "original_rule": original_rule, + "wrapper_using_call_by_type": wrapper_using_call_by_type, "wrapper_using_call_by_name": wrapper_using_call_by_name, "new_rule": new_rule, } ), DeleteRule.create(original_rule), + QueryRule(int, [WrapperUsingCallByTypeRequest]), QueryRule(int, [WrapperUsingCallByNameRequest]), ], ) + result = rule_runner.request(int, [WrapperUsingCallByTypeRequest()]) + assert result == 42 result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) assert result == 42 From c5897a5a29061ca90ee247bbf2b29e74593cc1cf Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sat, 14 Sep 2024 22:51:57 +0200 Subject: [PATCH 8/9] Split call_by_name and call_by_type tests --- .../engine/delete_rule_integration_test.py | 44 ++++++++++++++++--- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/python/pants/engine/delete_rule_integration_test.py b/src/python/pants/engine/delete_rule_integration_test.py index 22ab2d32c06..3037872b494 100644 --- a/src/python/pants/engine/delete_rule_integration_test.py +++ b/src/python/pants/engine/delete_rule_integration_test.py @@ -5,6 +5,7 @@ from pants.engine.rules import DeleteRule, collect_rules, rule from pants.testutil.rule_runner import QueryRule, RuleRunner from pants.engine.rules import Get +import pytest @dataclass(frozen=True) @@ -42,7 +43,7 @@ async def wrapper_using_call_by_name(request: WrapperUsingCallByNameRequest) -> return await original_rule(IntRequest()) -def test_delete() -> None: +def test_delete_call_by_type() -> None: rule_runner = RuleRunner( target_types=[], rules=[ @@ -50,18 +51,14 @@ def test_delete() -> None: { "original_rule": original_rule, "wrapper_using_call_by_type": wrapper_using_call_by_type, - "wrapper_using_call_by_name": wrapper_using_call_by_name, } ), QueryRule(int, [WrapperUsingCallByTypeRequest]), - QueryRule(int, [WrapperUsingCallByNameRequest]), ], ) result = rule_runner.request(int, [WrapperUsingCallByTypeRequest()]) assert result == 0 - result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) - assert result == 0 rule_runner = RuleRunner( target_types=[], @@ -70,17 +67,50 @@ def test_delete() -> None: { "original_rule": original_rule, "wrapper_using_call_by_type": wrapper_using_call_by_type, - "wrapper_using_call_by_name": wrapper_using_call_by_name, "new_rule": new_rule, } ), DeleteRule.create(original_rule), QueryRule(int, [WrapperUsingCallByTypeRequest]), - QueryRule(int, [WrapperUsingCallByNameRequest]), ], ) result = rule_runner.request(int, [WrapperUsingCallByTypeRequest()]) assert result == 42 + + +@pytest.mark.xfail +def test_delete_call_by_name() -> None: + rule_runner = RuleRunner( + target_types=[], + rules=[ + *collect_rules( + { + "original_rule": original_rule, + "wrapper_using_call_by_name": wrapper_using_call_by_name, + } + ), + QueryRule(int, [WrapperUsingCallByNameRequest]), + ], + ) + + result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) + assert result == 0 + + rule_runner = RuleRunner( + target_types=[], + rules=[ + *collect_rules( + { + "original_rule": original_rule, + "wrapper_using_call_by_name": wrapper_using_call_by_name, + "new_rule": new_rule, + } + ), + DeleteRule.create(original_rule), + QueryRule(int, [WrapperUsingCallByNameRequest]), + ], + ) + result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) assert result == 42 From 52c64474a1d68b6a4060f2f1998df8f4f7431f8d Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Mon, 16 Sep 2024 01:11:11 +0200 Subject: [PATCH 9/9] Trying to make it work --- .../pants/build_graph/build_configuration.py | 20 +++++----- .../engine/delete_rule_integration_test.py | 33 ++++++++-------- .../pants/engine/internals/rule_visitor.py | 11 ++++++ .../pants/engine/internals/scheduler.py | 39 ++++++++++++++++--- .../pants/engine/internals/selectors.py | 37 +++++++----------- src/python/pants/engine/rules.py | 39 ++++++++----------- src/python/pants/init/engine_initializer.py | 6 ++- 7 files changed, 107 insertions(+), 78 deletions(-) diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index 23c7b743a70..5c4b2958d50 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -8,13 +8,13 @@ from collections.abc import Iterable from dataclasses import dataclass, field from enum import Enum -from typing import Any, Callable, DefaultDict +from typing import Any, Callable, DefaultDict, FrozenSet from pants.backend.project_info.filter_targets import FilterSubsystem from pants.build_graph.build_file_aliases import BuildFileAliases from pants.core.util_rules.environments import EnvironmentsSubsystem from pants.engine.goal import GoalSubsystem -from pants.engine.rules import DeleteRule, Rule, RuleIndex, TaskRule +from pants.engine.rules import DeleteRule, Rule, RuleIndex from pants.engine.target import Target from pants.engine.unions import UnionRule from pants.option.alias import CliOptions @@ -60,6 +60,7 @@ class BuildConfiguration: subsystem_to_providers: FrozenDict[type[Subsystem], tuple[str, ...]] target_type_to_providers: FrozenDict[type[Target], tuple[str, ...]] rule_to_providers: FrozenDict[Rule, tuple[str, ...]] + delete_rules: FrozenSet[DeleteRule] union_rule_to_providers: FrozenDict[UnionRule, tuple[str, ...]] allow_unknown_options: bool remote_auth_plugin_func: Callable | None @@ -134,7 +135,7 @@ class Builder: default_factory=lambda: defaultdict(list) ) _rule_to_providers: dict[Rule, list[str]] = field(default_factory=lambda: defaultdict(list)) - _delete_rules: set[str] = field(default_factory=set) + _delete_rules: set[DeleteRule] = field(default_factory=set) _union_rule_to_providers: dict[UnionRule, list[str]] = field( default_factory=lambda: defaultdict(list) ) @@ -190,9 +191,9 @@ def _register_exposed_context_aware_object_factory( "Overwriting!".format(alias) ) - self._exposed_context_aware_object_factory_by_alias[ - alias - ] = context_aware_object_factory + self._exposed_context_aware_object_factory_by_alias[alias] = ( + context_aware_object_factory + ) def register_subsystems( self, plugin_or_backend: str, subsystems: Iterable[type[Subsystem]] @@ -229,7 +230,7 @@ def register_rules( for rule in rules_and_queries: self._rule_to_providers[rule].append(plugin_or_backend) for delete_rule in rule_index.delete_rules: - self._delete_rules.add(delete_rule.canonical_name) + self._delete_rules.add(delete_rule) for union_rule in rule_index.union_rules: self._union_rule_to_providers[union_rule].append(plugin_or_backend) self.register_subsystems( @@ -314,10 +315,9 @@ def create(self) -> BuildConfiguration: (k, tuple(v)) for k, v in self._target_type_to_providers.items() ), rule_to_providers=FrozenDict( - (k, tuple(v)) - for k, v in self._rule_to_providers.items() - if (not isinstance(k, TaskRule) or k.canonical_name not in self._delete_rules) + (k, tuple(v)) for k, v in self._rule_to_providers.items() ), + delete_rules=frozenset(self._delete_rules), union_rule_to_providers=FrozenDict( (k, tuple(v)) for k, v in self._union_rule_to_providers.items() ), diff --git a/src/python/pants/engine/delete_rule_integration_test.py b/src/python/pants/engine/delete_rule_integration_test.py index 3037872b494..1cab35e525b 100644 --- a/src/python/pants/engine/delete_rule_integration_test.py +++ b/src/python/pants/engine/delete_rule_integration_test.py @@ -78,24 +78,25 @@ def test_delete_call_by_type() -> None: result = rule_runner.request(int, [WrapperUsingCallByTypeRequest()]) assert result == 42 + assert 0 -@pytest.mark.xfail -def test_delete_call_by_name() -> None: - rule_runner = RuleRunner( - target_types=[], - rules=[ - *collect_rules( - { - "original_rule": original_rule, - "wrapper_using_call_by_name": wrapper_using_call_by_name, - } - ), - QueryRule(int, [WrapperUsingCallByNameRequest]), - ], - ) - result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) - assert result == 0 +def test_delete_call_by_name() -> None: + # rule_runner = RuleRunner( + # target_types=[], + # rules=[ + # *collect_rules( + # { + # "original_rule": original_rule, + # "wrapper_using_call_by_name": wrapper_using_call_by_name, + # } + # ), + # QueryRule(int, [WrapperUsingCallByNameRequest]), + # ], + # ) + + # result = rule_runner.request(int, [WrapperUsingCallByNameRequest()]) + # assert result == 0 rule_runner = RuleRunner( target_types=[], diff --git a/src/python/pants/engine/internals/rule_visitor.py b/src/python/pants/engine/internals/rule_visitor.py index 11675143177..06f9ee5b224 100644 --- a/src/python/pants/engine/internals/rule_visitor.py +++ b/src/python/pants/engine/internals/rule_visitor.py @@ -251,6 +251,13 @@ def _get_legacy_awaitable(self, call_node: ast.Call, is_effect: bool) -> Awaitab is_effect, ) + def _get_input_types(self, input_nodes: Sequence[Any]) -> tuple[type, ...]: + input_nodes, input_type_nodes = self._get_inputs(input_nodes) + return tuple( + self._check_constraint_arg_type(input_type, input_node) + for input_type, input_node in zip(input_type_nodes, input_nodes) + ) + def _get_byname_awaitable( self, rule_id: str, rule_func: Callable, call_node: ast.Call ) -> AwaitableConstraints: @@ -265,6 +272,9 @@ def _get_byname_awaitable( # argument names of kwargs. But positional-only callsites can avoid those allocations. explicit_args_arity = len(call_node.args) + # if "delete_rule_integration_test" in rule_id.split("."): + sys.stderr.write(f"{call_node=}\n") + input_types: tuple[type, ...] if not call_node.keywords: input_types = () @@ -295,6 +305,7 @@ def _get_byname_awaitable( # TODO: Extract this from the callee? Currently only intrinsics can be Effects, so need # to figure out their new syntax first. is_effect=False, + rule_func=rule_func, ) def visit_Call(self, call_node: ast.Call) -> None: diff --git a/src/python/pants/engine/internals/scheduler.py b/src/python/pants/engine/internals/scheduler.py index 8977d6b52d6..68c4323bddd 100644 --- a/src/python/pants/engine/internals/scheduler.py +++ b/src/python/pants/engine/internals/scheduler.py @@ -4,12 +4,13 @@ from __future__ import annotations import logging +import sys import os import time from dataclasses import dataclass from pathlib import PurePath from types import CoroutineType -from typing import Any, Callable, Dict, Iterable, NoReturn, Sequence, cast +from typing import Any, Callable, Dict, Iterable, NoReturn, Sequence, cast, get_type_hints from typing_extensions import TypedDict @@ -66,7 +67,7 @@ Process, ProcessResultMetadata, ) -from pants.engine.rules import Rule, RuleIndex, TaskRule +from pants.engine.rules import DeleteRule, RuleIndex, TaskRule from pants.engine.unions import UnionMembership, is_union, union_in_scope_types from pants.option.global_options import ( LOCAL_STORE_LEASE_TIME_SECS, @@ -119,7 +120,7 @@ def __init__( local_execution_root_dir: str, named_caches_dir: str, ca_certs_path: str | None, - rules: Iterable[Rule], + rule_index: RuleIndex, union_membership: UnionMembership, execution_options: ExecutionOptions, local_store_options: LocalStoreOptions, @@ -150,7 +151,6 @@ def __init__( self._visualize_to_dir = visualize_to_dir self._visualize_run_count = 0 # Validate and register all provided and intrinsic tasks. - rule_index = RuleIndex.create(rules) tasks = register_rules(rule_index, union_membership) # Create the native Scheduler and Session. @@ -685,6 +685,12 @@ def register_task(rule: TaskRule) -> None: ) for awaitable in rule.awaitables: + if "delete_rule_integration_test" in task_rule.canonical_name.split("."): + sys.stderr.write(f"{awaitable=}\n") + sys.stderr.write(f" {awaitable.input_types=}\n") + sys.stderr.write(f" {awaitable.output_type=}\n") + sys.stderr.write(f" {awaitable.explicit_args_arity=}\n") + unions = [t for t in awaitable.input_types if is_union(t)] if len(unions) == 1: # Register the union by recording a copy of the Get for each union member. @@ -702,6 +708,20 @@ def register_task(rule: TaskRule) -> None: raise TypeError( "Only one @union may be used in a Get, but {awaitable} used: {unions}." ) + elif ( + awaitable.rule_id is not None + and DeleteRule(awaitable.rule_id) in rule_index.delete_rules + ): + # This is a call to a known rule, but some plugin has deleted + # it, so it wants to override it with some other rule. We have + # to call it by type to make it possible. + awaitable.rule_id = None + native_engine.tasks_add_get( + tasks, + awaitable.output_type, + [v for k, v in get_type_hints(awaitable.rule_func).items() if k != "return"], + ) + sys.stderr.write(f"add_get for deleted {awaitable=}\n") elif awaitable.rule_id is not None: # Is a call to a known rule. native_engine.tasks_add_call( @@ -718,7 +738,16 @@ def register_task(rule: TaskRule) -> None: native_engine.tasks_task_end(tasks) for task_rule in rule_index.rules: - register_task(task_rule) + do_register = DeleteRule(rule_id=task_rule.canonical_name) not in rule_index.delete_rules + if "delete_rule_integration_test" in task_rule.canonical_name.split("."): + sys.stderr.write( + f"register {task_rule.canonical_name=}, {task_rule.func=}: {do_register}\n" + ) + for awaitable in task_rule.awaitables: + sys.stderr.write(f" {awaitable=}\n") + sys.stderr.write(f" {awaitable.rule_id=}\n") + if do_register: + register_task(task_rule) for query in rule_index.queries: native_engine.tasks_add_query( tasks, diff --git a/src/python/pants/engine/internals/selectors.py b/src/python/pants/engine/internals/selectors.py index bdeba709990..127b79cf995 100644 --- a/src/python/pants/engine/internals/selectors.py +++ b/src/python/pants/engine/internals/selectors.py @@ -9,6 +9,7 @@ from typing import ( TYPE_CHECKING, Any, + Callable, Coroutine, Generator, Generic, @@ -62,6 +63,7 @@ class AwaitableConstraints: explicit_args_arity: int input_types: tuple[type, ...] is_effect: bool + rule_func: Callable | None = None def __repr__(self) -> str: name = "Effect" if self.is_effect else "Get" @@ -192,9 +194,8 @@ def __await__(self) -> Generator[tuple[Get | Coroutine, ...], None, tuple]: @overload async def MultiGet( - __gets: Iterable[Get[_Output] | Coroutine[Any, Any, _Output]] -) -> tuple[_Output, ...]: - ... + __gets: Iterable[Get[_Output] | Coroutine[Any, Any, _Output]], +) -> tuple[_Output, ...]: ... @overload @@ -211,8 +212,7 @@ async def MultiGet( __get9: Get[_Output] | Coroutine[Any, Any, _Output], __get10: Get[_Output] | Coroutine[Any, Any, _Output], *__gets: Get[_Output] | Coroutine[Any, Any, _Output], -) -> tuple[_Output, ...]: - ... +) -> tuple[_Output, ...]: ... @overload @@ -227,8 +227,7 @@ async def MultiGet( __get7: Get[_Out7] | Coroutine[Any, Any, _Out7], __get8: Get[_Out8] | Coroutine[Any, Any, _Out8], __get9: Get[_Out9] | Coroutine[Any, Any, _Out9], -) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6, _Out7, _Out8, _Out9]: - ... +) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6, _Out7, _Out8, _Out9]: ... @overload @@ -242,8 +241,7 @@ async def MultiGet( __get6: Get[_Out6] | Coroutine[Any, Any, _Out6], __get7: Get[_Out7] | Coroutine[Any, Any, _Out7], __get8: Get[_Out8] | Coroutine[Any, Any, _Out8], -) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6, _Out7, _Out8]: - ... +) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6, _Out7, _Out8]: ... @overload @@ -256,8 +254,7 @@ async def MultiGet( __get5: Get[_Out5] | Coroutine[Any, Any, _Out5], __get6: Get[_Out6] | Coroutine[Any, Any, _Out6], __get7: Get[_Out7] | Coroutine[Any, Any, _Out7], -) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6, _Out7]: - ... +) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6, _Out7]: ... @overload @@ -269,8 +266,7 @@ async def MultiGet( __get4: Get[_Out4] | Coroutine[Any, Any, _Out4], __get5: Get[_Out5] | Coroutine[Any, Any, _Out5], __get6: Get[_Out6] | Coroutine[Any, Any, _Out6], -) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6]: - ... +) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5, _Out6]: ... @overload @@ -281,8 +277,7 @@ async def MultiGet( __get3: Get[_Out3] | Coroutine[Any, Any, _Out3], __get4: Get[_Out4] | Coroutine[Any, Any, _Out4], __get5: Get[_Out5] | Coroutine[Any, Any, _Out5], -) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5]: - ... +) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4, _Out5]: ... @overload @@ -292,8 +287,7 @@ async def MultiGet( __get2: Get[_Out2] | Coroutine[Any, Any, _Out2], __get3: Get[_Out3] | Coroutine[Any, Any, _Out3], __get4: Get[_Out4] | Coroutine[Any, Any, _Out4], -) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4]: - ... +) -> tuple[_Out0, _Out1, _Out2, _Out3, _Out4]: ... @overload @@ -302,8 +296,7 @@ async def MultiGet( __get1: Get[_Out1] | Coroutine[Any, Any, _Out1], __get2: Get[_Out2] | Coroutine[Any, Any, _Out2], __get3: Get[_Out3] | Coroutine[Any, Any, _Out3], -) -> tuple[_Out0, _Out1, _Out2, _Out3]: - ... +) -> tuple[_Out0, _Out1, _Out2, _Out3]: ... @overload @@ -311,16 +304,14 @@ async def MultiGet( __get0: Get[_Out0] | Coroutine[Any, Any, _Out0], __get1: Get[_Out1] | Coroutine[Any, Any, _Out1], __get2: Get[_Out2] | Coroutine[Any, Any, _Out2], -) -> tuple[_Out0, _Out1, _Out2]: - ... +) -> tuple[_Out0, _Out1, _Out2]: ... @overload async def MultiGet( __get0: Get[_Out0] | Coroutine[Any, Any, _Out0], __get1: Get[_Out1] | Coroutine[Any, Any, _Out1], -) -> tuple[_Out0, _Out1]: - ... +) -> tuple[_Out0, _Out1]: ... async def MultiGet( diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index 70547eb934c..1adadac51c2 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -3,6 +3,7 @@ from __future__ import annotations +import traceback import functools import inspect import sys @@ -357,20 +358,17 @@ def wrapper(*args): @overload -def rule(func: Callable[P, Coroutine[Any, Any, R]]) -> Callable[P, Coroutine[Any, Any, R]]: - ... +def rule(func: Callable[P, Coroutine[Any, Any, R]]) -> Callable[P, Coroutine[Any, Any, R]]: ... @overload -def rule(func: Callable[P, R]) -> Callable[P, Coroutine[Any, Any, R]]: - ... +def rule(func: Callable[P, R]) -> Callable[P, Coroutine[Any, Any, R]]: ... @overload def rule( *args, func: None = None, **kwargs: Any -) -> Callable[[Union[SyncRuleT, AsyncRuleT]], AsyncRuleT]: - ... +) -> Callable[[Union[SyncRuleT, AsyncRuleT]], AsyncRuleT]: ... def rule(*args, **kwargs): @@ -378,20 +376,17 @@ def rule(*args, **kwargs): @overload -def goal_rule(func: Callable[P, Coroutine[Any, Any, R]]) -> Callable[P, Coroutine[Any, Any, R]]: - ... +def goal_rule(func: Callable[P, Coroutine[Any, Any, R]]) -> Callable[P, Coroutine[Any, Any, R]]: ... @overload -def goal_rule(func: Callable[P, R]) -> Callable[P, Coroutine[Any, Any, R]]: - ... +def goal_rule(func: Callable[P, R]) -> Callable[P, Coroutine[Any, Any, R]]: ... @overload def goal_rule( *args, func: None = None, **kwargs: Any -) -> Callable[[Union[SyncRuleT, AsyncRuleT]], AsyncRuleT]: - ... +) -> Callable[[Union[SyncRuleT, AsyncRuleT]], AsyncRuleT]: ... def goal_rule(*args, **kwargs): @@ -402,21 +397,18 @@ def goal_rule(*args, **kwargs): @overload def _uncacheable_rule( - func: Callable[P, Coroutine[Any, Any, R]] -) -> Callable[P, Coroutine[Any, Any, R]]: - ... + func: Callable[P, Coroutine[Any, Any, R]], +) -> Callable[P, Coroutine[Any, Any, R]]: ... @overload -def _uncacheable_rule(func: Callable[P, R]) -> Callable[P, Coroutine[Any, Any, R]]: - ... +def _uncacheable_rule(func: Callable[P, R]) -> Callable[P, Coroutine[Any, Any, R]]: ... @overload def _uncacheable_rule( *args, func: None = None, **kwargs: Any -) -> Callable[[Union[SyncRuleT, AsyncRuleT]], AsyncRuleT]: - ... +) -> Callable[[Union[SyncRuleT, AsyncRuleT]], AsyncRuleT]: ... # This has a "private" name, as we don't (yet?) want it to be part of the rule API, at least @@ -474,14 +466,14 @@ def iter_rules(): @dataclass(frozen=True) class DeleteRule: - canonical_name: str + rule_id: str @classmethod def create(cls, f: Any): if not hasattr(f, "rule_id"): raise ValueError(f"`{f.__qualname__}` is not a rule") - return DeleteRule(canonical_name=f.rule_id) + return DeleteRule(rule_id=f.rule_id) @dataclass(frozen=True) @@ -565,9 +557,12 @@ def create(cls, rule_entries: Iterable[Rule | UnionRule | DeleteRule]) -> RuleIn "extend Rule or UnionRule, or are static functions decorated with @rule." ) - return RuleIndex( + rule_index = RuleIndex( rules=FrozenOrderedSet(rules), delete_rules=FrozenOrderedSet(delete_rules), queries=FrozenOrderedSet(queries), union_rules=FrozenOrderedSet(union_rules), ) + sys.stderr.write(f"create {id(rule_index)=} with {delete_rules}\n") + # traceback.print_stack() + return rule_index diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index f0f7fa16a94..ea9be13d74e 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -3,6 +3,7 @@ from __future__ import annotations +from itertools import chain import logging from dataclasses import dataclass from pathlib import Path @@ -34,7 +35,7 @@ from pants.engine.internals.scheduler import Scheduler, SchedulerSession from pants.engine.internals.selectors import Params from pants.engine.internals.session import SessionValues -from pants.engine.rules import QueryRule, collect_rules, rule +from pants.engine.rules import QueryRule, RuleIndex, collect_rules, rule from pants.engine.streaming_workunit_handler import rules as streaming_workunit_handler_rules from pants.engine.target import RegisteredTargetTypes from pants.engine.unions import UnionMembership, UnionRule @@ -335,6 +336,7 @@ def ensure_optional_absolute_path(v: str | None) -> str | None: return None return ensure_absolute_path(v) + rule_index = RuleIndex.create(chain(rules, build_configuration.delete_rules)) scheduler = Scheduler( ignore_patterns=pants_ignore_patterns, use_gitignore=use_gitignore, @@ -342,7 +344,7 @@ def ensure_optional_absolute_path(v: str | None) -> str | None: local_execution_root_dir=ensure_absolute_path(local_execution_root_dir), named_caches_dir=ensure_absolute_path(named_caches_dir), ca_certs_path=ensure_optional_absolute_path(ca_certs_path), - rules=rules, + rule_index=rule_index, union_membership=union_membership, executor=executor, execution_options=execution_options,