Skip to content

Commit 839ce70

Browse files
authored
feat(too/cmd/migrate): support parse BUILD.bazel for java migrate (#4335)
Supports parsing java specifc fields from BUILD.bazel and populate to librarian.yaml in migration. `include_samples` and `rest_numeric_enums` has majority and missing default of `true`, they are included in reverse in librarian.yaml, so only a few needs to be explicitly set. For #4306 For #4130
1 parent 6b62c4d commit 839ce70

8 files changed

Lines changed: 259 additions & 28 deletions

File tree

doc/config-schema.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,15 @@ This document describes the schema for the librarian.yaml.
141141
| `module_path_version` | string | Is the version of the Go module path. |
142142
| `nested_module` | string | Is the name of a nested module directory. |
143143

144+
## JavaAPI Configuration
145+
146+
| Field | Type | Description |
147+
| :--- | :--- | :--- |
148+
| `additional_protos` | list of string | Is a list of additional proto files to include in generation. |
149+
| `no_samples` | bool | Determines whether to generate samples for the API. |
150+
| `path` | string | Is the source path. |
151+
| `no_rest_numeric_enums` | bool | Determines whether to use numeric enums in REST requests for the API. |
152+
144153
## JavaModule Configuration
145154

146155
| Field | Type | Description |
@@ -160,6 +169,7 @@ This document describes the schema for the librarian.yaml.
160169
| `library_type_override` | string | Allows the "library_type" field in .repo-metadata.json to be overridden. |
161170
| `min_java_version` | int | Is the minimum Java version required. |
162171
| `name_pretty_override` | string | Allows the "name_pretty" field in .repo-metadata.json to be overridden. |
172+
| `java_apis` | list of [JavaAPI](#javaapi-configuration) (optional) | Is a list of Java-specific API configurations. |
163173
| `product_documentation_override` | string | Allows the "product_documentation" field in .repo-metadata.json to be overridden. |
164174
| `recommended_package` | string | Is the recommended package name. |
165175
| `billing_not_required` | bool | Indicates whether the API does NOT require billing. This is typically false. |

internal/config/language.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,9 @@ type JavaModule struct {
488488
// to be overridden.
489489
NamePrettyOverride string `yaml:"name_pretty_override,omitempty"`
490490

491+
// JavaAPIs is a list of Java-specific API configurations.
492+
JavaAPIs []*JavaAPI `yaml:"java_apis,omitempty"`
493+
491494
// ProductDocumentationOverride allows the "product_documentation" field in
492495
// .repo-metadata.json to be overridden.
493496
ProductDocumentationOverride string `yaml:"product_documentation_override,omitempty"`
@@ -505,3 +508,18 @@ type JavaModule struct {
505508
// RpcDocumentation is the URL for the RPC documentation.
506509
RpcDocumentation string `yaml:"rpc_documentation,omitempty"`
507510
}
511+
512+
// JavaAPI represents configuration for a single API within a Java module.
513+
type JavaAPI struct {
514+
// AdditionalProtos is a list of additional proto files to include in generation.
515+
AdditionalProtos []string `yaml:"additional_protos,omitempty"`
516+
517+
// NoSamples determines whether to generate samples for the API.
518+
NoSamples bool `yaml:"no_samples,omitempty"`
519+
520+
// Path is the source path.
521+
Path string `yaml:"path,omitempty"`
522+
523+
// NoRestNumericEnums determines whether to use numeric enums in REST requests for the API.
524+
NoRestNumericEnums bool `yaml:"no_rest_numeric_enums,omitempty"`
525+
}

tool/cmd/migrate/java.go

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,60 @@ const (
2929
generationConfigFileName = "generation_config.yaml"
3030
)
3131

32+
type javaGAPICInfo struct {
33+
NoRestNumericEnums bool
34+
NoSamples bool
35+
AdditionalProtos []string
36+
}
37+
38+
func parseJavaBazel(googleapisDir, dir string) (*javaGAPICInfo, error) {
39+
file, err := parseBazel(googleapisDir, dir)
40+
if err != nil {
41+
return nil, err
42+
}
43+
if file == nil {
44+
return nil, nil
45+
}
46+
info := &javaGAPICInfo{}
47+
// 1. From java_gapic_library
48+
if rules := file.Rules("java_gapic_library"); len(rules) > 0 {
49+
if len(rules) > 1 {
50+
log.Printf("Warning: multiple java_gapic_library in %s/BUILD.bazel, using first", dir)
51+
}
52+
rule := rules[0]
53+
info.NoRestNumericEnums = rule.AttrLiteral("rest_numeric_enums") == "False"
54+
}
55+
// 2. From java_gapic_assembly_gradle_pkg
56+
if rules := file.Rules("java_gapic_assembly_gradle_pkg"); len(rules) > 0 {
57+
if len(rules) > 1 {
58+
log.Printf("Warning: multiple java_gapic_assembly_gradle_pkg in %s/BUILD.bazel, using first", dir)
59+
}
60+
rule := rules[0]
61+
info.NoSamples = rule.AttrLiteral("include_samples") == "False"
62+
}
63+
// 3. From proto_library_with_info
64+
if rules := file.Rules("proto_library_with_info"); len(rules) > 0 {
65+
if len(rules) > 1 {
66+
log.Printf("Warning: multiple proto_library_with_info in %s/BUILD.bazel, using first", dir)
67+
}
68+
rule := rules[0]
69+
// Search for specific common resource targets in deps
70+
if deps := rule.AttrStrings("deps"); len(deps) > 0 {
71+
protoMappings := map[string]string{
72+
"//google/cloud:common_resources_proto": "google/cloud/common_resources.proto",
73+
"//google/cloud/location:location_proto": "google/cloud/location/locations.proto",
74+
"//google/iam/v1:iam_policy_proto": "google/iam/v1/iam_policy.proto",
75+
}
76+
for _, dep := range deps {
77+
if protoPath, ok := protoMappings[dep]; ok {
78+
info.AdditionalProtos = append(info.AdditionalProtos, protoPath)
79+
}
80+
}
81+
}
82+
}
83+
return info, nil
84+
}
85+
3286
// GAPICConfig represents the GAPIC configuration in generation_config.yaml.
3387
type GAPICConfig struct {
3488
ProtoPath string `yaml:"proto_path"`
@@ -73,10 +127,17 @@ func runJavaMigration(ctx context.Context, repoPath string) error {
73127
if err != nil {
74128
return err
75129
}
76-
cfg := buildConfig(gen)
130+
src, err := fetchSource(ctx)
131+
if err != nil {
132+
return errFetchSource
133+
}
134+
cfg := buildConfig(gen, src.Dir)
77135
if cfg == nil {
78136
return fmt.Errorf("no libraries found to migrate")
79137
}
138+
// The directory name in Googleapis is present for migration code to look
139+
// up API details. It shouldn't be persisted.
140+
cfg.Sources.Googleapis.Dir = ""
80141
if err := librarian.RunTidyOnConfig(ctx, cfg); err != nil {
81142
return errTidyFailed
82143
}
@@ -89,18 +150,36 @@ func readGenerationConfig(path string) (*GenerationConfig, error) {
89150
}
90151

91152
// buildConfig converts a GenerationConfig to a Librarian Config.
92-
func buildConfig(gen *GenerationConfig) *config.Config {
153+
func buildConfig(gen *GenerationConfig, googleapisDir string) *config.Config {
93154
var libs []*config.Library
94155
for _, l := range gen.Libraries {
95156
name := l.LibraryName
96157
if name == "" {
97158
name = l.APIShortName
98159
}
99160
var apis []*config.API
161+
var javaAPIs []*config.JavaAPI
100162
for _, g := range l.GAPICs {
101-
if g.ProtoPath != "" {
102-
apis = append(apis, &config.API{Path: g.ProtoPath})
163+
if g.ProtoPath == "" {
164+
continue
165+
}
166+
apis = append(apis, &config.API{Path: g.ProtoPath})
167+
168+
info, err := parseJavaBazel(googleapisDir, g.ProtoPath)
169+
if err != nil {
170+
log.Printf("Warning: failed to parse BUILD.bazel for %s: %v", g.ProtoPath, err)
171+
continue
172+
}
173+
if info == nil {
174+
continue
175+
}
176+
javaAPI := &config.JavaAPI{
177+
Path: g.ProtoPath,
178+
NoRestNumericEnums: info.NoRestNumericEnums,
179+
AdditionalProtos: info.AdditionalProtos,
180+
NoSamples: info.NoSamples,
103181
}
182+
javaAPIs = append(javaAPIs, javaAPI)
104183
}
105184
libs = append(libs, &config.Library{
106185
Name: name,
@@ -119,6 +198,7 @@ func buildConfig(gen *GenerationConfig) *config.Config {
119198
ExcludedDependencies: l.ExcludedDependencies,
120199
ExcludedPoms: l.ExcludedPoms,
121200
ExtraVersionedModules: l.ExtraVersionedModules,
201+
JavaAPIs: javaAPIs,
122202
GroupID: l.GroupID,
123203
IssueTrackerOverride: l.IssueTracker,
124204
LibraryTypeOverride: l.LibraryType,
@@ -139,8 +219,7 @@ func buildConfig(gen *GenerationConfig) *config.Config {
139219
Language: "java",
140220
Default: &config.Default{},
141221
Sources: &config.Sources{
142-
// hardcoded for local testing
143-
Googleapis: &config.Source{Dir: "../../googleapis"},
222+
Googleapis: &config.Source{Dir: googleapisDir},
144223
},
145224
Libraries: libs,
146225
}

tool/cmd/migrate/java_test.go

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package main
1616

1717
import (
18+
"context"
1819
"errors"
1920
"os"
2021
"testing"
@@ -24,6 +25,13 @@ import (
2425
)
2526

2627
func TestRunJavaMigration(t *testing.T) {
28+
fetchSource = func(ctx context.Context) (*config.Source, error) {
29+
return &config.Source{
30+
Commit: "abcd123",
31+
SHA256: "sha123",
32+
Dir: "../../internal/testdata/googleapis",
33+
}, nil
34+
}
2735
for _, test := range []struct {
2836
name string
2937
repoPath string
@@ -88,7 +96,7 @@ func TestBuildConfig(t *testing.T) {
8896
Language: "java",
8997
Default: &config.Default{},
9098
Sources: &config.Sources{
91-
Googleapis: &config.Source{Dir: "../../googleapis"},
99+
Googleapis: &config.Source{Dir: "../../internal/testdata/googleapis"},
92100
},
93101
Libraries: []*config.Library{
94102
{
@@ -118,7 +126,7 @@ func TestBuildConfig(t *testing.T) {
118126
Language: "java",
119127
Default: &config.Default{},
120128
Sources: &config.Sources{
121-
Googleapis: &config.Source{Dir: "../../googleapis"},
129+
Googleapis: &config.Source{Dir: "../../internal/testdata/googleapis"},
122130
},
123131
Libraries: []*config.Library{
124132
{
@@ -154,7 +162,7 @@ func TestBuildConfig(t *testing.T) {
154162
Language: "java",
155163
Default: &config.Default{},
156164
Sources: &config.Sources{
157-
Googleapis: &config.Source{Dir: "../../googleapis"},
165+
Googleapis: &config.Source{Dir: "../../internal/testdata/googleapis"},
158166
},
159167
Libraries: []*config.Library{
160168
{
@@ -215,7 +223,7 @@ func TestBuildConfig(t *testing.T) {
215223
Language: "java",
216224
Default: &config.Default{},
217225
Sources: &config.Sources{
218-
Googleapis: &config.Source{Dir: "../../googleapis"},
226+
Googleapis: &config.Source{Dir: "../../internal/testdata/googleapis"},
219227
},
220228
Libraries: []*config.Library{
221229
{
@@ -254,7 +262,48 @@ func TestBuildConfig(t *testing.T) {
254262
},
255263
} {
256264
t.Run(test.name, func(t *testing.T) {
257-
got := buildConfig(test.gen)
265+
got := buildConfig(test.gen, "../../internal/testdata/googleapis")
266+
if diff := cmp.Diff(test.want, got); diff != "" {
267+
t.Errorf("mismatch (-want +got):\n%s", diff)
268+
}
269+
})
270+
}
271+
}
272+
273+
func TestParseJavaBazel(t *testing.T) {
274+
for _, test := range []struct {
275+
name string
276+
googleapisDir string
277+
buildPath string
278+
want *javaGAPICInfo
279+
}{
280+
{
281+
name: "success",
282+
googleapisDir: "testdata/parse-bazel/success",
283+
buildPath: "google/cloud/bigquery/analyticshub/v1",
284+
want: &javaGAPICInfo{
285+
NoRestNumericEnums: true,
286+
NoSamples: false,
287+
AdditionalProtos: []string{
288+
"google/cloud/common_resources.proto",
289+
},
290+
},
291+
},
292+
{
293+
name: "no GAPIC rules",
294+
googleapisDir: "testdata/parse-bazel/no-gapic-rule",
295+
want: &javaGAPICInfo{
296+
AdditionalProtos: []string{
297+
"google/cloud/common_resources.proto",
298+
},
299+
},
300+
},
301+
} {
302+
t.Run(test.name, func(t *testing.T) {
303+
got, err := parseJavaBazel(test.googleapisDir, test.buildPath)
304+
if err != nil {
305+
t.Fatal(err)
306+
}
258307
if diff := cmp.Diff(test.want, got); diff != "" {
259308
t.Errorf("mismatch (-want +got):\n%s", diff)
260309
}

tool/cmd/migrate/legacylibrarian.go

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ package main
1717
import (
1818
"cmp"
1919
"context"
20-
"errors"
2120
"fmt"
2221
"os"
2322
"path/filepath"
@@ -26,7 +25,6 @@ import (
2625
"sort"
2726
"strings"
2827

29-
"github.com/bazelbuild/buildtools/build"
3028
"github.com/googleapis/librarian/internal/config"
3129
"github.com/googleapis/librarian/internal/fetch"
3230
"github.com/googleapis/librarian/internal/legacylibrarian/legacyconfig"
@@ -283,7 +281,7 @@ func buildGoLibraries(input *MigrationInput) ([]*config.Library, error) {
283281
}
284282
// Read Go GAPIC configurations from BUILD.bazel.
285283
for _, api := range library.APIs {
286-
info, err := parseBazel(input.googleapisDir, api.Path)
284+
info, err := parseGoBazel(input.googleapisDir, api.Path)
287285
if err != nil {
288286
return nil, err
289287
}
@@ -379,28 +377,22 @@ func readRepoConfig(path string) (*RepoConfig, error) {
379377
return yaml.Read[RepoConfig](configFile)
380378
}
381379

382-
// parseBazel parses the BUILD.bazel file in the given directory to extract information from
380+
// parseGoBazel parses the BUILD.bazel file in the given directory to extract information from
383381
// the go_gapic_library rule.
384-
func parseBazel(googleapisDir, dir string) (*goGAPICInfo, error) {
385-
path := filepath.Join(googleapisDir, dir, "BUILD.bazel")
386-
if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
387-
// Skip Not Exist error for testing purpose.
388-
return nil, nil
389-
}
390-
data, err := os.ReadFile(path)
382+
func parseGoBazel(googleapisDir, dir string) (*goGAPICInfo, error) {
383+
file, err := parseBazel(googleapisDir, dir)
391384
if err != nil {
392385
return nil, err
393386
}
394-
file, err := build.ParseBuild(path, data)
395-
if err != nil {
396-
return nil, err
387+
if file == nil {
388+
return nil, nil
397389
}
398390
rules := file.Rules("go_gapic_library")
399391
if len(rules) == 0 {
400392
return &goGAPICInfo{DisableGAPIC: true}, nil
401393
}
402394
if len(rules) > 1 {
403-
return nil, fmt.Errorf("file %s contains multiple go_gapic_library rules", path)
395+
return nil, fmt.Errorf("%s/BUILD.bazel contains multiple go_gapic_library rules", dir)
404396
}
405397
rule := rules[0]
406398
importPath, clientPkg := parseImportPathFromBuild(rule.AttrString("importpath"))

tool/cmd/migrate/legacylibrarian_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ func TestParseBazel(t *testing.T) {
900900
},
901901
} {
902902
t.Run(test.name, func(t *testing.T) {
903-
got, err := parseBazel(test.googleapisDir, test.buildPath)
903+
got, err := parseGoBazel(test.googleapisDir, test.buildPath)
904904
if err != nil {
905905
t.Fatal(err)
906906
}
@@ -924,7 +924,7 @@ func TestParseBazel_Error(t *testing.T) {
924924
},
925925
} {
926926
t.Run(test.name, func(t *testing.T) {
927-
_, err := parseBazel("", test.dir)
927+
_, err := parseGoBazel("", test.dir)
928928
if err == nil {
929929
t.Fatalf("parseBazel(%q): expected error", test.dir)
930930
}

0 commit comments

Comments
 (0)