Add await Get(Addresses, UnparsedAddressInputs) (pantsbuild#10913)

### Problem There are now ~dozen instances where we allow the user to specify an address somewhere other than the `dependencies` field, such as options like `--protoc-runtime-targets`. We had lots of duplication for parsing these raw string values. Likewise, every single implementation failed to handle subprojects correctly. ### Solution Factor up a new `UnparsedAddressInputs` type. This stores strings, rather than `AddressInput`, so that we can make sure we consider subprojects correctly without callers needing to know this exists. This is a much simpler implementation than the dependencies field, e.g. not allowing for `!` and `!!` ignores. [ci skip-rust] [ci skip-build-wheels]
awesome1128 · Oct 6, 2020 · 45c44f6 · 45c44f6
1 parent 3b6fbd7
commit 45c44f6
Show file tree

Hide file tree

Showing 14 changed files with 153 additions and 94 deletions.
diff --git a/src/python/pants/backend/codegen/protobuf/target_types.py b/src/python/pants/backend/codegen/protobuf/target_types.py
@@ -2,8 +2,8 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 from pants.backend.codegen.protobuf.protoc import Protoc
-from pants.engine.addresses import Address, AddressInput
-from pants.engine.rules import Get, MultiGet, collect_rules, rule
+from pants.engine.addresses import Addresses, UnparsedAddressInputs
+from pants.engine.rules import Get, collect_rules, rule
 from pants.engine.target import (
     COMMON_TARGET_FIELDS,
     Dependencies,
@@ -48,8 +48,8 @@ class InjectProtobufDependencies(InjectDependenciesRequest):
 async def inject_dependencies(
     _: InjectProtobufDependencies, protoc: Protoc
 ) -> InjectedDependencies:
-    addresses = await MultiGet(
-        Get(Address, AddressInput, AddressInput.parse(addr)) for addr in protoc.runtime_targets
+    addresses = await Get(
+        Addresses, UnparsedAddressInputs((v for v in protoc.runtime_targets), owning_address=None)
     )
     return InjectedDependencies(addresses)
 

diff --git a/src/python/pants/backend/python/goals/pytest_runner.py b/src/python/pants/backend/python/goals/pytest_runner.py
@@ -41,7 +41,7 @@
     TestSubsystem,
 )
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
-from pants.engine.addresses import Address, Addresses, AddressInput
+from pants.engine.addresses import Addresses, UnparsedAddressInputs
 from pants.engine.fs import AddPrefix, Digest, DigestSubset, MergeDigests, PathGlobs, Snapshot
 from pants.engine.process import FallibleProcessResult, InteractiveProcess, Process
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
@@ -160,18 +160,14 @@ async def setup_pytest_for_target(
         request.field_set.runtime_package_dependencies.value
         or request.field_set.runtime_binary_dependencies.value
     ):
-        runtime_package_addresses = await MultiGet(
-            Get(
-                Address,
-                AddressInput,
-                AddressInput.parse(v, relative_to=request.field_set.address.spec_path),
-            )
-            for v in (
-                *(request.field_set.runtime_package_dependencies.value or ()),
-                *(request.field_set.runtime_binary_dependencies.value or ()),
-            )
+        unparsed_addresses = (
+            *(request.field_set.runtime_package_dependencies.value or ()),
+            *(request.field_set.runtime_binary_dependencies.value or ()),
+        )
+        runtime_package_targets = await Get(
+            Targets,
+            UnparsedAddressInputs(unparsed_addresses, owning_address=request.field_set.address),
         )
-        runtime_package_targets = await Get(Targets, Addresses(runtime_package_addresses))
         field_sets_per_target = await Get(
             FieldSetsPerTarget,
             FieldSetsPerTargetRequest(PackageFieldSet, runtime_package_targets),

diff --git a/src/python/pants/backend/python/goals/setup_py.py b/src/python/pants/backend/python/goals/setup_py.py
@@ -40,7 +40,7 @@
 )
 from pants.core.target_types import FilesSources, ResourcesSources
 from pants.core.util_rules.distdir import DistDir
-from pants.engine.addresses import Address, Addresses, AddressInput
+from pants.engine.addresses import Address, Addresses, UnparsedAddressInputs
 from pants.engine.collection import Collection, DeduplicatedCollection
 from pants.engine.fs import (
     AddPrefix,
@@ -557,17 +557,10 @@ async def generate_chroot(request: SetupPyChrootRequest) -> SetupPyChroot:
         }
     )
     key_to_binary_spec = exported_target.provides.binaries
-    keys = list(key_to_binary_spec.keys())
-    addresses = await MultiGet(
-        Get(
-            Address,
-            AddressInput,
-            AddressInput.parse(key_to_binary_spec[key], relative_to=target.address.spec_path),
-        )
-        for key in keys
+    binaries = await Get(
+        Targets, UnparsedAddressInputs(key_to_binary_spec.values(), owning_address=target.address)
     )
-    binaries = await Get(Targets, Addresses(addresses))
-    for key, binary in zip(keys, binaries):
+    for key, binary in zip(key_to_binary_spec.keys(), binaries):
         binary_entry_point = binary.get(PythonEntryPoint).value
         if not binary_entry_point:
             raise InvalidEntryPoint(

diff --git a/src/python/pants/backend/python/lint/pylint/rules.py b/src/python/pants/backend/python/lint/pylint/rules.py
@@ -27,7 +27,7 @@
 )
 from pants.core.goals.lint import LintRequest, LintResult, LintResults
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
-from pants.engine.addresses import Address, Addresses, AddressInput
+from pants.engine.addresses import Addresses, UnparsedAddressInputs
 from pants.engine.fs import (
     EMPTY_DIGEST,
     AddPrefix,
@@ -232,9 +232,7 @@ async def pylint_lint(
     if pylint.skip:
         return LintResults([], linter_name="Pylint")
 
-    plugin_target_addresses = await MultiGet(
-        Get(Address, AddressInput, plugin_addr) for plugin_addr in pylint.source_plugins
-    )
+    plugin_target_addresses = await Get(Addresses, UnparsedAddressInputs, pylint.source_plugins)
     plugin_targets_request = Get(TransitiveTargets, Addresses(plugin_target_addresses))
     linted_targets_request = Get(
         Targets, Addresses(field_set.address for field_set in request.field_sets)

diff --git a/src/python/pants/backend/python/lint/pylint/subsystem.py b/src/python/pants/backend/python/lint/pylint/subsystem.py
@@ -1,10 +1,10 @@
 # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-from typing import List, Optional, Tuple, cast
+from typing import List, Optional, cast
 
 from pants.backend.python.subsystems.python_tool_base import PythonToolBase
-from pants.engine.addresses import AddressInput
+from pants.engine.addresses import UnparsedAddressInputs
 from pants.option.custom_types import file_option, shell_str, target_option
 
 
@@ -66,5 +66,5 @@ def config(self) -> Optional[str]:
         return cast(Optional[str], self.options.config)
 
     @property
-    def source_plugins(self) -> Tuple[AddressInput, ...]:
-        return tuple(AddressInput.parse(v) for v in self.options.source_plugins)
+    def source_plugins(self) -> UnparsedAddressInputs:
+        return UnparsedAddressInputs(self.options.source_plugins, owning_address=None)
diff --git a/src/python/pants/backend/python/target_types.py b/src/python/pants/backend/python/target_types.py
@@ -13,8 +13,8 @@
 from pants.backend.python.subsystems.pytest import PyTest
 from pants.base.deprecated import warn_or_error
 from pants.core.goals.package import OutputPathField
-from pants.engine.addresses import Address, AddressInput
-from pants.engine.rules import Get, MultiGet, collect_rules, rule
+from pants.engine.addresses import Address, Addresses, UnparsedAddressInputs
+from pants.engine.rules import Get, collect_rules, rule
 from pants.engine.target import (
     COMMON_TARGET_FIELDS,
     BoolField,
@@ -569,13 +569,11 @@ async def inject_dependencies(
         return InjectedDependencies()
     # Note that we don't validate that these are all `python_binary` targets; we don't care about
     # that here. `setup_py.py` will do that validation.
-    addresses = await MultiGet(
-        Get(
-            Address,
-            AddressInput,
-            AddressInput.parse(addr, relative_to=request.dependencies_field.address.spec_path),
-        )
-        for addr in with_binaries.values()
+    addresses = await Get(
+        Addresses,
+        UnparsedAddressInputs(
+            with_binaries.values(), owning_address=request.dependencies_field.address
+        ),
     )
     return InjectedDependencies(addresses)
 

diff --git a/src/python/pants/backend/python/typecheck/mypy/rules.py b/src/python/pants/backend/python/typecheck/mypy/rules.py
@@ -31,7 +31,7 @@
 )
 from pants.core.goals.typecheck import TypecheckRequest, TypecheckResult, TypecheckResults
 from pants.core.util_rules import pants_bin
-from pants.engine.addresses import Address, Addresses, AddressInput
+from pants.engine.addresses import Address, Addresses, UnparsedAddressInputs
 from pants.engine.fs import (
     CreateDigest,
     Digest,
@@ -186,10 +186,7 @@ def determine_python_files(files: Iterable[str]) -> Tuple[str, ...]:
 
 @rule
 async def mypy_typecheck_partition(partition: MyPyPartition, mypy: MyPy) -> TypecheckResult:
-    plugin_target_addresses = await MultiGet(
-        Get(Address, AddressInput, plugin_addr) for plugin_addr in mypy.source_plugins
-    )
-
+    plugin_target_addresses = await Get(Addresses, UnparsedAddressInputs, mypy.source_plugins)
     plugin_transitive_targets_request = Get(TransitiveTargets, Addresses(plugin_target_addresses))
     plugin_transitive_targets, launcher_script = await MultiGet(
         plugin_transitive_targets_request, Get(Digest, CreateDigest([LAUNCHER_FILE]))

diff --git a/src/python/pants/backend/python/typecheck/mypy/subsystem.py b/src/python/pants/backend/python/typecheck/mypy/subsystem.py
@@ -4,7 +4,7 @@
 from typing import Optional, Tuple, cast
 
 from pants.backend.python.subsystems.python_tool_base import PythonToolBase
-from pants.engine.addresses import AddressInput
+from pants.engine.addresses import UnparsedAddressInputs
 from pants.option.custom_types import file_option, shell_str, target_option
 
 
@@ -70,5 +70,5 @@ def config(self) -> Optional[str]:
         return cast(Optional[str], self.options.config)
 
     @property
-    def source_plugins(self) -> Tuple[AddressInput, ...]:
-        return tuple(AddressInput.parse(v) for v in self.options.source_plugins)
+    def source_plugins(self) -> UnparsedAddressInputs:
+        return UnparsedAddressInputs(self.options.source_plugins, owning_address=None)
diff --git a/src/python/pants/build_graph/address_test.py b/src/python/pants/build_graph/address_test.py
@@ -96,7 +96,7 @@ def assert_bad_path_component(spec: str) -> None:
 def test_address_input_parse_bad_target_component() -> None:
     def assert_bad_target_component(spec: str) -> None:
         with pytest.raises(InvalidTargetName):
-            print(repr(AddressInput.parse(spec)))
+            repr(AddressInput.parse(spec))
 
     # Missing target_component
     assert_bad_target_component("")

diff --git a/src/python/pants/core/target_types.py b/src/python/pants/core/target_types.py
@@ -6,7 +6,7 @@
 
 from pants.core.goals.package import BuiltPackage, OutputPathField, PackageFieldSet
 from pants.core.util_rules.archive import ArchiveFormat, CreateArchive
-from pants.engine.addresses import AddressInput
+from pants.engine.addresses import AddressInput, UnparsedAddressInputs
 from pants.engine.fs import AddPrefix, Digest, MergeDigests, RemovePrefix, Snapshot
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import (
@@ -22,6 +22,7 @@
     StringField,
     StringSequenceField,
     Target,
+    Targets,
     WrappedTarget,
 )
 from pants.engine.unions import UnionRule
@@ -153,6 +154,7 @@ class RelocateFilesViaCodegenRequest(GenerateSourcesRequest):
 async def relocate_files(request: RelocateFilesViaCodegenRequest) -> GeneratedSources:
     # Unlike normal codegen, we operate the on the sources of the `files_targets` field, not the
     # `sources` of the original `relocated_sources` target.
+    # TODO: using `await Get(Addresses, UnparsedAddressInputs)` causes a graph failure.
     original_files_targets = await MultiGet(
         Get(
             WrappedTarget,
@@ -290,44 +292,33 @@ class ArchiveFieldSet(PackageFieldSet):
 async def package_archive_target(
     field_set: ArchiveFieldSet, global_options: GlobalOptions
 ) -> BuiltPackage:
-    package_targets = await MultiGet(
+    package_targets, files_targets = await MultiGet(
         Get(
-            WrappedTarget,
-            AddressInput,
-            AddressInput.parse(v, relative_to=field_set.address.spec_path),
-        )
-        for v in field_set.packages.value or ()
+            Targets,
+            UnparsedAddressInputs(field_set.packages.value or (), owning_address=field_set.address),
+        ),
+        Get(
+            Targets,
+            UnparsedAddressInputs(field_set.files.value or (), owning_address=field_set.address),
+        ),
     )
+
     package_field_sets_per_target = await Get(
-        FieldSetsPerTarget,
-        FieldSetsPerTargetRequest(
-            PackageFieldSet,
-            (wrapped_tgt.target for wrapped_tgt in package_targets),
-        ),
+        FieldSetsPerTarget, FieldSetsPerTargetRequest(PackageFieldSet, package_targets)
     )
     packages = await MultiGet(
         Get(BuiltPackage, PackageFieldSet, field_set)
         for field_set in package_field_sets_per_target.field_sets
     )
 
-    files_targets = await MultiGet(
-        Get(
-            WrappedTarget,
-            AddressInput,
-            AddressInput.parse(v, relative_to=field_set.address.spec_path),
-        )
-        for v in field_set.files.value or ()
-    )
     files_sources = await MultiGet(
         Get(
             HydratedSources,
             HydrateSourcesRequest(
-                wrapped_tgt.target.get(Sources),
-                for_sources_types=(FilesSources,),
-                enable_codegen=True,
+                tgt.get(Sources), for_sources_types=(FilesSources,), enable_codegen=True
             ),
         )
-        for wrapped_tgt in files_targets
+        for tgt in files_targets
     )
 
     input_snapshot = await Get(

diff --git a/src/python/pants/engine/addresses.py b/src/python/pants/engine/addresses.py
@@ -2,14 +2,15 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 from dataclasses import dataclass
-from typing import Sequence
+from typing import Iterable, Optional, Sequence, Tuple
 
 from pants.base.exceptions import ResolveError
 from pants.base.specs import Spec
 from pants.build_graph.address import Address as Address
 from pants.build_graph.address import AddressInput as AddressInput  # noqa: F401: rexport.
 from pants.build_graph.address import BuildFileAddress as BuildFileAddress  # noqa: F401: rexport.
 from pants.engine.collection import Collection
+from pants.util.meta import frozen_after_init
 
 
 def assert_single_address(addresses: Sequence[Address]) -> None:
@@ -42,3 +43,32 @@ class AddressesWithOrigins(Collection[AddressWithOrigin]):
     def expect_single(self) -> AddressWithOrigin:
         assert_single_address([address_with_origin.address for address_with_origin in self])
         return self[0]
+
+
+@frozen_after_init
+@dataclass(unsafe_hash=True)
+class UnparsedAddressInputs:
+    """Raw addresses that have not been parsed yet.
+
+    You can convert these into fully normalized `Addresses` and `Targets` like this:
+
+        await Get(Addresses, UnparsedAddressInputs(["//:tgt1", "//:tgt2"], owning_address=None)
+        await Get(Targets, UnparsedAddressInputs([...], owning_address=Address("original"))
+
+    This is intended for contexts where the user specifies addresses outside of the `dependencies`
+    field, such as through an option or a special field on a target that is not normal
+    `dependencies`. You should not use this to resolve the `dependencies` field; use
+    `await Get(Addresses, DependenciesRequest)` for that.
+
+    If the addresses are coming from a target's fields, set `owning_address` so that relative
+    references like `:sibling` work properly.
+
+    Unlike the `dependencies` field, this type does not work with `!` and `!!` ignores.
+    """
+
+    values: Tuple[str, ...]
+    relative_to: Optional[str]
+
+    def __init__(self, values: Iterable[str], *, owning_address: Optional[Address]) -> None:
+        self.values = tuple(values)
+        self.relative_to = owning_address.spec_path if owning_address else None