fix: ignore version override if library flag is not specified (#1822)

Fixes #1823

Author: JoeWang1127

internal/librarian/release_init.go

@@ -125,9 +125,11 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
 				continue
 			}
 			// Only update one library with the given library ID.
-			if err := updateLibrary(r, r.state, i); err != nil {
+			updatedLibrary, err := updateLibrary(r.repo, library, r.cfg.LibraryVersion)
+			if err != nil {
 				return err
 			}
+			r.state.Libraries[i] = updatedLibrary
 			if err := copyLibrary(dst, src, library); err != nil {
 				return err
 			}
@@ -136,9 +138,11 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
 		}
 
 		// Update all libraries.
-		if err := updateLibrary(r, r.state, i); err != nil {
+		updatedLibrary, err := updateLibrary(r.repo, library, r.cfg.LibraryVersion)
+		if err != nil {
 			return err
 		}
+		r.state.Libraries[i] = updatedLibrary
 		if err := copyLibrary(dst, src, library); err != nil {
 			return err
 		}
@@ -188,28 +192,25 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
 	return cleanAndCopyGlobalAllowlist(r.librarianConfig, r.repo.GetDir(), outputDir)
 }
 
-// updateLibrary updates the library which is the index-th library in the given
-// [config.LibrarianState].
-func updateLibrary(r *initRunner, state *config.LibrarianState, index int) error {
-	library := state.Libraries[index]
-	updatedLibrary, err := getChangesOf(r.repo, library)
+// updateLibrary updates the given library in the following way:
+//
+// 1. Get the library's commit history in the given git repository.
+//
+// 2. Override the library version if libraryVersion is not empty.
+//
+// 3. Set the library's release trigger to true.
+func updateLibrary(repo gitrepo.Repository, library *config.LibraryState, libraryVersion string) (*config.LibraryState, error) {
+	updatedLibrary, err := getChangesOf(repo, library)
 	if err != nil {
-		return fmt.Errorf("failed to update library, %s: %w", library.ID, err)
+		return nil, fmt.Errorf("failed to update library, %s: %w", library.ID, err)
 	}
 
-	setReleaseTrigger(updatedLibrary, r.cfg.LibraryVersion, true)
-	state.Libraries[index] = updatedLibrary
-
-	return nil
-}
-
-// setReleaseTrigger sets the release trigger for the given library and
-// overrides the version, if provided.
-func setReleaseTrigger(library *config.LibraryState, libraryVersion string, trigger bool) {
 	if libraryVersion != "" {
-		library.Version = libraryVersion
+		updatedLibrary.Version = libraryVersion
 	}
-	library.ReleaseTriggered = trigger
+	updatedLibrary.ReleaseTriggered = true
+
+	return updatedLibrary, nil
 }
 
 // getChangesOf gets commit history of the given library.

internal/librarian/release_init_test.go

@@ -385,61 +385,14 @@ func TestInitRun(t *testing.T) {
 	}
 }
 
-func TestSetReleaseTrigger(t *testing.T) {
-	t.Parallel()
-	for _, test := range []struct {
-		name           string
-		library        *config.LibraryState
-		libraryID      string
-		libraryVersion string
-		trigger        bool
-		want           *config.LibraryState
-	}{
-		{
-			name: "set trigger for a library",
-			library: &config.LibraryState{
-				ID:      "one-example-id",
-				Version: "1.0.0",
-			},
-			trigger: true,
-			want: &config.LibraryState{
-				ID:               "one-example-id",
-				Version:          "1.0.0",
-				ReleaseTriggered: true,
-			},
-		},
-		{
-			name: "set trigger for one library and override version",
-			library: &config.LibraryState{
-				ID:      "one-example-id",
-				Version: "1.0.0",
-			},
-			trigger:        true,
-			libraryVersion: "2.0.0",
-			want: &config.LibraryState{
-				ID:               "one-example-id",
-				Version:          "2.0.0",
-				ReleaseTriggered: true,
-			},
-		},
-	} {
-		t.Run(test.name, func(t *testing.T) {
-			setReleaseTrigger(test.library, test.libraryVersion, test.trigger)
-			if diff := cmp.Diff(test.want, test.library); diff != "" {
-				t.Errorf("state mismatch (-want +got):\n%s", diff)
-			}
-		})
-	}
-}
-
 func TestUpdateLibrary(t *testing.T) {
 	t.Parallel()
 	for _, test := range []struct {
 		name            string
 		pathAndMessages []pathAndMessage
 		tags            []string
-		runner          *initRunner
-		state           *config.LibrarianState
+		libraryVersion  string
+		library         *config.LibraryState
 		repo            gitrepo.Repository
 		want            *config.LibraryState
 		wantErr         bool
@@ -468,28 +421,13 @@ func TestUpdateLibrary(t *testing.T) {
 			tags: []string{
 				"one-id-1.2.3",
 			},
-			runner: &initRunner{
-				cfg: &config.Config{
-					LibraryVersion: "2.0.0",
-				},
-			},
-			state: &config.LibrarianState{
-				Libraries: []*config.LibraryState{
-					{
-						ID:      "another-id",
-						Version: "2.3.4",
-						SourceRoots: []string{
-							"another/path",
-						},
-					},
-					{
-						ID:      "one-id",
-						Version: "1.2.3",
-						SourceRoots: []string{
-							"one/path",
-							"two/path",
-						},
-					},
+			libraryVersion: "2.0.0",
+			library: &config.LibraryState{
+				ID:      "one-id",
+				Version: "1.2.3",
+				SourceRoots: []string{
+					"one/path",
+					"two/path",
 				},
 			},
 			want: &config.LibraryState{
@@ -532,26 +470,13 @@ func TestUpdateLibrary(t *testing.T) {
 			},
 			tags: []string{
 				"one-id-1.2.3",
-				"another-id-2.3.4",
 			},
-			runner: &initRunner{cfg: &config.Config{}},
-			state: &config.LibrarianState{
-				Libraries: []*config.LibraryState{
-					{
-						ID:      "another-id",
-						Version: "2.3.4",
-						SourceRoots: []string{
-							"another/path",
-						},
-					},
-					{
-						ID:      "one-id",
-						Version: "1.2.3",
-						SourceRoots: []string{
-							"one/path",
-							"two/path",
-						},
-					},
+			library: &config.LibraryState{
+				ID:      "one-id",
+				Version: "1.2.3",
+				SourceRoots: []string{
+					"one/path",
+					"two/path",
 				},
 			},
 			want: &config.LibraryState{
@@ -576,25 +501,13 @@ func TestUpdateLibrary(t *testing.T) {
 			},
 		},
 		{
-			name:   "failed to get commit history of one library",
-			runner: &initRunner{cfg: &config.Config{}},
-			state: &config.LibrarianState{
-				Libraries: []*config.LibraryState{
-					{
-						ID:      "another-id",
-						Version: "2.3.4",
-						SourceRoots: []string{
-							"another/path",
-						},
-					},
-					{
-						ID:      "one-id",
-						Version: "1.2.3",
-						SourceRoots: []string{
-							"one/path",
-							"two/path",
-						},
-					},
+			name: "failed to get commit history of one library",
+			library: &config.LibraryState{
+				ID:      "one-id",
+				Version: "1.2.3",
+				SourceRoots: []string{
+					"one/path",
+					"two/path",
 				},
 			},
 			repo: &MockRepository{
@@ -606,12 +519,12 @@ func TestUpdateLibrary(t *testing.T) {
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			var err error
+			var updated *config.LibraryState
 			if test.repo != nil {
-				test.runner.repo = test.repo
-				err = updateLibrary(test.runner, test.state, 1)
+				updated, err = updateLibrary(test.repo, test.library, test.libraryVersion)
 			} else {
-				test.runner.repo = setupRepoForGetCommits(t, test.pathAndMessages, test.tags)
-				err = updateLibrary(test.runner, test.state, 1)
+				repo := setupRepoForGetCommits(t, test.pathAndMessages, test.tags)
+				updated, err = updateLibrary(repo, test.library, test.libraryVersion)
 			}
 
 			if test.wantErr {
@@ -625,10 +538,11 @@ func TestUpdateLibrary(t *testing.T) {
 
 				return
 			}
+
 			if err != nil {
 				t.Errorf("failed to run getChangesOf(): %q", err.Error())
 			}
-			if diff := cmp.Diff(test.want, test.state.Libraries[1], cmpopts.IgnoreFields(config.Change{}, "CommitHash")); diff != "" {
+			if diff := cmp.Diff(test.want, updated, cmpopts.IgnoreFields(config.Change{}, "CommitHash")); diff != "" {
 				t.Errorf("state mismatch (-want +got):\n%s", diff)
 			}
 		})