jvm: resolve protobuf codegen ambiguity per requesting language - #23647
Closed
robertpi wants to merge 4 commits into
Closed
jvm: resolve protobuf codegen ambiguity per requesting language#23647robertpi wants to merge 4 commits into
robertpi wants to merge 4 commits into
Conversation
… per component Thread the direct dependent's own ClasspathEntryRequest type through classpath_dependency_requests as a `preferred_impl` hint. When a codegen input (e.g. protobuf_sources) is consumed by more than one JVM language's codegen, this lets classify_impl prefer whichever language is actually asking for it on that dependency edge, instead of requiring the shared target to declare which language it's for. A single, unparametrized protobuf_sources() declaration can now be depended on by java_sources and scala_sources (and non-JVM consumers) without any disambiguating field. Root-level resolution (no requester) is unchanged and still raises ClasspathSourceAmbiguity, but protobuf_sources/protobuf_source are never valid roots for JVM compile/check, so this doesn't arise in practice. Environment: Datadog workspace
…e_sources gap Adds `test_protobuf_consumed_by_java_and_scala`, a real BUILD-level integration test with a fresh JVM lockfile fixture, showing that a single unparametrized `protobuf_sources` target can be depended on by both `java_sources` and `scala_sources` without any disambiguating field or additional target declaration. The test is marked `xfail(strict=True)`: it currently fails with `AmbiguousCodegenImplementationsException` because `hydrate_sources` is a separate codegen-dispatch mechanism from the classpath-entry dispatch that `preferred_impl` already fixes. `compile_scala_source` hydrates its own component's sources with `for_sources_types=(ScalaSourceField, JavaSourceField)` to support scalac's mixed-source compilation, so `hydrate_sources` still can't choose between `GenerateJavaFromProtobufRequest` and `GenerateScalaFromProtobufRequest` for the shared `protos` target. Environment: Datadog workspace
…types order `hydrate_sources` raised `AmbiguousCodegenImplementationsException` whenever more than one registered code generator could satisfy a `for_sources_types` request, even when the caller's tuple already expresses a preference order (e.g. `compile_scala_source` passes `(ScalaSourceField, JavaSourceField)` to accept Java sources coarsened into the same compile, alongside its own Scala sources). This meant a single `protobuf_sources` target depended on by both `java_sources` and `scala_sources` could not be hydrated on the Scala side at all, independent of the classpath-entry-level ambiguity already fixed by `preferred_impl` in `jvm/compile.py`. Narrow to the first `for_sources_types` entry that has any matching generator before raising: if exactly one generator remains, use it. Genuine ambiguity (multiple generators producing the same output type) still raises, unchanged. Removes the `xfail` marker from `test_protobuf_consumed_by_java_and_scala` now that it passes, and fixes its classpath-entry-name assertions to match the real jar naming. Environment: Datadog workspace
robertpi
force-pushed
the
jvm/protobuf-classpath-per-consumer-language
branch
from
August 25, 2026 09:42
d789595 to
3740582
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #23619: a single, unparametrized
protobuf_sources()target can now be depended on byjava_sources,scala_sources, andpython_sourcesat the same time, with no disambiguating field and no need to declare multipleprotobuf_sourcestargets.Previously, depending on the same
protobuf_sourcestarget from both ajava_sourcesand ascala_sourcestarget raised ambiguity errors, because two independent parts of the JVM/codegen machinery couldn't tell which generated language a given consumer wanted:ClasspathEntryRequestFactory.classify_implinjvm/compile.py) couldn't decide betweenCompileJavaSourceRequestandCompileScalaSourceRequestfor the shared codegen target.hydrate_sourcesinengine/internals/graph.py) couldn't decide betweenGenerateJavaFromProtobufRequestandGenerateScalaFromProtobufRequestwhencompile_scala_sourcehydrates its own component's sources (it requests bothScalaSourceFieldandJavaSourceFieldto support scalac's mixed Java/Scala compilation).This PR fixes both:
classify_impl/for_targetsgain apreferred_implparam;classpath_dependency_requestspasses the concrete requester's ownClasspathEntryRequesttype as the preference, so classpath-entry ambiguity resolves per(component, requesting-language)edge instead of per-component alone.hydrate_sourcesnarrows ambiguous codegen candidates using the order of the caller'sfor_sources_typestuple: if restricting to the first type in that tuple with any matching generator leaves exactly one candidate, it's used instead of raising. Genuine ambiguity (multiple generators producing the same output type) still raises, unchanged.Neither fix requires new fields, new BUILD syntax, or any per-backend wiring — dispatch is resolved implicitly from context that already exists at each call site.
Added
test_protobuf_consumed_by_java_and_scala(jvm/compile_test.py), a real BUILD-level integration test with a fresh JVM lockfile fixture, proving a singleprotobuf_sourcestarget compiles correctly for both a Java and a Scala consumer in the same graph.Design choice — why not a
jvm_codegen_typefield?An earlier, narrower version of this fix lives on
jvm/protobuf-codegen-type-parametrize(see3435af0). That approach added ajvm_codegen_typefield toprotobuf_sources/protobuf_sourceand usedparametrizeto generate a distinct target per JVM language, letting users write e.g.protobuf_sources(jvm_codegen_type=parametrize("java", "scala")).That branch is kept around, unmerged, as a smaller and more conservative alternative, but has real drawbacks surfaced during review:
python_sourceshas no reason to know aboutjvm_codegen_type.protobuf_sourcestargets (one per JVM language viaparametrize), which leaks the JVM implementation detail into the user's BUILD files instead of hiding it.This PR trades a bit more engine-internal surface area for a fix that requires zero BUILD-visible changes and generalizes to any two conflicting codegen outputs sharing a dependency edge, not just protobuf's java/scala split.
Test plan
pants test src/python/pants/jvm/compile_test.py— new end-to-end test passes; unit-leveltest_request_classificationcoverage ofpreferred_implpassespants testonbackend/codegen/protobuf/{java,scala}/rules_integration_test.py,backend/{java,scala,kotlin}/compile/*_test.py,engine/internals/graph_test.py,core/util_rules/source_files_test.py— no regressionspants lint fmt checkon all touched files