Skip to content

Commit 1935ee5

Browse files
committed
fix(rust): service-only libs, plugin order, output_mappings for path-mismatched pkgs
Three independent failures surfaced by running protoc-gen-prost, protoc-gen-prost-serde, and protoc-gen-tonic together across a real monorepo: 1. Service-only proto_libraries (no messages/enums) caused tonic to error with "Tried to insert into file that doesn't exist": tonic appends client/server code into prost's {package}.rs at the @@protoc_insertion_point(module) marker, but ProtocGenProstPlugin skipped libraries with no messages or enums. shouldApply and outputs now also fire on HasServices() so prost emits a stub .rs for tonic to insert into. Two new tests in protoc-gen-prost_test. 2. Plugin order was non-deterministic: BUILD.bazel attr ordering (preserved by bazel-gazelle's standard list-merge) put tonic before prost, so tonic's insertion ran before prost created its target file — yielding the "Tried to insert into file that doesn't exist" / "Tried to write the same file twice" pair. proto_compile.bzl now sorts plugins by name inside the rule impl, so protoc CLI ordering is independent of attr ordering. 3. proto packages whose path doesn't match the bazel package dir (e.g. "trumid.common.auth" living at //trumid/common/auth/proto, or "grpc.health.v1" living at //thirdparty/.../grpc/health/v1) failed with `mv: ... No such file or directory` because prost writes to its proto-package-derived path while proto_compile.bzl expected the output at the bazel pkg. The output_mappings computation in proto_rust_library.ProvideRule only handled the Rust-keyword-escape case (e.g. google.type → google/r#type). Generalized: emit mappings whenever RustProtocOutputDir(pkg) differs from pc.Rel, regardless of whether keywords are involved. Extracted RustProtocOutputDir helper; existing RustKeywordEscapeMappings retained for back-compat with the exposed Starlark function.
1 parent 9d81be5 commit 1935ee5

5 files changed

Lines changed: 114 additions & 25 deletions

File tree

pkg/plugin/neoeinstein/prost/protoc-gen-prost.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,15 @@ func needsCompileWellKnownTypes(opts []string, ownPackages map[string]bool) bool
9292
return false
9393
}
9494

95-
// shouldApply returns true if the library has files with messages or enums.
95+
// shouldApply returns true if the library has any file with messages, enums,
96+
// or services. Services are included because protoc-gen-tonic's generated
97+
// code is inserted into prost's {package}.rs at the @@protoc_insertion_point
98+
// marker (see protoc-gen-prost's append_to_file mechanism). If prost isn't
99+
// invoked, the file doesn't exist and tonic's insert fails with
100+
// "Tried to insert into file that doesn't exist".
96101
func (p *ProtocGenProstPlugin) shouldApply(lib protoc.ProtoLibrary) bool {
97102
for _, f := range lib.Files() {
98-
if f.HasMessages() || f.HasEnums() {
103+
if f.HasMessages() || f.HasEnums() || f.HasServices() {
99104
return true
100105
}
101106
}
@@ -105,12 +110,17 @@ func (p *ProtocGenProstPlugin) shouldApply(lib protoc.ProtoLibrary) bool {
105110
// outputs computes the output files for the plugin. Prost generates one .rs
106111
// file per proto package, named {proto_package}.rs. The path includes the
107112
// file's directory so that mergeSources can handle the rel stripping.
113+
//
114+
// Packages contributed by service-only files (no messages/enums) are
115+
// included — prost still emits a stub .rs containing the
116+
// @@protoc_insertion_point(module) marker, which tonic relies on to inject
117+
// its client/server code via append_to_file.
108118
func (p *ProtocGenProstPlugin) outputs(lib protoc.ProtoLibrary) []string {
109119
seen := make(map[string]bool)
110120
outputs := make([]string, 0)
111121

112122
for _, f := range lib.Files() {
113-
if !(f.HasMessages() || f.HasEnums()) {
123+
if !(f.HasMessages() || f.HasEnums() || f.HasServices()) {
114124
continue
115125
}
116126
pkg := f.Package()

pkg/plugin/neoeinstein/prost/protoc-gen-prost_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,39 @@ func TestProtocGenProstPlugin(t *testing.T) {
4242
),
4343
SkipIntegration: true,
4444
},
45+
"service only - emits stub .rs for tonic to insert into": {
46+
// Without this case the protoc plugin chain breaks: tonic
47+
// appends its client/server code at the @@protoc_insertion_point
48+
// in {package}.rs, and protoc errors with
49+
// "Tried to insert into file that doesn't exist" if prost
50+
// hasn't created that file first.
51+
Input: "package example.v1;\nservice Greeter { rpc Hello(Req) returns (Resp); }\nmessage Req {}\nmessage Resp {}",
52+
Directives: plugintest.WithDirectives(
53+
"proto_plugin", "protoc-gen-prost implementation neoeinstein:prost:protoc-gen-prost",
54+
),
55+
PluginName: "protoc-gen-prost",
56+
Configuration: plugintest.WithConfiguration(
57+
plugintest.WithLabel(t, "@build_stack_rules_proto//plugin/neoeinstein/prost:protoc-gen-prost"),
58+
plugintest.WithOutputs("example.v1.rs"),
59+
),
60+
SkipIntegration: true,
61+
},
62+
"service only - no messages": {
63+
// Same as above but the proto has no messages/enums of its own
64+
// (could be e.g. a separate proto_library that imports its
65+
// request/response types from a sibling). Prost still has to
66+
// emit the .rs stub so tonic's append succeeds.
67+
Input: "package example.v1;\nimport \"google/protobuf/empty.proto\";\nservice Pinger { rpc Ping(google.protobuf.Empty) returns (google.protobuf.Empty); }",
68+
Directives: plugintest.WithDirectives(
69+
"proto_plugin", "protoc-gen-prost implementation neoeinstein:prost:protoc-gen-prost",
70+
),
71+
PluginName: "protoc-gen-prost",
72+
Configuration: plugintest.WithConfiguration(
73+
plugintest.WithLabel(t, "@build_stack_rules_proto//plugin/neoeinstein/prost:protoc-gen-prost"),
74+
plugintest.WithOutputs("example.v1.rs"),
75+
),
76+
SkipIntegration: true,
77+
},
4578
"no package - skipped": {
4679
Input: "message Foo {}",
4780
Directives: plugintest.WithDirectives(

pkg/protoc/rust_keywords.go

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,50 @@ var rustKeywords = map[string]bool{
6262
"yield": true,
6363
}
6464

65+
// RustProtocOutputDir returns the directory that protoc-gen-prost (and its
66+
// siblings) will write outputs into for a given proto package, with Rust
67+
// keyword segments escaped via the r# prefix.
68+
//
69+
// Examples:
70+
// - "google.type" → "google/r#type"
71+
// - "trumid.common.auth" → "trumid/common/auth"
72+
// - "" → ""
73+
//
74+
// Note this is independent of the bazel package location — protoc-gen-prost
75+
// always derives the output path from the proto package name unless given
76+
// flat_output_dir=true. Use the result to compare against pc.Rel and decide
77+
// whether output_mappings are needed.
78+
func RustProtocOutputDir(pkg string) string {
79+
if pkg == "" {
80+
return ""
81+
}
82+
segments := strings.Split(pkg, ".")
83+
for i, seg := range segments {
84+
if rustKeywords[seg] {
85+
segments[i] = "r#" + seg
86+
}
87+
}
88+
return strings.Join(segments, "/")
89+
}
90+
6591
// RustKeywordEscapeMappings computes output mappings needed when
6692
// protoc-gen-prost escapes Rust keywords with the r# prefix in directory paths.
6793
//
6894
// For example, proto package "google.type" causes prost to write files to
6995
// "google/r#type/" instead of "google/type/". This function returns a mapping
7096
// from each declared output filename to the actual prost output path.
7197
//
72-
// Returns an empty map if no package segments are Rust keywords.
98+
// Returns an empty map if no package segments are Rust keywords. Callers who
99+
// also need to handle the more general case of proto-package path differing
100+
// from the bazel package path should use RustProtocOutputDir directly.
73101
func RustKeywordEscapeMappings(pkg string, outputs []string) map[string]string {
74102
if pkg == "" || len(outputs) == 0 {
75103
return nil
76104
}
77105

78-
segments := strings.Split(pkg, ".")
79-
80106
// Check if any segment is a Rust keyword.
81107
needsEscape := false
82-
for _, seg := range segments {
108+
for _, seg := range strings.Split(pkg, ".") {
83109
if rustKeywords[seg] {
84110
needsEscape = true
85111
break
@@ -89,17 +115,7 @@ func RustKeywordEscapeMappings(pkg string, outputs []string) map[string]string {
89115
return nil
90116
}
91117

92-
// Build the escaped directory path.
93-
escaped := make([]string, len(segments))
94-
for i, seg := range segments {
95-
if rustKeywords[seg] {
96-
escaped[i] = "r#" + seg
97-
} else {
98-
escaped[i] = seg
99-
}
100-
}
101-
escapedDir := strings.Join(escaped, "/")
102-
118+
escapedDir := RustProtocOutputDir(pkg)
103119
mappings := make(map[string]string, len(outputs))
104120
for _, output := range outputs {
105121
base := path.Base(output)

pkg/rule/rules_rust/proto_rust_library.go

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package rules_rust
22

33
import (
4+
"path"
45
"strings"
56

67
"github.com/bazelbuild/bazel-gazelle/label"
@@ -57,16 +58,32 @@ func (s *protoRustLibrary) ProvideRule(cfg *protoc.LanguageRuleConfig, pc *proto
5758
return nil
5859
}
5960

60-
// Compute Rust keyword escape mappings for proto packages containing
61-
// Rust reserved keywords (e.g., "google.type" → prost writes to
62-
// "google/r#type/" instead of "google/type/").
61+
// Compute output_mappings whenever the directory protoc-gen-prost writes
62+
// to (derived from the proto package name, with Rust keyword segments
63+
// r#-escaped) differs from the bazel package the rule lives in. Two
64+
// distinct causes:
65+
//
66+
// 1. Rust keyword escapes — proto package "google.type" → prost writes
67+
// to "google/r#type/" while the bazel pkg is "google/type".
68+
//
69+
// 2. Proto package path simply differs from bazel package path —
70+
// e.g. proto package "trumid.common.auth" living at bazel pkg
71+
// "trumid/common/auth/proto", or "grpc.health.v1" living at
72+
// "thirdparty/protobuf/grpc/src/main/protobuf/grpc/health/v1".
73+
// Without a mapping, proto_compile.bzl's rename step looks for the
74+
// output at <bazel-bin>/<bazel_pkg>/<file>.rs and fails with
75+
// `mv: ... No such file or directory`.
6376
if files := pc.Library.Files(); len(files) > 0 {
6477
pkg := files[0].Package().Name
65-
for output, escapedPath := range protoc.RustKeywordEscapeMappings(pkg, outputs) {
78+
protocDir := protoc.RustProtocOutputDir(pkg)
79+
if protocDir != "" && protocDir != pc.Rel {
6680
if pc.Mappings == nil {
6781
pc.Mappings = make(map[string]string)
6882
}
69-
pc.Mappings[output] = escapedPath
83+
for _, output := range outputs {
84+
base := path.Base(output)
85+
pc.Mappings[base] = path.Join(protocDir, base)
86+
}
7087
}
7188
}
7289

rules/proto_compile.bzl

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,21 @@ def _proto_compile_impl(ctx):
159159
# const <ProtoInfo> primary proto provider (for descriptor path resolution)
160160
primary_proto_info = proto_infos[0]
161161

162-
# const <list<ProtoPluginInfo>> plugins to be applied
163-
plugins = [plugin[ProtoPluginInfo] for plugin in ctx.attr.plugins]
162+
# const <list<ProtoPluginInfo>> plugins to be applied. Sort by plugin name
163+
# so protoc invocation order is deterministic and independent of attr
164+
# ordering. This matters when plugins use protoc's @@protoc_insertion_point
165+
# mechanism — e.g. protoc-gen-tonic inserts client/server code at the
166+
# `module` insertion point of protoc-gen-prost's {package}.rs output. If
167+
# tonic runs before prost, the insertion target doesn't exist yet and
168+
# protoc fails with "Tried to insert into file that doesn't exist" (and
169+
# then "Tried to write the same file twice" when prost's subsequent
170+
# create collides with the failed-insertion entry). Alphabetical sort
171+
# puts protoc-gen-prost before protoc-gen-tonic, satisfying the
172+
# producer-before-consumer ordering this protocol requires.
173+
plugins = sorted(
174+
[plugin[ProtoPluginInfo] for plugin in ctx.attr.plugins],
175+
key = lambda p: p.name,
176+
)
164177

165178
# const <dict<string,string>>
166179
outs = {_plugin_label_key(Label(k)): v for k, v in ctx.attr.outs.items()}

0 commit comments

Comments
 (0)