diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 42c83a4357e..93063cb18c4 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 a bug where a single, unparametrized `protobuf_sources` target could not be depended on by both a `java_sources` and a `scala_sources` target at the same time: Pants would raise an ambiguity error when resolving the classpath entry and when hydrating the generated sources, since it couldn't tell which language's codegen a given dependent wanted. Both are now resolved per dependency edge, based on which language is actually asking, with no BUILD file changes required. + #### 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/engine/internals/graph.py b/src/python/pants/engine/internals/graph.py index 451c492b864..38176785de2 100644 --- a/src/python/pants/engine/internals/graph.py +++ b/src/python/pants/engine/internals/graph.py @@ -1429,9 +1429,25 @@ async def hydrate_sources( and issubclass(generate_request_type.output, request.for_sources_types) ] if request.enable_codegen and len(relevant_generate_request_types) > 1: - raise AmbiguousCodegenImplementationsException.create( - relevant_generate_request_types, for_sources_types=request.for_sources_types - ) + # `for_sources_types` is ordered by the caller's preference (e.g. `compile_scala_source` + # requests `(ScalaSourceField, JavaSourceField)` to also accept Java sources coarsened + # into the same compile, but would rather generate Scala than Java from a shared codegen + # input like protobuf). If narrowing to the first `for_sources_types` entry that has any + # matching generator leaves exactly one candidate, use it instead of raising. + preferred_generate_request_types: list[type[GenerateSourcesRequest]] = [] + for preferred_sources_type in request.for_sources_types: + preferred_generate_request_types = [ + generate_request_type + for generate_request_type in relevant_generate_request_types + if issubclass(generate_request_type.output, preferred_sources_type) + ] + if preferred_generate_request_types: + break + if len(preferred_generate_request_types) != 1: + raise AmbiguousCodegenImplementationsException.create( + relevant_generate_request_types, for_sources_types=request.for_sources_types + ) + relevant_generate_request_types = preferred_generate_request_types generate_request_type = next(iter(relevant_generate_request_types), None) # Now, determine if any of the `for_sources_types` may be used, either because the diff --git a/src/python/pants/jvm/compile.py b/src/python/pants/jvm/compile.py index e1cd85744fb..c47f6ffe8e5 100644 --- a/src/python/pants/jvm/compile.py +++ b/src/python/pants/jvm/compile.py @@ -94,6 +94,14 @@ class ClasspathEntryRequest(metaclass=ABCMeta): class ClasspathEntryRequestFactory: impls: tuple[type[ClasspathEntryRequest], ...] generator_sources: FrozenDict[type[ClasspathEntryRequest], frozenset[type[SourcesField]]] + # Inverse of `generator_sources`: for each `SourcesField` that some codegen implementation + # generates from, the impls whose codegen consumes it. When more than one impl shares an + # input (e.g. both the Java and Scala protobuf codegen backends consume `ProtobufSourceField`), + # `classify_impl` uses this to tell that a target is only ambiguous in the abstract -- a + # concrete dependency edge can disambiguate by preferring whichever impl is doing the asking. + impls_by_generator_source: FrozenDict[ + type[SourcesField], frozenset[type[ClasspathEntryRequest]] + ] def for_targets( self, @@ -101,11 +109,17 @@ def for_targets( resolve: CoursierResolveKey, *, root: bool = False, + preferred_impl: type[ClasspathEntryRequest] | None = None, ) -> ClasspathEntryRequest: """Constructs a subclass compatible with the members of the CoarsenedTarget. If the CoarsenedTarget is a root of a compile graph, pass `root=True` to allow usage of request types which are marked `root_only`. + + If this component is being resolved as the dependency of another JVM component (rather + than as a root), pass that component's own request type as `preferred_impl`: it lets an + otherwise-ambiguous codegen input (consumed by more than one JVM language's codegen) be + resolved unambiguously, by preferring whichever language is actually asking for it. """ compatible = [] @@ -113,7 +127,7 @@ def for_targets( consume_only = [] impls = self.impls for impl in impls: - classification = self.classify_impl(impl, component) + classification = self.classify_impl(impl, component, preferred_impl=preferred_impl) if classification == _ClasspathEntryRequestClassification.INCOMPATIBLE: continue elif classification == _ClasspathEntryRequestClassification.COMPATIBLE: @@ -155,24 +169,36 @@ def for_targets( ) def classify_impl( - self, impl: type[ClasspathEntryRequest], component: CoarsenedTarget + self, + impl: type[ClasspathEntryRequest], + component: CoarsenedTarget, + *, + preferred_impl: type[ClasspathEntryRequest] | None = None, ) -> _ClasspathEntryRequestClassification: targets = component.members generator_sources = self.generator_sources.get(impl) or frozenset() + def is_claimed_by(field: type[SourcesField]) -> bool: + sharing_impls = self.impls_by_generator_source.get(field) or frozenset() + if preferred_impl is not None and impl in sharing_impls and len(sharing_impls) > 1: + # More than one impl's codegen can consume this field: only the preferred impl + # (i.e. the concrete dependent that is asking) claims it. + return impl == preferred_impl + return True + def is_compatible(target: Target) -> bool: return ( # Is directly applicable. 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(g) and is_claimed_by(g) for g in generator_sources) or # Is applicable via a generator. ( isinstance(target, TargetFilesGenerator) and any( - field in target.generated_target_cls.core_fields + field in target.generated_target_cls.core_fields and is_claimed_by(field) for field in generator_sources ) ) @@ -206,17 +232,28 @@ 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 + # Classify code generator sources by their CPE impl, and build the reverse mapping from a + # generator's input field to every impl whose codegen consumes it. sources_by_impl_: dict[type[ClasspathEntryRequest], list[type[SourcesField]]] = defaultdict( list ) + impls_by_generator_source_: dict[type[SourcesField], set[type[ClasspathEntryRequest]]] = ( + defaultdict(set) + ) 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) + impl = impls_by_source[g.output] + sources_by_impl_[impl].append(g.input) + impls_by_generator_source_[g.input].add(impl) sources_by_impl = FrozenDict((key, frozenset(value)) for key, value in sources_by_impl_.items()) + impls_by_generator_source = FrozenDict( + (key, frozenset(value)) for key, value in impls_by_generator_source_.items() + ) - return ClasspathEntryRequestFactory(tuple(cpe_impls), sources_by_impl) + return ClasspathEntryRequestFactory( + tuple(cpe_impls), sources_by_impl, impls_by_generator_source + ) @dataclass(frozen=True) @@ -442,7 +479,9 @@ def ignore_because_file(coarsened_dep: CoarsenedTarget) -> bool: return ClasspathEntryRequests( classpath_entry_request.for_targets( - component=coarsened_dep, resolve=request.request.resolve + component=coarsened_dep, + resolve=request.request.resolve, + preferred_impl=type(request.request), ) for coarsened_dep in request.request.component.dependencies if not ignore_because_generated(coarsened_dep) and not ignore_because_file(coarsened_dep) diff --git a/src/python/pants/jvm/compile_test.py b/src/python/pants/jvm/compile_test.py index 7faa36f8797..3e4ca22ee1a 100644 --- a/src/python/pants/jvm/compile_test.py +++ b/src/python/pants/jvm/compile_test.py @@ -11,6 +11,7 @@ from __future__ import annotations +from collections import defaultdict from collections.abc import Sequence from textwrap import dedent from typing import cast @@ -24,6 +25,7 @@ ) 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.scala.rules import rules as scala_protobuf_rules from pants.backend.codegen.protobuf.target_types import ( ProtobufSourceField, ProtobufSourcesGeneratorTarget, @@ -121,6 +123,7 @@ def rule_runner() -> RuleRunner: *util_rules(), *testutil.rules(), *protobuf_rules(), + *scala_protobuf_rules(), *stripped_source_files.rules(), *protobuf_target_types_rules(), QueryRule(Classpath, (Addresses,)), @@ -221,12 +224,26 @@ def classify( targets: Sequence[Target], members: Sequence[type[ClasspathEntryRequest]], generators: FrozenDict[type[ClasspathEntryRequest], frozenset[type[SourcesField]]], + preferred_impl: type[ClasspathEntryRequest] | None = None, ) -> tuple[type[ClasspathEntryRequest], type[ClasspathEntryRequest] | None]: - factory = ClasspathEntryRequestFactory(tuple(members), generators) + impls_by_generator_source: dict[type[SourcesField], set[type[ClasspathEntryRequest]]] = ( + defaultdict(set) + ) + for impl, fields in generators.items(): + for field in fields: + impls_by_generator_source[field].add(impl) + factory = ClasspathEntryRequestFactory( + tuple(members), + generators, + FrozenDict( + (field, frozenset(impls)) for field, impls in impls_by_generator_source.items() + ), + ) req = factory.for_targets( CoarsenedTarget(targets, ()), CoursierResolveKey("example", "path", EMPTY_DIGEST), + preferred_impl=preferred_impl, ) return (type(req), type(req.prerequisite) if req.prerequisite else None) @@ -292,6 +309,127 @@ def classify( with pytest.raises(ClasspathSourceAmbiguity): classify([java], [CompileJavaSourceRequest, CompileMockSourceRequest], generators) + # A codegen input consumed by more than one impl (e.g. protobuf, generated into both Java and + # Scala) is ambiguous in the abstract... + shared_generators = FrozenDict( + { + CompileJavaSourceRequest: frozenset([cast(type[SourcesField], ProtobufSourceField)]), + CompileScalaSourceRequest: frozenset([cast(type[SourcesField], ProtobufSourceField)]), + } + ) + with pytest.raises(ClasspathSourceAmbiguity): + classify([protos], all_members, shared_generators) + with pytest.raises(ClasspathSourceAmbiguity): + classify([proto], all_members, shared_generators) + + # ...but is resolved unambiguously once a `preferred_impl` is supplied, as happens when the + # component is being resolved as the dependency of a concrete Java or Scala compile request + # (see `classpath_dependency_requests`). + assert (CompileJavaSourceRequest, None) == classify( + [protos], all_members, shared_generators, preferred_impl=CompileJavaSourceRequest + ) + assert (CompileScalaSourceRequest, None) == classify( + [protos], all_members, shared_generators, preferred_impl=CompileScalaSourceRequest + ) + assert (CompileJavaSourceRequest, None) == classify( + [proto], all_members, shared_generators, preferred_impl=CompileJavaSourceRequest + ) + assert (CompileScalaSourceRequest, None) == classify( + [proto], all_members, shared_generators, preferred_impl=CompileScalaSourceRequest + ) + + +@pytest.fixture +def protobuf_multi_lang_lockfile_def() -> JVMLockfileFixtureDefinition: + return JVMLockfileFixtureDefinition( + "protobuf-multi-lang.test.lock", + [ + "com.google.protobuf:protobuf-java:4.33.2", + "com.thesamet.scalapb:scalapb-runtime_2.13:0.11.6", + "org.scala-lang:scala-library:2.13.6", + ], + ) + + +@pytest.fixture +def protobuf_multi_lang_lockfile( + protobuf_multi_lang_lockfile_def: JVMLockfileFixtureDefinition, request +) -> JVMLockfileFixture: + return protobuf_multi_lang_lockfile_def.load(request) + + +@maybe_skip_jdk_test +def test_protobuf_consumed_by_java_and_scala( + rule_runner: RuleRunner, protobuf_multi_lang_lockfile: JVMLockfileFixture +) -> None: + """A single, unparametrized `protobuf_sources` target can be depended on by both a + `java_sources` and a `scala_sources` target without any disambiguating field, and each + consumer resolves its own compiled classpath entry for it via the `preferred_impl` mechanism + in `classpath_dependency_requests` -- rather than raising `ClasspathSourceAmbiguity`. + """ + rule_runner.set_options( + args=["--scala-version-for-resolve={'jvm-default': '2.13.6'}"], + env_inherit=PYTHON_BOOTSTRAP_ENV, + ) + rule_runner.write_files( + { + "3rdparty/jvm/default.lock": protobuf_multi_lang_lockfile.serialized_lockfile, + "3rdparty/jvm/BUILD": protobuf_multi_lang_lockfile.requirements_as_jvm_artifact_targets(), + "protos/BUILD": "protobuf_sources()", + "protos/f.proto": proto_source(), + "java/BUILD": "java_sources(dependencies=['//protos'])", + "java/C.java": dedent( + """\ + package org.pantsbuild.example.java; + + public class C { + public static String HELLO = "hello!"; + } + """ + ), + "scala/BUILD": "scala_sources(dependencies=['//protos'])", + "scala/Main.scala": dedent( + """\ + package org.pantsbuild.example.scala + + object Main { + def main(args: Array[String]): Unit = { + println("hello!") + } + } + """ + ), + } + ) + + rendered_classpath = rule_runner.request( + RenderedClasspath, + [ + Addresses( + [ + Address("java", relative_file_path="C.java"), + Address("scala", relative_file_path="Main.scala"), + ] + ) + ], + ) + + # The Java root's dependency on `protos` was resolved via `CompileJavaSourceRequest`: the + # generated sources are Java, compiled by javac. + assert rendered_classpath.content["java.C.java.javac.jar"] == { + "org/pantsbuild/example/java/C.class", + } + assert "protos.f.proto.javac.jar" in rendered_classpath.content + + # The Scala root's dependency on the *same* `protos` target was resolved via + # `CompileScalaSourceRequest`: the generated sources are Scala, compiled by scalac -- proving + # that the same, single, unparametrized `protobuf_sources` declaration serves both languages. + assert rendered_classpath.content["scala.Main.scala.scalac.jar"] == { + "org/pantsbuild/example/scala/Main$.class", + "org/pantsbuild/example/scala/Main.class", + } + assert "protos.f.proto.scalac.jar" in rendered_classpath.content + @maybe_skip_jdk_test def test_compile_mixed( diff --git a/src/python/pants/jvm/protobuf-multi-lang.test.lock b/src/python/pants/jvm/protobuf-multi-lang.test.lock new file mode 100644 index 00000000000..d2a70e655b2 --- /dev/null +++ b/src/python/pants/jvm/protobuf-multi-lang.test.lock @@ -0,0 +1,158 @@ +# This lockfile was autogenerated by Pants. To regenerate, run: +# +# pants internal-generate-test-lockfile-fixtures :: +# +# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE --- +# { +# "version": 1, +# "generated_with_requirements": [ +# "com.google.protobuf:protobuf-java:4.33.2,url=not_provided,jar=not_provided", +# "com.thesamet.scalapb:scalapb-runtime_2.13:0.11.6,url=not_provided,jar=not_provided", +# "org.scala-lang:scala-library:2.13.6,url=not_provided,jar=not_provided" +# ] +# } +# --- END PANTS LOCKFILE METADATA --- + +[[entries]] +directDependencies = [] +dependencies = [] +file_name = "com.google.protobuf_protobuf-java_4.33.2.jar" + +[entries.coord] +group = "com.google.protobuf" +artifact = "protobuf-java" +version = "4.33.2" +packaging = "jar" +[entries.file_digest] +fingerprint = "c5b582aa127fb62c5fc3077329d522dfb7930b4e9a625c08760b681b2ba5aab7" +serialized_bytes_length = 1886420 +[[entries]] +file_name = "com.thesamet.scalapb_lenses_2.13_0.11.6.jar" +[[entries.directDependencies]] +group = "org.scala-lang.modules" +artifact = "scala-collection-compat_2.13" +version = "2.5.0" +packaging = "jar" + +[[entries.directDependencies]] +group = "org.scala-lang" +artifact = "scala-library" +version = "2.13.6" +packaging = "jar" + +[[entries.dependencies]] +group = "org.scala-lang.modules" +artifact = "scala-collection-compat_2.13" +version = "2.5.0" +packaging = "jar" + +[[entries.dependencies]] +group = "org.scala-lang" +artifact = "scala-library" +version = "2.13.6" +packaging = "jar" + + +[entries.coord] +group = "com.thesamet.scalapb" +artifact = "lenses_2.13" +version = "0.11.6" +packaging = "jar" +[entries.file_digest] +fingerprint = "d1882eebf6deb4b1bb85c3c188790b28985214ac0dcfd79617fd04cc2e6df2de" +serialized_bytes_length = 34801 +[[entries]] +file_name = "com.thesamet.scalapb_scalapb-runtime_2.13_0.11.6.jar" +[[entries.directDependencies]] +group = "com.google.protobuf" +artifact = "protobuf-java" +version = "4.33.2" +packaging = "jar" + +[[entries.directDependencies]] +group = "com.thesamet.scalapb" +artifact = "lenses_2.13" +version = "0.11.6" +packaging = "jar" + +[[entries.directDependencies]] +group = "org.scala-lang.modules" +artifact = "scala-collection-compat_2.13" +version = "2.5.0" +packaging = "jar" + +[[entries.directDependencies]] +group = "org.scala-lang" +artifact = "scala-library" +version = "2.13.6" +packaging = "jar" + +[[entries.dependencies]] +group = "com.google.protobuf" +artifact = "protobuf-java" +version = "4.33.2" +packaging = "jar" + +[[entries.dependencies]] +group = "com.thesamet.scalapb" +artifact = "lenses_2.13" +version = "0.11.6" +packaging = "jar" + +[[entries.dependencies]] +group = "org.scala-lang.modules" +artifact = "scala-collection-compat_2.13" +version = "2.5.0" +packaging = "jar" + +[[entries.dependencies]] +group = "org.scala-lang" +artifact = "scala-library" +version = "2.13.6" +packaging = "jar" + + +[entries.coord] +group = "com.thesamet.scalapb" +artifact = "scalapb-runtime_2.13" +version = "0.11.6" +packaging = "jar" +[entries.file_digest] +fingerprint = "439b613f40b9ac43db4d68de5bef36befc56393d9c9cd002e2b965ce94f6f793" +serialized_bytes_length = 2426575 +[[entries]] +file_name = "org.scala-lang.modules_scala-collection-compat_2.13_2.5.0.jar" +[[entries.directDependencies]] +group = "org.scala-lang" +artifact = "scala-library" +version = "2.13.6" +packaging = "jar" + +[[entries.dependencies]] +group = "org.scala-lang" +artifact = "scala-library" +version = "2.13.6" +packaging = "jar" + + +[entries.coord] +group = "org.scala-lang.modules" +artifact = "scala-collection-compat_2.13" +version = "2.5.0" +packaging = "jar" +[entries.file_digest] +fingerprint = "93f8bf202ac28c4ca13562e31f6881a7770768e12b056b568139f37c025a3841" +serialized_bytes_length = 5610 +[[entries]] +directDependencies = [] +dependencies = [] +file_name = "org.scala-lang_scala-library_2.13.6.jar" + +[entries.coord] +group = "org.scala-lang" +artifact = "scala-library" +version = "2.13.6" +packaging = "jar" +[entries.file_digest] +fingerprint = "f19ed732e150d3537794fd3fe42ee18470a3f707efd499ecd05a99e727ff6c8a" +serialized_bytes_length = 5955737