chore(internal/config): remove PushConfig flag (#1526)

Fixes #1517

Author: JoeWang1127

internal/config/config.go

@@ -128,18 +128,6 @@ type Config struct {
 	// Push is specified with the -push flag. No value is required.
 	Push bool
 
-	// PushConfig specifies the email address and display name used in Git commits,
-	// in the format "email,name".
-	//
-	// PushConfig is used in all commands that create commits in a language repository:
-	// create-release-pr, configure and update-apis.
-	//
-	// PushConfig is optional. If unspecified, commits will use a default name of
-	// "Google Cloud SDK" and a default email of [email protected].
-	//
-	// PushConfig is specified with the -push-config flag.
-	PushConfig string
-
 	// Repo specifies the language repository to use, as either a local root directory
 	// or a URL to clone from. If a local directory is specified, it can
 	// be relative to the current working directory. The repository must
@@ -187,7 +175,6 @@ type Config struct {
 func New() *Config {
 	return &Config{
 		GitHubToken: os.Getenv("LIBRARIAN_GITHUB_TOKEN"),
-		PushConfig:  "",
 	}
 }
 
@@ -213,10 +200,6 @@ func (c *Config) IsValid() (bool, error) {
 		return false, errors.New("no GitHub token supplied for push")
 	}
 
-	if _, err := validatePushConfig(c.PushConfig, ""); err != nil {
-		return false, err
-	}
-
 	if _, err := validateHostMount(c.HostMount, ""); err != nil {
 		return false, err
 	}
@@ -236,16 +219,3 @@ func validateHostMount(hostMount, defaultValue string) (bool, error) {
 
 	return true, nil
 }
-
-func validatePushConfig(pushConfig, defaultValue string) (bool, error) {
-	if pushConfig == defaultValue {
-		return true, nil
-	}
-
-	parts := strings.Split(pushConfig, ",")
-	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
-		return false, errors.New("unable to parse push config")
-	}
-
-	return true, nil
-}

internal/config/config_test.go

@@ -36,15 +36,13 @@ func TestNew(t *testing.T) {
 			},
 			want: Config{
 				GitHubToken: "gh_token",
-				PushConfig:  "",
 			},
 		},
 		{
 			name:    "No environment variables set",
 			envVars: map[string]string{},
 			want: Config{
 				GitHubToken: "",
-				PushConfig:  "",
 			},
 		},
 		{
@@ -54,7 +52,6 @@ func TestNew(t *testing.T) {
 			},
 			want: Config{
 				GitHubToken: "gh_token",
-				PushConfig:  "",
 			},
 		},
 	} {
@@ -137,71 +134,55 @@ func TestIsValid(t *testing.T) {
 		wantErr   bool
 	}{
 		{
-			name: "Valid config - Push false, push config valid",
+			name: "Valid config - Push false",
 			cfg: Config{
 				Push:        false,
 				GitHubToken: "",
-				PushConfig:  "[email protected],abc",
 			},
 			wantValid: true,
 			wantErr:   false,
 		},
 		{
-			name: "Valid config - Push true, token present, push config valid",
+			name: "Valid config - Push true, token present",
 			cfg: Config{
 				Push:        true,
 				GitHubToken: "some_token",
-				PushConfig:  "[email protected],abc",
 			},
 			wantValid: true,
 			wantErr:   false,
 		},
 		{
-			name: "Invalid config - Push true, token missing, push config valid",
+			name: "Invalid config - Push true, token missing",
 			cfg: Config{
 				Push:        true,
 				GitHubToken: "",
-				PushConfig:  "[email protected],abc",
-			},
-			wantValid: false,
-			wantErr:   true,
-		},
-		{
-			name: "Invalid config - Push true, token present, push config invalid",
-			cfg: Config{
-				Push:        true,
-				GitHubToken: "some_token",
-				PushConfig:  "abc:[email protected]",
 			},
 			wantValid: false,
 			wantErr:   true,
 		},
 		{
 			name: "Invalid config - host mount invalid, missing local-dir",
 			cfg: Config{
-				Push:       false,
-				PushConfig: "[email protected],abc",
-				HostMount:  "host-dir:",
+				Push:      false,
+				HostMount: "host-dir:",
 			},
 			wantValid: false,
 			wantErr:   true,
 		},
 		{
 			name: "Invalid config - host mount invalid, missing host-dir",
 			cfg: Config{
-				Push:       false,
-				PushConfig: "[email protected],abc",
-				HostMount:  ":local-dir",
+				Push:      false,
+				HostMount: ":local-dir",
 			},
 			wantValid: false,
 			wantErr:   true,
 		},
 		{
 			name: "Invalid config - host mount invalid, missing separator",
 			cfg: Config{
-				Push:       false,
-				PushConfig: "[email protected],abc",
-				HostMount:  "host-dir/local-dir",
+				Push:      false,
+				HostMount: "host-dir/local-dir",
 			},
 			wantValid: false,
 			wantErr:   true,

internal/gitrepo/gitrepo.go

@@ -18,20 +18,17 @@ package gitrepo
 import (
 	"errors"
 	"fmt"
+	"github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/plumbing"
 	"log/slog"
 	"os"
 	"strings"
-	"time"
-
-	"github.com/go-git/go-git/v5"
-	"github.com/go-git/go-git/v5/plumbing"
-	"github.com/go-git/go-git/v5/plumbing/object"
 )
 
 // Repository defines the interface for git repository operations.
 type Repository interface {
 	AddAll() (git.Status, error)
-	Commit(msg string, userName, userEmail string) error
+	Commit(msg string) error
 	IsClean() (bool, error)
 	Remotes() ([]*git.Remote, error)
 	GetDir() string
@@ -145,7 +142,7 @@ func (r *LocalRepository) AddAll() (git.Status, error) {
 
 // Commit creates a new commit with the provided message and author
 // information.
-func (r *LocalRepository) Commit(msg string, userName, userEmail string) error {
+func (r *LocalRepository) Commit(msg string) error {
 	worktree, err := r.repo.Worktree()
 	if err != nil {
 		return err
@@ -158,13 +155,8 @@ func (r *LocalRepository) Commit(msg string, userName, userEmail string) error {
 	if status.IsClean() {
 		return fmt.Errorf("no modifications to commit")
 	}
-	hash, err := worktree.Commit(msg, &git.CommitOptions{
-		Author: &object.Signature{
-			Name:  userName,
-			Email: userEmail,
-			When:  time.Now(),
-		},
-	})
+	// The author of the commit will be read from git config.
+	hash, err := worktree.Commit(msg, &git.CommitOptions{})
 	if err != nil {
 		return err
 	}

internal/gitrepo/gitrepo_test.go

@@ -21,7 +21,7 @@ import (
 	"testing"
 
 	"github.com/go-git/go-git/v5"
-	gogitConfig "github.com/go-git/go-git/v5/config"
+	goGitConfig "github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/google/go-cmp/cmp"
 )
@@ -367,15 +367,33 @@ func TestAddAll(t *testing.T) {
 
 func TestCommit(t *testing.T) {
 	t.Parallel()
+	name, email := "tester", "[email protected]"
 	// setupRepo is a helper to create a repository with an initial commit.
 	setupRepo := func(t *testing.T) *LocalRepository {
 		t.Helper()
 		dir := t.TempDir()
-		gogitRepo, err := git.PlainInit(dir, false)
+		goGitRepo, err := git.PlainInit(dir, false)
 		if err != nil {
 			t.Fatalf("git.PlainInit failed: %v", err)
 		}
-		w, err := gogitRepo.Worktree()
+
+		author := struct {
+			Name  string
+			Email string
+		}{
+			Name:  name,
+			Email: email,
+		}
+		config, err := goGitRepo.Config()
+		if err != nil {
+			t.Fatalf("gitRepo.Config failed: %v", err)
+		}
+		config.User = author
+		if err := goGitRepo.SetConfig(config); err != nil {
+			t.Fatalf("gitRepo.SetConfig failed: %v", err)
+		}
+
+		w, err := goGitRepo.Worktree()
 		if err != nil {
 			t.Fatalf("Worktree() failed: %v", err)
 		}
@@ -385,15 +403,13 @@ func TestCommit(t *testing.T) {
 		}); err != nil {
 			t.Fatalf("initial commit failed: %v", err)
 		}
-		return &LocalRepository{Dir: dir, repo: gogitRepo}
+		return &LocalRepository{Dir: dir, repo: goGitRepo}
 	}
 
 	for _, tc := range []struct {
 		name       string
 		setup      func(t *testing.T) *LocalRepository
 		commitMsg  string
-		userName   string
-		userEmail  string
 		wantErr    bool
 		wantErrMsg string
 		check      func(t *testing.T, repo *LocalRepository, commitMsg string)
@@ -417,8 +433,6 @@ func TestCommit(t *testing.T) {
 				return repo
 			},
 			commitMsg: "feat: add new file",
-			userName:  "tester",
-			userEmail: "[email protected]",
 			check: func(t *testing.T, repo *LocalRepository, commitMsg string) {
 				head, err := repo.repo.Head()
 				if err != nil {
@@ -446,8 +460,6 @@ func TestCommit(t *testing.T) {
 				return setupRepo(t)
 			},
 			commitMsg:  "no-op",
-			userName:   "tester",
-			userEmail:  "[email protected]",
 			wantErr:    true,
 			wantErrMsg: "no modifications to commit",
 		},
@@ -456,15 +468,13 @@ func TestCommit(t *testing.T) {
 			setup: func(t *testing.T) *LocalRepository {
 				dir := t.TempDir()
 				// Create a bare repository which has no worktree.
-				gogitRepo, err := git.PlainInit(dir, true)
+				goGitRepo, err := git.PlainInit(dir, true)
 				if err != nil {
 					t.Fatalf("git.PlainInit failed: %v", err)
 				}
-				return &LocalRepository{Dir: dir, repo: gogitRepo}
+				return &LocalRepository{Dir: dir, repo: goGitRepo}
 			},
 			commitMsg:  "any message",
-			userName:   "tester",
-			userEmail:  "[email protected]",
 			wantErr:    true,
 			wantErrMsg: "worktree not available",
 		},
@@ -497,8 +507,6 @@ func TestCommit(t *testing.T) {
 				return repo
 			},
 			commitMsg:  "any message",
-			userName:   "tester",
-			userEmail:  "[email protected]",
 			wantErr:    true,
 			wantErrMsg: "permission denied",
 		},
@@ -507,7 +515,7 @@ func TestCommit(t *testing.T) {
 			t.Parallel()
 			repo := tc.setup(t)
 
-			err := repo.Commit(tc.commitMsg, tc.userName, tc.userEmail)
+			err := repo.Commit(tc.commitMsg)
 
 			if tc.wantErr {
 				if err == nil {
@@ -564,7 +572,7 @@ func TestRemotes(t *testing.T) {
 			}
 
 			for name, urls := range tt.setupRemotes {
-				if _, err := gogitRepo.CreateRemote(&gogitConfig.RemoteConfig{
+				if _, err := gogitRepo.CreateRemote(&goGitConfig.RemoteConfig{
 					Name: name,
 					URLs: urls,
 				}); err != nil {

internal/librarian/command.go

@@ -151,24 +151,22 @@ func createWorkRoot(t time.Time, workRootOverride string) (string, error) {
 	return path, nil
 }
 
-// commitAndPush creates a commit and push request to Github for the generated changes.
-// It uses the GitHub client to create a PR with the specified branch, title, and description to the repository.
-func commitAndPush(ctx context.Context, repo gitrepo.Repository, ghClient GitHubClient, pushConfig, commitMessage string) error {
-	if pushConfig == "" {
-		slog.Info("PushConfig flag not specified, skipping")
+// commitAndPush creates a commit and push request to GitHub for the generated
+// changes.
+// It uses the GitHub client to create a PR with the specified branch, title, and
+// description to the repository.
+func commitAndPush(ctx context.Context, r *generateRunner, commitMessage string) error {
+	if !r.cfg.Push {
+		slog.Info("Push flag is not specified, skipping")
 		return nil
 	}
 	// Ensure we have a GitHub repository
-	gitHubRepo, err := github.FetchGitHubRepoFromRemote(repo)
+	gitHubRepo, err := github.FetchGitHubRepoFromRemote(r.repo)
 	if err != nil {
 		return err
 	}
 
-	userEmail, userName, err := parsePushConfig(pushConfig)
-	if err != nil {
-		return err
-	}
-	status, err := repo.AddAll()
+	status, err := r.repo.AddAll()
 	if err != nil {
 		return err
 	}
@@ -178,7 +176,7 @@ func commitAndPush(ctx context.Context, repo gitrepo.Repository, ghClient GitHub
 	}
 
 	// TODO: get correct language for message (https://github.com/googleapis/librarian/issues/885)
-	if err := repo.Commit(commitMessage, userName, userEmail); err != nil {
+	if err := r.repo.Commit(commitMessage); err != nil {
 		return err
 	}
 
@@ -188,18 +186,8 @@ func commitAndPush(ctx context.Context, repo gitrepo.Repository, ghClient GitHub
 	branch := fmt.Sprintf("librarian-%s", datetimeNow)
 	title := fmt.Sprintf("%s: %s", titlePrefix, datetimeNow)
 
-	if _, err = ghClient.CreatePullRequest(ctx, gitHubRepo, branch, title, commitMessage); err != nil {
+	if _, err = r.ghClient.CreatePullRequest(ctx, gitHubRepo, branch, title, commitMessage); err != nil {
 		return fmt.Errorf("failed to create pull request: %w", err)
 	}
 	return nil
 }
-
-func parsePushConfig(pushConfig string) (string, string, error) {
-	parts := strings.Split(pushConfig, ",")
-	if len(parts) != 2 {
-		return "", "", fmt.Errorf("invalid pushConfig format: expected 'email,user', got %q", pushConfig)
-	}
-	userEmail := parts[0]
-	userName := parts[1]
-	return userEmail, userName, nil
-}