Skip to content

Commit b95e2d6

Browse files
ldetmerzhumin8
andauthored
chore: refactor release cmd so its source of truth is librarian.yaml not cargo files (#3316)
"release -all" was failing because it was trying to find releases for packages specified in cargo.toml file but then not finding them in librarian.yaml. Cargo.toml files should not be source of truth as it contains entries that don't map to a library, ex: [user-guide-samples](https://github.com/googleapis/google-cloud-rust/blob/c37cdf6e9c80acd9f1501e1d39714d31e4f07ac4/Cargo.lock#L7939). This refactor changes so that we use librarian.yaml as the source of truth for getting libraries to release. It moves this logic to librarian release.go file as this should not be language specific. As part of moving this logic I also migrated all release-all logic to librarian release.go, since this is also is not language specific. Example PR after running release-all: https://github.com/googleapis/google-cloud-rust/compare/main...ldetmer:google-cloud-rust:test-release?expand=1 Fixes #3308 --------- Signed-off-by: ldetmer <[email protected]> Co-authored-by: Min Zhu <[email protected]>
1 parent c142bed commit b95e2d6

File tree

5 files changed

+202
-173
lines changed

5 files changed

+202
-173
lines changed

internal/librarian/internal/rust/release.go

Lines changed: 40 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,16 @@ package rust
1717

1818
import (
1919
"errors"
20-
"fmt"
21-
"io/fs"
2220
"os"
2321
"path/filepath"
22+
"strings"
2423

2524
"github.com/googleapis/librarian/internal/config"
2625
"github.com/googleapis/librarian/internal/semver"
2726
rustrelease "github.com/googleapis/librarian/internal/sidekick/rust_release"
2827
"github.com/pelletier/go-toml/v2"
2928
)
3029

31-
var errLibraryNotFound = errors.New("library not found")
32-
3330
type cargoPackage struct {
3431
Name string `toml:"name"`
3532
Version string `toml:"version"`
@@ -39,85 +36,53 @@ type cargoManifest struct {
3936
Package *cargoPackage `toml:"package"`
4037
}
4138

42-
// ReleaseAll bumps versions for all Cargo.toml files and updates librarian.yaml.
43-
func ReleaseAll(cfg *config.Config) (*config.Config, error) {
44-
return release(cfg, "")
45-
}
46-
47-
// ReleaseLibrary bumps the version for a specific library and updates librarian.yaml.
48-
func ReleaseLibrary(cfg *config.Config, name string) (*config.Config, error) {
49-
return release(cfg, name)
50-
}
39+
var errCouldNotDeriveSrcPath = errors.New("could not derive source path for library")
5140

52-
func release(cfg *config.Config, name string) (*config.Config, error) {
53-
shouldRelease := func(pkgName string) bool {
54-
// If name is the empty string, release everything.
55-
if name == "" {
56-
return true
57-
}
58-
return name == pkgName
41+
// ReleaseLibrary bumps version for Cargo.toml files and updates librarian config version.
42+
func ReleaseLibrary(cfg *config.Config, library *config.Library) error {
43+
srcPath := deriveSrcPath(library, cfg)
44+
if srcPath == "" {
45+
return errCouldNotDeriveSrcPath
5946
}
60-
61-
var found bool
62-
err := filepath.WalkDir(".", func(path string, d fs.DirEntry, err error) error {
63-
if err != nil {
64-
return err
65-
}
66-
if d.IsDir() || d.Name() != "Cargo.toml" {
67-
return nil
68-
}
69-
contents, err := os.ReadFile(path)
70-
if err != nil {
71-
return err
72-
}
73-
74-
var manifest cargoManifest
75-
if err := toml.Unmarshal(contents, &manifest); err != nil {
76-
return err
77-
}
78-
if manifest.Package == nil {
79-
return nil
80-
}
81-
if !shouldRelease(manifest.Package.Name) {
82-
return nil
83-
}
84-
85-
found = true
86-
newVersion, err := semver.DeriveNextOptions{
87-
BumpVersionCore: true,
88-
DowngradePreGAChanges: true,
89-
}.DeriveNext(semver.Minor, manifest.Package.Version)
90-
if err != nil {
91-
return err
92-
}
93-
if err := rustrelease.UpdateCargoVersion(path, newVersion); err != nil {
94-
return err
95-
}
96-
library, err := libraryByName(cfg, manifest.Package.Name)
97-
if err != nil {
98-
return err
99-
}
100-
library.Version = newVersion
101-
return nil
102-
})
47+
cargoFile := filepath.Join(srcPath, "Cargo.toml")
48+
cargoContents, err := os.ReadFile(cargoFile)
10349
if err != nil {
104-
return nil, err
50+
return err
10551
}
106-
if name != "" && !found {
107-
return nil, fmt.Errorf("library %q not found", name)
52+
var manifest cargoManifest
53+
if err := toml.Unmarshal(cargoContents, &manifest); err != nil {
54+
return err
10855
}
109-
return cfg, nil
56+
if manifest.Package == nil {
57+
return err
58+
}
59+
newVersion, err := semver.DeriveNextOptions{
60+
BumpVersionCore: true,
61+
DowngradePreGAChanges: true,
62+
}.DeriveNext(semver.Minor, manifest.Package.Version)
63+
if err != nil {
64+
return err
65+
}
66+
if err := rustrelease.UpdateCargoVersion(cargoFile, newVersion); err != nil {
67+
return err
68+
}
69+
library.Version = newVersion
70+
return nil
11071
}
11172

112-
// libraryByName returns a library with the given name from the config.
113-
func libraryByName(c *config.Config, name string) (*config.Library, error) {
114-
if c.Libraries == nil {
115-
return nil, errLibraryNotFound
73+
func deriveSrcPath(libCfg *config.Library, cfg *config.Config) string {
74+
if libCfg.Output != "" {
75+
return libCfg.Output
11676
}
117-
for _, library := range c.Libraries {
118-
if library.Name == name {
119-
return library, nil
77+
libSrcDir := ""
78+
if len(libCfg.Channels) > 0 && libCfg.Channels[0].Path != "" {
79+
libSrcDir = libCfg.Channels[0].Path
80+
} else {
81+
libSrcDir = strings.ReplaceAll(libCfg.Name, "-", "/")
82+
if cfg.Default == nil {
83+
return ""
12084
}
12185
}
122-
return nil, errLibraryNotFound
86+
return DefaultOutput(libSrcDir, cfg.Default.Output)
87+
12388
}

internal/librarian/internal/rust/release_test.go

Lines changed: 60 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,12 @@
1515
package rust
1616

1717
import (
18-
"errors"
1918
"fmt"
2019
"os"
2120
"path/filepath"
2221
"strings"
2322
"testing"
2423

25-
"github.com/google/go-cmp/cmp"
2624
"github.com/googleapis/librarian/internal/config"
2725
"github.com/googleapis/librarian/internal/testhelpers"
2826
)
@@ -34,37 +32,23 @@ const (
3432
storageInitial = "1.0.0"
3533
storageReleased = "1.1.0"
3634

37-
secretmanagerDir = "src/secretmanager"
38-
secretmanagerCargo = "src/secretmanager/Cargo.toml"
39-
secretmanagerName = "google-cloud-secretmanager-v1"
40-
secretmanagerInitial = "1.5.3"
41-
secretmanagerReleased = "1.6.0"
35+
secretmanagerDir = "src/secretmanager"
36+
secretmanagerCargo = "src/secretmanager/Cargo.toml"
37+
secretmanagerName = "google-cloud-secretmanager-v1"
38+
secretmanagerInitial = "1.5.3"
4239
)
4340

44-
func TestReleaseAll(t *testing.T) {
45-
cfg := setupRelease(t)
46-
got, err := ReleaseAll(cfg)
47-
if err != nil {
48-
t.Fatal(err)
49-
}
50-
51-
checkCargoVersion(t, storageCargo, storageReleased)
52-
checkCargoVersion(t, secretmanagerCargo, secretmanagerReleased)
53-
checkLibraryVersion(t, got, storageName, storageReleased)
54-
checkLibraryVersion(t, got, secretmanagerName, secretmanagerReleased)
55-
}
56-
5741
func TestReleaseOne(t *testing.T) {
5842
cfg := setupRelease(t)
59-
got, err := ReleaseLibrary(cfg, storageName)
43+
err := ReleaseLibrary(cfg, cfg.Libraries[0])
6044
if err != nil {
6145
t.Fatal(err)
6246
}
6347

6448
checkCargoVersion(t, storageCargo, storageReleased)
6549
checkCargoVersion(t, secretmanagerCargo, secretmanagerInitial)
66-
checkLibraryVersion(t, got, storageName, storageReleased)
67-
checkLibraryVersion(t, got, secretmanagerName, secretmanagerInitial)
50+
checkLibraryVersion(t, cfg.Libraries[0], storageReleased)
51+
checkLibraryVersion(t, cfg.Libraries[1], secretmanagerInitial)
6852
}
6953

7054
func setupRelease(t *testing.T) *config.Config {
@@ -81,10 +65,12 @@ func setupRelease(t *testing.T) *config.Config {
8165
{
8266
Name: storageName,
8367
Version: storageInitial,
68+
Output: storageDir,
8469
},
8570
{
8671
Name: secretmanagerName,
8772
Version: secretmanagerInitial,
73+
Output: secretmanagerDir,
8874
},
8975
},
9076
}
@@ -120,71 +106,79 @@ func checkCargoVersion(t *testing.T, path, wantVersion string) {
120106
}
121107
}
122108

123-
func checkLibraryVersion(t *testing.T, cfg *config.Config, name, wantVersion string) {
109+
func checkLibraryVersion(t *testing.T, library *config.Library, wantVersion string) {
124110
t.Helper()
125-
for _, lib := range cfg.Libraries {
126-
if lib.Name == name {
127-
if lib.Version != wantVersion {
128-
t.Errorf("library %q version mismatch: want %q, got %q", name, wantVersion, lib.Version)
129-
}
130-
return
131-
}
111+
if library.Version != wantVersion {
112+
t.Errorf("library %q version mismatch: want %q, got %q", library.Name, wantVersion, library.Version)
132113
}
133-
t.Errorf("library %q not found in config", name)
134114
}
135115

136-
func TestLibraryByName(t *testing.T) {
116+
func TestDeriveSrcPath(t *testing.T) {
137117
for _, test := range []struct {
138-
name string
139-
libraryName string
140-
config *config.Config
141-
want *config.Library
142-
wantErr error
118+
name string
119+
config *config.Config
120+
want string
143121
}{
144122
{
145-
name: "find_a_library",
146-
libraryName: "example-library",
123+
name: "use library output",
147124
config: &config.Config{
125+
Default: &config.Default{
126+
Output: "ignored",
127+
},
148128
Libraries: []*config.Library{
149-
{Name: "example-library"},
150-
{Name: "another-library"},
129+
{Output: "src/lib/dir"},
151130
},
152131
},
153-
want: &config.Library{Name: "example-library"},
132+
want: "src/lib/dir",
154133
},
155134
{
156-
name: "no_library_in_config",
157-
libraryName: "example-library",
158-
config: &config.Config{},
159-
wantErr: errLibraryNotFound,
135+
name: "use channel path",
136+
config: &config.Config{
137+
Default: &config.Default{
138+
Output: "src/",
139+
},
140+
Libraries: []*config.Library{{
141+
Channels: []*config.Channel{
142+
{Path: "channel/dir"},
143+
},
144+
},
145+
},
146+
},
147+
want: "src/channel/dir",
160148
},
161149
{
162-
name: "does_not_find_a_library",
163-
libraryName: "non-existent-library",
150+
name: "use library name",
164151
config: &config.Config{
165-
Libraries: []*config.Library{
166-
{Name: "example-library"},
167-
{Name: "another-library"},
152+
Default: &config.Default{
153+
Output: "src/",
154+
},
155+
Libraries: []*config.Library{{
156+
Name: "lib-name",
157+
},
168158
},
169159
},
170-
wantErr: errLibraryNotFound,
160+
want: "src/lib/name",
171161
},
172162
} {
173163
t.Run(test.name, func(t *testing.T) {
174-
got, err := libraryByName(test.config, test.libraryName)
175-
if test.wantErr != nil {
176-
if !errors.Is(err, test.wantErr) {
177-
t.Errorf("got error %v, want %v", err, test.wantErr)
178-
}
179-
return
180-
}
181-
if err != nil {
182-
t.Errorf("libraryByName(%q): %v", test.libraryName, err)
183-
return
184-
}
185-
if diff := cmp.Diff(test.want, got); diff != "" {
186-
t.Errorf("mismatch (-want +got):\n%s", diff)
164+
got := deriveSrcPath(test.config.Libraries[0], test.config)
165+
if got != test.want {
166+
t.Errorf("got derived source path %s, wanted %s", got, test.want)
187167
}
188168
})
189169
}
190170
}
171+
172+
func TestInvalidDerivedSource(t *testing.T) {
173+
cfg := &config.Config{
174+
Libraries: []*config.Library{
175+
{
176+
Name: storageName,
177+
},
178+
},
179+
}
180+
err := ReleaseLibrary(cfg, cfg.Libraries[0])
181+
if err != errCouldNotDeriveSrcPath {
182+
t.Errorf("wanted error %v, got %v", errCouldNotDeriveSrcPath, err)
183+
}
184+
}

0 commit comments

Comments
 (0)