Skip to content

Commit cc3ee38

Browse files
amanda-tarafaamanda-tarafa
authored
fix(librarian/rust): Sample generation flags are propagated all the way (#3518)
Default values for setter and RPC sample generation propagate all the way, including to Veneer modules.
1 parent c02145f commit cc3ee38

7 files changed

Lines changed: 96 additions & 36 deletions

File tree

‎internal/config/language.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ type RustDefault struct {
4242

4343
// GenerateSetterSamples indicates whether to generate setter samples.
4444
GenerateSetterSamples string `yaml:"generate_setter_samples,omitempty"`
45+
46+
// GenerateRpcSamples indicates whether to generate RPC samples.
47+
GenerateRpcSamples string `yaml:"generate_rpc_samples,omitempty"`
4548
}
4649

4750
// RustModule defines a generation target within a veneer crate.
@@ -59,7 +62,10 @@ type RustModule struct {
5962
ExtendGrpcTransport bool `yaml:"extend_grpc_transport,omitempty"`
6063

6164
// GenerateSetterSamples indicates whether to generate setter samples.
62-
GenerateSetterSamples bool `yaml:"generate_setter_samples,omitempty"`
65+
GenerateSetterSamples string `yaml:"generate_setter_samples,omitempty"`
66+
67+
// GenerateRpcSamples indicates whether to generate RPC samples.
68+
GenerateRpcSamples string `yaml:"generate_rpc_samples,omitempty"`
6369

6470
// HasVeneer indicates whether this module has a handwritten wrapper.
6571
HasVeneer bool `yaml:"has_veneer,omitempty"`
@@ -160,9 +166,6 @@ type RustCrate struct {
160166
// IncludeGrpcOnlyMethods indicates whether to include gRPC-only methods.
161167
IncludeGrpcOnlyMethods bool `yaml:"include_grpc_only_methods,omitempty"`
162168

163-
// GenerateRpcSamples indicates whether to generate RPC samples.
164-
GenerateRpcSamples bool `yaml:"generate_rpc_samples,omitempty"`
165-
166169
// PostProcessProtos indicates whether to post-process protos.
167170
PostProcessProtos string `yaml:"post_process_protos,omitempty"`
168171

‎internal/librarian/library.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,17 @@ func fillRust(lib *config.Library, d *config.Default) *config.Library {
5656
if lib.Rust.GenerateSetterSamples == "" {
5757
lib.Rust.GenerateSetterSamples = d.Rust.GenerateSetterSamples
5858
}
59+
if lib.Rust.GenerateRpcSamples == "" {
60+
lib.Rust.GenerateRpcSamples = d.Rust.GenerateRpcSamples
61+
}
62+
for _, mod := range lib.Rust.Modules {
63+
if mod.GenerateSetterSamples == "" {
64+
mod.GenerateSetterSamples = lib.Rust.GenerateSetterSamples
65+
}
66+
if mod.GenerateRpcSamples == "" {
67+
mod.GenerateRpcSamples = lib.Rust.GenerateRpcSamples
68+
}
69+
}
5970
return lib
6071
}
6172

‎internal/librarian/library_test.go‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ func TestFillDefaults_Rust(t *testing.T) {
9393
},
9494
DisabledRustdocWarnings: []string{"broken_intra_doc_links"},
9595
GenerateSetterSamples: "true",
96+
GenerateRpcSamples: "true",
9697
},
9798
}
9899
for _, test := range []struct {
@@ -102,7 +103,11 @@ func TestFillDefaults_Rust(t *testing.T) {
102103
}{
103104
{
104105
name: "fills rust defaults",
105-
lib: &config.Library{},
106+
lib: &config.Library{
107+
Rust: &config.RustCrate{
108+
Modules: []*config.RustModule{{}},
109+
},
110+
},
106111
want: &config.Library{
107112
Rust: &config.RustCrate{
108113
RustDefault: config.RustDefault{
@@ -112,6 +117,13 @@ func TestFillDefaults_Rust(t *testing.T) {
112117
},
113118
DisabledRustdocWarnings: []string{"broken_intra_doc_links"},
114119
GenerateSetterSamples: "true",
120+
GenerateRpcSamples: "true",
121+
},
122+
Modules: []*config.RustModule{
123+
{
124+
GenerateSetterSamples: "true",
125+
GenerateRpcSamples: "true",
126+
},
115127
},
116128
},
117129
},
@@ -125,6 +137,7 @@ func TestFillDefaults_Rust(t *testing.T) {
125137
{Name: "custom", Package: "custom-pkg"},
126138
},
127139
GenerateSetterSamples: "true",
140+
GenerateRpcSamples: "true",
128141
},
129142
},
130143
},
@@ -138,6 +151,7 @@ func TestFillDefaults_Rust(t *testing.T) {
138151
},
139152
DisabledRustdocWarnings: []string{"broken_intra_doc_links"},
140153
GenerateSetterSamples: "true",
154+
GenerateRpcSamples: "true",
141155
},
142156
},
143157
},
@@ -151,6 +165,7 @@ func TestFillDefaults_Rust(t *testing.T) {
151165
{Name: "wkt", Package: "custom-wkt"},
152166
},
153167
GenerateSetterSamples: "false",
168+
GenerateRpcSamples: "false",
154169
},
155170
},
156171
},
@@ -163,6 +178,7 @@ func TestFillDefaults_Rust(t *testing.T) {
163178
},
164179
DisabledRustdocWarnings: []string{"broken_intra_doc_links"},
165180
GenerateSetterSamples: "false",
181+
GenerateRpcSamples: "false",
166182
},
167183
},
168184
},
@@ -185,6 +201,39 @@ func TestFillDefaults_Rust(t *testing.T) {
185201
},
186202
DisabledRustdocWarnings: []string{"custom_warning"},
187203
GenerateSetterSamples: "true",
204+
GenerateRpcSamples: "true",
205+
},
206+
},
207+
},
208+
},
209+
{
210+
name: "module overrides defaults",
211+
lib: &config.Library{
212+
Rust: &config.RustCrate{
213+
Modules: []*config.RustModule{
214+
{
215+
GenerateSetterSamples: "false",
216+
GenerateRpcSamples: "false",
217+
},
218+
},
219+
},
220+
},
221+
want: &config.Library{
222+
Rust: &config.RustCrate{
223+
RustDefault: config.RustDefault{
224+
PackageDependencies: []*config.RustPackageDependency{
225+
{Name: "wkt", Package: "google-cloud-wkt", Source: "google.protobuf"},
226+
{Name: "iam_v1", Package: "google-cloud-iam-v1", Source: "google.iam.v1"},
227+
},
228+
DisabledRustdocWarnings: []string{"broken_intra_doc_links"},
229+
GenerateSetterSamples: "true",
230+
GenerateRpcSamples: "true",
231+
},
232+
Modules: []*config.RustModule{
233+
{
234+
GenerateSetterSamples: "false",
235+
GenerateRpcSamples: "false",
236+
},
188237
},
189238
},
190239
},

‎internal/librarian/rust/codec.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,8 @@ func buildCodec(library *config.Library) map[string]string {
158158
if rust.GenerateSetterSamples != "" {
159159
codec["generate-setter-samples"] = rust.GenerateSetterSamples
160160
}
161-
if rust.GenerateRpcSamples {
162-
codec["generate-rpc-samples"] = "true"
161+
if rust.GenerateRpcSamples != "" {
162+
codec["generate-rpc-samples"] = rust.GenerateRpcSamples
163163
}
164164
if rust.NameOverrides != "" {
165165
codec["name-overrides"] = rust.NameOverrides
@@ -277,8 +277,11 @@ func moduleToSidekickConfig(library *config.Library, module *config.RustModule,
277277

278278
func buildModuleCodec(library *config.Library, module *config.RustModule) map[string]string {
279279
codec := newLibraryCodec(library)
280-
if module.GenerateSetterSamples {
281-
codec["generate-setter-samples"] = "true"
280+
if module.GenerateSetterSamples != "" {
281+
codec["generate-setter-samples"] = module.GenerateSetterSamples
282+
}
283+
if module.GenerateRpcSamples != "" {
284+
codec["generate-rpc-samples"] = module.GenerateRpcSamples
282285
}
283286
if module.HasVeneer {
284287
codec["has-veneer"] = "true"

‎internal/librarian/rust/codec_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,14 @@ func TestToSidekickConfig(t *testing.T) {
129129
RustDefault: config.RustDefault{
130130
DisabledRustdocWarnings: []string{"broken_intra_doc_links"},
131131
GenerateSetterSamples: "true",
132+
GenerateRpcSamples: "true",
132133
},
133134
ModulePath: "gcs",
134135
PerServiceFeatures: true,
135136
IncludeGrpcOnlyMethods: true,
136137
DetailedTracingAttributes: true,
137138
HasVeneer: true,
138139
RoutingRequired: true,
139-
GenerateRpcSamples: true,
140140
DisabledClippyWarnings: []string{"too_many_arguments"},
141141
DefaultFeatures: []string{"default-feature"},
142142
TemplateOverride: "custom-template",

‎tool/cmd/migrate/main.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,7 @@ func buildGAPIC(files []string, repoPath string) (map[string]*config.Library, er
423423
PackageDependencies: packageDeps,
424424
DisabledRustdocWarnings: strToSlice(disabledRustdocWarnings, false),
425425
GenerateSetterSamples: generateSetterSamples,
426+
GenerateRpcSamples: generateRpcSamples,
426427
},
427428
PerServiceFeatures: strToBool(perServiceFeatures),
428429
ModulePath: modulePath,
@@ -438,7 +439,6 @@ func buildGAPIC(files []string, repoPath string) (map[string]*config.Library, er
438439
HasVeneer: strToBool(hasVeneer),
439440
RoutingRequired: strToBool(routingRequired),
440441
IncludeGrpcOnlyMethods: strToBool(includeGrpcOnlyMethods),
441-
GenerateRpcSamples: strToBool(generateRpcSamples),
442442
PostProcessProtos: postProcessProtos,
443443
DetailedTracingAttributes: strToBool(detailedTracingAttributes),
444444
DocumentationOverrides: documentationOverrides,
@@ -609,11 +609,8 @@ func buildModules(rootDir string, repoPath string) ([]*config.RustModule, error)
609609
nameOverrides := sidekick.Codec["name-overrides"]
610610
postProcessProtos := sidekick.Codec["post-process-protos"]
611611
templateOverride := sidekick.Codec["template-override"]
612+
generateSetterSamples := sidekick.Codec["generate-setter-samples"]
612613

613-
generateSetterSamples, ok := sidekick.Codec["generate-setter-samples"]
614-
if !ok {
615-
generateSetterSamples = "true"
616-
}
617614
// Parse documentation overrides
618615
var documentationOverrides []config.RustDocumentationOverride
619616
for _, do := range sidekick.CommentOverrides {
@@ -629,7 +626,7 @@ func buildModules(rootDir string, repoPath string) ([]*config.RustModule, error)
629626
}
630627
module := &config.RustModule{
631628
DocumentationOverrides: documentationOverrides,
632-
GenerateSetterSamples: strToBool(generateSetterSamples),
629+
GenerateSetterSamples: generateSetterSamples,
633630
HasVeneer: strToBool(hasVeneer),
634631
IncludedIds: strToSlice(includedIds, false),
635632
IncludeGrpcOnlyMethods: strToBool(includeGrpcOnlyMethods),
@@ -683,7 +680,7 @@ func buildConfig(libraries map[string]*config.Library, defaults *config.Config)
683680
// Check if library has extra configuration beyond just name/api/version
684681
hasExtraConfig := lib.CopyrightYear != "" ||
685682
(lib.Rust != nil && (lib.Rust.PerServiceFeatures || len(lib.Rust.DisabledRustdocWarnings) > 0 ||
686-
lib.Rust.GenerateSetterSamples != "" || lib.Rust.GenerateRpcSamples ||
683+
lib.Rust.GenerateSetterSamples != "" || lib.Rust.GenerateRpcSamples != "" ||
687684
len(lib.Rust.PackageDependencies) > 0 || len(lib.Rust.PaginationOverrides) > 0 ||
688685
lib.Rust.NameOverrides != ""))
689686
// Only include in libraries section if specific data needs to be retained

‎tool/cmd/migrate/main_test.go‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ func TestBuildGAPIC(t *testing.T) {
197197
RustDefault: config.RustDefault{
198198
DisabledRustdocWarnings: []string{"bare_urls", "broken_intra_doc_links", "redundant_explicit_links"},
199199
GenerateSetterSamples: "true",
200+
GenerateRpcSamples: "true",
200201
},
201202
PerServiceFeatures: true,
202203
ModulePath: "crate",
@@ -212,7 +213,6 @@ func TestBuildGAPIC(t *testing.T) {
212213
HasVeneer: true,
213214
RoutingRequired: true,
214215
IncludeGrpcOnlyMethods: true,
215-
GenerateRpcSamples: true,
216216
PostProcessProtos: "example post processing",
217217
DetailedTracingAttributes: true,
218218
NameOverrides: ".google.cloud.security/publicca.v1.Storage=StorageControl",
@@ -458,7 +458,6 @@ func TestBuildVeneer(t *testing.T) {
458458
Modules: []*config.RustModule{
459459
{
460460
DisabledRustdocWarnings: []string{},
461-
GenerateSetterSamples: true,
462461
ModuleRoots: nil,
463462
HasVeneer: true,
464463
IncludedIds: []string{
@@ -478,7 +477,7 @@ func TestBuildVeneer(t *testing.T) {
478477
TitleOverride: "Cloud Firestore API",
479478
},
480479
{
481-
GenerateSetterSamples: false,
480+
GenerateSetterSamples: "false",
482481
ModulePath: "crate::generated::gapic_control::model",
483482
ModuleRoots: map[string]string{
484483
"project-root": ".",
@@ -523,7 +522,6 @@ func TestBuildVeneer(t *testing.T) {
523522
Replace: "The service helps to manage cloud storage resources.",
524523
},
525524
},
526-
GenerateSetterSamples: true,
527525
HasVeneer: true,
528526
IncludeGrpcOnlyMethods: true,
529527
NameOverrides: ".google.storage.v2.Storage=StorageControl",
@@ -581,9 +579,10 @@ func TestBuildVeneer(t *testing.T) {
581579
ModuleRoots: map[string]string{
582580
"project-root": ".",
583581
},
584-
Output: "tests/common/src/generated",
585-
Source: "src/wkt/tests/protos",
586-
Template: "mod",
582+
Output: "tests/common/src/generated",
583+
Source: "src/wkt/tests/protos",
584+
Template: "mod",
585+
GenerateSetterSamples: "false",
587586
},
588587
},
589588
},
@@ -597,12 +596,11 @@ func TestBuildVeneer(t *testing.T) {
597596
Rust: &config.RustCrate{
598597
Modules: []*config.RustModule{
599598
{
600-
GenerateSetterSamples: true,
601-
IncludeList: "api.proto,source_context.proto,type.proto,descriptor.proto",
602-
ModulePath: "crate",
603-
Output: "src/generated",
604-
Source: "google/protobuf",
605-
Template: "mod",
599+
IncludeList: "api.proto,source_context.proto,type.proto,descriptor.proto",
600+
ModulePath: "crate",
601+
Output: "src/generated",
602+
Source: "google/protobuf",
603+
Template: "mod",
606604
},
607605
},
608606
},
@@ -627,7 +625,6 @@ func TestBuildVeneer(t *testing.T) {
627625
Modules: []*config.RustModule{
628626
{
629627
DisabledRustdocWarnings: []string{},
630-
GenerateSetterSamples: true,
631628
ModuleRoots: nil,
632629
HasVeneer: true,
633630
IncludedIds: []string{
@@ -647,7 +644,7 @@ func TestBuildVeneer(t *testing.T) {
647644
TitleOverride: "Cloud Firestore API",
648645
},
649646
{
650-
GenerateSetterSamples: false,
647+
GenerateSetterSamples: "false",
651648
ModulePath: "crate::generated::gapic_control::model",
652649
ModuleRoots: map[string]string{
653650
"project-root": ".",
@@ -727,9 +724,9 @@ func TestBuildConfig(t *testing.T) {
727724
RustDefault: config.RustDefault{
728725
DisabledRustdocWarnings: []string{"bare_urls", "broken_intra_doc_links", "redundant_explicit_links"},
729726
GenerateSetterSamples: "true",
727+
GenerateRpcSamples: "true",
730728
},
731729
PerServiceFeatures: true,
732-
GenerateRpcSamples: true,
733730
NameOverrides: ".google.cloud.security/publicca.v1.Storage=StorageControl",
734731
},
735732
},
@@ -761,9 +758,9 @@ func TestBuildConfig(t *testing.T) {
761758
RustDefault: config.RustDefault{
762759
DisabledRustdocWarnings: []string{"bare_urls", "broken_intra_doc_links", "redundant_explicit_links"},
763760
GenerateSetterSamples: "true",
761+
GenerateRpcSamples: "true",
764762
},
765763
PerServiceFeatures: true,
766-
GenerateRpcSamples: true,
767764
NameOverrides: ".google.cloud.security/publicca.v1.Storage=StorageControl",
768765
},
769766
},
@@ -787,9 +784,9 @@ func TestBuildConfig(t *testing.T) {
787784
RustDefault: config.RustDefault{
788785
DisabledRustdocWarnings: []string{"bare_urls", "broken_intra_doc_links", "redundant_explicit_links"},
789786
GenerateSetterSamples: "true",
787+
GenerateRpcSamples: "true",
790788
},
791789
PerServiceFeatures: true,
792-
GenerateRpcSamples: true,
793790
NameOverrides: ".google.cloud.orgpolicy.v1.OrgPolicy=OrgPolicyControl",
794791
},
795792
},
@@ -810,9 +807,9 @@ func TestBuildConfig(t *testing.T) {
810807
RustDefault: config.RustDefault{
811808
DisabledRustdocWarnings: []string{"bare_urls", "broken_intra_doc_links", "redundant_explicit_links"},
812809
GenerateSetterSamples: "true",
810+
GenerateRpcSamples: "true",
813811
},
814812
PerServiceFeatures: true,
815-
GenerateRpcSamples: true,
816813
NameOverrides: ".google.cloud.orgpolicy.v1.OrgPolicy=OrgPolicyControl",
817814
},
818815
},

0 commit comments

Comments
 (0)