Deprecate TransitiveTargetsRequestLite and `DependenciesRequestLite…

…` now that graph cycle is fixed (pantsbuild#11188)

Closes pantsbuild#10917. This cycle made sense. We were trying to use `HydratedSources` in a dep inference rule when `HydratedSources` already uses dependencies. This is why with the rule for `-> Subtargets`, we were avoiding `HydratedSources` in the first place.

pantsbuild#11094 ended up fixing this without us realizing.

This PR also factors up `SourcesPaths` and `StrippedSourceFileNames`, which are the reason we were able to fix this cycle. We'll want to use these types in other places, such as pantsbuild#11184.

[ci skip-rust]
[ci skip-build-wheels]

src/python/pants/backend/codegen/protobuf/python/rules.py

@@ -37,7 +37,7 @@
     GenerateSourcesRequest,
     Sources,
     TransitiveTargets,
-    TransitiveTargetsRequestLite,
+    TransitiveTargetsRequest,
 )
 from pants.engine.unions import UnionRule
 from pants.source.source_root import SourceRoot, SourceRootRequest
@@ -66,9 +66,8 @@ async def generate_python_from_protobuf(
     # Protoc needs all transitive dependencies on `protobuf_libraries` to work properly. It won't
     # actually generate those dependencies; it only needs to look at their .proto files to work
     # with imports.
-    # TODO(#10917): Use TransitiveTargets instead of TransitiveTargetsLite.
     transitive_targets = await Get(
-        TransitiveTargets, TransitiveTargetsRequestLite([request.protocol_target.address])
+        TransitiveTargets, TransitiveTargetsRequest([request.protocol_target.address])
     )
 
     # NB: By stripping the source roots, we avoid having to set the value `--proto_path`

src/python/pants/backend/python/dependency_inference/module_mapper.py

@@ -12,14 +12,11 @@
     PythonSources,
 )
 from pants.base.specs import AddressSpecs, DescendantAddresses
+from pants.core.util_rules.stripped_source_files import StrippedSourceFileNames
 from pants.engine.addresses import Address
 from pants.engine.collection import Collection
-from pants.engine.fs import PathGlobs, Paths
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
-from pants.engine.target import Targets
-from pants.option.global_options import GlobalOptions
-from pants.source.source_root import SourceRoot, SourceRootRequest
-from pants.util.dirutil import fast_relpath
+from pants.engine.target import SourcesPathsRequest, Targets
 from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 
@@ -68,54 +65,23 @@ def addresses_for_module(self, module: str) -> Tuple[Address, ...]:
         return self.mapping.get(parent_module, ())
 
 
-@dataclass(frozen=True)
-class _StrippedFileNamesRequest:
-    sources: PythonSources
-
-
-class _StrippedFileNames(Collection[str]):
-    pass
-
-
-@rule
-async def _stripped_file_names(
-    request: _StrippedFileNamesRequest, global_options: GlobalOptions
-) -> _StrippedFileNames:
-    sources_field_path_globs = request.sources.path_globs(
-        global_options.options.files_not_found_behavior
-    )
-    source_root, paths = await MultiGet(
-        Get(SourceRoot, SourceRootRequest, SourceRootRequest.for_address(request.sources.address)),
-        Get(Paths, PathGlobs, sources_field_path_globs),
-    )
-    if source_root.path == ".":
-        return _StrippedFileNames(paths.files)
-    return _StrippedFileNames(fast_relpath(f, source_root.path) for f in paths.files)
-
-
 @rule(desc="Creating map of first party targets to Python modules", level=LogLevel.DEBUG)
 async def map_first_party_modules_to_addresses() -> FirstPartyModuleToAddressMapping:
     all_expanded_targets = await Get(Targets, AddressSpecs([DescendantAddresses("")]))
     candidate_targets = tuple(tgt for tgt in all_expanded_targets if tgt.has_field(PythonSources))
-    # NB: We use a custom implementation to resolve the stripped source paths, rather than
-    # `StrippedSourceFiles`, so that we can use `Get(Paths, PathGlobs)` instead of
-    # `Get(Snapshot, PathGlobs)`, which is much faster.
-    #
-    # This implementation is kept private because it's not fully comprehensive, such as not looking
-    # at codegen. That's fine for dep inference, but not in other contexts.
-    stripped_sources_per_explicit_target = await MultiGet(
-        Get(_StrippedFileNames, _StrippedFileNamesRequest(tgt[PythonSources]))
+    stripped_sources_per_target = await MultiGet(
+        Get(StrippedSourceFileNames, SourcesPathsRequest(tgt[PythonSources]))
         for tgt in candidate_targets
     )
 
     modules_to_addresses: DefaultDict[str, List[Address]] = defaultdict(list)
     modules_with_multiple_implementations: Set[str] = set()
-    for tgt, stripped_sources in zip(candidate_targets, stripped_sources_per_explicit_target):
+    for tgt, stripped_sources in zip(candidate_targets, stripped_sources_per_target):
         for stripped_f in stripped_sources:
             module = PythonModule.create_from_stripped_path(PurePath(stripped_f)).module
             if module in modules_to_addresses:
-                # We check if one of the targets is an implementation (.py file) and the other is a type stub (.pyi
-                # file), which we allow. Otherwise, we have ambiguity.
+                # We check if one of the targets is an implementation (.py file) and the other is
+                # a type stub (.pyi file), which we allow. Otherwise, we have ambiguity.
                 either_targets_are_type_stubs = len(modules_to_addresses[module]) == 1 and (
                     tgt.address.filename.endswith(".pyi")
                     or modules_to_addresses[module][0].filename.endswith(".pyi")

src/python/pants/core/util_rules/source_files.py

@@ -14,11 +14,7 @@
 
 @dataclass(frozen=True)
 class SourceFiles:
-    """A merged snapshot of the `sources` fields of multiple targets.
-
-    Possibly containing a subset of the `sources` when using `SpecifiedSourceFilesRequest` (instead
-    of `AllSourceFilesRequest`).
-    """
+    """A merged snapshot of the `sources` fields of multiple targets."""
 
     snapshot: Snapshot
 

src/python/pants/core/util_rules/stripped_source_files.py

@@ -6,15 +6,26 @@
 
 from pants.core.util_rules.source_files import SourceFiles
 from pants.core.util_rules.source_files import rules as source_files_rules
+from pants.engine.collection import Collection
 from pants.engine.fs import Digest, DigestSubset, MergeDigests, PathGlobs, RemovePrefix, Snapshot
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
-from pants.source.source_root import SourceRootsRequest, SourceRootsResult
+from pants.engine.target import SourcesPaths
+from pants.source.source_root import (
+    SourceRoot,
+    SourceRootRequest,
+    SourceRootsRequest,
+    SourceRootsResult,
+)
 from pants.source.source_root import rules as source_root_rules
+from pants.util.dirutil import fast_relpath
 
 
 @dataclass(frozen=True)
 class StrippedSourceFiles:
-    """Wrapper for a snapshot of files whose source roots have been stripped."""
+    """Wrapper for a snapshot of files whose source roots have been stripped.
+
+    Use via `Get(StrippedSourceFiles, SourceFilesRequest([tgt.get(Sources)])`.
+    """
 
     snapshot: Snapshot
 
@@ -78,5 +89,24 @@ async def strip_source_roots(source_files: SourceFiles) -> StrippedSourceFiles:
     return StrippedSourceFiles(resulting_snapshot)
 
 
+class StrippedSourceFileNames(Collection[str]):
+    """The file names from a target's `sources` field, with source roots stripped.
+
+    Use via `Get(StrippedSourceFileNames, SourcePathsRequest(tgt.get(Sources))`.
+    """
+
+
+@rule
+async def strip_sources_paths(sources_paths: SourcesPaths) -> StrippedSourceFileNames:
+    if not sources_paths.files:
+        return StrippedSourceFileNames()
+    source_root = await Get(
+        SourceRoot, SourceRootRequest, SourceRootRequest.for_file(sources_paths.files[0])
+    )
+    if source_root.path == ".":
+        return StrippedSourceFileNames(sources_paths.files)
+    return StrippedSourceFileNames(fast_relpath(f, source_root.path) for f in sources_paths.files)
+
+
 def rules():
     return (*collect_rules(), *source_root_rules(), *source_files_rules())

src/python/pants/core/util_rules/stripped_source_files_test.py

@@ -1,46 +1,45 @@
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-import json
-from typing import List, Optional
+from typing import List, Sequence
 
 import pytest
 
 from pants.core.util_rules import stripped_source_files
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
-from pants.core.util_rules.stripped_source_files import StrippedSourceFiles
+from pants.core.util_rules.stripped_source_files import StrippedSourceFileNames, StrippedSourceFiles
+from pants.engine.addresses import Address
 from pants.engine.fs import EMPTY_SNAPSHOT
 from pants.engine.internals.scheduler import ExecutionError
+from pants.engine.target import Sources, SourcesPathsRequest, Target
 from pants.testutil.rule_runner import QueryRule, RuleRunner
 
 
+class TargetWithSources(Target):
+    alias = "target"
+    core_fields = (Sources,)
+
+
 @pytest.fixture
 def rule_runner() -> RuleRunner:
     return RuleRunner(
         rules=[
             *stripped_source_files.rules(),
-            QueryRule(SourceFiles, (SourceFilesRequest,)),
-            QueryRule(StrippedSourceFiles, (SourceFiles,)),
-        ]
+            QueryRule(SourceFiles, [SourceFilesRequest]),
+            QueryRule(StrippedSourceFiles, [SourceFiles]),
+            QueryRule(StrippedSourceFileNames, [SourcesPathsRequest]),
+        ],
+        target_types=[TargetWithSources],
     )
 
 
 def get_stripped_files(
     rule_runner: RuleRunner,
     request: SourceFiles,
     *,
-    args: Optional[List[str]] = None,
+    source_root_patterns: Sequence[str] = ("src/python", "src/java", "tests/python"),
 ) -> List[str]:
-    args = args or []
-    has_source_root_patterns = False
-    for arg in args:
-        if arg.startswith("--source-root-patterns"):
-            has_source_root_patterns = True
-            break
-    if not has_source_root_patterns:
-        source_root_patterns = ["src/python", "src/java", "tests/python"]
-        args.append(f"--source-root-patterns={json.dumps(source_root_patterns)}")
-    rule_runner.set_options(args)
+    rule_runner.set_options([f"--source-root-patterns={repr(source_root_patterns)}"])
     result = rule_runner.request(StrippedSourceFiles, [request])
     return list(result.snapshot.files)
 
@@ -49,11 +48,11 @@ def test_strip_snapshot(rule_runner: RuleRunner) -> None:
     def get_stripped_files_for_snapshot(
         paths: List[str],
         *,
-        args: Optional[List[str]] = None,
+        source_root_patterns: Sequence[str] = ("src/python", "src/java", "tests/python"),
     ) -> List[str]:
         input_snapshot = rule_runner.make_snapshot_of_empty_files(paths)
         request = SourceFiles(input_snapshot, ())
-        return get_stripped_files(rule_runner, request, args=args)
+        return get_stripped_files(rule_runner, request, source_root_patterns=source_root_patterns)
 
     # Normal source roots
     assert get_stripped_files_for_snapshot(["src/python/project/example.py"]) == [
@@ -90,23 +89,51 @@ def get_stripped_files_for_snapshot(
 
     # Test a source root at the repo root. We have performance optimizations for this case
     # because there is nothing to strip.
-    source_root_config = [f"--source-root-patterns={json.dumps(['/'])}"]
+    assert get_stripped_files_for_snapshot(
+        ["project/f1.py", "project/f2.py"], source_root_patterns=["/"]
+    ) == ["project/f1.py", "project/f2.py"]
 
-    assert (
-        get_stripped_files_for_snapshot(
-            ["project/f1.py", "project/f2.py"],
-            args=source_root_config,
-        )
-        == ["project/f1.py", "project/f2.py"]
+    assert get_stripped_files_for_snapshot(
+        ["dir1/f.py", "dir2/f.py"], source_root_patterns=["/"]
+    ) == ["dir1/f.py", "dir2/f.py"]
+
+    # Gracefully handle an empty snapshot
+    assert get_stripped_files(rule_runner, SourceFiles(EMPTY_SNAPSHOT, ())) == []
+
+
+def test_strip_source_file_names(rule_runner: RuleRunner) -> None:
+    def assert_stripped_source_file_names(
+        address: Address, *, source_root: str, expected: List[str]
+    ) -> None:
+        rule_runner.set_options([f"--source-root-patterns=['{source_root}']"])
+        tgt = rule_runner.get_target(address)
+        result = rule_runner.request(StrippedSourceFileNames, [SourcesPathsRequest(tgt[Sources])])
+        assert set(result) == set(expected)
+
+    rule_runner.create_file("src/java/com/project/example.java")
+    rule_runner.add_to_build_file("src/java/com/project", "target(sources=['*.java'])")
+    assert_stripped_source_file_names(
+        Address("src/java/com/project"),
+        source_root="src/java",
+        expected=["com/project/example.java"],
     )
 
-    assert (
-        get_stripped_files_for_snapshot(
-            ["dir1/f.py", "dir2/f.py"],
-            args=source_root_config,
-        )
-        == ["dir1/f.py", "dir2/f.py"]
+    rule_runner.create_file("src/python/script.py")
+    rule_runner.add_to_build_file("src/python", "target(sources=['*.py'])")
+    assert_stripped_source_file_names(
+        Address("src/python"), source_root="src/python", expected=["script.py"]
     )
 
-    # Gracefully handle an empty snapshot
-    assert get_stripped_files(rule_runner, SourceFiles(EMPTY_SNAPSHOT, ())) == []
+    # Test a source root at the repo root. We have performance optimizations for this case
+    # because there is nothing to strip.
+    rule_runner.create_file("data.json")
+    rule_runner.add_to_build_file("", "target(name='json', sources=['*.json'])\n")
+    assert_stripped_source_file_names(
+        Address("", target_name="json"), source_root="/", expected=["data.json"]
+    )
+
+    # Gracefully handle an empty sources field.
+    rule_runner.add_to_build_file("", "target(name='empty', sources=[])")
+    assert_stripped_source_file_names(
+        Address("", target_name="empty"), source_root="/", expected=[]
+    )

src/python/pants/engine/internals/graph.py

@@ -53,6 +53,8 @@
     InjectedDependencies,
     RegisteredTargetTypes,
     Sources,
+    SourcesPaths,
+    SourcesPathsRequest,
     SpecialCasedDependencies,
     Subtargets,
     Target,
@@ -89,7 +91,7 @@ async def resolve_unexpanded_targets(addresses: Addresses) -> UnexpandedTargets:
 
 
 @rule
-async def generate_subtargets(address: Address, global_options: GlobalOptions) -> Subtargets:
+async def generate_subtargets(address: Address) -> Subtargets:
     if address.is_file_target:
         raise ValueError(f"Cannot generate file Targets for a file Address: {address}")
     wrapped_build_target = await Get(WrappedTarget, Address, address)
@@ -100,15 +102,8 @@ async def generate_subtargets(address: Address, global_options: GlobalOptions) -
         # the BUILD target from depending on its splits.
         return Subtargets(build_target, ())
 
-    # Create subtargets for matched sources.
-    sources_field = build_target[Sources]
-    sources_field_path_globs = sources_field.path_globs(
-        global_options.options.files_not_found_behavior
-    )
-
     # Generate a subtarget per source.
-    paths = await Get(Paths, PathGlobs, sources_field_path_globs)
-    sources_field.validate_resolved_files(paths.files)
+    paths = await Get(SourcesPaths, SourcesPathsRequest(build_target[Sources]))
     wrapped_subtargets = await MultiGet(
         Get(
             WrappedTarget,
@@ -117,7 +112,6 @@ async def generate_subtargets(address: Address, global_options: GlobalOptions) -
         )
         for subtarget_file in paths.files
     )
-
     return Subtargets(build_target, tuple(wt.target for wt in wrapped_subtargets))
 
 
@@ -669,6 +663,17 @@ def compatible_with_sources_field(valid_type: Type[Sources]) -> bool:
     )
 
 
+@rule(desc="Resolve `sources` field file names")
+async def resolve_source_paths(
+    request: SourcesPathsRequest, global_options: GlobalOptions
+) -> SourcesPaths:
+    sources_field = request.field
+    path_globs = sources_field.path_globs(global_options.options.files_not_found_behavior)
+    paths = await Get(Paths, PathGlobs, path_globs)
+    sources_field.validate_resolved_files(paths.files)
+    return SourcesPaths(files=paths.files, dirs=paths.dirs)
+
+
 # -----------------------------------------------------------------------------------------------
 # Resolve addresses, including the Dependencies field
 # -----------------------------------------------------------------------------------------------