Skip to content

Commit 9781651

Browse files
authored
feat(rust): flat-crate proto_rust_library + prost/tonic interop fixes (#425)
* Auto-sync root Cargo.toml workspace members and proto_compile_assets deps After GenerateRules has run for every package, rewrite two marker-delimited sections at the repo root: - Cargo.toml between `# gazelle:proto_rust_members start/end` — populated with one entry per package that emitted a proto_rust_library. - BUILD.bazel between `# gazelle:vendor_proto_sources_deps start/end` — populated with the underlying _lib target of each proto_rust_library plus every proto_compiled_sources rule. Both rewrites are no-ops when the markers (or the target file) are absent, so existing repos without the markers see no change. Entries are sorted and deduplicated; the file is only rewritten when content actually changes. * proto_rust_library: flat-crate convention — drop pkg attr, _rs suffix, nested extern paths The consuming Starlark proto_rust_library macro now exposes all generated types at the crate root, so several pieces of namespace plumbing that the gazelle extension was emitting are no longer needed and have been removed: - pkg attribute on the generated rule (rust_library.go): the macro doesn't accept it anymore. RustLibrary.Pkg() is retained as an internal helper that still feeds Name() and Reexports() — only the SetAttr("pkg", ...) emission is gone. - _rs suffix on crate names (rust_keywords.go): RustCrateName returns the package with dots replaced by underscores, with no trailing "_rs". E.g. "google.api" -> "google_api" (was "google_api_rs"). The crate name now IS the namespace, matching the flat-root layout. - Nested ::module::path on extern_path entries (extern_paths.go): `extern_path=.{pkg}=::{crate}::{module}` is shortened to `extern_path=.{pkg}=::{crate}` for transitive deps, and the self-extern override `extern_path=.{pkg}=crate::{module}::...` is shortened to `extern_path=.{pkg}=crate`. Both reflect the same flat-root convention. * gencopy: print a ProTip with the .update target when a check fails When `bazel test //path:foo_proto_compile_test` fails because a checked-in generated file has drifted from what `protoc` would produce, the existing error tells you which file pair mismatched and shows the diff, but leaves the developer to figure out which target regenerates the source-of-truth copy. Append a one-line ProTip to the mismatch error message, naming the `.update` target so it can be copy-pasted into `bazel run`: gencopy mismatch "foo.pb.go.gen" vs. "foo.pb.go" (-want +got): ... ProTip: to regenerate, run: bazel run //proto:foo_proto_compile.update The label is derived from pkg.TargetLabel (the proto_compile rule's full label, including any external-repo prefix) by replacing the rule-name component with cfg.UpdateTargetLabelName -- both already populated by the proto_compile_gencopy_test Starlark rule via gencopy_config(). * fix: enumerate all protos * fix: extern path resolution for WKT * 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. * checkpoint: per-file progress * per-file-imports checkpoint * checkpoint: refactor without per_file * checkpoint: all rlibs compile * apply buildifier.fix * fix(rust): deduplicate extern paths and apply rewrites * fix(rust): exclude per-file crates from Cargo workspace * fix(rust): enroll per-file Cargo manifests * fix(rust): enroll per-file Cargo deps transitively * fix(rust): manage per-file Cargo exclusions * fix(rust): expose per-file Cargo path dependencies * fix(rust): document Cargo path patches * Revert "fix(rust): document Cargo path patches" This reverts commit 4aa32a1. * Revert "fix(rust): expose per-file Cargo path dependencies" This reverts commit b5078bb.
1 parent 4518136 commit 9781651

31 files changed

Lines changed: 1845 additions & 157 deletions

‎cmd/gencopy/gencopy.go‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"os"
1212
"path/filepath"
1313
"strconv"
14+
"strings"
1415

1516
"github.com/google/go-cmp/cmp"
1617
)
@@ -105,7 +106,7 @@ func readFileAsString(filename string) (string, error) {
105106
return string(bytes), nil
106107
}
107108

108-
func check(_ *Config, pkg *PackageConfig, pairs []*SrcDst) error {
109+
func check(cfg *Config, pkg *PackageConfig, pairs []*SrcDst) error {
109110
for _, pair := range pairs {
110111
expected, err := readFileAsString(pair.Src)
111112
if err != nil {
@@ -117,7 +118,10 @@ func check(_ *Config, pkg *PackageConfig, pairs []*SrcDst) error {
117118
}
118119

119120
if diff := cmp.Diff(expected, actual); diff != "" {
120-
return fmt.Errorf("gencopy mismatch %q vs. %q (-want +got):\n%s", pair.Src, pair.Dst, diff)
121+
return fmt.Errorf(
122+
"gencopy mismatch %q vs. %q (-want +got):\n%s\n%s",
123+
pair.Src, pair.Dst, diff, regenerateProTip(pkg.TargetLabel, cfg.UpdateTargetLabelName),
124+
)
121125
}
122126
}
123127

@@ -129,6 +133,22 @@ func check(_ *Config, pkg *PackageConfig, pairs []*SrcDst) error {
129133
return nil
130134
}
131135

136+
// regenerateProTip returns a friendly hint pointing the developer at the
137+
// `.update` target that regenerates the checked-in copies. targetLabel is the
138+
// proto_compile rule's label (e.g. "//proto:foo_proto_compile" or
139+
// "@@repo//proto:foo_proto_compile"); updateName is the .update target's
140+
// rule name (e.g. "foo_proto_compile.update").
141+
func regenerateProTip(targetLabel, updateName string) string {
142+
if updateName == "" {
143+
return ""
144+
}
145+
updateLabel := updateName
146+
if idx := strings.LastIndex(targetLabel, ":"); idx >= 0 {
147+
updateLabel = targetLabel[:idx+1] + updateName
148+
}
149+
return fmt.Sprintf("\nProTip: to regenerate, run:\n bazel run %s\n", updateLabel)
150+
}
151+
132152
func update(cfg *Config, pkg *PackageConfig, pairs []*SrcDst) error {
133153
for _, pair := range pairs {
134154
pair.Dst += cfg.Extension

‎cmd/gencopy/gencopy_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,42 @@ func TestRunPkg(t *testing.T) {
144144
}
145145
}
146146

147+
func TestRegenerateProTip(t *testing.T) {
148+
for name, tc := range map[string]struct {
149+
targetLabel string
150+
updateName string
151+
want string
152+
}{
153+
"empty update name": {
154+
targetLabel: "//proto:foo_proto_compile",
155+
updateName: "",
156+
want: "",
157+
},
158+
"local target": {
159+
targetLabel: "//proto:foo_proto_compile",
160+
updateName: "foo_proto_compile.update",
161+
want: "\nProTip: to regenerate, run:\n bazel run //proto:foo_proto_compile.update\n",
162+
},
163+
"external repo target": {
164+
targetLabel: "@@some_repo//pkg:foo_proto_compile",
165+
updateName: "foo_proto_compile.update",
166+
want: "\nProTip: to regenerate, run:\n bazel run @@some_repo//pkg:foo_proto_compile.update\n",
167+
},
168+
"target with no colon": {
169+
targetLabel: "raw_label_no_colon",
170+
updateName: "foo_proto_compile.update",
171+
want: "\nProTip: to regenerate, run:\n bazel run foo_proto_compile.update\n",
172+
},
173+
} {
174+
t.Run(name, func(t *testing.T) {
175+
got := regenerateProTip(tc.targetLabel, tc.updateName)
176+
if got != tc.want {
177+
t.Errorf("regenerateProTip: got %q, want %q", got, tc.want)
178+
}
179+
})
180+
}
181+
}
182+
147183
// listFiles - convenience debugging function to log the files under a given dir
148184
func listFiles(t *testing.T, dir string) error {
149185
return filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {

‎example/thing/BUILD.bazel‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ proto_library(
1111
name = "thing_proto",
1212
srcs = ["thing.proto"],
1313
visibility = ["//visibility:public"],
14-
deps = ["@protobufapis//google/protobuf:timestamp_proto"],
14+
deps = ["@com_google_protobuf//:timestamp_proto"],
1515
)
1616

1717
proto_cc_library(

‎pkg/language/protobuf/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ go_library(
2828
go_test(
2929
name = "protobuf_test",
3030
srcs = [
31+
"config_filemode_test.go",
3132
"generate_test.go",
3233
"override_test.go",
3334
],
@@ -37,6 +38,7 @@ go_test(
3738
"@bazel_gazelle//config",
3839
"@bazel_gazelle//label",
3940
"@bazel_gazelle//language",
41+
"@bazel_gazelle//language/proto",
4042
"@bazel_gazelle//resolve",
4143
"@bazel_gazelle//rule",
4244
"@bazel_gazelle//testtools",

‎pkg/language/protobuf/config.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,18 @@ func (pl *protobufLang) Configure(c *config.Config, rel string, f *rule.File) {
117117

118118
// getOrCreatePackageConfig either inserts a new config into the map under the
119119
// language name or replaces it with a clone.
120+
//
121+
// When cloning from the parent directory's config we must overwrite the
122+
// embedded *config.Config pointer to the current directory's config. Without
123+
// this, lookups that walk `cfg.Config.Exts[...]` (e.g.
124+
// `IsProtoFileMode` reading the standard proto language's per-directory mode)
125+
// resolve against the parent's `Exts` and miss directives like
126+
// `# gazelle:proto file` that were applied to the child only.
120127
func (pl *protobufLang) getOrCreatePackageConfig(config *config.Config) *protoc.PackageConfig {
121128
var cfg *protoc.PackageConfig
122129
if existingExt, ok := config.Exts[pl.name]; ok {
123130
cfg = existingExt.(*protoc.PackageConfig).Clone()
131+
cfg.Config = config
124132
} else {
125133
cfg = protoc.NewPackageConfig(config)
126134
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package protobuf
2+
3+
import (
4+
"testing"
5+
6+
"github.com/bazelbuild/bazel-gazelle/config"
7+
gproto "github.com/bazelbuild/bazel-gazelle/language/proto"
8+
)
9+
10+
// Regression: getOrCreatePackageConfig must point the cloned PackageConfig's
11+
// embedded *config.Config at the CURRENT directory's config, not the parent's.
12+
// Otherwise IsProtoFileMode reads the parent's proto-language config and
13+
// misses a child directory that opted into `# gazelle:proto file`.
14+
func TestGetOrCreatePackageConfig_RebindsConfig(t *testing.T) {
15+
pl := &protobufLang{name: "protobuf"}
16+
17+
// Parent: default mode.
18+
parent := &config.Config{Exts: map[string]interface{}{}}
19+
parent.Exts["proto"] = &gproto.ProtoConfig{Mode: gproto.DefaultMode}
20+
pl.getOrCreatePackageConfig(parent)
21+
22+
// Child: clone via gazelle's Config.Clone, then proto-lang flips mode to FileMode
23+
// for this dir only.
24+
child := parent.Clone()
25+
childProto := &gproto.ProtoConfig{Mode: gproto.FileMode}
26+
child.Exts["proto"] = childProto
27+
28+
cfg := pl.getOrCreatePackageConfig(child)
29+
if cfg.Config != child {
30+
t.Fatalf("cloned PackageConfig.Config not rebound to child: got %p, want %p", cfg.Config, child)
31+
}
32+
33+
// IsProtoFileMode-equivalent check.
34+
if gproto.GetProtoConfig(cfg.Config).Mode != gproto.FileMode {
35+
t.Errorf("expected FileMode via cfg.Config, got %v", gproto.GetProtoConfig(cfg.Config).Mode)
36+
}
37+
}

‎pkg/language/protobuf/generate.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package protobuf
33
import (
44
"log"
55
"path"
6+
"strings"
67

78
"github.com/bazelbuild/bazel-gazelle/config"
89
"github.com/bazelbuild/bazel-gazelle/label"
@@ -104,7 +105,14 @@ func (pl *protobufLang) GenerateRules(args language.GenerateArgs) language.Gener
104105
protoc.GlobalRuleIndex().Put(internalLabel, r)
105106
switch r.Kind() {
106107
case "proto_rust_library":
107-
pl.protoRustLibraryPackages = append(pl.protoRustLibraryPackages, args.Rel)
108+
if protoRustLibraryIsExplicitWorkspaceMember(r.Name()) {
109+
pl.protoRustLibraryPackages = append(pl.protoRustLibraryPackages, args.Rel)
110+
} else {
111+
pl.protoRustPerFilePackageDirs = append(
112+
pl.protoRustPerFilePackageDirs,
113+
path.Join(args.Rel, "_rust"),
114+
)
115+
}
108116
// The proto_rust_library macro's underlying _proto_rust_lib rule
109117
// (named "<name>_lib") is what provides ProtoCompileInfo for the
110118
// wrapper lib.rs + Cargo.toml; that's the label that belongs in
@@ -136,6 +144,14 @@ func (pl *protobufLang) GenerateRules(args language.GenerateArgs) language.Gener
136144
}
137145
}
138146

147+
// protoRustLibraryIsExplicitWorkspaceMember reports whether the generated
148+
// crate belongs in the root Cargo.toml marker section. Per-file crates are
149+
// path dependencies of these package-level roots, so Cargo enrolls them in
150+
// the workspace transitively after their manifests are vendored.
151+
func protoRustLibraryIsExplicitWorkspaceMember(name string) bool {
152+
return !strings.Contains(name, "__")
153+
}
154+
139155
func matchingFiles(files map[string]*protoc.File, srcs []label.Label) []*protoc.File {
140156
matching := make([]*protoc.File, 0)
141157
for _, src := range srcs {

‎pkg/language/protobuf/generate_test.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,28 @@ import "google/protobuf/any.proto";
151151
}
152152
}
153153

154+
func TestProtoRustLibraryIsExplicitWorkspaceMember(t *testing.T) {
155+
for name, tc := range map[string]struct {
156+
ruleName string
157+
want bool
158+
}{
159+
"package-level crate": {
160+
ruleName: "example_proto_rs",
161+
want: true,
162+
},
163+
"per-file crate": {
164+
ruleName: "example_proto__message_proto_rs",
165+
want: false,
166+
},
167+
} {
168+
t.Run(name, func(t *testing.T) {
169+
if got := protoRustLibraryIsExplicitWorkspaceMember(tc.ruleName); got != tc.want {
170+
t.Errorf("protoRustLibraryIsExplicitWorkspaceMember(%q) = %t, want %t", tc.ruleName, got, tc.want)
171+
}
172+
})
173+
}
174+
}
175+
154176
type testGenerateRulesState struct {
155177
t *testing.T
156178
tmpdir string

‎pkg/language/protobuf/lang.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,17 @@ type protobufLang struct {
4040
starlarkRules arrayFlags
4141
// starlarkPlugins stores custom starlark proto plugin names in the form filename%pluginname
4242
starlarkPlugins arrayFlags
43-
// protoRustLibraryPackages collects the workspace-relative path of every
44-
// package that emits a proto_rust_library rule. Populated in
43+
// protoRustLibraryPackages collects the workspace-relative path of each
44+
// package-level proto_rust_library. Per-file crates remain standalone path
45+
// dependencies under excluded _rust directories. Populated in
4546
// GenerateRules and consumed in DoneGeneratingRules to update the root
4647
// Cargo.toml [workspace] members list.
4748
protoRustLibraryPackages []string
49+
// protoRustPerFilePackageDirs collects the workspace-relative _rust
50+
// directories that contain standalone per-file crates. These directories
51+
// are excluded from the root Cargo workspace so each generated manifest can
52+
// be used directly or as a path dependency.
53+
protoRustPerFilePackageDirs []string
4854
// vendorAssetLabels collects bazel labels of every generated rule that
4955
// provides ProtoCompileInfo and should appear in the root
5056
// `proto_compile_assets` aggregator. Populated in GenerateRules and

‎pkg/language/protobuf/lifecycle.go‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,23 @@ func (pl *protobufLang) Before(context.Context) {
1616

1717
// DoneGeneratingRules implements part of the language.LifecycleManager interface.
1818
//
19-
// Performs two cross-package syncs that need every GenerateRules call to
19+
// Performs three cross-package syncs that need every GenerateRules call to
2020
// have completed first:
2121
//
2222
// 1. Root Cargo.toml [workspace] members list — the lines between the
2323
// `# gazelle:proto_rust_members start/end` markers are replaced with
24-
// one entry per package that emitted a proto_rust_library.
24+
// one entry per package-level proto_rust_library.
2525
//
2626
// 2. Root BUILD.bazel proto_compile_assets aggregator deps — the lines
2727
// between the `# gazelle:vendor_proto_sources_deps start/end` markers
2828
// are replaced with one entry per generated proto_compiled_sources rule
2929
// and one entry per proto_rust_library's underlying _lib target.
3030
//
31-
// Both syncs are no-ops when the corresponding markers are absent (or the
31+
// 3. Root Cargo.toml [workspace] exclude list — the lines between the
32+
// `# gazelle:proto_rust_excludes start/end` markers are replaced with
33+
// one entry per Bazel package containing standalone per-file crates.
34+
//
35+
// All syncs are no-ops when the corresponding markers are absent (or the
3236
// target file does not exist).
3337
func (pl *protobufLang) DoneGeneratingRules() {
3438
if pl.repoRoot == "" {
@@ -37,6 +41,9 @@ func (pl *protobufLang) DoneGeneratingRules() {
3741
if err := updateRootCargoMembers(pl.repoRoot, pl.protoRustLibraryPackages); err != nil {
3842
log.Printf("warning: could not update root Cargo.toml proto_rust_members: %v", err)
3943
}
44+
if err := updateRootCargoExcludes(pl.repoRoot, pl.protoRustPerFilePackageDirs); err != nil {
45+
log.Printf("warning: could not update root Cargo.toml proto_rust_excludes: %v", err)
46+
}
4047
if err := updateRootVendorAssetsDeps(pl.repoRoot, pl.vendorAssetLabels); err != nil {
4148
log.Printf("warning: could not update root BUILD.bazel vendor_proto_sources_deps: %v", err)
4249
}
@@ -47,10 +54,12 @@ func (pl *protobufLang) AfterResolvingDeps(context.Context) {
4754
}
4855

4956
const (
50-
cargoMembersStartMarker = "# gazelle:proto_rust_members start"
51-
cargoMembersEndMarker = "# gazelle:proto_rust_members end"
52-
vendorAssetsDepsStartMarker = "# gazelle:vendor_proto_sources_deps start"
53-
vendorAssetsDepsEndMarker = "# gazelle:vendor_proto_sources_deps end"
57+
cargoMembersStartMarker = "# gazelle:proto_rust_members start"
58+
cargoMembersEndMarker = "# gazelle:proto_rust_members end"
59+
cargoExcludesStartMarker = "# gazelle:proto_rust_excludes start"
60+
cargoExcludesEndMarker = "# gazelle:proto_rust_excludes end"
61+
vendorAssetsDepsStartMarker = "# gazelle:vendor_proto_sources_deps start"
62+
vendorAssetsDepsEndMarker = "# gazelle:vendor_proto_sources_deps end"
5463
)
5564

5665
// updateRootCargoMembers rewrites the gazelle:proto_rust_members marker
@@ -67,6 +76,19 @@ func updateRootCargoMembers(repoRoot string, packages []string) error {
6776
)
6877
}
6978

79+
// updateRootCargoExcludes rewrites the gazelle:proto_rust_excludes marker
80+
// section in the root Cargo.toml with directories containing standalone
81+
// per-file crates.
82+
func updateRootCargoExcludes(repoRoot string, packages []string) error {
83+
return rewriteMarkerSection(
84+
filepath.Join(repoRoot, "Cargo.toml"),
85+
cargoExcludesStartMarker,
86+
cargoExcludesEndMarker,
87+
packages,
88+
"[workspace] exclude list",
89+
)
90+
}
91+
7092
// updateRootVendorAssetsDeps rewrites the gazelle:vendor_proto_sources_deps
7193
// marker section in the root BUILD.bazel with a sorted, deduplicated list
7294
// of `"<label>",` entries. No-op if the file is missing or the markers are

0 commit comments

Comments
 (0)