Skip to content

Commit 2c00fa1

Browse files
authored
refactor(internal/librarian/golang): refactor library cleaning logic (#4293)
This change refactors the Go library Clean operation to use a more explicit and targeted approach. Previously, the logic relied on a complex, recursive walk with a broad regex to identify generated files. The new implementation splits this into two focused functions, improving readability and reducing the risk of accidental deletion. Changes: - Refactored `Clean` logic: Replaced the generic clean function and generatedRegex with specialized helpers. - Added `cleanRootFiles`: Specifically manages the removal of standard root files like `README.md` and `internal/version.go`, respecting the keep list. - Added `cleanClientDirectory`: Iterates through API directories to remove generated Go client files (e.g., `_client.go`, `.pb.go`) based on explicit suffix matching. - Improved Snippet Cleaning: Updated snippet directory cleanup to be more direct. - Updated Tests: Refactored clean_test.go to align with the new logic, including more descriptive test cases and updated expectations for nested modules and API paths. For #3617 Fixes #4294
1 parent 5903063 commit 2c00fa1

2 files changed

Lines changed: 259 additions & 95 deletions

File tree

internal/librarian/golang/clean.go

Lines changed: 86 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,32 +20,33 @@ import (
2020
"io/fs"
2121
"os"
2222
"path/filepath"
23-
"regexp"
23+
"strings"
24+
"syscall"
2425

2526
"github.com/googleapis/librarian/internal/config"
2627
)
2728

2829
var (
29-
// generatedRegex defines patterns to identify files produced by the generator.
30-
// These patterns are essential to ensure the 'Clean' operation only removes
31-
// files that are known to be generated, protecting handwritten code or
32-
// configuration that may reside in the same directory.
33-
// TODO(https://github.com/googleapis/librarian/issues/4217): document each regex about
30+
rootFiles = []string{"README.md", "internal/version.go"}
31+
// TODO(https://github.com/googleapis/librarian/issues/4217), document each file about
3432
// what are matched and why it is necessary.
35-
generatedRegex = func() []*regexp.Regexp {
36-
prefix := `.*/(?:apiv(\d+).*/)?`
37-
return []*regexp.Regexp{
38-
regexp.MustCompile(prefix + `\.repo-metadata\.json$`),
39-
regexp.MustCompile(prefix + `(auxiliary(?:_go123)?|doc|operations)\.go$`),
40-
regexp.MustCompile(prefix + `.*_client\.go$`),
41-
regexp.MustCompile(prefix + `.*_client_example_go123_test\.go$`),
42-
regexp.MustCompile(prefix + `.*_client_example_test\.go$`),
43-
regexp.MustCompile(prefix + `gapic_metadata\.json$`),
44-
regexp.MustCompile(prefix + `helpers\.go$`),
45-
regexp.MustCompile(`.*pb/.*\.pb\.go$`),
46-
regexp.MustCompile(`(^|.*/)internal/generated/snippets/.*$`),
47-
}
48-
}()
33+
// Separate generated files to filename and filename suffix allow us to match
34+
// the files as accurate as possible.
35+
generatedClientFiles = []string{
36+
".repo-metadata.json",
37+
"auxiliary.go",
38+
"auxiliary_go123.go",
39+
"doc.go",
40+
"gapic_metadata.json",
41+
"helpers.go",
42+
"operations.go",
43+
}
44+
generatedClientFileSuffixes = []string{
45+
".pb.go",
46+
"_client.go",
47+
"_client_example_go123_test.go",
48+
"_client_example_test.go",
49+
}
4950
)
5051

5152
// Clean cleans up a Go library and its associated snippets.
@@ -55,15 +56,11 @@ func Clean(library *config.Library) error {
5556
if err != nil {
5657
return err
5758
}
58-
var nestedModule string
59-
if library.Go != nil {
60-
nestedModule = library.Go.NestedModule
61-
}
62-
if err := clean(libraryDir, nestedModule, keepSet); err != nil {
59+
60+
if err := cleanRootFiles(libraryDir, keepSet); err != nil {
6361
return err
6462
}
65-
snippetDir := snippetDirectory(library.Output, library.Name)
66-
if err := clean(snippetDir, nestedModule, nil); err != nil {
63+
if err := cleanClientDirectory(library, libraryDir, keepSet); err != nil {
6764
return err
6865
}
6966
return nil
@@ -87,7 +84,7 @@ func check(dir string, keep []string) (map[string]bool, error) {
8784
for _, k := range keep {
8885
path := filepath.Join(dir, k)
8986
if _, err := os.Stat(path); errors.Is(err, fs.ErrNotExist) {
90-
return nil, fmt.Errorf("keep file %q does not exist", k)
87+
return nil, fmt.Errorf("error keeping %s: %w", k, err)
9188
}
9289
// Effectively get a canonical relative path. While in most cases
9390
// this will be equal to k, it might not be - in particular,
@@ -102,36 +99,80 @@ func check(dir string, keep []string) (map[string]bool, error) {
10299
return keepSet, nil
103100
}
104101

105-
// clean recursively removes files in dir that are not in keepSet.
106-
// If nestedModule is non-empty, any directory with that name is skipped.
107-
func clean(dir, nestedModule string, keepSet map[string]bool) error {
108-
if _, err := os.Stat(dir); os.IsNotExist(err) {
102+
// cleanRootFiles removes predefined root files from the library directory unless
103+
// they are explicitly marked to be kept.
104+
func cleanRootFiles(libraryDir string, keepSet map[string]bool) error {
105+
for _, rootFile := range rootFiles {
106+
// Handwritten/veneer libraries may have handwritten root files, README.md for example,
107+
// defined in the keep list.
108+
// Skip cleaning these files.
109+
if keepSet[rootFile] {
110+
continue
111+
}
112+
rootFilePath := filepath.Join(libraryDir, rootFile)
113+
if err := os.Remove(rootFilePath); err != nil {
114+
if errors.Is(err, syscall.ENOENT) {
115+
// The file doesn't exist during deletion, it's fine to ignore this error.
116+
continue
117+
}
118+
return err
119+
}
120+
}
121+
return nil
122+
}
123+
124+
// cleanClientDirectory walks through each API directory in the library and
125+
// removes generated Go client files and snippets.
126+
func cleanClientDirectory(library *config.Library, libraryDir string, keepSet map[string]bool) error {
127+
for _, api := range library.APIs {
128+
goAPI := findGoAPI(library, api.Path)
129+
if goAPI == nil {
130+
return fmt.Errorf("could not find Go API associated with %s: %w", api.Path, errGoAPINotFound)
131+
}
132+
clientPath := filepath.Join(library.Output, goAPI.ImportPath)
133+
if err := cleanGeneratedClientFiles(clientPath, libraryDir, keepSet); err != nil {
134+
return err
135+
}
136+
snippetDir := snippetDirectory(library.Output, goAPI.ImportPath)
137+
if err := os.RemoveAll(snippetDir); err != nil {
138+
return err
139+
}
140+
}
141+
return nil
142+
}
143+
144+
func cleanGeneratedClientFiles(clientPath, libraryDir string, keepSet map[string]bool) error {
145+
// clientPath doesn't exist, which means this is a new library, skip cleaning.
146+
if _, err := os.Stat(clientPath); errors.Is(err, fs.ErrNotExist) {
109147
return nil
110148
}
111-
return filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
149+
return filepath.WalkDir(clientPath, func(path string, d fs.DirEntry, err error) error {
112150
if err != nil {
113151
return err
114152
}
115153
if d.IsDir() {
116-
if d.Name() == nestedModule {
117-
return fs.SkipDir
118-
}
119154
return nil
120155
}
121-
rel, err := filepath.Rel(dir, path)
156+
relPath, err := filepath.Rel(libraryDir, path)
122157
if err != nil {
123158
return err
124159
}
125-
isGenerated := false
126-
for _, re := range generatedRegex {
127-
if re.MatchString(path) {
128-
isGenerated = true
129-
break
160+
// Some libraries may have a non-generated file that has one of the suffixes in generatedClientFileSuffixes,
161+
// e.g., iam_policy_client.go.
162+
// These files will be listed in the keep configuration, so we need to check and potentially skip cleaning.
163+
if keepSet[relPath] {
164+
return nil
165+
}
166+
for _, file := range generatedClientFiles {
167+
if d.Name() == file {
168+
return os.Remove(path)
130169
}
131170
}
132-
if keepSet[rel] || !isGenerated {
133-
return nil
171+
for _, file := range generatedClientFileSuffixes {
172+
if strings.HasSuffix(filepath.Base(path), file) {
173+
return os.Remove(path)
174+
}
134175
}
135-
return os.Remove(path)
176+
return nil
136177
})
137178
}

0 commit comments

Comments
 (0)