chore(internal/config): remove version map in librarian config (#3073)

Remove version map in librarian config, use `library.Version` instead.

Fixes #3070

Author: JoeWang1127

internal/config/config.go

@@ -17,12 +17,15 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"os"
 
 	"gopkg.in/yaml.v3"
 )
 
+var errLibraryNotFound = errors.New("library not found")
+
 // Config represents the complete librarian.yaml configuration file.
 type Config struct {
 	// Language is the primary language for this repository (go, python, rust).
@@ -39,13 +42,7 @@ type Config struct {
 
 	// Libraries contains configuration overrides for libraries that need special handling.
 	// Only include libraries that differ from defaults.
-	// Versions are looked up from the Versions map below.
 	Libraries []*Library `yaml:"libraries,omitempty"`
-
-	// Versions contains version numbers for all libraries.
-	// This is the source of truth for release versions.
-	// Key is library name, value is version string.
-	Versions map[string]string `yaml:"versions,omitempty"`
 }
 
 // Sources contains references to external source repositories.
@@ -220,3 +217,18 @@ func (c *Config) Write(path string) error {
 
 	return nil
 }
+
+// LibraryByName returns a library with the given name.
+func (c *Config) LibraryByName(name string) (*Library, error) {
+	if c.Libraries == nil {
+		return nil, errLibraryNotFound
+	}
+
+	for _, library := range c.Libraries {
+		if library.Name == name {
+			return library, nil
+		}
+	}
+
+	return nil, errLibraryNotFound
+}

internal/config/config_test.go

@@ -16,6 +16,7 @@ package config
 
 import (
 	"bytes"
+	"errors"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -47,3 +48,71 @@ func TestReadWrite(t *testing.T) {
 		t.Errorf("mismatch (-want +got):\n%s", diff)
 	}
 }
+
+func TestLibraryByName(t *testing.T) {
+	for _, test := range []struct {
+		name        string
+		libraryName string
+		config      *Config
+		want        *Library
+		wantErr     error
+	}{
+		{
+			name:        "find_a_library",
+			libraryName: "example-library",
+			config: &Config{
+				Libraries: []*Library{
+					{
+						Name: "example-library",
+					},
+					{
+						Name: "another-library",
+					},
+				},
+			},
+			want: &Library{
+				Name: "example-library",
+			},
+		},
+		{
+			name:        "no_library_in_config",
+			libraryName: "example-library",
+			config:      &Config{},
+			wantErr:     errLibraryNotFound,
+		},
+		{
+			name:        "does_not_find_a_library",
+			libraryName: "non-existent-library",
+			config: &Config{
+				Libraries: []*Library{
+					{
+						Name: "example-library",
+					},
+					{
+						Name: "another-library",
+					},
+				},
+			},
+			wantErr: errLibraryNotFound,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			got, err := test.config.LibraryByName(test.libraryName)
+			if test.wantErr != nil {
+				if !errors.Is(err, test.wantErr) {
+					t.Errorf("got error %v, want %v", err, test.wantErr)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Errorf("LibraryByName(%q): %v", test.libraryName, err)
+				return
+			}
+
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

internal/config/testdata/rust/librarian.yaml

@@ -40,12 +40,11 @@ libraries:
   - name: google-cloud-secretmanager-v1
     channels:
       - path: google/cloud/secretmanager/v1
+    version: 1.2.3
   - name: google-cloud-storage-v2
     channels:
       - path: google/cloud/storage/v2
     rust:
       disabled_rustdoc_warnings:
         - rustdoc::bare_urls
-versions:
-  google-cloud-secretmanager-v1: 0.1.0
-  google-cloud-storage-v2: 0.2.0
+    version: 2.3.4

internal/language/internal/rust/release.go

@@ -46,10 +46,6 @@ func ReleaseLibrary(cfg *config.Config, name string) (*config.Config, error) {
 }
 
 func release(cfg *config.Config, name string) (*config.Config, error) {
-	if cfg.Versions == nil {
-		cfg.Versions = make(map[string]string)
-	}
-
 	shouldRelease := func(pkgName string) bool {
 		// If name is the empty string, release everything.
 		if name == "" {
@@ -93,7 +89,11 @@ func release(cfg *config.Config, name string) (*config.Config, error) {
 		if err := rustrelease.UpdateCargoVersion(path, newVersion); err != nil {
 			return err
 		}
-		cfg.Versions[manifest.Package.Name] = newVersion
+		library, err := cfg.LibraryByName(manifest.Package.Name)
+		if err != nil {
+			return err
+		}
+		library.Version = newVersion
 		return nil
 	})
 	if err != nil {

internal/language/internal/rust/release_test.go

@@ -21,7 +21,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	cmdtest "github.com/googleapis/librarian/internal/command"
 	"github.com/googleapis/librarian/internal/config"
 )
@@ -49,13 +48,8 @@ func TestReleaseAll(t *testing.T) {
 
 	checkCargoVersion(t, storageCargo, storageReleased)
 	checkCargoVersion(t, secretmanagerCargo, secretmanagerReleased)
-	want := map[string]string{
-		storageName:       storageReleased,
-		secretmanagerName: secretmanagerReleased,
-	}
-	if diff := cmp.Diff(want, got.Versions); diff != "" {
-		t.Errorf("mismatch (-want +got):\n%s", diff)
-	}
+	checkLibraryVersion(t, got, storageName, storageReleased)
+	checkLibraryVersion(t, got, secretmanagerName, secretmanagerReleased)
 }
 
 func TestReleaseOne(t *testing.T) {
@@ -67,13 +61,8 @@ func TestReleaseOne(t *testing.T) {
 
 	checkCargoVersion(t, storageCargo, storageReleased)
 	checkCargoVersion(t, secretmanagerCargo, secretmanagerInitial)
-	want := map[string]string{
-		storageName:       storageReleased,
-		secretmanagerName: secretmanagerInitial,
-	}
-	if diff := cmp.Diff(want, got.Versions); diff != "" {
-		t.Errorf("mismatch (-want +got):\n%s", diff)
-	}
+	checkLibraryVersion(t, got, storageName, storageReleased)
+	checkLibraryVersion(t, got, secretmanagerName, secretmanagerInitial)
 }
 
 func setupRelease(t *testing.T) *config.Config {
@@ -86,9 +75,15 @@ func setupRelease(t *testing.T) *config.Config {
 	createCrate(t, storageDir, storageName, storageInitial)
 	createCrate(t, secretmanagerDir, secretmanagerName, secretmanagerInitial)
 	return &config.Config{
-		Versions: map[string]string{
-			storageName:       storageInitial,
-			secretmanagerName: secretmanagerInitial,
+		Libraries: []*config.Library{
+			{
+				Name:    storageName,
+				Version: storageInitial,
+			},
+			{
+				Name:    secretmanagerName,
+				Version: secretmanagerInitial,
+			},
 		},
 	}
 }
@@ -122,3 +117,16 @@ func checkCargoVersion(t *testing.T, path, wantVersion string) {
 		t.Errorf("%s version mismatch:\nwant line: %q\ngot:\n%s", path, wantLine, got)
 	}
 }
+
+func checkLibraryVersion(t *testing.T, cfg *config.Config, name, wantVersion string) {
+	t.Helper()
+	for _, lib := range cfg.Libraries {
+		if lib.Name == name {
+			if lib.Version != wantVersion {
+				t.Errorf("library %q version mismatch: want %q, got %q", name, wantVersion, lib.Version)
+			}
+			return
+		}
+	}
+	t.Errorf("library %q not found in config", name)
+}

internal/language/release_test.go

@@ -24,41 +24,64 @@ import (
 func TestReleaseAll(t *testing.T) {
 	cfg := &config.Config{
 		Language: "testhelper",
-		Versions: map[string]string{
-			"lib1": "0.1.0",
-			"lib2": "0.2.0",
+		Libraries: []*config.Library{
+			{
+				Name:    "lib-1",
+				Version: "0.1.0",
+			},
+			{
+				Name:    "lib-2",
+				Version: "0.2.0",
+			},
 		},
 	}
+
 	cfg, err := ReleaseAll(cfg)
 	if err != nil {
 		t.Fatal(err)
 	}
+	gotVersions := make(map[string]string)
+	for _, lib := range cfg.Libraries {
+		gotVersions[lib.Name] = lib.Version
+	}
 	want := map[string]string{
-		"lib1": TestReleaseVersion,
-		"lib2": TestReleaseVersion,
+		"lib-1": TestReleaseVersion,
+		"lib-2": TestReleaseVersion,
 	}
-	if diff := cmp.Diff(want, cfg.Versions); diff != "" {
+	if diff := cmp.Diff(want, gotVersions); diff != "" {
 		t.Errorf("mismatch (-want +got):\n%s", diff)
 	}
 }
 
 func TestReleaseLibrary(t *testing.T) {
 	cfg := &config.Config{
 		Language: "testhelper",
-		Versions: map[string]string{
-			"lib1": "0.1.0",
-			"lib2": "0.2.0",
+		Libraries: []*config.Library{
+			{
+				Name:    "lib-1",
+				Version: "0.1.0",
+			},
+			{
+				Name:    "lib-2",
+				Version: "0.2.0",
+			},
 		},
 	}
-	cfg, err := ReleaseLibrary(cfg, "lib1")
+
+	cfg, err := ReleaseLibrary(cfg, "lib-1")
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	gotVersions := make(map[string]string)
+	for _, lib := range cfg.Libraries {
+		gotVersions[lib.Name] = lib.Version
+	}
 	want := map[string]string{
-		"lib1": TestReleaseVersion,
-		"lib2": "0.2.0",
+		"lib-1": TestReleaseVersion,
+		"lib-2": "0.2.0",
 	}
-	if diff := cmp.Diff(want, cfg.Versions); diff != "" {
+	if diff := cmp.Diff(want, gotVersions); diff != "" {
 		t.Errorf("mismatch (-want +got):\n%s", diff)
 	}
 }

internal/language/testhelper.go

@@ -27,21 +27,20 @@ import (
 const TestReleaseVersion = "1.2.3"
 
 func testReleaseAll(cfg *config.Config) (*config.Config, error) {
-	if cfg.Versions == nil {
-		cfg.Versions = make(map[string]string)
-	}
-	for k := range cfg.Versions {
-		cfg.Versions[k] = TestReleaseVersion
+	for _, lib := range cfg.Libraries {
+		lib.Version = TestReleaseVersion
 	}
 	return cfg, nil
 }
 
 func testReleaseLibrary(cfg *config.Config, name string) (*config.Config, error) {
-	if cfg.Versions == nil {
-		cfg.Versions = make(map[string]string)
+	for _, lib := range cfg.Libraries {
+		if lib.Name == name {
+			lib.Version = TestReleaseVersion
+			return cfg, nil
+		}
 	}
-	cfg.Versions[name] = TestReleaseVersion
-	return cfg, nil
+	return nil, fmt.Errorf("library %q not found", name)
 }
 
 func testGenerate(library *config.Library) error {

internal/librarian/release_test.go

@@ -66,8 +66,9 @@ func TestReleaseCommand(t *testing.T) {
 
 			configPath := filepath.Join(tempDir, librarianConfigPath)
 			configContent := fmt.Sprintf(`language: testhelper
-versions:
-  %s: 0.1.0
+libraries:
+  - name: %s
+    version: 0.1.0
 `, testlib)
 			if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil {
 				t.Fatal(err)
@@ -86,7 +87,11 @@ versions:
 				if err != nil {
 					t.Fatal(err)
 				}
-				if diff := cmp.Diff(test.wantVersions, cfg.Versions); diff != "" {
+				gotVersions := make(map[string]string)
+				for _, lib := range cfg.Libraries {
+					gotVersions[lib.Name] = lib.Version
+				}
+				if diff := cmp.Diff(test.wantVersions, gotVersions); diff != "" {
 					t.Errorf("mismatch (-want +got):\n%s", diff)
 				}
 			}