Skip to content

Commit

Permalink
Revert tags changes, except for the python_tests bug fix (#15553)
Browse files Browse the repository at this point in the history
Revert #15413. It did fix two bugs, but was too risky.

In particular, it resulted in #15544. I wasn't able to cherry-pick the fix #15546 because the tests have diverged too much. So instead, we should revert.

However, we keep the fix for `python_tests`, which was a 2.11-specific regression.

[ci skip-rust]
  • Loading branch information
Eric-Arellano authored May 20, 2022
1 parent a042388 commit fd13827
Show file tree
Hide file tree
Showing 13 changed files with 38 additions and 148 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/backend/codegen/export_codegen_goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import (
FilteredTargets,
GenerateSourcesRequest,
HydratedSources,
HydrateSourcesRequest,
RegisteredTargetTypes,
SourcesField,
Targets,
)
from pants.engine.unions import UnionMembership

Expand All @@ -35,7 +35,7 @@ class ExportCodegen(Goal):

@goal_rule
async def export_codegen(
targets: FilteredTargets,
targets: Targets,
union_membership: UnionMembership,
workspace: Workspace,
dist_dir: DistDir,
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/core/goals/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.process import FallibleProcessResult
from pants.engine.rules import Get, MultiGet, QueryRule, collect_rules, goal_rule
from pants.engine.target import FilteredTargets
from pants.engine.target import Targets
from pants.engine.unions import UnionMembership, union
from pants.option.option_types import StrListOption
from pants.util.logging import LogLevel
Expand Down Expand Up @@ -160,7 +160,7 @@ class Check(Goal):
async def check(
console: Console,
workspace: Workspace,
targets: FilteredTargets,
targets: Targets,
dist_dir: DistDir,
union_membership: UnionMembership,
check_subsystem: CheckSubsystem,
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/core/goals/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import os
from dataclasses import dataclass
from typing import Iterable, Mapping, Sequence, cast
from typing import Iterable, Mapping, cast

from pants.base.build_root import BuildRoot
from pants.core.util_rules.distdir import DistDir
Expand All @@ -17,7 +17,7 @@
from pants.engine.internals.selectors import Effect, Get, MultiGet
from pants.engine.process import InteractiveProcess, InteractiveProcessResult
from pants.engine.rules import collect_rules, goal_rule
from pants.engine.target import FilteredTargets, Target
from pants.engine.target import Targets
from pants.engine.unions import UnionMembership, union
from pants.util.dirutil import safe_rmtree
from pants.util.frozendict import FrozenDict
Expand All @@ -36,7 +36,7 @@ class ExportRequest:
Subclass and install a member of this type to export data.
"""

targets: Sequence[Target]
targets: Targets


@frozen_after_init
Expand Down Expand Up @@ -107,7 +107,7 @@ class Export(Goal):
@goal_rule
async def export(
console: Console,
targets: FilteredTargets,
targets: Targets,
workspace: Workspace,
union_membership: UnionMembership,
build_root: BuildRoot,
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/core/goals/fmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.process import FallibleProcessResult, ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule, rule
from pants.engine.target import FilteredTargets, SourcesField, Targets
from pants.engine.target import SourcesField, Targets
from pants.engine.unions import UnionMembership, union
from pants.option.option_types import IntOption, StrListOption
from pants.util.collections import partition_sequentially
Expand Down Expand Up @@ -155,7 +155,7 @@ class Fmt(Goal):
@goal_rule
async def fmt(
console: Console,
targets: FilteredTargets,
targets: Targets,
fmt_subsystem: FmtSubsystem,
workspace: Workspace,
union_membership: UnionMembership,
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/core/goals/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.process import FallibleProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import FieldSet, FilteredTargets
from pants.engine.target import FieldSet, Targets
from pants.engine.unions import UnionMembership, union
from pants.option.option_types import IntOption, StrListOption
from pants.util.collections import partition_sequentially
Expand Down Expand Up @@ -185,7 +185,7 @@ class Lint(Goal):
async def lint(
console: Console,
workspace: Workspace,
targets: FilteredTargets,
targets: Targets,
specs_snapshot: SpecsSnapshot,
lint_subsystem: LintSubsystem,
union_membership: UnionMembership,
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/core/goals/repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from abc import ABC
from dataclasses import dataclass
from pathlib import PurePath
from typing import ClassVar, Iterable, Mapping, Optional, Sequence, Tuple
from typing import ClassVar, Iterable, Mapping, Optional, Tuple

from pants.base.build_root import BuildRoot
from pants.engine.addresses import Addresses
Expand All @@ -17,7 +17,7 @@
from pants.engine.internals.native_engine import EMPTY_DIGEST
from pants.engine.process import InteractiveProcess, InteractiveProcessResult
from pants.engine.rules import Effect, Get, collect_rules, goal_rule
from pants.engine.target import FilteredTargets, Target
from pants.engine.target import Targets
from pants.engine.unions import UnionMembership, union
from pants.option.global_options import GlobalOptions
from pants.option.option_types import BoolOption, StrOption
Expand All @@ -37,7 +37,7 @@ class ReplImplementation(ABC):

name: ClassVar[str]

targets: Sequence[Target]
targets: Targets
chroot: str # Absolute path of the chroot the sources will be materialized to.

def in_chroot(self, relpath: str) -> str:
Expand Down Expand Up @@ -102,7 +102,7 @@ async def run_repl(
console: Console,
workspace: Workspace,
repl_subsystem: ReplSubsystem,
specified_targets: FilteredTargets,
specified_targets: Targets,
build_root: BuildRoot,
union_membership: UnionMembership,
global_options: GlobalOptions,
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/engine/internals/build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from pants.engine.addresses import Address, Addresses, AddressInput, BuildFileAddress
from pants.engine.engine_aware import EngineAwareParameter
from pants.engine.fs import DigestContents, GlobMatchErrorBehavior, PathGlobs, Paths
from pants.engine.internals.mapper import AddressFamily, AddressMap, SpecsFilter
from pants.engine.internals.mapper import AddressFamily, AddressMap, AddressSpecsFilter
from pants.engine.internals.parametrize import _TargetParametrizations
from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser, error_on_imports
from pants.engine.internals.target_adaptor import TargetAdaptor
Expand Down Expand Up @@ -172,8 +172,8 @@ async def find_target_adaptor(address: Address) -> TargetAdaptor:


@rule
def setup_address_specs_filter(global_options: GlobalOptions) -> SpecsFilter:
return SpecsFilter(
def setup_address_specs_filter(global_options: GlobalOptions) -> AddressSpecsFilter:
return AddressSpecsFilter(
tags=global_options.tag, exclude_target_regexps=global_options.exclude_target_regexp
)

Expand All @@ -182,7 +182,7 @@ def setup_address_specs_filter(global_options: GlobalOptions) -> SpecsFilter:
async def addresses_from_address_specs(
address_specs: AddressSpecs,
build_file_options: BuildFileOptions,
specs_filter: SpecsFilter,
specs_filter: AddressSpecsFilter,
) -> Addresses:
matched_addresses: OrderedSet[Address] = OrderedSet()
filtering_disabled = address_specs.filter_by_global_options is False
Expand Down
31 changes: 7 additions & 24 deletions src/python/pants/engine/internals/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
SpecsSnapshot,
)
from pants.engine.internals import native_engine
from pants.engine.internals.mapper import SpecsFilter
from pants.engine.internals.parametrize import Parametrize, _TargetParametrization
from pants.engine.internals.parametrize import ( # noqa: F401
_TargetParametrizations as _TargetParametrizations,
Expand All @@ -60,7 +59,6 @@
FieldSet,
FieldSetsPerTarget,
FieldSetsPerTargetRequest,
FilteredTargets,
GeneratedSources,
GeneratedTargets,
GenerateSourcesRequest,
Expand Down Expand Up @@ -344,11 +342,6 @@ async def resolve_targets(
return Targets(expanded_targets)


@rule
def filter_targets(targets: Targets, specs_filter: SpecsFilter) -> FilteredTargets:
return FilteredTargets(tgt for tgt in targets if specs_filter.matches(tgt))


@rule(desc="Find all targets in the project", level=LogLevel.DEBUG)
async def find_all_targets(_: AllTargetsRequest) -> AllTargets:
tgts = await Get(Targets, AddressSpecs([MaybeEmptyDescendantAddresses("")]))
Expand Down Expand Up @@ -619,7 +612,6 @@ class OwnersRequest:

sources: tuple[str, ...]
owners_not_found_behavior: OwnersNotFoundBehavior = OwnersNotFoundBehavior.ignore
filter_by_global_options: bool = False


class Owners(Collection[Address]):
Expand All @@ -642,18 +634,10 @@ async def find_owners(owners_request: OwnersRequest) -> Owners:
# glob.
live_candidate_specs = tuple(AscendantAddresses(directory=d) for d in live_dirs)
deleted_candidate_specs = tuple(AscendantAddresses(directory=d) for d in deleted_dirs)
live_get: Get[FilteredTargets | Targets, AddressSpecs]
if owners_request.filter_by_global_options:
live_get = Get(
FilteredTargets, AddressSpecs(live_candidate_specs, filter_by_global_options=True)
)
deleted_get = Get(
UnexpandedTargets, AddressSpecs(deleted_candidate_specs, filter_by_global_options=True)
)
else:
live_get = Get(Targets, AddressSpecs(live_candidate_specs))
deleted_get = Get(UnexpandedTargets, AddressSpecs(deleted_candidate_specs))
live_candidate_tgts, deleted_candidate_tgts = await MultiGet(live_get, deleted_get)
live_candidate_tgts, deleted_candidate_tgts = await MultiGet(
Get(Targets, AddressSpecs(live_candidate_specs)),
Get(UnexpandedTargets, AddressSpecs(deleted_candidate_specs)),
)

matching_addresses: OrderedSet[Address] = OrderedSet()
unmatched_sources = set(owners_request.sources)
Expand All @@ -678,7 +662,7 @@ async def find_owners(owners_request: OwnersRequest) -> Owners:
# primary ownership, but the target still should match the file. We can't use
# `tgt.get()` because this is a mixin, and there technically may be >1 field.
secondary_owner_fields = tuple(
field
field # type: ignore[misc]
for field in candidate_tgt.field_values.values()
if isinstance(field, SecondaryOwnerMixin)
)
Expand Down Expand Up @@ -763,8 +747,7 @@ async def addresses_from_filesystem_specs(
for spec in filesystem_specs.file_includes
)
owners_per_include = await MultiGet(
Get(Owners, OwnersRequest(paths.files, filter_by_global_options=True))
for paths in paths_per_include
Get(Owners, OwnersRequest(sources=paths.files)) for paths in paths_per_include
)
addresses: set[Address] = set()
for spec, owners in zip(filesystem_specs.file_includes, owners_per_include):
Expand Down Expand Up @@ -1320,7 +1303,7 @@ async def find_valid_field_sets_for_target_roots(
) -> TargetRootsToFieldSets:
# NB: This must be in an `await Get`, rather than the rule signature, to avoid a rule graph
# issue.
targets = await Get(FilteredTargets, Specs, specs)
targets = await Get(Targets, Specs, specs)
field_sets_per_target = await Get(
FieldSetsPerTarget, FieldSetsPerTargetRequest(request.field_set_superclass, targets)
)
Expand Down
Loading

0 comments on commit fd13827

Please sign in to comment.