Skip to content

Commit e90a5fe

Browse files
authored
fix(tool/cmd/migrate): resolve duplicate API paths and regex failures (#4445)
The regular expression used to parse base API paths from .OwlBot.yaml source patterns is updated to support a wider variety of directory structures. Previously, the regex assumed the presence of parentheses used for version matching (e.g. (v1|v1beta1)), causing failures when migrating APIs with specific unversioned directory paths like grafeas-nodejs. The parseOwlBotAPIPaths function has been rewritten to recursively walk the matched base path and inspect the nodejs_gapic_library rules in the BUILD.bazel files. It now explicitly matches the Bazel rule's package_name property to the Node.js library's package.json name, ensuring packages sharing the same base path (like google-cloud-monitoring and google-monitoring-dashboard) are assigned the correct API paths. Additionally, runNodejsMigration now wraps errors returned by librarian.RunTidyOnConfig to surface the exact cause of any librarian tidy failed errors.
1 parent b833f14 commit e90a5fe

1 file changed

Lines changed: 36 additions & 23 deletions

File tree

tool/cmd/migrate/nodejs.go

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,10 @@ type nodejsGapicInfo struct {
5858
}
5959

6060
// owlBotSourceRegex extracts the base API path from an .OwlBot.yaml
61-
// deep-copy-regex source pattern. The pattern is always of the form:
62-
// /some/path/(version-regex)/.*-nodejs.
63-
var owlBotSourceRegex = regexp.MustCompile(`^/(.+?)/\(`)
61+
// deep-copy-regex source pattern. The pattern is usually of the form:
62+
// /some/path/(version-regex)/.*-nodejs, or /some/path/[^/]+-nodejs,
63+
// or /some_path-nodejs.
64+
var owlBotSourceRegex = regexp.MustCompile(`^/(?:(.+?)/(?:\(|v\d|[^/]+-nodejs)|([^/]+)-nodejs)`)
6465

6566
func runNodejsMigration(ctx context.Context, repoPath string) error {
6667
src, err := fetchSource(ctx)
@@ -93,7 +94,7 @@ func runNodejsMigration(ctx context.Context, repoPath string) error {
9394
cfg.Sources.Googleapis.Dir = ""
9495

9596
if err := librarian.RunTidyOnConfig(ctx, cfg); err != nil {
96-
return errTidyFailed
97+
return fmt.Errorf("librarian tidy failed: %w", err)
9798
}
9899
return nil
99100
}
@@ -131,7 +132,7 @@ func buildNodejsLibraries(repoPath, googleapisDir string) ([]*config.Library, er
131132
if err != nil {
132133
return nil, fmt.Errorf("reading .OwlBot.yaml for %s: %w", libraryName, err)
133134
}
134-
apis, err := parseOwlBotAPIPaths(owlBot, googleapisDir)
135+
apis, err := parseOwlBotAPIPaths(owlBot, googleapisDir, pkgJSON.Name)
135136
if err != nil {
136137
return nil, fmt.Errorf("parsing API paths for %s: %w", libraryName, err)
137138
}
@@ -201,8 +202,9 @@ func readNodejsPackageJSON(path string) (*nodejsPackageJSON, error) {
201202

202203
// parseOwlBotAPIPaths extracts API paths from .OwlBot.yaml deep-copy-regex
203204
// source patterns by finding the base path and then discovering version
204-
// directories in googleapis that contain a nodejs_gapic_library rule.
205-
func parseOwlBotAPIPaths(owlBot *owlBotYAML, googleapisDir string) ([]*config.API, error) {
205+
// directories in googleapis that contain a nodejs_gapic_library rule matching
206+
// the provided npm package name.
207+
func parseOwlBotAPIPaths(owlBot *owlBotYAML, googleapisDir, pkgName string) ([]*config.API, error) {
206208
if len(owlBot.DeepCopyRegex) == 0 {
207209
return nil, nil
208210
}
@@ -213,32 +215,43 @@ func parseOwlBotAPIPaths(owlBot *owlBotYAML, googleapisDir string) ([]*config.AP
213215
return nil, fmt.Errorf("cannot parse API path from .OwlBot.yaml source: %q", source)
214216
}
215217
basePath := matches[1]
218+
if basePath == "" {
219+
basePath = matches[2]
220+
}
216221

217-
// Find version directories in googleapis.
222+
// Find version directories in googleapis by walking the base path.
218223
dir := filepath.Join(googleapisDir, basePath)
219-
entries, err := os.ReadDir(dir)
220-
if err != nil {
221-
return nil, fmt.Errorf("reading googleapis directory %s: %w", dir, err)
222-
}
223224
var apis []*config.API
224-
for _, entry := range entries {
225-
if !entry.IsDir() {
226-
continue
225+
err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error {
226+
if err != nil {
227+
return nil // ignore inaccessible directories
227228
}
228-
name := entry.Name()
229-
// Version directories start with "v".
230-
if !strings.HasPrefix(name, "v") {
231-
continue
229+
if d.IsDir() {
230+
return nil
231+
}
232+
if d.Name() != "BUILD.bazel" {
233+
return nil
234+
}
235+
apiDir := filepath.Dir(path)
236+
apiPath, err := filepath.Rel(googleapisDir, apiDir)
237+
if err != nil {
238+
return fmt.Errorf("getting relative path for %s: %w", apiDir, err)
232239
}
233-
// Check that a BUILD.bazel exists with a nodejs_gapic_library rule.
234-
apiPath := filepath.Join(basePath, name)
235240
info, err := parseBazelNodejsInfo(googleapisDir, apiPath)
236241
if err != nil {
237-
return nil, err
242+
return fmt.Errorf("parsing bazel info for %s: %w", apiPath, err)
238243
}
239-
if info != nil {
244+
if info == nil {
245+
return nil
246+
}
247+
// Match the npm package name.
248+
if info.packageName == pkgName {
240249
apis = append(apis, &config.API{Path: apiPath})
241250
}
251+
return nil
252+
})
253+
if err != nil {
254+
return nil, fmt.Errorf("walking googleapis directory %s: %w", dir, err)
242255
}
243256
sort.Slice(apis, func(i, j int) bool {
244257
return apis[i].Path < apis[j].Path

0 commit comments

Comments
 (0)