diff --git a/src/python/pants/base/specs.py b/src/python/pants/base/specs.py index 77a5418b05b..83a7b69a205 100644 --- a/src/python/pants/base/specs.py +++ b/src/python/pants/base/specs.py @@ -5,7 +5,18 @@ import re from abc import ABC, ABCMeta, abstractmethod from dataclasses import dataclass -from typing import Any, Dict, Iterable, Iterator, List, Optional, Sequence, Tuple, Union, cast +from typing import ( + TYPE_CHECKING, + Dict, + Iterable, + Iterator, + List, + Optional, + Sequence, + Tuple, + Union, + cast, +) from pants.engine.collection import Collection from pants.engine.fs import PathGlobs @@ -17,6 +28,9 @@ from pants.util.memo import memoized_property from pants.util.meta import frozen_after_init +if TYPE_CHECKING: + from pants.engine.internals.mapper import AddressFamily, AddressMapper + class Spec(ABC): """A specification for what Pants should operate on.""" @@ -40,7 +54,9 @@ class AddressFamilyResolutionError(Exception): pass @abstractmethod - def matching_address_families(self, address_families_dict: Dict[str, Any]) -> List[Any]: + def matching_address_families( + self, address_families_dict: Dict[str, "AddressFamily"] + ) -> List["AddressFamily"]: """Given a dict of (namespace path) -> AddressFamily, return the values matching this address spec. @@ -49,8 +65,8 @@ def matching_address_families(self, address_families_dict: Dict[str, Any]) -> Li @classmethod def address_families_for_dir( - cls, address_families_dict: Dict[str, Any], spec_dir_path: str - ) -> List[Any]: + cls, address_families_dict: Dict[str, "AddressFamily"], spec_dir_path: str + ) -> List["AddressFamily"]: """Implementation of `matching_address_families()` for address specs matching at most one directory.""" maybe_af = address_families_dict.get(spec_dir_path, None) @@ -64,7 +80,7 @@ class AddressResolutionError(Exception): pass @abstractmethod - def address_target_pairs_from_address_families(self, address_families: List[Any]): + def address_target_pairs_from_address_families(self, address_families: List["AddressFamily"]): """Given a list of AddressFamily, return (address, target) pairs matching this address spec. :raises: :class:`SingleAddress._SingleAddressResolutionError` for resolution errors with a @@ -84,11 +100,11 @@ def all_address_target_pairs(cls, address_families): return addr_tgt_pairs @abstractmethod - def make_glob_patterns(self, address_mapper: Any) -> List[str]: + def make_glob_patterns(self, address_mapper: "AddressMapper") -> List[str]: """Generate glob patterns matching exactly all the BUILD files this address spec covers.""" @classmethod - def globs_in_single_dir(cls, spec_dir_path: str, address_mapper: Any) -> List[str]: + def globs_in_single_dir(cls, spec_dir_path: str, address_mapper: "AddressMapper") -> List[str]: """Implementation of `make_glob_patterns()` which only allows a single base directory.""" return [os.path.join(spec_dir_path, pat) for pat in address_mapper.build_patterns] @@ -109,16 +125,20 @@ def __post_init__(self) -> None: def to_spec_string(self) -> str: return "{}:{}".format(self.directory, self.name) - def matching_address_families(self, address_families_dict: Dict[str, Any]) -> List[Any]: + def matching_address_families( + self, address_families_dict: Dict[str, "AddressFamily"] + ) -> List["AddressFamily"]: return self.address_families_for_dir(address_families_dict, self.directory) class _SingleAddressResolutionError(Exception): - def __init__(self, single_address_family: Any, name: str) -> None: + def __init__(self, single_address_family: "AddressFamily", name: str) -> None: super().__init__() self.single_address_family = single_address_family self.name = name - def address_target_pairs_from_address_families(self, address_families: Sequence[Any]): + def address_target_pairs_from_address_families( + self, address_families: Sequence["AddressFamily"] + ): """Return the pair for the single target matching the single AddressFamily, or error. :raises: :class:`SingleAddress._SingleAddressResolutionError` if no targets could be found for a @@ -137,7 +157,7 @@ def address_target_pairs_from_address_families(self, address_families: Sequence[ assert len(addr_tgt_pairs) == 1 return addr_tgt_pairs - def make_glob_patterns(self, address_mapper: Any) -> List[str]: + def make_glob_patterns(self, address_mapper: "AddressMapper") -> List[str]: return self.globs_in_single_dir(self.directory, address_mapper) @@ -150,13 +170,17 @@ class SiblingAddresses(AddressSpec): def to_spec_string(self) -> str: return f"{self.directory}:" - def matching_address_families(self, address_families_dict: Dict[str, Any]) -> List[Any]: + def matching_address_families( + self, address_families_dict: Dict[str, "AddressFamily"] + ) -> List["AddressFamily"]: return self.address_families_for_dir(address_families_dict, self.directory) - def address_target_pairs_from_address_families(self, address_families: Sequence[Any]): + def address_target_pairs_from_address_families( + self, address_families: Sequence["AddressFamily"] + ): return self.all_address_target_pairs(address_families) - def make_glob_patterns(self, address_mapper: Any) -> List[str]: + def make_glob_patterns(self, address_mapper: "AddressMapper") -> List[str]: return self.globs_in_single_dir(self.directory, address_mapper) @@ -170,14 +194,18 @@ class DescendantAddresses(AddressSpec): def to_spec_string(self) -> str: return f"{self.directory}::" - def matching_address_families(self, address_families_dict: Dict[str, Any]) -> List[Any]: + def matching_address_families( + self, address_families_dict: Dict[str, "AddressFamily"] + ) -> List["AddressFamily"]: return [ af for ns, af in address_families_dict.items() if fast_relpath_optional(ns, self.directory) is not None ] - def address_target_pairs_from_address_families(self, address_families: Sequence[Any]): + def address_target_pairs_from_address_families( + self, address_families: Sequence["AddressFamily"] + ): addr_tgt_pairs = self.all_address_target_pairs(address_families) if self.error_if_no_matches and len(addr_tgt_pairs) == 0: raise self.AddressResolutionError( @@ -185,7 +213,7 @@ def address_target_pairs_from_address_families(self, address_families: Sequence[ ) return addr_tgt_pairs - def make_glob_patterns(self, address_mapper: Any) -> List[str]: + def make_glob_patterns(self, address_mapper: "AddressMapper") -> List[str]: return [os.path.join(self.directory, "**", pat) for pat in address_mapper.build_patterns] @@ -198,17 +226,21 @@ class AscendantAddresses(AddressSpec): def to_spec_string(self) -> str: return f"{self.directory}^" - def matching_address_families(self, address_families_dict: Dict[str, Any]) -> List[Any]: + def matching_address_families( + self, address_families_dict: Dict[str, "AddressFamily"] + ) -> List["AddressFamily"]: return [ af for ns, af in address_families_dict.items() if fast_relpath_optional(self.directory, ns) is not None ] - def address_target_pairs_from_address_families(self, address_families): + def address_target_pairs_from_address_families( + self, address_families: Sequence["AddressFamily"] + ): return self.all_address_target_pairs(address_families) - def make_glob_patterns(self, address_mapper: Any) -> List[str]: + def make_glob_patterns(self, address_mapper: "AddressMapper") -> List[str]: return [ os.path.join(f, pattern) for pattern in address_mapper.build_patterns diff --git a/src/python/pants/engine/fs.py b/src/python/pants/engine/fs.py index 99116e58695..fd40a620654 100644 --- a/src/python/pants/engine/fs.py +++ b/src/python/pants/engine/fs.py @@ -4,7 +4,7 @@ import os from dataclasses import dataclass from pathlib import Path -from typing import Any, Iterable, Optional, Tuple +from typing import TYPE_CHECKING, Iterable, Optional, Tuple from pants.engine.collection import Collection from pants.engine.rules import RootRule, side_effecting @@ -18,6 +18,9 @@ ) from pants.util.meta import frozen_after_init +if TYPE_CHECKING: + from pants.engine.internals.scheduler import SchedulerSession + @dataclass(frozen=True) class FileContent: @@ -282,8 +285,7 @@ class UrlToFetch: class Workspace: """Abstract handle for operations that touch the real local filesystem.""" - # TODO: SchedulerSession. Untyped because `fs.py` and `scheduler.py` have a cycle. - _scheduler: Any + _scheduler: "SchedulerSession" def materialize_directory(self, directory_to_materialize: DirectoryToMaterialize): """Materialize one single directory digest to disk. diff --git a/src/python/pants/engine/interactive_process.py b/src/python/pants/engine/interactive_process.py index a750a43ca4f..23470e8a034 100644 --- a/src/python/pants/engine/interactive_process.py +++ b/src/python/pants/engine/interactive_process.py @@ -3,13 +3,16 @@ import itertools from dataclasses import dataclass -from typing import Any, Iterable, Mapping, Optional, Tuple, cast +from typing import TYPE_CHECKING, Iterable, Mapping, Optional, Tuple from pants.base.exception_sink import ExceptionSink from pants.engine.fs import EMPTY_DIGEST, Digest from pants.engine.rules import RootRule, side_effecting from pants.util.meta import frozen_after_init +if TYPE_CHECKING: + from pants.engine.internals.scheduler import SchedulerSession + @dataclass(frozen=True) class InteractiveProcessResult: @@ -56,13 +59,11 @@ def __post_init__(self): @side_effecting @dataclass(frozen=True) class InteractiveRunner: - _scheduler: Any + _scheduler: "SchedulerSession" def run_process(self, request: InteractiveProcess) -> InteractiveProcessResult: ExceptionSink.toggle_ignoring_sigint_v2_engine(True) - return cast( - InteractiveProcessResult, self._scheduler.run_local_interactive_process(request) - ) + return self._scheduler.run_local_interactive_process(request) def rules(): diff --git a/src/python/pants/engine/internals/graph.py b/src/python/pants/engine/internals/graph.py index 1b3cdb9dd75..6b93f2790fc 100644 --- a/src/python/pants/engine/internals/graph.py +++ b/src/python/pants/engine/internals/graph.py @@ -141,8 +141,32 @@ async def transitive_target(wrapped_root: WrappedTarget) -> TransitiveTarget: if not root.has_field(Dependencies): return TransitiveTarget(root, ()) dependency_addresses = await Get(Addresses, DependenciesRequest(root[Dependencies])) - dependencies = await MultiGet(Get(TransitiveTarget, Address, d) for d in dependency_addresses) - return TransitiveTarget(root, dependencies) + + # For generated subtargets, we use a weak Get, which means that any dependency cycles will + # return None, rather than TransitiveTarget. + transitive_dependencies = await MultiGet( + Get(TransitiveTarget, Address, addr, weak=bool(addr.generated_base_target_name)) + for addr in dependency_addresses + ) + non_cyclic_dependencies = [] + cyclic_addresses = [] + for transitive_dep, address in zip(transitive_dependencies, dependency_addresses): + if transitive_dep: + non_cyclic_dependencies.append(transitive_dep) + else: + cyclic_addresses.append(address) + cyclic_dependencies = await MultiGet( + Get(WrappedTarget, Address, addr) for addr in cyclic_addresses + ) + + all_dependencies = ( + *non_cyclic_dependencies, + *( + TransitiveTarget(wrapped_tgt.target, dependencies=()) + for wrapped_tgt in cyclic_dependencies + ), + ) + return TransitiveTarget(root, tuple(all_dependencies)) @rule diff --git a/src/python/pants/engine/internals/graph_test.py b/src/python/pants/engine/internals/graph_test.py index aa8ae965f94..3bf8da0abb0 100644 --- a/src/python/pants/engine/internals/graph_test.py +++ b/src/python/pants/engine/internals/graph_test.py @@ -133,6 +133,35 @@ def get_target(name: str) -> Target: assert transitive_targets.dependencies == FrozenOrderedSet([d1, d2, d3, t2, t1]) assert transitive_targets.closure == FrozenOrderedSet([root, d2, d1, d3, t2, t1]) + def test_transitive_targets_tolerates_cycles(self) -> None: + """For generated subtargets, we should tolerate cycles between targets. + + This only works with generated subtargets, so we use explicit file dependencies in this + test. + """ + self.create_files("", ["dep.txt", "t1.txt", "t2.txt"]) + self.add_to_build_file( + "", + dedent( + """\ + target(name='dep', sources=['dep.txt']) + target(name='t1', sources=['t1.txt'], dependencies=['dep.txt', 't2.txt']) + target(name='t2', sources=['t2.txt'], dependencies=['t1.txt']) + """ + ), + ) + result = self.request_single_product( + TransitiveTargets, + Params(Addresses([Address.parse("//:t2")]), create_options_bootstrapper()), + ) + assert len(result.roots) == 1 + assert result.roots[0].address == Address.parse("//:t2") + assert [tgt.address for tgt in result.dependencies] == [ + Address("", target_name="t1.txt", generated_base_target_name="t1"), + Address("", target_name="dep.txt", generated_base_target_name="dep"), + Address("", target_name="t2.txt", generated_base_target_name="t2"), + ] + def test_resolve_generated_subtarget(self) -> None: self.add_to_build_file("demo", "target(sources=['f1.txt', 'f2.txt'])") generated_target_addresss = Address(