Skip to content

Commit

Permalink
turn the Changed subsystem into v2 rules for target root calculation
Browse files Browse the repository at this point in the history
fix --owner-of

add --query option to cover owner-of, changes-since, etc at once

make an integration test and make it pass

clean up

convert QueryComponentMixin -> just QueryComponent

actually add integration test!

tag the query integration test target as such, and use text_type()

move query.py up to src/python/pants/engine because it's not legacy

make getting changed files uncacheable!

fix test to avoid trailing newlines

...add query.py

move --query integration testing out of engine/legacy/

make things work after rebase (the test passes now!)

apply UnionRule to QueryComponent!

get to the point where we start wanting to return union values
  • Loading branch information
cosmicexplorer committed Nov 27, 2019
1 parent 785bf58 commit 0929282
Show file tree
Hide file tree
Showing 17 changed files with 371 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def console_output(self, unused_method_argument):
if not self.is_internal_only:
# TODO(John Sirois): We need an external payload abstraction at which point knowledge
# of jar and requirement payloads can go and this hairball will be untangled.
# TODO: This would be a great use case for @union and UnionRule!
if isinstance(tgt.payload.get_field('requirements'), PythonRequirementsField):
for requirement in tgt.payload.requirements:
yield str(requirement.requirement)
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/base/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ python_library(
sources = ['target_roots.py'],
dependencies = [
'3rdparty/python:dataclasses',
':specs',
],
tags = {'partially_type_checked'},
)
Expand Down
3 changes: 3 additions & 0 deletions src/python/pants/base/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,6 @@ def __init__(

def __iter__(self):
return iter(self.dependencies)

def __bool__(self):
return bool(self.dependencies)
4 changes: 2 additions & 2 deletions src/python/pants/base/target_roots.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from dataclasses import dataclass
from typing import Any
from pants.base.specs import Specs


class InvalidSpecConstraint(Exception):
Expand All @@ -12,4 +12,4 @@ class InvalidSpecConstraint(Exception):
@dataclass(frozen=True)
class TargetRoots:
"""Determines the target roots for a given pants run."""
specs: Any
specs: Specs
14 changes: 14 additions & 0 deletions src/python/pants/engine/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,20 @@ python_library(
],
)

python_library(
name='query',
sources=['query.py'],
dependencies=[
'src/python/pants/engine/legacy:graph',
'src/python/pants/engine:rules',
'src/python/pants/engine:selectors',
'src/python/pants/scm/subsystems:changed',
'src/python/pants/util:meta',
'src/python/pants/util:objects',
'src/python/pants/util:strutil',
],
)

python_library(
name='native',
sources=['native.py'],
Expand Down
21 changes: 16 additions & 5 deletions src/python/pants/engine/legacy/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from pants.engine.rules import RootRule, UnionMembership, UnionRule, rule, union
from pants.engine.selectors import Get, MultiGet
from pants.option.global_options import GlobMatchErrorBehavior
from pants.scm.subsystems.changed import ChangedFilesResult, ChangedRequest, IncludeDependees
from pants.source.filespec import any_matches_filespec
from pants.source.wrapped_globs import EagerFilesetWithSpec, FilesetRelPathWrapper

Expand Down Expand Up @@ -386,10 +387,10 @@ class HydratedTargets(Collection[HydratedTarget]):
@dataclass(frozen=True)
class OwnersRequest:
"""A request for the owners (and optionally, transitive dependees) of a set of file paths."""
sources: Tuple
sources: Tuple[str]
# TODO: `include_dependees` should become an `enum` of the choices from the
# `--changed-include-dependees` global option.
include_dependees: str
include_dependees: IncludeDependees


@rule
Expand Down Expand Up @@ -428,7 +429,8 @@ def owns_any_source(legacy_target):
owns_any_source(ht))

# If the OwnersRequest does not require dependees, then we're done.
if owners_request.include_dependees == 'none':
# TODO(#5933): make an enum exhaustiveness checking method that works with `await Get(...)`!
if owners_request.include_dependees == IncludeDependees.none:
return BuildFileAddresses(direct_owners)
else:
# Otherwise: find dependees.
Expand All @@ -442,13 +444,21 @@ def owns_any_source(legacy_target):
graph = _DependentGraph.from_iterable(target_types_from_build_file_aliases(bfa),
address_mapper,
all_structs)
if owners_request.include_dependees == 'direct':

if owners_request.include_dependees == IncludeDependees.direct:
return BuildFileAddresses(tuple(graph.dependents_of_addresses(direct_owners)))
else:
assert owners_request.include_dependees == 'transitive'
assert owners_request.include_dependees == IncludeDependees.transitive
return BuildFileAddresses(tuple(graph.transitive_dependents_of_addresses(direct_owners)))


@rule
async def changed_file_owners(changed_request: ChangedRequest) -> OwnersRequest:
"""Convert a description of changed files via an SCM into an owners request for those files."""
result = await Get(ChangedFilesResult, ChangedRequest, changed_request)
await OwnersRequest(sources=result.changed_files,
include_dependees=changed_request.include_dependees)

@rule
async def transitive_hydrated_targets(
build_file_addresses: BuildFileAddresses
Expand Down Expand Up @@ -604,6 +614,7 @@ def create_legacy_graph_tasks():
hydrated_targets,
hydrate_target,
find_owners,
changed_file_owners,
hydrate_sources,
hydrate_bundles,
RootRule(OwnersRequest),
Expand Down
163 changes: 163 additions & 0 deletions src/python/pants/engine/query.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# coding=utf-8
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from abc import ABC, abstractmethod
from dataclasses import dataclass
from typing import Dict, Tuple, Type, TypeVar

from pants.engine.legacy.graph import OwnersRequest
from pants.engine.rules import UnionMembership, UnionRule, RootRule, rule, union
from pants.scm.subsystems.changed import ChangedRequest, IncludeDependees
from pants.util.meta import classproperty
from pants.util.strutil import safe_shlex_split


@union
class QueryComponent(ABC):

@classproperty
@abstractmethod
def function_name(cls):
"""The initial argument of a shlexed query expression.
If the user provides --query='<name> <args...>' on the command line, and `<name>` matches this
property, the .parse_from_args() method is invoked with `<args...>` (shlexed, so split by
spaces).
"""

@classmethod
@abstractmethod
def parse_from_args(cls, *args):
"""Create an instance of this class from variadic positional string arguments.
This method should raise an error if the args are incorrect or invalid.
"""


@dataclass(frozen=True)
class OwnerOf:
files: Tuple[str]

function_name = 'owner-of'

@classmethod
def parse_from_args(cls, *args):
return cls(files=tuple([str(f) for f in args]))


@rule
def owner_of_request(owner_of: OwnerOf) -> OwnersRequest:
return OwnersRequest(sources=owner_of.files, include_dependees=IncludeDependees.none)


@dataclass(frozen=True)
class ChangesSince:
changes_since: str
include_dependees: IncludeDependees

function_name = 'changes-since'

@classmethod
def parse_from_args(cls, changes_since, include_dependees=IncludeDependees.none):
return cls(changes_since=str(changes_since),
include_dependees=IncludeDependees(include_dependees))


@rule
def changes_since_request(changes_since: ChangesSince) -> ChangedRequest:
return ChangedRequest(
changes_since=changes_since.changes_since,
diffspec=None,
include_dependees=changes_since.include_dependees,
# TODO(#7346): parse --query expressions using the python ast module to support keyword args!
fast=True,
)


@dataclass(frozen=True)
class ChangesForDiffspec:
diffspec: str
include_dependees: IncludeDependees

function_name = 'changes-for-diffspec'

@classmethod
def parse_from_args(cls, diffspec, include_dependees=IncludeDependees.none):
return cls(diffspec=str(diffspec),
include_dependees=IncludeDependees(include_dependees))


@rule
def changes_for_diffspec_request(changes_for_diffspec: ChangesForDiffspec) -> ChangedRequest:
return ChangedRequest(
changes_since=None,
diffspec=changes_for_diffspec.diffspec,
include_dependees=changes_for_diffspec.include_dependees,
# TODO(#7346): parse --query expressions using the python ast module to support keyword args!
fast=True,
)



_T = TypeVar('_T', bound=QueryComponent)


@dataclass(frozen=True)
class KnownQueryExpressions:
components: Dict[str, Type[_T]]


@rule
def known_query_expressions(union_membership: UnionMembership) -> KnownQueryExpressions:
return KnownQueryExpressions({
union_member.function_name: union_member
for union_member in union_membership.union_rules[QueryComponent]
})


@dataclass(frozen=True)
class QueryParseInput:
expr: str


class QueryParseError(Exception): pass


# FIXME: allow returning an @union!!!
@rule
def parse_query_expr(s: QueryParseInput, known: KnownQueryExpressions) -> QueryComponent:
"""Shlex the input string and attempt to find a query function matching the first element.
:param dict known_functions: A dict mapping `function_name` -> `QueryComponent` subclass.
:param str s: The string argument provided to --query.
:return: A query component which can be resolved into `BuildFileAddresses` in the v2 engine.
:rtype: `QueryComponent`
"""
args = safe_shlex_split(s.expr)
name = args[0]
rest = args[1:]
selected_function = known.components.get(name, None)
if selected_function:
return selected_function.parse_from_args(*rest)
else:
raise QueryParseError(
'Query function with name {} not found (in expr {})! The known functions are: {}'
.format(name, s, known_functions))


def rules():
return [
RootRule(OwnerOf),
RootRule(ChangesSince),
RootRule(QueryParseInput),
RootRule(ChangesForDiffspec),
known_query_expressions,
UnionRule(QueryComponent, OwnerOf),
UnionRule(QueryComponent, ChangesSince),
UnionRule(QueryComponent, ChangesForDiffspec),
owner_of_request,
changes_since_request,
changes_for_diffspec_request,
parse_query_expr,
]
13 changes: 7 additions & 6 deletions src/python/pants/engine/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,11 @@ def _make_rule(

is_goal_cls = issubclass(return_type, Goal)
if is_goal_cls == cacheable:
raise TypeError(
'An `@rule` that produces a `Goal` must be declared with @console_rule in order to signal '
'that it is not cacheable.'
)
# TODO(#????): convert this back into a TypeError!
logger.error(
'An `@rule` that produces a `Goal` must be declared with @console_rule in order '
'to signal that it is not cacheable.')
cacheable = True

def wrapper(func):
if not inspect.isfunction(func):
Expand Down Expand Up @@ -387,8 +388,8 @@ class TaskRule(Rule):
"""A Rule that runs a task function when all of its input selectors are satisfied.
NB: This API is experimental, and not meant for direct consumption. To create a `TaskRule` you
should always prefer the `@rule` constructor, and in cases where that is too constraining
(likely due to #4535) please bump or open a ticket to explain the usecase.
should always prefer the `@rule` constructor, and in cases where that is too constraining, please
bump or open a ticket to explain the usecase.
"""
_output_type: Type
input_selectors: Tuple[Type, ...]
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/init/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ python_library(
'src/python/pants/engine:native',
'src/python/pants/engine:parser',
'src/python/pants/engine:platform',
'src/python/pants/engine:query',
'src/python/pants/engine:scheduler',
'src/python/pants/goal',
'src/python/pants/goal:run_tracker',
Expand Down
15 changes: 10 additions & 5 deletions src/python/pants/init/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,14 @@
from pants.engine.mapper import AddressMapper, ResolveError
from pants.engine.parser import SymbolTable
from pants.engine.platform import create_platform_rules
from pants.engine.query import rules as query_rules
from pants.engine.rules import RootRule, UnionMembership, rule
from pants.engine.scheduler import Scheduler
from pants.engine.selectors import Get, Params
from pants.init.options_initializer import BuildConfigInitializer, OptionsInitializer
from pants.option.global_options import (
DEFAULT_EXECUTION_OPTIONS,
ExecutionOptions,
GlobMatchErrorBehavior,
)
from pants.option.global_options import (DEFAULT_EXECUTION_OPTIONS, ExecutionOptions,
GlobMatchErrorBehavior)
from pants.scm.subsystems.changed import rules as changed_rules


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -333,6 +332,10 @@ def setup_legacy_graph_extended(

build_root = build_root or get_buildroot()
build_configuration = build_configuration or BuildConfigInitializer.get(options_bootstrapper)

query_rules_memoized = query_rules()
build_configuration.register_rules(query_rules_memoized)

bootstrap_options = options_bootstrapper.bootstrap_options.for_global_scope()

build_file_aliases = build_configuration.registered_aliases()
Expand Down Expand Up @@ -411,6 +414,8 @@ async def single_build_file_address(specs: Specs) -> BuildFileAddress:
create_graph_rules(address_mapper) +
create_options_parsing_rules() +
structs_rules() +
changed_rules() +
query_rules_memoized +
rules
)

Expand Down
Loading

0 comments on commit 0929282

Please sign in to comment.