fix: get conventional commits from source repository (#1867)

JoeWang1127 · web-flow · commit 40e7bc375ad5 · 2025-08-28T11:41:18.000-04:00
Function `updateChangesSinceLastGeneration` may not be needed since we don't need to pass api changes to language container. Fixes #1866
diff --git a/internal/librarian/generate.go b/internal/librarian/generate.go
@@ -126,7 +126,7 @@ func newGenerateRunner(cfg *config.Config) (*generateRunner, error) {
 func (r *generateRunner) run(ctx context.Context) error {
 	outputDir := filepath.Join(r.workRoot, "output")
 	if err := os.Mkdir(outputDir, 0755); err != nil {
-		return err
+		return fmt.Errorf("failed to make output directory, %s: %w", outputDir, err)
 	}
 	slog.Info("Code will be generated", "dir", outputDir)
 
@@ -219,7 +219,7 @@ func (r *generateRunner) needsConfigure() bool {
 func (r *generateRunner) updateChangesSinceLastGeneration(libraryID string) error {
 	for _, library := range r.state.Libraries {
 		if library.ID == libraryID {
-			commits, err := GetConventionalCommitsSinceLastGeneration(r.repo, library)
+			commits, err := GetConventionalCommitsSinceLastGeneration(r.sourceRepo, library)
 			if err != nil {
 				return fmt.Errorf("failed to fetch conventional commits for library, %s: %w", library.ID, err)
 			}
diff --git a/internal/librarian/generate_test.go b/internal/librarian/generate_test.go
@@ -24,6 +24,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/google/go-cmp/cmp"
 	"github.com/googleapis/librarian/internal/config"
 	"github.com/googleapis/librarian/internal/gitrepo"
@@ -750,6 +751,7 @@ func TestGenerateScenarios(t *testing.T) {
 			build:             true,
 			wantGenerateCalls: 1,
 			wantErr:           true,
+			wantErrMsg:        "failed to make output directory",
 		},
 		{
 			name: "generate error",
@@ -873,6 +875,17 @@ func TestGenerateScenarios(t *testing.T) {
 				t.Fatal(err)
 			}
 
+			// Create a symlink in the output directory to trigger an error.
+			if test.name == "symlink in output" {
+				outputDir := filepath.Join(r.workRoot, "output")
+				if err := os.MkdirAll(outputDir, 0755); err != nil {
+					t.Fatalf("os.MkdirAll() = %v", err)
+				}
+				if err := os.Symlink("target", filepath.Join(outputDir, "symlink")); err != nil {
+					t.Fatalf("os.Symlink() = %v", err)
+				}
+			}
+
 			err := r.run(context.Background())
 			if test.wantErr {
 				if err == nil {
@@ -1468,3 +1481,139 @@ func TestCompileRegexps(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateChangesSinceLastGeneration(t *testing.T) {
+	t.Parallel()
+	hash1 := plumbing.NewHash("1234567")
+	hash2 := plumbing.NewHash("abcdefg")
+	for _, test := range []struct {
+		name       string
+		libraryID  string
+		libraries  []*config.LibraryState
+		repo       gitrepo.Repository
+		want       *config.LibraryState
+		wantErr    bool
+		wantErrMsg string
+	}{
+		{
+			name:      "update changes in a library",
+			libraryID: "another-id",
+			libraries: []*config.LibraryState{
+				{
+					ID: "example-d",
+				},
+				{
+					ID:                  "another-id",
+					LastGeneratedCommit: "fake-sha",
+					APIs: []*config.API{
+						{
+							Path: "api/one/path",
+						},
+						{
+							Path: "api/another/path",
+						},
+					},
+				},
+			},
+			repo: &MockRepository{
+				GetCommitsForPathsSinceLastGenByPath: map[string][]*gitrepo.Commit{
+					"api/one/path": {
+						{
+							Message: "feat: new feature\n\nThis is body.\n\nPiperOrigin-RevId: 98765",
+							Hash:    hash1,
+						},
+					},
+					"api/another/path": {
+						{
+							Message: "fix: a bug fix\n\nThis is another body.\n\nPiperOrigin-RevId: 573342",
+							Hash:    hash2,
+						},
+					},
+				},
+				ChangedFilesInCommitValueByHash: map[string][]string{
+					hash1.String(): {
+						"api/one/path/file.txt",
+						"api/another/path/example.txt",
+					},
+					hash2.String(): {
+						"api/one/path/another-file.txt",
+						"api/another/path/another-example.txt",
+					},
+				},
+			},
+			want: &config.LibraryState{
+				ID:                  "another-id",
+				LastGeneratedCommit: "fake-sha",
+				APIs: []*config.API{
+					{
+						Path: "api/one/path",
+					},
+					{
+						Path: "api/another/path",
+					},
+				},
+				Changes: []*config.Change{
+					{
+						Type:       "feat",
+						Subject:    "new feature",
+						Body:       "This is body.",
+						ClNum:      "98765",
+						CommitHash: hash1.String(),
+					},
+					{
+						Type:       "fix",
+						Subject:    "a bug fix",
+						Body:       "This is another body.",
+						ClNum:      "573342",
+						CommitHash: hash2.String(),
+					},
+				},
+			},
+		},
+		{
+			name:      "failed to get conventional commits",
+			libraryID: "another-id",
+			libraries: []*config.LibraryState{
+				{
+					ID: "example-d",
+				},
+				{
+					ID: "another-id",
+				},
+			},
+			repo: &MockRepository{
+				GetCommitsForPathsSinceLastGenError: errors.New("simulated error"),
+			},
+			wantErr:    true,
+			wantErrMsg: "failed to fetch conventional commits for library",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			runner := &generateRunner{
+				sourceRepo: test.repo,
+				state: &config.LibrarianState{
+					Libraries: test.libraries,
+				},
+			}
+			err := runner.updateChangesSinceLastGeneration(test.libraryID)
+			if test.wantErr {
+				if err == nil {
+					t.Error("updateChangesSinceLastGeneration() should fail")
+				}
+				if !strings.Contains(err.Error(), test.wantErrMsg) {
+					t.Errorf("want error message %s, got %s", test.wantErrMsg, err.Error())
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Errorf("updateChangesSinceLastGeneration() failed: %q", err)
+			}
+
+			if diff := cmp.Diff(test.want, runner.state.Libraries[1]); diff != "" {
+				t.Errorf("updateChangesSinceLastGeneration() dirs mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
diff --git a/internal/librarian/mocks_test.go b/internal/librarian/mocks_test.go
@@ -273,6 +273,7 @@ type MockRepository struct {
 	GetCommitsForPathsSinceTagValueByTag map[string][]*gitrepo.Commit
 	GetCommitsForPathsSinceTagError      error
 	GetCommitsForPathsSinceLastGenValue  []*gitrepo.Commit
+	GetCommitsForPathsSinceLastGenByPath map[string][]*gitrepo.Commit
 	GetCommitsForPathsSinceLastGenError  error
 	ChangedFilesInCommitValue            []string
 	ChangedFilesInCommitValueByHash      map[string][]string
@@ -327,6 +328,16 @@ func (m *MockRepository) GetCommitsForPathsSinceCommit(paths []string, sinceComm
 	if m.GetCommitsForPathsSinceLastGenError != nil {
 		return nil, m.GetCommitsForPathsSinceLastGenError
 	}
+	if m.GetCommitsForPathsSinceLastGenByPath != nil {
+		allCommits := make([]*gitrepo.Commit, 0)
+		for _, path := range paths {
+			if commits, ok := m.GetCommitsForPathsSinceLastGenByPath[path]; ok {
+				allCommits = append(allCommits, commits...)
+			}
+		}
+
+		return allCommits, nil
+	}
 	return m.GetCommitsForPathsSinceLastGenValue, nil
 }
 
diff --git a/internal/librarian/release_please_lite.go b/internal/librarian/release_please_lite.go
@@ -39,9 +39,14 @@ func GetConventionalCommitsSinceLastRelease(repo gitrepo.Repository, library *co
 }
 
 // GetConventionalCommitsSinceLastGeneration returns all conventional commits for
-// the given library since the last generation.
+// all API paths in given library since the last generation.
 func GetConventionalCommitsSinceLastGeneration(repo gitrepo.Repository, library *config.LibraryState) ([]*conventionalcommits.ConventionalCommit, error) {
-	commits, err := repo.GetCommitsForPathsSinceCommit(library.SourceRoots, library.LastGeneratedCommit)
+	apiPaths := make([]string, 0)
+	for _, oneAPI := range library.APIs {
+		apiPaths = append(apiPaths, oneAPI.Path)
+	}
+
+	commits, err := repo.GetCommitsForPathsSinceCommit(apiPaths, library.LastGeneratedCommit)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get commits for library %s: %w", library.ID, err)
 	}
