Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename @console_rule to @goal_rule #8942

Merged
merged 1 commit into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from pants.engine.addressable import BuildFileAddresses
from pants.engine.console import Console
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.rules import console_rule
from pants.engine.rules import goal_rule


class ListAndDieForTestingOptions(GoalSubsystem):
Expand All @@ -16,7 +16,7 @@ class ListAndDieForTesting(Goal):
subsystem_cls = ListAndDieForTestingOptions


@console_rule
@goal_rule
def fast_list_and_die_for_testing(
console: Console, addresses: BuildFileAddresses
) -> ListAndDieForTesting:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from pants.engine.goal import Goal, GoalSubsystem, LineOriented
from pants.engine.legacy.graph import HydratedTarget
from pants.engine.objects import union
from pants.engine.rules import console_rule, rule
from pants.engine.rules import goal_rule, rule
from pants.engine.selectors import Get, MultiGet
from pants.rules.core.distdir import DistDir

Expand Down Expand Up @@ -39,7 +39,7 @@ class AWSLambdaGoal(Goal):
subsystem_cls = AWSLambdaOptions


@console_rule
@goal_rule
async def create_awslambda(
addresses: BuildFileAddresses,
console: Console,
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/project_info/rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ python_tests(
name='tests',
dependencies=[
':rules',
'src/python/pants/testutil:console_rule_test_base',
'src/python/pants/testutil:goal_rule_test_base',
],
tags = {'partially_type_checked'},
)
4 changes: 2 additions & 2 deletions src/python/pants/backend/project_info/rules/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pants.engine.console import Console
from pants.engine.goal import Goal, GoalSubsystem, LineOriented
from pants.engine.legacy.graph import HydratedTargets, TransitiveHydratedTargets
from pants.engine.rules import console_rule
from pants.engine.rules import goal_rule
from pants.engine.selectors import Get


Expand All @@ -29,7 +29,7 @@ def register_options(cls, register):
class Dependencies(Goal):
subsystem_cls = DependenciesOptions

@console_rule
@goal_rule
async def dependencies(
console: Console, address_specs: AddressSpecs, options: DependenciesOptions,
) -> Dependencies:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
from pants.backend.project_info.rules import dependencies
from pants.backend.python.targets.python_library import PythonLibrary
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.testutil.console_rule_test_base import ConsoleRuleTestBase
from pants.testutil.goal_rule_test_base import GoalRuleTestBase


class FastDependenciesTest(ConsoleRuleTestBase):
class FastDependenciesTest(GoalRuleTestBase):
goal_cls = dependencies.Dependencies

@classmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.legacy.graph import HydratedTarget, HydratedTargets
from pants.engine.objects import Collection
from pants.engine.rules import console_rule, rule, subsystem_rule
from pants.engine.rules import goal_rule, rule, subsystem_rule
from pants.engine.selectors import Get, MultiGet
from pants.subsystem.subsystem import Subsystem
from pants.util.memo import memoized_method
Expand Down Expand Up @@ -217,7 +217,7 @@ def get_applicable_content_pattern_names(self, path):

# TODO: Switch this to `lint` once we figure out a good way for v1 tasks and v2 rules
# to share goal names.
@console_rule
@goal_rule
async def validate(
console: Console, hydrated_targets: HydratedTargets, validate_options: ValidateOptions
) -> Validate:
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/rules/run_setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
from pants.engine.legacy.graph import HydratedTarget, HydratedTargets, TransitiveHydratedTargets
from pants.engine.legacy.structs import PythonBinaryAdaptor, PythonTargetAdaptor, ResourcesAdaptor
from pants.engine.objects import Collection
from pants.engine.rules import console_rule, rule, subsystem_rule
from pants.engine.rules import goal_rule, rule, subsystem_rule
from pants.engine.selectors import Get, MultiGet
from pants.option.custom_types import shell_str
from pants.rules.core.distdir import DistDir
Expand Down Expand Up @@ -228,7 +228,7 @@ def validate_args(args: Tuple[str, ...]):
raise InvalidSetupPyArgs('Cannot use the `upload` or `register` setup.py commands')


@console_rule
@goal_rule
async def run_setup_pys(targets: HydratedTargets, options: SetupPyOptions, console: Console,
provenance_map: AddressProvenanceMap,
distdir: DistDir, workspace: Workspace) -> SetupPy:
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/bin/goal_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def create(self):
class GoalRunner:
"""Lists installed goals or else executes a named goal.

NB: GoalRunner represents a v1-only codepath. v2 goals are registered via `@console_rule` and
NB: GoalRunner represents a v1-only codepath. v2 goals are registered via `@goal_rule` and
the `pants.engine.goal.Goal` class.
"""

Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/bin/local_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,18 +283,18 @@ def _maybe_run_v1(self):
).create().run()

def _maybe_run_v2(self):
# N.B. For daemon runs, @console_rules are invoked pre-fork -
# N.B. For daemon runs, @goal_rules are invoked pre-fork -
# so this path only serves the non-daemon run mode.
if self._is_daemon:
return PANTS_SUCCEEDED_EXIT_CODE

_, ambiguous_goals, v2_goals = self._options.goals_by_version
goals = v2_goals + (ambiguous_goals if self._global_options.v2 else tuple())
self._run_tracker.set_v2_console_rule_names(goals)
self._run_tracker.set_v2_goal_rule_names(goals)
if not goals:
return PANTS_SUCCEEDED_EXIT_CODE

return self._graph_session.run_console_rules(
return self._graph_session.run_goal_rules(
self._options_bootstrapper,
self._options,
goals,
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/engine/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def values(self):

@dataclass(frozen=True)
class Goal:
"""The named product of a `@console_rule`.
"""The named product of a `@goal_rule`.

This class should be subclassed and linked to a corresponding `GoalSubsystem`:

Expand All @@ -99,8 +99,8 @@ class List(Goal):
subsystem_cls = ListOptions
```

Since `@console_rules` always run in order to produce side effects (generally: console output),
they are not cacheable, and the `Goal` product of a `@console_rule` contains only a exit_code
Since `@goal_rules` always run in order to produce side effects (generally: console output),
they are not cacheable, and the `Goal` product of a `@goal_rule` contains only a exit_code
value to indicate whether the rule exited cleanly.
"""
exit_code: int
Expand Down
17 changes: 6 additions & 11 deletions src/python/pants/engine/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,9 @@ def _make_rule(

has_goal_return_type = issubclass(return_type, Goal)
if cacheable and has_goal_return_type:
raise TypeError(
'An `@rule` that returns a `Goal` must be declared with `@console_rule` in order to signal '
'that it is not cacheable.'
)
raise TypeError("An `@rule` that returns a `Goal` must instead be declared with `@goal_rule`.")
if not cacheable and not has_goal_return_type:
raise TypeError(
'An `@console_rule` must return a subclass of `engine.goal.Goal`.'
)
raise TypeError("An `@goal_rule` must return a subclass of `engine.goal.Goal`.")
is_goal_cls = has_goal_return_type

def wrapper(func):
Expand Down Expand Up @@ -145,7 +140,7 @@ def resolve_type(name):
Get.create_statically_for_rule_graph(resolve_type(p), resolve_type(s))
for p, s in rule_visitor.gets)

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

# Set a default name for Goal classes if one is not explicitly provided
Expand Down Expand Up @@ -201,7 +196,7 @@ def _ensure_type_annotation(

PUBLIC_RULE_DECORATOR_ARGUMENTS = {'name'}
# We don't want @rule-writers to use 'cacheable' as a kwarg directly, but rather
# set it implicitly based on whether the rule annotation is @rule or @console_rule.
# set it implicitly based on whether the rule annotation is @rule or @goal_rule.
# So we leave it out of PUBLIC_RULE_DECORATOR_ARGUMENTS.
IMPLICIT_PRIVATE_RULE_DECORATOR_ARGUMENTS = {'cacheable'}

Expand All @@ -215,7 +210,7 @@ def rule_decorator(*args, **kwargs) -> Callable:

if len(set(kwargs) - PUBLIC_RULE_DECORATOR_ARGUMENTS - IMPLICIT_PRIVATE_RULE_DECORATOR_ARGUMENTS) != 0:
raise UnrecognizedRuleArgument(
f"`@rule`s and `@console_rule`s only accept the following keyword arguments: {PUBLIC_RULE_DECORATOR_ARGUMENTS}"
f"`@rule`s and `@goal_rule`s only accept the following keyword arguments: {PUBLIC_RULE_DECORATOR_ARGUMENTS}"
)

func = args[0]
Expand Down Expand Up @@ -256,7 +251,7 @@ def rule(*args, **kwargs) -> Callable:
return inner_rule(*args, **kwargs, cacheable=True)


def console_rule(*args, **kwargs) -> Callable:
def goal_rule(*args, **kwargs) -> Callable:
return inner_rule(*args, **kwargs, cacheable=False)


Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def _trace_on_error(self, unique_exceptions, request):
unique_exceptions
)

def run_console_rule(self, product, subject):
def run_goal_rule(self, product, subject):
"""
:param product: A Goal subtype.
:param subject: subject for the request.
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/fs/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ python_tests(
dependencies = [
'src/python/pants/util:contextutil',
'src/python/pants/testutil:test_base',
'src/python/pants/testutil:console_rule_test_base',
'src/python/pants/testutil:goal_rule_test_base',
'src/python/pants/testutil/engine:util',
],
tags = {"partially_type_checked"},
Expand Down
24 changes: 12 additions & 12 deletions src/python/pants/fs/fs_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@
Workspace,
)
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.rules import RootRule, console_rule
from pants.engine.rules import RootRule, goal_rule
from pants.engine.selectors import Get
from pants.fs.fs import is_child_of
from pants.testutil.console_rule_test_base import ConsoleRuleTestBase
from pants.testutil.goal_rule_test_base import GoalRuleTestBase
from pants.testutil.test_base import TestBase


@dataclass(frozen=True)
class MessageToConsoleRule:
class MessageToGoalRule:
input_files_content: InputFilesContent


Expand All @@ -38,26 +38,26 @@ class MockWorkspaceGoal(Goal):
subsystem_cls = MockWorkspaceGoalOptions


@console_rule
async def workspace_console_rule(console: Console, workspace: Workspace, msg: MessageToConsoleRule) -> MockWorkspaceGoal:
@goal_rule
async def workspace_goal_rule(console: Console, workspace: Workspace, msg: MessageToGoalRule) -> MockWorkspaceGoal:
digest = await Get[Digest](InputFilesContent, msg.input_files_content)
output = workspace.materialize_directory(DirectoryToMaterialize(digest))
console.print_stdout(output.output_paths[0], end='')
return MockWorkspaceGoal(exit_code=0)


class WorkspaceInConsoleRuleTest(ConsoleRuleTestBase):
class WorkspaceInGoalRuleTest(GoalRuleTestBase):
"""This test is meant to ensure that the Workspace type successfully
invokes the rust FFI function to write to disk in the context of a @console_rule,
invokes the rust FFI function to write to disk in the context of a @goal_rule,
without crashing or otherwise failing."""
goal_cls = MockWorkspaceGoal

@classmethod
def rules(cls):
return super().rules() + [RootRule(MessageToConsoleRule), workspace_console_rule]
return super().rules() + [RootRule(MessageToGoalRule), workspace_goal_rule]

def test(self):
msg = MessageToConsoleRule(
msg = MessageToGoalRule(
input_files_content=InputFilesContent([FileContent(path='a.txt', content=b'hello')])
)
output_path = Path(self.build_root, 'a.txt')
Expand All @@ -66,11 +66,11 @@ def test(self):


#TODO(gshuflin) - it would be nice if this test, which tests that the MaterializeDirectoryResults value
# is valid, could be subsumed into the above @console_rule-based test, but it's a bit awkward
# to get the MaterializeDirectoriesResult out of a @console_rule at the moment.
# is valid, could be subsumed into the above @goal_rule-based test, but it's a bit awkward
# to get the MaterializeDirectoriesResult out of a @goal_rule at the moment.
class FileSystemTest(TestBase):
def test_workspace_materialize_directories_result(self):
#TODO(#8336): at some point, this test should require that Workspace only be invoked from a console_role
#TODO(#8336): at some point, this test should require that Workspace only be invoked from an @goal_rule
workspace = Workspace(self.scheduler)

input_files_content = InputFilesContent((
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/goal/run_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def __init__(self, *args, **kwargs):
self._run_timestamp = time.time()
self._cmd_line = ' '.join(['pants'] + sys.argv[1:])
self._sorted_goal_infos = tuple()
self._v2_console_rule_names = tuple()
self._v2_goal_rule_names = tuple()

self.run_uuid = uuid.uuid4().hex
# Select a globally unique ID for the run, that sorts by time.
Expand Down Expand Up @@ -184,8 +184,8 @@ def __init__(self, *args, **kwargs):
def set_sorted_goal_infos(self, sorted_goal_infos):
self._sorted_goal_infos = sorted_goal_infos

def set_v2_console_rule_names(self, v2_console_rule_names):
self._v2_console_rule_names = v2_console_rule_names
def set_v2_goal_rule_names(self, v2_goal_rule_names):
self._v2_goal_rule_names = v2_goal_rule_names

def register_thread(self, parent_workunit):
"""Register the parent workunit for all work in the calling thread.
Expand Down Expand Up @@ -527,7 +527,7 @@ def end(self):
if self._sorted_goal_infos and self.run_info.get_info("computed_goals") is None:
self.run_info.add_info(
"computed_goals",
self._v2_console_rule_names + tuple(goal.goal.name for goal in self._sorted_goal_infos),
self._v2_goal_rule_names + tuple(goal.goal.name for goal in self._sorted_goal_infos),
stringify=False,
# If the goal is clean-all then the run info dir no longer exists, so ignore that error.
ignore_errors=True,
Expand Down
11 changes: 5 additions & 6 deletions src/python/pants/init/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,13 @@ class InvalidGoals(Exception):
"""Raised when invalid v2 goals are passed in a v2-only mode."""

def __init__(self, invalid_goals):
super(LegacyGraphSession.InvalidGoals, self).__init__(
'could not satisfy the following goals with @console_rules: {}'
.format(', '.join(invalid_goals))
super().__init__(
f"could not satisfy the following goals with @goal_rules: {', '.join(invalid_goals)}"
)
self.invalid_goals = invalid_goals

def run_console_rules(self, options_bootstrapper, options, goals, target_roots):
"""Runs @console_rules sequentially and interactively by requesting their implicit Goal products.
def run_goal_rules(self, options_bootstrapper, options, goals, target_roots):
"""Runs @goal_rules sequentially and interactively by requesting their implicit Goal products.

For retryable failures, raises scheduler.ExecutionError.

Expand All @@ -223,7 +222,7 @@ def run_console_rules(self, options_bootstrapper, options, goals, target_roots):
params = Params(subject, options_bootstrapper, console, workspace, interactive_runner)
logger.debug(f'requesting {goal_product} to satisfy execution of `{goal}` goal')
try:
exit_code = self.scheduler_session.run_console_rule(goal_product, params)
exit_code = self.scheduler_session.run_goal_rule(goal_product, params)
finally:
console.flush()

Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/option/global_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,14 +511,14 @@ def register_options(cls, register):
register('--v1', advanced=True, type=bool, default=True,
help='Enables execution of v1 Tasks.')
register('--v2', advanced=True, type=bool, default=False,
help='Enables execution of v2 @console_rules.')
help='Enables execution of v2 @goal_rules.')
register('--v2-ui', default=False, type=bool, daemon=False,
help='Whether to show v2 engine execution progress. '
'This requires the --v2 flag to take effect.')

loop_flag = '--loop'
register(loop_flag, type=bool,
help='Run v2 @console_rules continuously as file changes are detected. Requires '
help='Run v2 @goal_rules continuously as file changes are detected. Requires '
'`--v2`, and is best utilized with `--v2 --no-v1`.')
register('--loop-max', type=int, default=2**32, advanced=True,
help=f'The maximum number of times to loop when `{loop_flag}` is specified.')
Expand Down Expand Up @@ -562,7 +562,7 @@ def validate_instance(cls, opts):
Raises pants.option.errors.OptionsError on validation failure.
"""
if opts.loop and (not opts.v2 or opts.v1):
raise OptionsError('The `--loop` option only works with @console_rules, and thus requires '
raise OptionsError('The `--loop` option only works with @goal_rules, and thus requires '
'`--v2 --no-v1` to function as expected.')
if opts.loop and not opts.enable_pantsd:
raise OptionsError('The `--loop` option requires `--enable-pantsd`, in order to watch files.')
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/pantsd/service/scheduler_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ def _body(self, session, options, options_bootstrapper):
if v2_goals or (ambiguous_goals and global_options.v2):
goals = v2_goals + (ambiguous_goals if global_options.v2 else tuple())

# N.B. @console_rules run pre-fork in order to cache the products they request during execution.
exit_code = session.run_console_rules(
# N.B. @goal_rules run pre-fork in order to cache the products they request during execution.
exit_code = session.run_goal_rules(
options_bootstrapper,
options,
goals,
Expand Down
Loading