diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index f1cc0ca82f1..cfd3fbc934f 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -53,6 +53,8 @@ Fixed nailgun servers being left running after a `--no-pantsd` run exits. The Scala protobuf codegen backend (`pants.backend.codegen.protobuf.scala`) now honors `protobuf_sources`/`protobuf_source`'s `grpc=True` field: ScalaPB now generates `*Grpc.scala` service stubs, and the `scalapb-runtime-grpc` runtime is automatically inferred as a dependency, matching the existing behavior of the Java protobuf codegen backend. This is a behavior change for anyone using the Scala protobuf backend with `grpc=True` set on a `protobuf_sources` target: a `jvm_artifact` providing `com.thesamet.scalapb:scalapb-runtime-grpc_` must now be resolvable, or dependency inference will fail. Callers remain responsible for declaring their own gRPC transport dependency (e.g. `grpc-netty-shaded`, `grpc-netty`, or `grpc-okhttp`), since that is a deployment choice not dictated by the generated code. +Fixed JVM compilation raising `ClasspathSourceAmbiguity` ("More than one JVM classpath provider ... was compatible with the inputs") when both the Java and Scala protobuf codegen backends (`pants.backend.codegen.protobuf.java` and `pants.backend.codegen.protobuf.scala`) are active and a `java_sources` or `scala_sources` target directly depends on a `protobuf_sources`/`protobuf_source` target: Pants could not tell whether `javac` or `scalac` should compile the generated code. New `skip_java`/`skip_scala` fields on `protobuf_source`/`protobuf_sources` let you disambiguate, e.g. `protobuf_sources(skip_scala=True)` for a target whose generated code should only be compiled as Java. + #### Python Support for creating multiplatform/foreign platform pexes when using the `uv` resolver. This includes support for FAAS (AWS Lambda/Google Cloud Functions). diff --git a/src/python/pants/backend/codegen/protobuf/java/rules.py b/src/python/pants/backend/codegen/protobuf/java/rules.py index ce0f2a2bcdc..332ae01eaf2 100644 --- a/src/python/pants/backend/codegen/protobuf/java/rules.py +++ b/src/python/pants/backend/codegen/protobuf/java/rules.py @@ -22,6 +22,7 @@ from pants.core.util_rules.source_files import SourceFilesRequest from pants.core.util_rules.stripped_source_files import strip_source_roots from pants.engine.fs import ( + EMPTY_SNAPSHOT, AddPrefix, CreateDigest, Digest, @@ -41,7 +42,12 @@ from pants.engine.platform import Platform from pants.engine.process import Process, fallible_to_exec_result_or_raise from pants.engine.rules import collect_rules, concurrently, implicitly, rule -from pants.engine.target import GeneratedSources, GenerateSourcesRequest, TransitiveTargetsRequest +from pants.engine.target import ( + BoolField, + GeneratedSources, + GenerateSourcesRequest, + TransitiveTargetsRequest, +) from pants.engine.unions import UnionRule from pants.jvm.resolve.coursier_fetch import ToolClasspathRequest, materialize_classpath_for_tool from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool @@ -50,9 +56,23 @@ from pants.util.logging import LogLevel +class SkipJavaProtobufField(BoolField): + alias = "skip_java" + default = False + help = ( + "If true, skips generation of Java sources from this target.\n\n" + "This is required to disambiguate JVM compilation when both the Java and Scala " + "protobuf codegen backends (`pants.backend.codegen.protobuf.java` and " + "`pants.backend.codegen.protobuf.scala`) are active, and a `java_sources` or " + "`scala_sources` target directly depends on this target: without disambiguation, " + "Pants cannot tell whether javac or scalac should compile the generated code." + ) + + class GenerateJavaFromProtobufRequest(GenerateSourcesRequest): input = ProtobufSourceField output = JavaSourceField + skip_field = SkipJavaProtobufField @dataclass(frozen=True) @@ -113,6 +133,9 @@ async def generate_java_from_protobuf( grpc_plugin: ProtobufJavaGrpcPlugin, # TODO: Don't access grpc plugin unless gRPC codegen is enabled. platform: Platform, ) -> GeneratedSources: + if request.protocol_target.get(SkipJavaProtobufField).value: + return GeneratedSources(EMPTY_SNAPSHOT) + download_protoc_request = download_external_tool(protoc.get_request(platform)) output_dir = "_generated_files" @@ -211,6 +234,8 @@ def rules(): ProtobufSourcesGeneratorTarget.register_plugin_field(PrefixedJvmJdkField), ProtobufSourceTarget.register_plugin_field(PrefixedJvmResolveField), ProtobufSourcesGeneratorTarget.register_plugin_field(PrefixedJvmResolveField), + ProtobufSourceTarget.register_plugin_field(SkipJavaProtobufField), + ProtobufSourcesGeneratorTarget.register_plugin_field(SkipJavaProtobufField), # Bring in the Java backend (since this backend compiles Java code) to avoid rule graph errors. # TODO: Figure out whether a subset of rules can be brought in to still avoid rule graph errors. *java_backend_rules(), diff --git a/src/python/pants/backend/codegen/protobuf/java/rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/java/rules_integration_test.py index 537519ac67d..03f57d3d9d2 100644 --- a/src/python/pants/backend/codegen/protobuf/java/rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/java/rules_integration_test.py @@ -250,6 +250,32 @@ def assert_gen(addr: Address, expected: str) -> None: _ = rule_runner.request(RenderedClasspath, [request]) +@maybe_skip_jdk_test +def test_skip_java_field(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + { + "src/protobuf/dir1/f.proto": dedent( + """\ + syntax = "proto3"; + + package dir1; + + message Person { + string name = 1; + } + """ + ), + "src/protobuf/dir1/BUILD": "protobuf_sources(skip_java=True)", + } + ) + generated_sources = _run_codegen( + rule_runner, + Address("src/protobuf/dir1", relative_file_path="f.proto"), + source_roots=["/src/protobuf"], + ) + assert generated_sources == frozenset() + + @pytest.fixture def protobuf_java_grpc_lockfile_def() -> JVMLockfileFixtureDefinition: return JVMLockfileFixtureDefinition( diff --git a/src/python/pants/backend/codegen/protobuf/scala/rules.py b/src/python/pants/backend/codegen/protobuf/scala/rules.py index 11630dfbf55..f7e515b62a1 100644 --- a/src/python/pants/backend/codegen/protobuf/scala/rules.py +++ b/src/python/pants/backend/codegen/protobuf/scala/rules.py @@ -29,6 +29,7 @@ from pants.core.util_rules.stripped_source_files import strip_source_roots from pants.engine.env_vars import EnvironmentVarsRequest from pants.engine.fs import ( + EMPTY_SNAPSHOT, AddPrefix, CreateDigest, Digest, @@ -50,7 +51,12 @@ from pants.engine.platform import Platform from pants.engine.process import fallible_to_exec_result_or_raise from pants.engine.rules import collect_rules, implicitly, rule -from pants.engine.target import GeneratedSources, GenerateSourcesRequest, TransitiveTargetsRequest +from pants.engine.target import ( + BoolField, + GeneratedSources, + GenerateSourcesRequest, + TransitiveTargetsRequest, +) from pants.engine.unions import UnionRule from pants.jvm.compile import ClasspathEntry from pants.jvm.dependency_inference import artifact_mapper @@ -73,9 +79,23 @@ from pants.util.resources import read_resource +class SkipScalaProtobufField(BoolField): + alias = "skip_scala" + default = False + help = ( + "If true, skips generation of Scala sources from this target.\n\n" + "This is required to disambiguate JVM compilation when both the Java and Scala " + "protobuf codegen backends (`pants.backend.codegen.protobuf.java` and " + "`pants.backend.codegen.protobuf.scala`) are active, and a `java_sources` or " + "`scala_sources` target directly depends on this target: without disambiguation, " + "Pants cannot tell whether javac or scalac should compile the generated code." + ) + + class GenerateScalaFromProtobufRequest(GenerateSourcesRequest): input = ProtobufSourceField output = ScalaSourceField + skip_field = SkipScalaProtobufField class ScalaPBShimCompiledClassfiles(ClasspathEntry): @@ -148,6 +168,9 @@ async def generate_scala_from_protobuf( jdk: InternalJdk, platform: Platform, ) -> GeneratedSources: + if request.protocol_target.get(SkipScalaProtobufField).value: + return GeneratedSources(EMPTY_SNAPSHOT) + output_dir = "_generated_files" toolcp_relpath = "__toolcp" shimcp_relpath = "__shimcp" @@ -348,6 +371,8 @@ def rules(): ProtobufSourcesGeneratorTarget.register_plugin_field(PrefixedJvmJdkField), ProtobufSourceTarget.register_plugin_field(PrefixedJvmResolveField), ProtobufSourcesGeneratorTarget.register_plugin_field(PrefixedJvmResolveField), + ProtobufSourceTarget.register_plugin_field(SkipScalaProtobufField), + ProtobufSourcesGeneratorTarget.register_plugin_field(SkipScalaProtobufField), # Rules to avoid rule graph errors. *artifact_mapper.rules(), *distdir.rules(), diff --git a/src/python/pants/backend/codegen/protobuf/scala/rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/scala/rules_integration_test.py index e009ce53e56..94b697e4b42 100644 --- a/src/python/pants/backend/codegen/protobuf/scala/rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/scala/rules_integration_test.py @@ -297,6 +297,32 @@ def test_top_level_proto_root( ) +@maybe_skip_jdk_test +def test_skip_scala_field(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + { + "protos/f.proto": dedent( + """\ + syntax = "proto3"; + + package protos; + """ + ), + "protos/BUILD": "protobuf_sources(skip_scala=True)", + } + ) + tgt = rule_runner.get_target(Address("protos", relative_file_path="f.proto")) + rule_runner.set_options([], env_inherit=PYTHON_BOOTSTRAP_ENV) + protocol_sources = rule_runner.request( + HydratedSources, [HydrateSourcesRequest(tgt[ProtobufSourceField])] + ) + generated_sources = rule_runner.request( + GeneratedSources, + [GenerateScalaFromProtobufRequest(protocol_sources.snapshot, tgt)], + ) + assert generated_sources.snapshot.files == () + + def test_generates_fs2_grpc_via_jvm_plugin( rule_runner: RuleRunner, scalapb_lockfile: JVMLockfileFixture ) -> None: diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 580010ecbd4..355c55a934f 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -2013,6 +2013,14 @@ def rules(): exportable: ClassVar[bool] = True + # Optional field that, when present and set to `True` on the `protocol_target`, indicates + # that this particular codegen implementation should be treated as inapplicable to that + # target. This is consulted by JVM classpath resolution (`pants.jvm.compile`) to disambiguate + # between multiple codegen implementations that share the same `input` SourcesField but + # produce different JVM-compilable `output` SourcesFields (e.g. Java and Scala protobuf + # codegen both consuming `ProtobufSourceField`). It has no effect outside of that use. + skip_field: ClassVar[type[BoolField] | None] = None + @dataclass(frozen=True) class GeneratedSources: diff --git a/src/python/pants/jvm/compile.py b/src/python/pants/jvm/compile.py index e1cd85744fb..b0cc583ad1c 100644 --- a/src/python/pants/jvm/compile.py +++ b/src/python/pants/jvm/compile.py @@ -28,7 +28,6 @@ Field, FieldSet, GenerateSourcesRequest, - SourcesField, Target, TargetFilesGenerator, ) @@ -93,7 +92,15 @@ class ClasspathEntryRequest(metaclass=ABCMeta): @dataclass(frozen=True) class ClasspathEntryRequestFactory: impls: tuple[type[ClasspathEntryRequest], ...] - generator_sources: FrozenDict[type[ClasspathEntryRequest], frozenset[type[SourcesField]]] + # For each `ClasspathEntryRequest` impl, the `GenerateSourcesRequest` subclasses whose + # `output` maps to that impl (see `calculate_jvm_request_types`). Multiple ` + # GenerateSourcesRequest`s may map to the same impl while sharing the same `input` + # SourcesField: for example, both the Java and Scala protobuf codegen backends consume + # `ProtobufSourceField`. In that case, `GenerateSourcesRequest.skip_field` allows a target to + # opt out of one of the two, so that `classify_impl` does not consider both impls compatible + # with the same `protobuf_sources` target (which would otherwise raise + # `ClasspathSourceAmbiguity`). + generators: FrozenDict[type[ClasspathEntryRequest], tuple[type[GenerateSourcesRequest], ...]] def for_targets( self, @@ -158,7 +165,12 @@ def classify_impl( self, impl: type[ClasspathEntryRequest], component: CoarsenedTarget ) -> _ClasspathEntryRequestClassification: targets = component.members - generator_sources = self.generator_sources.get(impl) or frozenset() + generators = self.generators.get(impl) or () + + def is_skipped(target: Target, generator: type[GenerateSourcesRequest]) -> bool: + # `target.get()` returns the field's default (`False`, for a `BoolField`) if the + # field isn't registered on this target type, so it's safe to call unconditionally. + return generator.skip_field is not None and target.get(generator.skip_field).value def is_compatible(target: Target) -> bool: return ( @@ -166,14 +178,18 @@ def is_compatible(target: Target) -> bool: any(fs.is_applicable(target) for fs in impl.field_sets) or # Is applicable via generated sources. - any(target.has_field(g) for g in generator_sources) + any( + target.has_field(generator.input) and not is_skipped(target, generator) + for generator in generators + ) or # Is applicable via a generator. ( isinstance(target, TargetFilesGenerator) and any( - field in target.generated_target_cls.core_fields - for field in generator_sources + generator.input in target.generated_target_cls.core_fields + and not is_skipped(target, generator) + for generator in generators ) ) ) @@ -206,17 +222,19 @@ async def calculate_jvm_request_types( # (note that subsequently, we only check for `SourceFields`, so no need to filter) impls_by_source[field] = impl - # Classify code generator sources by their CPE impl - sources_by_impl_: dict[type[ClasspathEntryRequest], list[type[SourcesField]]] = defaultdict( - list + # Classify code generators by their CPE impl. + generators_by_impl_: dict[type[ClasspathEntryRequest], list[type[GenerateSourcesRequest]]] = ( + defaultdict(list) ) for g in union_membership.get(GenerateSourcesRequest): if g.output in impls_by_source: - sources_by_impl_[impls_by_source[g.output]].append(g.input) - sources_by_impl = FrozenDict((key, frozenset(value)) for key, value in sources_by_impl_.items()) + generators_by_impl_[impls_by_source[g.output]].append(g) + generators_by_impl = FrozenDict( + (key, tuple(value)) for key, value in generators_by_impl_.items() + ) - return ClasspathEntryRequestFactory(tuple(cpe_impls), sources_by_impl) + return ClasspathEntryRequestFactory(tuple(cpe_impls), generators_by_impl) @dataclass(frozen=True) diff --git a/src/python/pants/jvm/compile_test.py b/src/python/pants/jvm/compile_test.py index 7faa36f8797..fe6e1a0c212 100644 --- a/src/python/pants/jvm/compile_test.py +++ b/src/python/pants/jvm/compile_test.py @@ -22,7 +22,10 @@ JVMLockfileFixture, JVMLockfileFixtureDefinition, ) -from pants.backend.codegen.protobuf.java.rules import GenerateJavaFromProtobufRequest +from pants.backend.codegen.protobuf.java.rules import ( + GenerateJavaFromProtobufRequest, + SkipJavaProtobufField, +) from pants.backend.codegen.protobuf.java.rules import rules as protobuf_rules from pants.backend.codegen.protobuf.target_types import ( ProtobufSourceField, @@ -36,13 +39,14 @@ from pants.backend.java.target_types import ( JavaFieldSet, JavaGeneratorFieldSet, + JavaSourceField, JavaSourcesGeneratorTarget, ) from pants.backend.java.target_types import rules as java_target_types_rules from pants.backend.scala.compile.scalac import CompileScalaSourceRequest from pants.backend.scala.compile.scalac import rules as scalac_rules from pants.backend.scala.dependency_inference.rules import rules as scala_dep_inf_rules -from pants.backend.scala.target_types import ScalaSourcesGeneratorTarget +from pants.backend.scala.target_types import ScalaSourceField, ScalaSourcesGeneratorTarget from pants.backend.scala.target_types import rules as scala_target_types_rules from pants.build_graph.address import Address from pants.core.target_types import FilesGeneratorTarget, RelocatedFiles @@ -53,9 +57,9 @@ from pants.engine.target import ( CoarsenedTarget, GeneratedSources, + GenerateSourcesRequest, HydratedSources, HydrateSourcesRequest, - SourcesField, Target, UnexpandedTargets, ) @@ -213,6 +217,17 @@ class CompileMockSourceRequest(ClasspathEntryRequest): field_sets = (JavaFieldSet, JavaGeneratorFieldSet) +class MockGenerateJavaFromProtobufRequest(GenerateSourcesRequest): + input = ProtobufSourceField + output = JavaSourceField + skip_field = SkipJavaProtobufField + + +class MockGenerateScalaFromProtobufRequest(GenerateSourcesRequest): + input = ProtobufSourceField + output = ScalaSourceField + + @maybe_skip_jdk_test def test_request_classification( rule_runner: RuleRunner, scala_stdlib_jvm_lockfile: JVMLockfileFixture @@ -220,7 +235,9 @@ def test_request_classification( def classify( targets: Sequence[Target], members: Sequence[type[ClasspathEntryRequest]], - generators: FrozenDict[type[ClasspathEntryRequest], frozenset[type[SourcesField]]], + generators: FrozenDict[ + type[ClasspathEntryRequest], tuple[type[GenerateSourcesRequest], ...] + ], ) -> tuple[type[ClasspathEntryRequest], type[ClasspathEntryRequest] | None]: factory = ClasspathEntryRequestFactory(tuple(members), generators) @@ -239,6 +256,7 @@ def classify( jvm_artifact(name='jvm_artifact', group='ex', artifact='ex', version='0.0.0') protobuf_source(name='proto', source="f.proto") protobuf_sources(name='protos') + protobuf_source(name='proto_scala_only', source="f.proto", skip_java=True) """ ), "f.proto": proto_source(), @@ -246,7 +264,7 @@ def classify( "3rdparty/jvm/default.lock": scala_stdlib_jvm_lockfile.serialized_lockfile, } ) - scala, java, jvm_artifact, proto, protos = rule_runner.request( + scala, java, jvm_artifact, proto, protos, proto_scala_only = rule_runner.request( UnexpandedTargets, [ Addresses( @@ -256,15 +274,18 @@ def classify( Address("", target_name="jvm_artifact"), Address("", target_name="proto"), Address("", target_name="protos"), + Address("", target_name="proto_scala_only"), ] ) ], ) all_members = [CompileJavaSourceRequest, CompileScalaSourceRequest, CoursierFetchRequest] - generators = FrozenDict( + generators: FrozenDict[ + type[ClasspathEntryRequest], tuple[type[GenerateSourcesRequest], ...] + ] = FrozenDict( { - CompileJavaSourceRequest: frozenset([cast(type[SourcesField], ProtobufSourceField)]), - CompileScalaSourceRequest: frozenset(), + CompileJavaSourceRequest: (MockGenerateJavaFromProtobufRequest,), + CompileScalaSourceRequest: (), } ) @@ -292,6 +313,26 @@ def classify( with pytest.raises(ClasspathSourceAmbiguity): classify([java], [CompileJavaSourceRequest, CompileMockSourceRequest], generators) + # Two codegen backends can both claim the same `input` SourcesField (as with the real Java + # and Scala protobuf codegen backends both consuming `ProtobufSourceField`): without a + # `skip_field` on one side, this is ambiguous... + ambiguous_generators: FrozenDict[ + type[ClasspathEntryRequest], tuple[type[GenerateSourcesRequest], ...] + ] = FrozenDict( + { + CompileJavaSourceRequest: (MockGenerateJavaFromProtobufRequest,), + CompileScalaSourceRequest: (MockGenerateScalaFromProtobufRequest,), + } + ) + with pytest.raises(ClasspathSourceAmbiguity): + classify([proto], all_members, ambiguous_generators) + + # ...but is resolved once the target opts out of one of the two via the relevant + # `skip_field` (here, `skip_java=True`). + assert (CompileScalaSourceRequest, None) == classify( + [proto_scala_only], all_members, ambiguous_generators + ) + @maybe_skip_jdk_test def test_compile_mixed(