Skip to content

Commit 1f8ee00

Browse files
authored
feat(add): handle Release Please config for google-cloud-node (#6569)
We had to manally add Release Please entry in the configuration files when we onboard a new library to google-cloud-node. googleapis/google-cloud-node#8693 is the example. Let's have Librarian to add the entries in the Release Please files. Python and Go already have the feature with the "bulk" files. Let's make the logic work for google-cloud-node's configuration files. They use the default names: release-please-config.json and .release-please-manifest.json. Note that when Librarian starts to touch the Release Please files, it sorts the JSON keys. googleapis/google-cloud-node#8777 is the outcome of an example invocation for the agentregistry package using the source tree before the package was introduced. The JSON keys are sorted. (The irrelevant changes in the mixin fields are due to recent change in Librarian for NodeJS.) Fixes #6528
1 parent 027103b commit 1f8ee00

3 files changed

Lines changed: 115 additions & 23 deletions

File tree

internal/librarian/add.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ func runAdd(ctx context.Context, cfg *config.Config, api string) error {
100100
if err != nil {
101101
return err
102102
}
103-
if cfg.Language == config.LanguageGo || cfg.Language == config.LanguagePython {
104-
if hasBulkReleasePleaseConfigs(".") {
103+
if cfg.Language == config.LanguageGo || cfg.Language == config.LanguagePython || cfg.Language == config.LanguageNodejs {
104+
if hasBulkReleasePleaseConfigs(".", cfg) {
105105
if err := syncToReleasePlease(".", cfg, name); err != nil {
106106
return err
107107
}

internal/librarian/release_please.go

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,33 @@ import (
3131
const (
3232
bulkManifestFile = ".release-please-bulk-manifest.json"
3333
bulkConfigFile = "release-please-bulk-config.json"
34+
defaultManifestFile = ".release-please-manifest.json"
35+
defaultConfigFile = "release-please-config.json"
3436
defaultReleasePleaseVersion = "0.0.0"
3537
)
3638

37-
func hasBulkReleasePleaseConfigs(dir string) bool {
38-
_, errM := os.Stat(filepath.Join(dir, bulkManifestFile))
39-
_, errC := os.Stat(filepath.Join(dir, bulkConfigFile))
39+
func hasBulkReleasePleaseConfigs(dir string, cfg *config.Config) bool {
40+
manifestFile, configFile := releasePleaseFiles(cfg)
41+
_, errM := os.Stat(filepath.Join(dir, manifestFile))
42+
_, errC := os.Stat(filepath.Join(dir, configFile))
4043
return !errors.Is(errM, fs.ErrNotExist) && !errors.Is(errC, fs.ErrNotExist)
4144
}
4245

46+
// releasePleaseFiles returns the file names for the Release Please manifest file
47+
// and config file in this order, depending on the SDK language.
48+
func releasePleaseFiles(cfg *config.Config) (string, string) {
49+
// google-cloud-node uses the default Release Please files to add a new library.
50+
// google-cloud-python and google-cloud-go use the "-bulk-" files.
51+
manifestFile := bulkManifestFile
52+
configFile := bulkConfigFile
53+
if cfg.Language == config.LanguageNodejs {
54+
// google-cloud-node uses the default files
55+
manifestFile = defaultManifestFile
56+
configFile = defaultConfigFile
57+
}
58+
return manifestFile, configFile
59+
}
60+
4361
// syncToReleasePlease updates the release-please configuration files with the
4462
// onboarded library's package name, initial version, and language-specific
4563
// extra files to track for release version bumps.
@@ -49,27 +67,29 @@ func syncToReleasePlease(dir string, cfg *config.Config, name string) error {
4967
return err
5068
}
5169

52-
manifestPath := filepath.Join(dir, bulkManifestFile)
70+
manifestFile, configFile := releasePleaseFiles(cfg)
71+
manifestPath := filepath.Join(dir, manifestFile)
5372
manifest, err := readJSONFile[map[string]string](manifestPath)
5473
if err != nil {
55-
return fmt.Errorf("failed to read bulk manifest file: %w", err)
74+
return fmt.Errorf("failed to read manifest file: %w", err)
5675
}
5776
if manifest == nil {
5877
manifest = make(map[string]string)
5978
}
6079

61-
configPath := filepath.Join(dir, bulkConfigFile)
80+
configPath := filepath.Join(dir, configFile)
6281
bulkConfig, err := readJSONFile[map[string]any](configPath)
6382
if err != nil {
64-
return fmt.Errorf("failed to read bulk config file: %w", err)
83+
return fmt.Errorf("failed to read config file: %w", err)
6584
}
6685
if bulkConfig == nil {
6786
bulkConfig = make(map[string]any)
6887
}
6988
packagesRaw, pkgsExist := bulkConfig["packages"]
7089
packages, isMap := packagesRaw.(map[string]any)
7190
if pkgsExist && !isMap {
72-
return fmt.Errorf("'packages' in bulk config is not an object: %v", packagesRaw)
91+
return fmt.Errorf("'packages' in %s is not an object: %v",
92+
configPath, packagesRaw)
7393
}
7494
if !isMap || packages == nil {
7595
packages = make(map[string]any)
@@ -78,12 +98,22 @@ func syncToReleasePlease(dir string, cfg *config.Config, name string) error {
7898

7999
var extraFiles []any
80100
pkgPath := lib.Name
81-
if cfg.Language == config.LanguagePython {
101+
switch cfg.Language {
102+
case config.LanguagePython:
82103
pkgPath = python.ReleasePleasePkgPrefix + lib.Name
83104
extraFiles = python.ReleasePleaseExtraFiles(lib)
105+
case config.LanguageNodejs:
106+
pkgPath = "packages/" + lib.Name
84107
}
85108

86-
if err := syncPackageToReleasePlease(manifest, packages, pkgPath, lib.Version, lib.Name, extraFiles); err != nil {
109+
component := lib.Name
110+
if cfg.Language == config.LanguageNodejs {
111+
// google-cloud-node does not need to override
112+
// component value in package.
113+
component = ""
114+
}
115+
116+
if err := syncPackageToReleasePlease(manifest, packages, pkgPath, lib.Version, component, extraFiles); err != nil {
87117
return err
88118
}
89119

@@ -140,7 +170,11 @@ func syncPackageToReleasePlease(manifest map[string]string, packages map[string]
140170
packages[pkgPath] = pkgCfg
141171
}
142172

143-
pkgCfg["component"] = component
173+
if component != "" {
174+
// Python and Go set component names for packages in the config file.
175+
// NodeJS does not do this and passes an empty string in the argument.
176+
pkgCfg["component"] = component
177+
}
144178

145179
if len(extraFiles) > 0 {
146180
var existing []any

internal/librarian/release_please_test.go

Lines changed: 68 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,50 +27,88 @@ import (
2727
func TestHasBulkReleasePleaseConfigs(t *testing.T) {
2828
for _, test := range []struct {
2929
name string
30+
language string
3031
createConfig bool
3132
createManifest bool
3233
want bool
3334
}{
3435
{
35-
name: "both missing",
36+
name: "both missing (Go)",
37+
language: config.LanguageGo,
3638
createConfig: false,
3739
createManifest: false,
3840
want: false,
3941
},
4042
{
41-
name: "config missing",
43+
name: "config missing (Go)",
44+
language: config.LanguageGo,
4245
createConfig: false,
4346
createManifest: true,
4447
want: false,
4548
},
4649
{
47-
name: "manifest missing",
50+
name: "manifest missing (Go)",
51+
language: config.LanguageGo,
4852
createConfig: true,
4953
createManifest: false,
5054
want: false,
5155
},
5256
{
53-
name: "both exist",
57+
name: "both exist (Go)",
58+
language: config.LanguageGo,
59+
createConfig: true,
60+
createManifest: true,
61+
want: true,
62+
},
63+
{
64+
name: "both missing (Nodejs)",
65+
language: config.LanguageNodejs,
66+
createConfig: false,
67+
createManifest: false,
68+
want: false,
69+
},
70+
{
71+
name: "config missing (Nodejs)",
72+
language: config.LanguageNodejs,
73+
createConfig: false,
74+
createManifest: true,
75+
want: false,
76+
},
77+
{
78+
name: "manifest missing (Nodejs)",
79+
language: config.LanguageNodejs,
80+
createConfig: true,
81+
createManifest: false,
82+
want: false,
83+
},
84+
{
85+
name: "both exist (Nodejs)",
86+
language: config.LanguageNodejs,
5487
createConfig: true,
5588
createManifest: true,
5689
want: true,
5790
},
5891
} {
5992
t.Run(test.name, func(t *testing.T) {
6093
tmp := t.TempDir()
94+
manifestFile, configFile := releasePleaseFiles(
95+
&config.Config{
96+
Language: test.language,
97+
},
98+
)
6199
if test.createConfig {
62-
if err := os.WriteFile(filepath.Join(tmp, "release-please-bulk-config.json"), []byte("{}"), 0644); err != nil {
100+
if err := os.WriteFile(filepath.Join(tmp, configFile), []byte("{}"), 0644); err != nil {
63101
t.Fatal(err)
64102
}
65103
}
66104
if test.createManifest {
67-
if err := os.WriteFile(filepath.Join(tmp, ".release-please-bulk-manifest.json"), []byte("{}"), 0644); err != nil {
105+
if err := os.WriteFile(filepath.Join(tmp, manifestFile), []byte("{}"), 0644); err != nil {
68106
t.Fatal(err)
69107
}
70108
}
71-
got := hasBulkReleasePleaseConfigs(tmp)
109+
got := hasBulkReleasePleaseConfigs(tmp, &config.Config{Language: test.language})
72110
if got != test.want {
73-
t.Errorf("hasBulkReleasePleaseConfigs(%s) = %t, want %t", tmp, got, test.want)
111+
t.Errorf("hasBulkReleasePleaseConfigs(%s, %s) = %t, want %t", tmp, test.language, got, test.want)
74112
}
75113
})
76114
}
@@ -101,6 +139,21 @@ func TestSyncToReleasePlease(t *testing.T) {
101139
wantManifest: `{"secretmanager":"1.0.0"}`,
102140
wantConfig: `{"packages":{"secretmanager":{"component":"secretmanager"}}}`,
103141
},
142+
{
143+
name: "new nodejs library",
144+
language: config.LanguageNodejs,
145+
initialManifest: `{}`,
146+
initialConfig: `{"packages": {}}`,
147+
library: &config.Library{
148+
Name: "google-cloud-secretmanager",
149+
Version: "1.0.0",
150+
APIs: []*config.API{
151+
{Path: "google/cloud/secretmanager/v1"},
152+
},
153+
},
154+
wantManifest: `{"packages/google-cloud-secretmanager":"1.0.0"}`,
155+
wantConfig: `{"packages":{"packages/google-cloud-secretmanager":{}}}`,
156+
},
104157

105158
{
106159
name: "new python library",
@@ -258,8 +311,13 @@ func TestSyncToReleasePlease(t *testing.T) {
258311
} {
259312
t.Run(test.name, func(t *testing.T) {
260313
tmp := t.TempDir()
261-
manifestPath := filepath.Join(tmp, ".release-please-bulk-manifest.json")
262-
configPath := filepath.Join(tmp, "release-please-bulk-config.json")
314+
manifestFile, configFile := releasePleaseFiles(
315+
&config.Config{
316+
Language: test.language,
317+
},
318+
)
319+
manifestPath := filepath.Join(tmp, manifestFile)
320+
configPath := filepath.Join(tmp, configFile)
263321
if err := os.WriteFile(manifestPath, []byte(test.initialManifest), 0644); err != nil {
264322
t.Fatal(err)
265323
}

0 commit comments

Comments
 (0)