Skip to content

Commit

Permalink
Refactor find_valid_field_sets. (#10732)
Browse files Browse the repository at this point in the history
- Now it's possible to get the FieldSets that subclass some
  given superclass for any arbitrary set of targets.
- The previous logic, that applies this to the target roots,
  now uses this new logic.
- Also removes FieldSetWithOrigin, which was unused,
  and would have complicated the refactoring.
- Also improves error messages in setup_py.py, that
  are unrelated to this change but didn't merit their
  own change.

[ci skip-rust]

[ci skip-build-wheels]
  • Loading branch information
benjyw committed Sep 4, 2020
1 parent 39cff4f commit 91921db
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 165 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from pants.engine.fs import Digest, MergeDigests, Workspace
from pants.engine.goal import Goal, GoalSubsystem, LineOriented
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import FieldSet, TargetsToValidFieldSets, TargetsToValidFieldSetsRequest
from pants.engine.target import FieldSet, TargetRootsToFieldSets, TargetRootsToFieldSetsRequest
from pants.engine.unions import union


Expand Down Expand Up @@ -49,11 +49,11 @@ async def create_awslambda(
workspace: Workspace,
) -> AWSLambdaGoal:
targets_to_valid_field_sets = await Get(
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest(
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest(
AWSLambdaFieldSet,
goal_description="the `awslambda` goal",
error_if_no_valid_targets=True,
error_if_no_applicable_targets=True,
),
)
awslambdas = await MultiGet(
Expand Down
37 changes: 24 additions & 13 deletions src/python/pants/backend/python/goals/setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,26 @@ class TargetNotExported(Exception):
"""Indicates a target that was expected to be exported is not."""


class NoOwnerError(Exception):
"""Indicates an exportable target has no owning exported target."""
class InvalidEntryPoint(Exception):
"""Indicates that a specified binary entry point was invalid."""


class AmbiguousOwnerError(Exception):
"""Indicates an exportable target has more than one owning exported target."""
class OwnershipError(Exception):
"""An error related to target ownership calculation."""

def __init__(self, msg: str):
super().__init__(
f"{msg} See https://www.pantsbuild.org/v2.0/docs/python-setup-py-goal for "
f"how python_library targets are mapped to distributions."
)

class UnsupportedPythonVersion(Exception):
"""Indicates that the Python version is unsupported for running setup.py commands."""

class NoOwnerError(OwnershipError):
"""Indicates an exportable target has no owning exported target."""

class InvalidEntryPoint(Exception):
"""Indicates that a specified binary entry point was invalid."""

class AmbiguousOwnerError(OwnershipError):
"""Indicates an exportable target has more than one owning exported target."""


@dataclass(frozen=True)
Expand Down Expand Up @@ -739,14 +745,19 @@ async def get_exporting_owner(owned_dependency: OwnedDependency) -> ExportedTarg
sibling_owners.append(sibling)
sibling = next(exported_ancestor_iter, None)
if sibling_owners:
all_owners = [exported_ancestor] + sibling_owners
raise AmbiguousOwnerError(
f"Exporting owners for {target.address} are "
f"ambiguous. Found {exported_ancestor.address} and "
f"{len(sibling_owners)} others: "
f'{", ".join(so.address.spec for so in sibling_owners)}'
f"Found multiple sibling python_distribution targets that are the closest "
f"ancestor dependees of {target.address} and are therefore candidates to "
f"own it: {', '.join(o.address.spec for o in all_owners)}. Only a "
f"single such owner is allowed, to avoid ambiguity."
)
return ExportedTarget(owner)
raise NoOwnerError(f"No exported target owner found for {target.address}")
raise NoOwnerError(
f"No python_distribution target found to own {target.address}. Note that "
f"the owner must be in or above the owned target's directory, and must "
f"depend on it (directly or indirectly)."
)


@rule(desc="Set up setuptools")
Expand Down
10 changes: 6 additions & 4 deletions src/python/pants/core/goals/binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from pants.engine.fs import Digest, MergeDigests, Snapshot, Workspace
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import FieldSet, TargetsToValidFieldSets, TargetsToValidFieldSetsRequest
from pants.engine.target import FieldSet, TargetRootsToFieldSets, TargetRootsToFieldSetsRequest
from pants.engine.unions import union

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -41,9 +41,11 @@ class Binary(Goal):
@goal_rule
async def create_binary(workspace: Workspace, dist_dir: DistDir) -> Binary:
targets_to_valid_field_sets = await Get(
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest(
BinaryFieldSet, goal_description="the `binary` goal", error_if_no_valid_targets=True
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest(
BinaryFieldSet,
goal_description="the `binary` goal",
error_if_no_applicable_targets=True,
),
)
binaries = await MultiGet(
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/core/goals/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.process import InteractiveProcess, InteractiveRunner
from pants.engine.rules import Get, collect_rules, goal_rule
from pants.engine.target import FieldSet, TargetsToValidFieldSets, TargetsToValidFieldSetsRequest
from pants.engine.target import FieldSet, TargetRootsToFieldSets, TargetRootsToFieldSetsRequest
from pants.engine.unions import union
from pants.option.custom_types import shell_str
from pants.option.global_options import GlobalOptions
Expand Down Expand Up @@ -89,11 +89,11 @@ async def run(
build_root: BuildRoot,
) -> Run:
targets_to_valid_field_sets = await Get(
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest(
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest(
RunFieldSet,
goal_description="the `run` goal",
error_if_no_valid_targets=True,
error_if_no_applicable_targets=True,
expect_single_field_set=True,
),
)
Expand Down
10 changes: 5 additions & 5 deletions src/python/pants/core/goals/run_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
from pants.engine.process import InteractiveProcess, InteractiveRunner
from pants.engine.target import (
Target,
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest,
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest,
TargetWithOrigin,
)
from pants.option.global_options import GlobalOptions
Expand Down Expand Up @@ -72,9 +72,9 @@ class TestBinaryTarget(Target):
],
mock_gets=[
MockGet(
product_type=TargetsToValidFieldSets,
subject_type=TargetsToValidFieldSetsRequest,
mock=lambda _: TargetsToValidFieldSets({target_with_origin: [field_set]}),
product_type=TargetRootsToFieldSets,
subject_type=TargetRootsToFieldSetsRequest,
mock=lambda _: TargetRootsToFieldSets({target_with_origin: [field_set]}),
),
MockGet(
product_type=RunRequest,
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/core/goals/style_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

from pants.engine.collection import Collection
from pants.engine.fs import Snapshot
from pants.engine.target import FieldSetWithOrigin
from pants.engine.target import FieldSet
from pants.util.meta import frozen_after_init

_FS = TypeVar("_FS", bound=FieldSetWithOrigin)
_FS = TypeVar("_FS", bound=FieldSet)


@frozen_after_init
Expand Down
16 changes: 8 additions & 8 deletions src/python/pants/core/goals/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
from pants.engine.target import (
FieldSet,
Sources,
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest,
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest,
)
from pants.engine.unions import UnionMembership, union
from pants.util.frozendict import FrozenDict
Expand Down Expand Up @@ -376,9 +376,9 @@ async def run_tests(
) -> Test:
if test_subsystem.debug:
targets_to_valid_field_sets = await Get(
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest(
TestFieldSet, goal_description="`test --debug`", error_if_no_valid_targets=True
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest(
TestFieldSet, goal_description="`test --debug`", error_if_no_applicable_targets=True
),
)
debug_requests = await MultiGet(
Expand All @@ -395,11 +395,11 @@ async def run_tests(
return Test(exit_code)

targets_to_valid_field_sets = await Get(
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest(
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest(
TestFieldSet,
goal_description=f"the `{test_subsystem.name}` goal",
error_if_no_valid_targets=False,
error_if_no_applicable_targets=False,
),
)
field_sets_with_sources = await Get(
Expand Down
16 changes: 8 additions & 8 deletions src/python/pants/core/goals/test_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
from pants.engine.target import (
Sources,
Target,
TargetsToValidFieldSets,
TargetsToValidFieldSetsRequest,
TargetRootsToFieldSets,
TargetRootsToFieldSetsRequest,
TargetWithOrigin,
)
from pants.engine.unions import UnionMembership
Expand Down Expand Up @@ -132,11 +132,11 @@ def run_test_rule(
)

def mock_find_valid_field_sets(
_: TargetsToValidFieldSetsRequest,
) -> TargetsToValidFieldSets:
_: TargetRootsToFieldSetsRequest,
) -> TargetRootsToFieldSets:
if not valid_targets:
return TargetsToValidFieldSets({})
return TargetsToValidFieldSets(
return TargetRootsToFieldSets({})
return TargetRootsToFieldSets(
{
tgt_with_origin: [field_set.create(tgt_with_origin.target)]
for tgt_with_origin in targets
Expand Down Expand Up @@ -171,8 +171,8 @@ def mock_coverage_report_generation(
],
mock_gets=[
MockGet(
product_type=TargetsToValidFieldSets,
subject_type=TargetsToValidFieldSetsRequest,
product_type=TargetRootsToFieldSets,
subject_type=TargetRootsToFieldSetsRequest,
mock=mock_find_valid_field_sets,
),
MockGet(
Expand Down
Loading

0 comments on commit 91921db

Please sign in to comment.