proto: add Java proto-library targets#607
Open
smolkaj wants to merge 3 commits intop4lang:mainfrom
Open
Conversation
smolkaj
added a commit
to smolkaj/4ward
that referenced
this pull request
Apr 11, 2026
Every Kotlin target in 4ward must now declare every dependency whose symbols its source actually uses — no more silently relying on a transitive dep to bring in a class. The same kind of dep-leak that motivated promoting stf/ to a top-level package (#510) is now caught at analysis time, across the whole tree, before the reviewer has to notice. ## What's enabled `experimental_strict_kotlin_deps = "error"` on a custom Kotlin toolchain registered ahead of the rules_kotlin default (see `bazel/BUILD.bazel` and the `register_toolchains` in `MODULE.bazel`). `experimental_report_unused_deps` is deliberately left off: it misfires on 4ward's intentional mix of rules_jvm_external Maven artifacts and Bazel-native targets that provide the same `.class` files. 4ward uses the Maven style by design so BCR consumers don't need `known_contributing_modules` boilerplate (see the comment in `MODULE.bazel`). The strict_kotlin_deps half is where the actual value lives — it catches the real problem (undeclared transitive deps) without the noise of the unused-deps half. ## What had to change to make it green - **`//simulator`** gains four new `java_proto_library` wrappers (`p4info_java_proto`, `p4data_java_proto`, `p4types_java_proto` alongside the existing `p4runtime_java_proto`). Strict-deps needs a *direct* Java wrapper for each proto whose generated classes the Kotlin code references — transitively-materialized classes don't count. A TODO points at p4lang/p4runtime#607, an upstream PR adding these wrappers directly to the p4runtime module. Once that lands and a BCR release is out, the 4ward-side wrappers can go away. - **`//p4runtime`** drops its duplicate `p4info_java_proto` and `p4runtime_java_proto` wrappers and re-routes to the canonical ones in `//simulator`. Fewer targets, no behavior change. - **`MODULE.bazel`** gains direct `bazel_dep` entries for `googleapis` and `googleapis-java` so `@googleapis//google/rpc:rpc_java_proto` is reachable by apparent name (it was previously transitive via `@p4runtime`). - **`bazel/grpc_kotlin.patch`** fixes three strict-deps blockers in `grpc_kotlin@1.5.0`: the `compiler/.../generator` target needs the JVM variant of `kotlinx-coroutines-core` (the multiplatform JAR split is invisible to strict-deps), and the `kt_jvm_grpc_library` macro needs to forward `@grpc-java//api`, the JVM coroutines JAR, and the user's `java_proto_library` into the generated library's `kt_deps`. Wired via `single_version_override` + `patches`, the same pattern already used for grpc and behavioral_model. - **~50 `kt_jvm_library` / `kt_jvm_test` deps** across the tree get the right directly-named dep added. Applied mechanically via buildozer, then re-translated by hand where strict-deps reported a raw `proto_library` label that a Kotlin target can't actually consume (every such case had to route through a `java_proto_library` wrapper instead). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
9c33dc5 to
a88f348
Compare
Add first-class `java_proto_library` wrappers for `p4info_proto`, `p4types_proto`, `p4data_proto`, and `p4runtime_proto` alongside the existing cc/py/go targets. This lets Kotlin/Java consumers depend directly on the generated Java classes without having to define their own wrappers. Under Bazel rule sets like rules_kotlin with `strict_kotlin_deps = "error"`, users need a direct `java_proto_library` dep for every proto whose generated classes their code references — transitively-materialized Java classes don't satisfy the check. Today every Kotlin consumer of p4runtime (e.g. 4ward, DVaaS, sonic-pins) has to define these wrappers locally and duplicate them package by package; shipping them upstream removes that boilerplate. Signed-off-by: Steffen Smolka <steffen.smolka@gmail.com>
a88f348 to
a73a716
Compare
smolkaj
added a commit
to smolkaj/4ward
that referenced
this pull request
Apr 11, 2026
## Summary Every Kotlin target in 4ward must now declare every dependency whose symbols its source actually uses — no more silently relying on a transitive dep to bring in a class. The same kind of dep-leak that motivated promoting `stf/` to a top-level package (#510) is now caught at analysis time, across the whole tree, before a reviewer has to notice. This is a ~275-line BUILD-file-only change (no production code touched). If you've been wondering why a target has an over-broad dep list now, it's because strict mode requires calling out every symbol source by name. ## What's enabled - `experimental_strict_kotlin_deps = "error"` on a custom Kotlin toolchain registered ahead of the `rules_kotlin` default (`bazel/BUILD.bazel`, `MODULE.bazel`). - `experimental_report_unused_deps` is **deliberately not enabled**. It misfires on 4ward's intentional mix of `rules_jvm_external` Maven artifacts and Bazel-native targets that provide the same `.class` files. 4ward uses the Maven style by design so BCR consumers don't need `known_contributing_modules` boilerplate (see the comment in `MODULE.bazel` and `bcr_test_module/`). The strict-deps half is where the actual value lives — it catches the real problem (undeclared transitive deps) without the noise. ## What had to change to make it green **`//simulator` gains four `java_proto_library` wrappers.** Strict-deps needs a *direct* Java wrapper for each proto whose generated classes are referenced — transitively-materialized classes don't count. New: `p4info_java_proto`, `p4data_java_proto`, `p4types_java_proto` (alongside the existing `p4runtime_java_proto`). A `TODO` points at **[p4lang/p4runtime#607](p4lang/p4runtime#607, an upstream PR I filed that adds these wrappers directly to the p4runtime module. Once that lands and a BCR release is out, the 4ward-side wrappers can go away. **`//p4runtime` drops its duplicate Java-proto wrappers.** The package used to define its own `p4info_java_proto` and `p4runtime_java_proto` alongside `//simulator`'s copies. Both sets now route through `//simulator`'s, which is the lower-layer package. Fewer targets, no behavior change. **`MODULE.bazel` gains direct `bazel_dep` entries for `googleapis` and `googleapis-java`** so `@googleapis//google/rpc:rpc_java_proto` is reachable by apparent name (it was previously transitive via `@p4runtime`). **`bazel/grpc_kotlin.patch` is new.** It fixes two classes of strict-deps blocker in `grpc_kotlin@1.5.0`: 1. The `compiler/.../generator` target declares the common `kotlinx-coroutines-core` artifact, but strict-deps traces the JVM classes to the `-jvm` variant (the multiplatform JAR split is invisible to strict-deps). Fix: switch to the JVM variant. 2. The `kt_jvm_grpc_library` macro generates a `kt_jvm_library` whose Kotlin source references `io.grpc.*`, `kotlinx.coroutines.*`, and the user's own proto classes, without declaring them directly. Fix: extend the macro's hardcoded `kt_deps` to include `@grpc-java//api`, the JVM coroutines artifact, and forward the caller's `java_proto_library` dep. Wired via `single_version_override` + `patches` in `MODULE.bazel`, the same pattern already used for `grpc` and `behavioral_model`. The patch carries a `TODO` to drop it once an equivalent upstream change lands in grpc-kotlin. **~50 `kt_jvm_library` / `kt_jvm_test` deps** get the right directly-named dep added. Applied mechanically via `buildozer`, then re-translated by hand where strict-deps reported a raw `proto_library` label that a Kotlin target can't actually consume — every such case had to route through a `java_proto_library` wrapper instead. ## Not in scope - Switching 4ward off the `@fourward_maven//:...` Maven-style deps to `@grpc-java//...` native Bazel targets. That would break BCR-consumer ergonomics and is explicitly out of bounds. - Enabling `report_unused_deps`. Blocked on the same trade-off. ## Test plan - [x] `bazel build //...` — 1736 targets green - [x] `bazel test //... --test_tag_filters=-heavy` — 63/63 pass - [x] `./tools/format.sh` + `./tools/lint.sh` — clean - [ ] CI green (wait before merging) ## Follow-ups - p4lang/p4runtime#607 — upstream wrappers, then drop the ones in `//simulator`. - A grpc-kotlin upstream PR for the macro change, then drop `bazel/grpc_kotlin.patch`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Set Java options on all four .proto files so generated Java/Kotlin code
is consistent across build environments and reads naturally:
- java_package matches the proto package, pinning the OSS-Bazel
default and blocking google3's "com.google.protos.*" default from
diverging silently.
- java_outer_classname uses "<File>Proto" suffix uniformly, matching
the convention used throughout protobuf's own well-known types
(TimestampProto, AnyProto, WrappersProto, ...). Necessary anyway
for p4runtime/p4data/p4info where the natural name collides with
a top-level message or service.
- java_multiple_files = true flattens generated classes so each
top-level message lives in its own .java file. Imports collapse
from "p4.v1.P4RuntimeOuterClass.ForwardingPipelineConfig" to
"p4.v1.ForwardingPipelineConfig".
No existing Java consumers to break — this PR is what introduces Java
proto-library targets in the first place.
Signed-off-by: Steffen Smolka <steffen.smolka@gmail.com>
The new java_package, java_outer_classname, and java_multiple_files options change the FileDescriptorProto bytes embedded into the generated Go and Python descriptors, so check-codegen requires a fresh codegen run. No source-level changes — only the embedded descriptor blobs differ. Signed-off-by: Steffen Smolka <steffen.smolka@gmail.com>
Member
Author
|
Would be grateful for a sanity check. |
Member
Author
|
@matthewtlam could you PTAL? |
matthewtlam
approved these changes
Apr 30, 2026
Contributor
matthewtlam
left a comment
There was a problem hiding this comment.
LGTM, thanks Steffen!
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.
Adds first-class
java_proto_libraryBazel targets for all four protos(
p4runtime,p4data,p4info,p4types) and sets the standard setof Java file options on each.
Why
Bazel: Today every Kotlin/Java consumer (4ward, DVaaS, sonic-pins,
...) re-declares these wrappers locally. Strict-deps rule sets
(e.g. rules_kotlin with
strict_kotlin_deps = "error") require adirect
java_proto_libraryper referenced proto — transitiveexposure isn't enough. Shipping them upstream removes the boilerplate.
Java options: Without explicit options, generated Java packages
fall back to defaults that vary by environment (e.g. google3 defaults
to
com.google.protos.<package>), so the same imports break acrossbuild systems. Setting them explicitly pins behavior.
Conventions
All choices align with how protobuf's own well-known types are
configured (
wrappers.proto,timestamp.proto, …):java_package = "<proto package>"— matches the proto package, noreverse-DNS rebrand. Pins OSS Bazel behavior and blocks google3's
alternate default.
java_outer_classname = "<File>Proto"— standardProtosuffix,used uniformly even when there's no name collision (forward-compat:
adding a same-named message later won't force a rename).
java_multiple_files = true— top-level messages get their own.javafile, so imports collapse fromp4.v1.P4RuntimeOuterClass.ForwardingPipelineConfigtop4.v1.ForwardingPipelineConfig.No existing Java consumers to break — this PR introduces the Java
proto-library targets in the first place.