fix: save old version in release (#1953)

Run command:
```
go run ./cmd/librarian release init -repo=. -push
```

Release pull request created in
#1955

The release note is not well formatted, will address in
#1964.

Fixes #1932

Author: JoeWang1127

internal/config/state.go

@@ -19,6 +19,8 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+
+	"github.com/googleapis/librarian/internal/conventionalcommits"
 )
 
 const (
@@ -106,11 +108,13 @@ type LibraryState struct {
 	LastGeneratedCommit string `yaml:"last_generated_commit" json:"last_generated_commit"`
 	// The changes from the language repository since the library was last released.
 	// This field is ignored when writing to state.yaml.
-	Changes []*Change `yaml:"-" json:"changes,omitempty"`
+	Changes []*conventionalcommits.ConventionalCommit `yaml:"-" json:"changes,omitempty"`
 	// A list of APIs that are part of this library.
 	APIs []*API `yaml:"apis" json:"apis"`
 	// A list of directories in the language repository where Librarian contributes code.
 	SourceRoots []string `yaml:"source_roots" json:"source_roots"`
+	// The previous release version, this field is only for bookkeeping.
+	PreviousVersion string `yaml:"-" json:"-"`
 	// A list of regular expressions for files and directories to preserve during the copy and remove process.
 	PreserveRegex []string `yaml:"preserve_regex" json:"preserve_regex"`
 	// A list of regular expressions for files and directories to remove before copying generated code.
@@ -223,20 +227,6 @@ func (a *API) Validate() error {
 	return nil
 }
 
-// Change represents the changelog of a library.
-type Change struct {
-	// The type of the change, should be one of the conventional type.
-	Type string `yaml:"type" json:"type"`
-	// The summary of the change.
-	Subject string `yaml:"subject" json:"subject"`
-	// The body of the change.
-	Body string `yaml:"body" json:"body"`
-	// The Changelist number in piper associated with this change.
-	ClNum string `yaml:"piper_cl_number" json:"piper_cl_number"`
-	// The commit hash in the source repository associated with this change.
-	CommitHash string `yaml:"source_commit_hash" json:"source_commit_hash"`
-}
-
 // invalidPathChars contains characters that are invalid in path components,
 // plus path separators and the null byte.
 const invalidPathChars = "<>:\"|?*/\\\x00"

internal/librarian/command.go

@@ -58,9 +58,9 @@ type commitInfo struct {
 	ghClient          GitHubClient
 	idToCommits       map[string]string
 	failedLibraries   []string
+	pullRequestLabels []string
 	commitMessage     string
 	prType            string
-	pullRequestLabels []string
 }
 
 type commandRunner struct {
@@ -400,11 +400,11 @@ func commitAndPush(ctx context.Context, info *commitInfo) error {
 // Passing in `nil` for labels will no-op and an empty list for labels will clear all labels on the PR.
 // TODO: Consolidate the params to a potential PullRequestInfo struct.
 func addLabelsToPullRequest(ctx context.Context, ghClient GitHubClient, pullRequestLabels []string, prMetadata *github.PullRequestMetadata) error {
-	// Do not update if there are aren't labels provided
+	// Do not update if there aren't labels provided
 	if pullRequestLabels == nil {
 		return nil
 	}
-	// Github API treats Issues and Pull Request the same
+	// GitHub API treats Issues and Pull Request the same
 	// https://docs.github.com/en/rest/issues/labels#add-labels-to-an-issue
 	if err := ghClient.AddLabelsToIssue(ctx, prMetadata.Repo, prMetadata.Number, pullRequestLabels); err != nil {
 		return fmt.Errorf("failed to add labels to pull request: %w", err)

internal/librarian/commit_version_analyzer_test.go

@@ -177,7 +177,6 @@ func TestGetConventionalCommitsSinceLastRelease(t *testing.T) {
 					Footers:     make(map[string]string),
 				},
 			},
-			wantErr: false,
 		},
 		{
 			name: "GetCommitsForPathsSinceTag error",

internal/librarian/release_init.go

@@ -21,19 +21,13 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/googleapis/librarian/internal/conventionalcommits"
-
 	"github.com/googleapis/librarian/internal/docker"
 
 	"github.com/googleapis/librarian/internal/cli"
 	"github.com/googleapis/librarian/internal/config"
 	"github.com/googleapis/librarian/internal/gitrepo"
 )
 
-const (
-	KeyClNum = "PiperOrigin-RevId"
-)
-
 // cmdInit is the command for the `release init` subcommand.
 var cmdInit = &cli.Command{
 	Short:     "init initiates a release by creating a release pull request.",
@@ -84,14 +78,14 @@ func newInitRunner(cfg *config.Config) (*initRunner, error) {
 	}
 	return &initRunner{
 		cfg:             runner.cfg,
-		workRoot:        runner.workRoot,
 		repo:            runner.repo,
-		partialRepo:     filepath.Join(runner.workRoot, "release-init"),
 		state:           runner.state,
 		librarianConfig: runner.librarianConfig,
-		image:           runner.image,
 		ghClient:        runner.ghClient,
 		containerClient: runner.containerClient,
+		workRoot:        runner.workRoot,
+		partialRepo:     filepath.Join(runner.workRoot, "release-init"),
+		image:           runner.image,
 	}, nil
 }
 
@@ -113,7 +107,7 @@ func (r *initRunner) run(ctx context.Context) error {
 		commitMessage: "chore: create a release",
 		prType:        release,
 		// Newly created PRs from the `release init` command should have a
-		// `release:pending` Github tab to be tracked for release.
+		// `release:pending` GitHub tab to be tracked for release.
 		pullRequestLabels: []string{"release:pending"},
 	}
 	if err := commitAndPush(ctx, commitInfo); err != nil {
@@ -128,15 +122,16 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
 	if err := os.MkdirAll(dst, 0755); err != nil {
 		return fmt.Errorf("failed to make directory: %w", err)
 	}
-	src := r.repo.GetDir()
 
+	src := r.repo.GetDir()
 	for _, library := range r.state.Libraries {
 		if r.cfg.Library != "" {
 			if r.cfg.Library != library.ID {
 				continue
 			}
+
 			// Only update one library with the given library ID.
-			if err := updateLibrary(r.repo, library, r.cfg.LibraryVersion); err != nil {
+			if err := r.updateLibrary(library); err != nil {
 				return err
 			}
 			if err := copyLibrary(dst, src, library); err != nil {
@@ -147,7 +142,7 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
 		}
 
 		// Update all libraries.
-		if err := updateLibrary(r.repo, library, r.cfg.LibraryVersion); err != nil {
+		if err := r.updateLibrary(library); err != nil {
 			return err
 		}
 		if err := copyLibrary(dst, src, library); err != nil {
@@ -201,24 +196,28 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
 
 // updateLibrary updates the given library in the following way:
 //
-// 1. Get the library's commit history in the given git repository.
+// 1. Update the library's previous version.
 //
-// 2. Override the library version if libraryVersion is not empty.
+// 2. Get the library's commit history in the given git repository.
 //
-// 3. Set the library's release trigger to true.
-func updateLibrary(repo gitrepo.Repository, library *config.LibraryState, libraryVersion string) error {
-	commits, err := GetConventionalCommitsSinceLastRelease(repo, library)
+// 3. Override the library version if libraryVersion is not empty.
+//
+// 4. Set the library's release trigger to true.
+func (r *initRunner) updateLibrary(library *config.LibraryState) error {
+	// Update the previous version, we need this value when creating release note.
+	library.PreviousVersion = library.Version
+	commits, err := GetConventionalCommitsSinceLastRelease(r.repo, library)
 	if err != nil {
 		return fmt.Errorf("failed to fetch conventional commits for library, %s: %w", library.ID, err)
 	}
 
-	library.Changes = coerceLibraryChanges(commits)
+	library.Changes = commits
 	if len(library.Changes) == 0 {
 		slog.Info("Skip releasing library since no eligible change is found", "library", library.ID)
 		return nil
 	}
 
-	nextVersion, err := NextVersion(commits, library.Version, libraryVersion)
+	nextVersion, err := NextVersion(commits, library.Version, r.cfg.LibraryVersion)
 	if err != nil {
 		return err
 	}
@@ -229,38 +228,6 @@ func updateLibrary(repo gitrepo.Repository, library *config.LibraryState, librar
 	return nil
 }
 
-func coerceLibraryChanges(commits []*conventionalcommits.ConventionalCommit) []*config.Change {
-	changes := make([]*config.Change, 0)
-	for _, commit := range commits {
-		clNum := ""
-		if cl, ok := commit.Footers[KeyClNum]; ok {
-			clNum = cl
-		}
-
-		changeType := getChangeType(commit)
-		changes = append(changes, &config.Change{
-			Type:       changeType,
-			Subject:    commit.Description,
-			Body:       commit.Body,
-			ClNum:      clNum,
-			CommitHash: commit.SHA,
-		})
-	}
-
-	return changes
-}
-
-// getChangeType gets the type of the commit, adding an escalation mark (!) if
-// it is a breaking change.
-func getChangeType(commit *conventionalcommits.ConventionalCommit) string {
-	changeType := commit.Type
-	if commit.IsBreaking {
-		changeType = changeType + "!"
-	}
-
-	return changeType
-}
-
 // copyGlobalAllowlist copies files in the global file allowlist excluding
 //
 //	read-only files and copies global files from src.

internal/librarian/release_init_test.go

@@ -22,6 +22,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/googleapis/librarian/internal/conventionalcommits"
+
 	"github.com/go-git/go-git/v5"
 
 	"github.com/googleapis/librarian/internal/gitrepo"
@@ -467,22 +469,26 @@ func TestUpdateLibrary(t *testing.T) {
 				},
 			},
 			want: &config.LibraryState{
-				ID:      "one-id",
-				Version: "2.0.0",
+				ID:              "one-id",
+				Version:         "2.0.0",
+				PreviousVersion: "1.2.3",
 				SourceRoots: []string{
 					"one/path",
 					"two/path",
 				},
-				Changes: []*config.Change{
+				Changes: []*conventionalcommits.ConventionalCommit{
 					{
-						Type:    "fix",
-						Subject: "change a typo",
+						Type:        "fix",
+						Description: "change a typo",
+						LibraryID:   "one-id",
+						Footers:     map[string]string{},
 					},
 					{
-						Type:    "feat",
-						Subject: "add a config file",
-						Body:    "This is the body.",
-						ClNum:   "12345",
+						Type:        "feat",
+						Description: "add a config file",
+						Body:        "This is the body.",
+						LibraryID:   "one-id",
+						Footers:     map[string]string{"PiperOrigin-RevId": "12345"},
 					},
 				},
 				ReleaseTriggered: true,
@@ -516,21 +522,30 @@ func TestUpdateLibrary(t *testing.T) {
 				},
 			},
 			want: &config.LibraryState{
-				ID:      "one-id",
-				Version: "2.0.0",
+				ID:              "one-id",
+				Version:         "2.0.0",
+				PreviousVersion: "1.2.3",
 				SourceRoots: []string{
 					"one/path",
 					"two/path",
 				},
-				Changes: []*config.Change{
+				Changes: []*conventionalcommits.ConventionalCommit{
 					{
-						Type:    "feat!",
-						Subject: "add another config file",
-						Body:    "This is the body",
+						Type:        "feat",
+						Description: "add another config file",
+						Body:        "This is the body",
+						LibraryID:   "one-id",
+						Footers: map[string]string{
+							"BREAKING CHANGE": "this is a breaking change",
+						},
+						IsBreaking: true,
 					},
 					{
-						Type:    "feat!",
-						Subject: "change a typo",
+						Type:        "feat",
+						Description: "change a typo",
+						LibraryID:   "one-id",
+						Footers:     map[string]string{},
+						IsBreaking:  true,
 					},
 				},
 				ReleaseTriggered: true,
@@ -554,12 +569,19 @@ func TestUpdateLibrary(t *testing.T) {
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
+			r := &initRunner{
+				cfg: &config.Config{
+					LibraryVersion: test.libraryVersion,
+				},
+				repo: test.repo,
+			}
 			var err error
 			if test.repo != nil {
-				err = updateLibrary(test.repo, test.library, test.libraryVersion)
+				err = r.updateLibrary(test.library)
 			} else {
 				repo := setupRepoForGetCommits(t, test.pathAndMessages, test.tags)
-				err = updateLibrary(repo, test.library, test.libraryVersion)
+				r.repo = repo
+				err = r.updateLibrary(test.library)
 			}
 
 			if test.wantErr {
@@ -577,7 +599,7 @@ func TestUpdateLibrary(t *testing.T) {
 			if err != nil {
 				t.Errorf("failed to run getChangesOf(): %q", err.Error())
 			}
-			if diff := cmp.Diff(test.want, test.library, cmpopts.IgnoreFields(config.Change{}, "CommitHash")); diff != "" {
+			if diff := cmp.Diff(test.want, test.library, cmpopts.IgnoreFields(conventionalcommits.ConventionalCommit{}, "SHA", "When")); diff != "" {
 				t.Errorf("state mismatch (-want +got):\n%s", diff)
 			}
 		})

internal/librarian/release_notes.go

@@ -274,19 +274,14 @@ func formatLibraryReleaseNotes(repo gitrepo.Repository, library *config.LibraryS
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch github repo from remote: %w", err)
 	}
-	previousTag := formatTag(library, "")
-	commits, err := GetConventionalCommitsSinceLastRelease(repo, library)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get conventional commits for library %s: %w", library.ID, err)
-	}
-	newVersion, err := NextVersion(commits, library.Version, "")
-	if err != nil {
-		return nil, fmt.Errorf("failed to get next version for library %s: %w", library.ID, err)
-	}
+
+	// The version should already be updated to the next version.
+	newVersion := library.Version
 	newTag := formatTag(library, newVersion)
+	previousTag := formatTag(library, library.PreviousVersion)
 
 	commitsByType := make(map[string][]*conventionalcommits.ConventionalCommit)
-	for _, commit := range commits {
+	for _, commit := range library.Changes {
 		commitsByType[commit.Type] = append(commitsByType[commit.Type], commit)
 	}