Skip to content

Commit

Permalink
Deprecate [python-protobuf].runtime_dependencies in favor of Pants …
Browse files Browse the repository at this point in the history
…discovering the dependency (#14691)

@tdyas had the idea that rather than you having to tell Pants where to load `protobuf`, `grpcio`, and `thrift`, we can simply discover it! We do this for Scala already and it works well. 

There's minimal performance hit because we already will have created a third-party module mapping.

Two motivations:

1. Ergonomics.
2. Unblock codegen supporting multiple resolves: #14484. Otherwise we would need to add `[python-protobuf].runtime_dependencies_per_resolve`, which violates our goal for resolves to not make things more complex for the average user who only wants one resolve.

We deprecate Protobuf, but completely update Thrift since it hasn't reached stable yet.

[ci skip-rust]
  • Loading branch information
Eric-Arellano committed Mar 3, 2022
1 parent e287aac commit 54e7d54
Show file tree
Hide file tree
Showing 12 changed files with 275 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@

from pants.backend.codegen.protobuf.java.rules import GenerateJavaFromProtobufRequest
from pants.backend.codegen.protobuf.java.rules import rules as protobuf_rules
from pants.backend.codegen.protobuf.python.python_protobuf_subsystem import (
rules as protobuf_subsystem_rules,
)
from pants.backend.codegen.protobuf.target_types import (
ProtobufSourceField,
ProtobufSourcesGeneratorTarget,
Expand Down Expand Up @@ -50,7 +47,6 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*protobuf_rules(),
*protobuf_subsystem_rules(),
*stripped_source_files.rules(),
*target_types_rules(),
QueryRule(HydratedSources, [HydrateSourcesRequest]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).


from pants.backend.codegen.protobuf.target_types import ProtobufDependenciesField
from pants.backend.codegen.protobuf.target_types import (
ProtobufDependenciesField,
ProtobufGrpcToggleField,
)
from pants.backend.codegen.utils import find_python_runtime_library_or_raise_error
from pants.backend.python.dependency_inference.module_mapper import ThirdPartyPythonModuleMapping
from pants.backend.python.goals import lockfile
from pants.backend.python.goals.lockfile import GeneratePythonLockfile
from pants.backend.python.subsystems.python_tool_base import PythonToolRequirementsBase
from pants.core.goals.generate_lockfiles import GenerateToolLockfileSentinel
from pants.engine.addresses import Addresses, UnparsedAddressInputs
from pants.engine.addresses import Address, Addresses, UnparsedAddressInputs
from pants.engine.rules import Get, collect_rules, rule
from pants.engine.target import InjectDependenciesRequest, InjectedDependencies
from pants.engine.target import InjectDependenciesRequest, InjectedDependencies, WrappedTarget
from pants.engine.unions import UnionRule
from pants.option.option_types import BoolOption, TargetListOption
from pants.option.subsystem import Subsystem
Expand All @@ -28,6 +33,17 @@ class PythonProtobufSubsystem(Subsystem):
"`['3rdparty/python:protobuf', '3rdparty/python:grpcio']`. These dependencies will "
"be automatically added to every `protobuf_sources` target"
),
).deprecated(
removal_version="2.11.0.dev0",
hint=(
"Pants can now infer dependencies on the Protobuf and gRPC runtime libraries for you. "
"Not only is this more convenient, it allows Pants to support the new "
"`[python].resolves` feature.\n\n"
"To use Pants's new mechanism, simply remove this option. Run "
"`./pants dependencies path/to/f.proto` to confirm that dependencies are still added "
"correctly. You can disable this new dependency inference feature by setting "
"`[python-protobuf].infer_runtime_dependency = false`."
),
)

mypy_plugin = BoolOption(
Expand All @@ -39,6 +55,19 @@ class PythonProtobufSubsystem(Subsystem):
),
)

infer_runtime_dependency = BoolOption(
"--infer-runtime-dependency",
default=True,
help=(
"If True, will add a dependency on a `python_requirement` target exposing the "
"`protobuf` module (usually from the `protobuf` requirement). If the `protobuf_source` "
"target sets `grpc=True`, will also add a dependency on the `python_requirement` "
"target exposing the `grpcio` module.\n\n"
"Unless this option is disabled, Pants will error if no relevant target is found or "
"if more than one is found which causes ambiguity."
),
).advanced()

@property
def runtime_dependencies(self) -> UnparsedAddressInputs:
return UnparsedAddressInputs(self._runtime_dependencies, owning_address=None)
Expand Down Expand Up @@ -81,10 +110,46 @@ class InjectPythonProtobufDependencies(InjectDependenciesRequest):

@rule
async def inject_dependencies(
_: InjectPythonProtobufDependencies, python_protobuf: PythonProtobufSubsystem
request: InjectPythonProtobufDependencies,
python_protobuf: PythonProtobufSubsystem,
# TODO(#12946): Make this a lazy Get once possible.
module_mapping: ThirdPartyPythonModuleMapping,
) -> InjectedDependencies:
addresses = await Get(Addresses, UnparsedAddressInputs, python_protobuf.runtime_dependencies)
return InjectedDependencies(addresses)
if python_protobuf.runtime_dependencies.values:
addresses = await Get(
Addresses, UnparsedAddressInputs, python_protobuf.runtime_dependencies
)
return InjectedDependencies(addresses)

if not python_protobuf.infer_runtime_dependency:
return InjectedDependencies()

result = [
find_python_runtime_library_or_raise_error(
module_mapping,
request.dependencies_field.address,
"google.protobuf",
recommended_requirement_name="protobuf",
recommended_requirement_url="https://pypi.org/project/protobuf/",
disable_inference_option=f"[{python_protobuf.options_scope}].infer_runtime_dependency",
)
]

wrapped_tgt = await Get(WrappedTarget, Address, request.dependencies_field.address)
if wrapped_tgt.target.get(ProtobufGrpcToggleField).value:
result.append(
find_python_runtime_library_or_raise_error(
module_mapping,
request.dependencies_field.address,
# Note that the library is called `grpcio`, but the module is `grpc`.
"grpc",
recommended_requirement_name="grpcio",
recommended_requirement_url="https://pypi.org/project/grpcio/",
disable_inference_option=f"[{python_protobuf.options_scope}].infer_runtime_dependency",
)
)

return InjectedDependencies(result)


def rules():
Expand Down
Original file line number Diff line number Diff line change
@@ -1,45 +1,62 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.codegen.protobuf import target_types
from pants.backend.codegen.protobuf.python import python_protobuf_subsystem
from pants.backend.codegen.protobuf.python.python_protobuf_subsystem import (
InjectPythonProtobufDependencies,
)
from pants.backend.codegen.protobuf.target_types import (
ProtobufDependenciesField,
ProtobufSourcesGeneratorTarget,
from pants.backend.codegen.protobuf.target_types import ProtobufSourcesGeneratorTarget
from pants.backend.codegen.utils import (
AmbiguousPythonCodegenRuntimeLibrary,
MissingPythonCodegenRuntimeLibrary,
)
from pants.core.target_types import GenericTarget
from pants.backend.python.dependency_inference import module_mapper
from pants.backend.python.target_types import PythonRequirementTarget
from pants.core.util_rules import stripped_source_files
from pants.engine.addresses import Address
from pants.engine.target import InjectedDependencies
from pants.testutil.rule_runner import QueryRule, RuleRunner
from pants.engine.target import Dependencies, InjectedDependencies
from pants.testutil.rule_runner import QueryRule, RuleRunner, engine_error


def test_inject_dependencies() -> None:
def test_find_protobuf_python_requirement() -> None:
rule_runner = RuleRunner(
rules=[
*python_protobuf_subsystem.rules(),
*target_types.rules(),
*module_mapper.rules(),
*stripped_source_files.rules(),
QueryRule(InjectedDependencies, (InjectPythonProtobufDependencies,)),
],
target_types=[ProtobufSourcesGeneratorTarget, GenericTarget],
target_types=[ProtobufSourcesGeneratorTarget, PythonRequirementTarget],
)
rule_runner.set_options(["--python-protobuf-runtime-dependencies=protos:injected_dep"])
# Note that injected deps can be any target type for `--python-protobuf-runtime-dependencies`.

rule_runner.write_files(
{
"protos/BUILD": "protobuf_sources()\ntarget(name='injected_dep')",
"protos/f.proto": "",
}
{"codegen/dir/f.proto": "", "codegen/dir/BUILD": "protobuf_sources(grpc=True)"}
)
proto_tgt = rule_runner.get_target(Address("codegen/dir", relative_file_path="f.proto"))
request = InjectPythonProtobufDependencies(proto_tgt[Dependencies])

def assert_injected(addr: Address) -> None:
tgt = rule_runner.get_target(addr)
injected = rule_runner.request(
InjectedDependencies, [InjectPythonProtobufDependencies(tgt[ProtobufDependenciesField])]
)
assert injected == InjectedDependencies([Address("protos", target_name="injected_dep")])
# Start with no relevant requirements.
with engine_error(MissingPythonCodegenRuntimeLibrary, contains="protobuf"):
rule_runner.request(InjectedDependencies, [request])
rule_runner.write_files({"proto1/BUILD": "python_requirement(requirements=['protobuf'])"})
with engine_error(MissingPythonCodegenRuntimeLibrary, contains="grpcio"):
rule_runner.request(InjectedDependencies, [request])

# If exactly one, match it.
rule_runner.write_files({"grpc1/BUILD": "python_requirement(requirements=['grpc'])"})
assert rule_runner.request(InjectedDependencies, [request]) == InjectedDependencies(
[Address("proto1"), Address("grpc1")]
)

assert_injected(Address("protos"))
assert_injected(Address("protos", relative_file_path="f.proto"))
# If multiple, error.
rule_runner.write_files({"grpc2/BUILD": "python_requirement(requirements=['grpc'])"})
with engine_error(
AmbiguousPythonCodegenRuntimeLibrary, contains="['grpc1:grpc1', 'grpc2:grpc2']"
):
rule_runner.request(InjectedDependencies, [request])
rule_runner.write_files({"proto2/BUILD": "python_requirement(requirements=['protobuf'])"})
with engine_error(
AmbiguousPythonCodegenRuntimeLibrary, contains="['proto1:proto1', 'proto2:proto2']"
):
rule_runner.request(InjectedDependencies, [request])
4 changes: 4 additions & 0 deletions src/python/pants/backend/codegen/protobuf/python/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
ProtobufSourceTarget,
)
from pants.backend.codegen.protobuf.target_types import rules as protobuf_target_rules
from pants.backend.python.dependency_inference import module_mapper
from pants.core.util_rules import stripped_source_files


def rules():
Expand All @@ -32,6 +34,8 @@ def rules():
*protobuf_tailor.rules(),
*export_codegen_goal.rules(),
*protobuf_target_rules(),
*module_mapper.rules(),
*stripped_source_files.rules(),
]


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
ProtobufSourcesGeneratorTarget,
)
from pants.backend.codegen.protobuf.target_types import rules as target_types_rules
from pants.backend.python.dependency_inference import module_mapper
from pants.core.util_rules import stripped_source_files
from pants.engine.addresses import Address
from pants.engine.target import GeneratedSources, HydratedSources, HydrateSourcesRequest
Expand Down Expand Up @@ -58,6 +59,7 @@ def rule_runner() -> RuleRunner:
*additional_fields.rules(),
*stripped_source_files.rules(),
*target_types_rules(),
*module_mapper.rules(),
QueryRule(HydratedSources, [HydrateSourcesRequest]),
QueryRule(GeneratedSources, [GeneratePythonFromProtobufRequest]),
],
Expand All @@ -74,7 +76,11 @@ def assert_files_generated(
mypy: bool = False,
extra_args: list[str] | None = None,
) -> None:
args = [f"--source-root-patterns={repr(source_roots)}", *(extra_args or ())]
args = [
f"--source-root-patterns={repr(source_roots)}",
"--no-python-protobuf-infer-runtime-dependency",
*(extra_args or ()),
]
if mypy:
args.append("--python-protobuf-mypy-plugin")
rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
ThriftSourcesGeneratorTarget,
ThriftSourceTarget,
)
from pants.backend.python.dependency_inference import module_mapper
from pants.core.util_rules import stripped_source_files


def target_types():
Expand All @@ -21,4 +23,6 @@ def rules():
*apache_thrift_rules(),
*apache_thrift_python_rules(),
*python_thrift_module_mapper.rules(),
*module_mapper.rules(),
*stripped_source_files.rules(),
]
26 changes: 21 additions & 5 deletions src/python/pants/backend/codegen/thrift/apache/python/rules.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

from pants.backend.codegen.thrift.apache.python import subsystem
from pants.backend.codegen.thrift.apache.python.subsystem import ThriftPythonSubsystem
from pants.backend.codegen.thrift.apache.rules import (
GeneratedThriftSources,
GenerateThriftSourcesRequest,
)
from pants.backend.codegen.thrift.target_types import ThriftDependenciesField, ThriftSourceField
from pants.backend.codegen.utils import find_python_runtime_library_or_raise_error
from pants.backend.python.dependency_inference.module_mapper import ThirdPartyPythonModuleMapping
from pants.backend.python.target_types import PythonSourceField
from pants.engine.addresses import Addresses, UnparsedAddressInputs
from pants.engine.fs import AddPrefix, Digest, Snapshot
from pants.engine.internals.selectors import Get
from pants.engine.rules import collect_rules, rule
Expand Down Expand Up @@ -63,11 +66,24 @@ class InjectApacheThriftPythonDependencies(InjectDependenciesRequest):


@rule
async def inject_apache_thrift_java_dependencies(
_: InjectApacheThriftPythonDependencies, thrift_python: ThriftPythonSubsystem
async def find_apache_thrift_python_requirement(
request: InjectApacheThriftPythonDependencies,
thrift_python: ThriftPythonSubsystem,
# TODO(#12946): Make this a lazy Get once possible.
module_mapping: ThirdPartyPythonModuleMapping,
) -> InjectedDependencies:
addresses = await Get(Addresses, UnparsedAddressInputs, thrift_python.runtime_dependencies)
return InjectedDependencies(addresses)
if not thrift_python.infer_runtime_dependency:
return InjectedDependencies()

addr = find_python_runtime_library_or_raise_error(
module_mapping,
request.dependencies_field.address,
"thrift",
recommended_requirement_name="thrift",
recommended_requirement_url="https://pypi.org/project/thrift/",
disable_inference_option=f"[{thrift_python.options_scope}].infer_runtime_dependency",
)
return InjectedDependencies([addr])


def rules():
Expand Down

0 comments on commit 54e7d54

Please sign in to comment.