diff --git a/internal/github/github.go b/internal/github/github.go index f81e5c51c8..c45ee05d17 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -119,8 +119,10 @@ func (c *Client) GetRawContent(ctx context.Context, path, ref string) ([]byte, e // which must have a GitHub HTTPS URL. We assume a base branch of "main". func (c *Client) CreatePullRequest(ctx context.Context, repo *Repository, remoteBranch, title, body string) (*PullRequestMetadata, error) { if body == "" { + slog.Warn("Provided PR body is empty, setting default.") body = "Regenerated all changed APIs. See individual commits for details." } + slog.Info("Creating PR", "branch", remoteBranch, "title", title, "body", body) newPR := &github.NewPullRequest{ Title: &title, Head: &remoteBranch, diff --git a/internal/gitrepo/gitrepo.go b/internal/gitrepo/gitrepo.go index 066baf1fd5..8ed29b0520 100644 --- a/internal/gitrepo/gitrepo.go +++ b/internal/gitrepo/gitrepo.go @@ -23,8 +23,10 @@ import ( "strings" "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + httpAuth "github.com/go-git/go-git/v5/plumbing/transport/http" ) // Repository defines the interface for git repository operations. @@ -37,12 +39,15 @@ type Repository interface { HeadHash() (string, error) ChangedFilesInCommit(commitHash string) ([]string, error) GetCommitsForPathsSinceTag(paths []string, tagName string) ([]*Commit, error) + CreateBranchAndCheckout(name string) error + Push(branchName string) error } // LocalRepository represents a git repository. type LocalRepository struct { - Dir string - repo *git.Repository + Dir string + repo *git.Repository + gitPassword string } // Commit represents a git commit. @@ -63,6 +68,8 @@ type RepositoryOptions struct { // CI is the type of Continuous Integration (CI) environment in which // the tool is executing. CI string + // GitPassword is used for HTTP basic auth. + GitPassword string } // NewRepository provides access to a git repository based on the provided options. @@ -72,6 +79,15 @@ type RepositoryOptions struct { // otherwise it clones from opts.RemoteURL. // If opts.Clone is CloneOptionAlways, it always clones from opts.RemoteURL. func NewRepository(opts *RepositoryOptions) (*LocalRepository, error) { + repo, err := newRepositoryWithoutUser(opts) + if err != nil { + return repo, err + } + repo.gitPassword = opts.GitPassword + return repo, nil +} + +func newRepositoryWithoutUser(opts *RepositoryOptions) (*LocalRepository, error) { if opts.Dir == "" { return nil, errors.New("gitrepo: dir is required") } @@ -100,6 +116,7 @@ func open(dir string) (*LocalRepository, error) { if err != nil { return nil, err } + return &LocalRepository{ Dir: dir, repo: repo, @@ -363,3 +380,41 @@ func (r *LocalRepository) ChangedFilesInCommit(commitHash string) ([]string, err } return files, nil } + +// CreateBranchAndCheckout creates a new git branch and checks out the +// branch in the local git repository. +func (r *LocalRepository) CreateBranchAndCheckout(name string) error { + slog.Info("Creating branch and checking out", "name", name) + worktree, err := r.repo.Worktree() + if err != nil { + return err + } + return worktree.Checkout(&git.CheckoutOptions{ + Branch: plumbing.NewBranchReferenceName(name), + Create: true, + Keep: true, + }) +} + +// Push pushes the local branch to the origin remote. +func (r *LocalRepository) Push(branchName string) error { + // https://stackoverflow.com/a/75727620 + refSpec := config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/heads/%s", branchName, branchName)) + slog.Info("Pushing changes", slog.Any("refspec", refSpec)) + var auth *httpAuth.BasicAuth + if r.gitPassword != "" { + slog.Info("Authenticating with basic auth") + auth = &httpAuth.BasicAuth{ + Password: r.gitPassword, + } + } + if err := r.repo.Push(&git.PushOptions{ + RemoteName: "origin", + RefSpecs: []config.RefSpec{refSpec}, + Auth: auth, + }); err != nil { + return err + } + slog.Info("Successfully pushed branch to remote 'origin", "branch", branchName) + return nil +} diff --git a/internal/gitrepo/gitrepo_test.go b/internal/gitrepo/gitrepo_test.go index 504ec77ed1..501060102b 100644 --- a/internal/gitrepo/gitrepo_test.go +++ b/internal/gitrepo/gitrepo_test.go @@ -975,6 +975,47 @@ func TestGetCommitsForPathsSinceTag(t *testing.T) { } } +func TestCreateBranchAndCheckout(t *testing.T) { + for _, test := range []struct { + name string + branchName string + wantErr bool + wantErrPhrase string + }{ + { + name: "works", + branchName: "test-branch", + }, + { + name: "invalid branch name", + branchName: "invalid branch name", + wantErr: true, + wantErrPhrase: "invalid", + }, + } { + t.Run(test.name, func(t *testing.T) { + repo, _ := setupRepoForGetCommitsTest(t) + err := repo.CreateBranchAndCheckout(test.branchName) + if test.wantErr { + if err == nil { + t.Errorf("%s should return error", test.name) + } + if !strings.Contains(err.Error(), test.wantErrPhrase) { + t.Errorf("CreateBranchAndCheckout() returned error %q, want to contain %q", err.Error(), test.wantErrPhrase) + } + return + } + if err != nil { + t.Fatal(err) + } + head, _ := repo.repo.Head() + if diff := cmp.Diff(test.branchName, head.Name().Short()); diff != "" { + t.Errorf("CreateBranchAndCheckout() mismatch (-want +got):\n%s", diff) + } + }) + } +} + // initTestRepo creates a new git repository in a temporary directory. func initTestRepo(t *testing.T) (*git.Repository, string) { t.Helper() diff --git a/internal/librarian/command.go b/internal/librarian/command.go index d75a7d6a11..a4f29b4544 100644 --- a/internal/librarian/command.go +++ b/internal/librarian/command.go @@ -46,7 +46,8 @@ func newCommandRunner(cfg *config.Config) (*commandRunner, error) { if cfg.APISource == "" { cfg.APISource = "https://github.com/googleapis/googleapis" } - languageRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.Repo, cfg.CI) + + languageRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.Repo, cfg.CI, cfg.GitHubToken) if err != nil { return nil, err } @@ -54,7 +55,7 @@ func newCommandRunner(cfg *config.Config) (*commandRunner, error) { var sourceRepo gitrepo.Repository var sourceRepoDir string if cfg.CommandName != tagAndReleaseCmdName { - sourceRepo, err = cloneOrOpenRepo(cfg.WorkRoot, cfg.APISource, cfg.CI) + sourceRepo, err = cloneOrOpenRepo(cfg.WorkRoot, cfg.APISource, cfg.CI, cfg.GitHubToken) if err != nil { return nil, err } @@ -99,7 +100,7 @@ func newCommandRunner(cfg *config.Config) (*commandRunner, error) { }, nil } -func cloneOrOpenRepo(workRoot, repo, ci string) (*gitrepo.LocalRepository, error) { +func cloneOrOpenRepo(workRoot, repo, ci string, gitPassword string) (*gitrepo.LocalRepository, error) { if repo == "" { return nil, errors.New("repo must be specified") } @@ -111,10 +112,11 @@ func cloneOrOpenRepo(workRoot, repo, ci string) (*gitrepo.LocalRepository, error repoName := path.Base(strings.TrimSuffix(repo, "/")) repoPath := filepath.Join(workRoot, repoName) return gitrepo.NewRepository(&gitrepo.RepositoryOptions{ - Dir: repoPath, - MaybeClone: true, - RemoteURL: repo, - CI: ci, + Dir: repoPath, + MaybeClone: true, + RemoteURL: repo, + CI: ci, + GitPassword: gitPassword, }) } // repo is a directory @@ -123,8 +125,9 @@ func cloneOrOpenRepo(workRoot, repo, ci string) (*gitrepo.LocalRepository, error return nil, err } githubRepo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{ - Dir: absRepoRoot, - CI: ci, + Dir: absRepoRoot, + CI: ci, + GitPassword: gitPassword, }) if err != nil { return nil, err @@ -204,17 +207,27 @@ func commitAndPush(ctx context.Context, r *generateRunner, commitMessage string) return nil } + datetimeNow := formatTimestamp(time.Now()) + branch := fmt.Sprintf("librarian-%s", datetimeNow) + slog.Info("Creating branch", slog.String("branch", branch)) + if err := r.repo.CreateBranchAndCheckout(branch); err != nil { + return err + } + // TODO: get correct language for message (https://github.com/googleapis/librarian/issues/885) + slog.Info("Committing", "message", commitMessage) if err := r.repo.Commit(commitMessage); err != nil { return err } + if err := r.repo.Push(branch); err != nil { + return err + } + // Create a new branch, set title and message for the PR. - datetimeNow := formatTimestamp(time.Now()) titlePrefix := "Librarian pull request" - branch := fmt.Sprintf("librarian-%s", datetimeNow) title := fmt.Sprintf("%s: %s", titlePrefix, datetimeNow) - + slog.Info("Creating pull request", slog.String("branch", branch), slog.String("title", title)) if _, err = r.ghClient.CreatePullRequest(ctx, gitHubRepo, branch, title, commitMessage); err != nil { return fmt.Errorf("failed to create pull request: %w", err) } diff --git a/internal/librarian/command_test.go b/internal/librarian/command_test.go index c715532e71..212f5a0b47 100644 --- a/internal/librarian/command_test.go +++ b/internal/librarian/command_test.go @@ -254,7 +254,7 @@ func TestCloneOrOpenLanguageRepo(t *testing.T) { } }() - repo, err := cloneOrOpenRepo(workRoot, test.repo, test.ci) + repo, err := cloneOrOpenRepo(workRoot, test.repo, test.ci, "") if test.wantErr { if err == nil { t.Error("cloneOrOpenLanguageRepo() expected an error but got nil") @@ -302,22 +302,14 @@ func TestCommitAndPush(t *testing.T) { { name: "Happy Path", setupMockRepo: func(t *testing.T) gitrepo.Repository { - repoDir := newTestGitRepoWithCommit(t, "") - // Add remote so FetchGitHubRepoFromRemote succeeds. - cmd := exec.Command("git", "remote", "add", "origin", "https://github.com/test-owner/test-repo.git") - cmd.Dir = repoDir - if err := cmd.Run(); err != nil { - t.Fatalf("git remote add: %v", err) - } - // Add a file to make the repo dirty, so there's something to commit. - if err := os.WriteFile(filepath.Join(repoDir, "new-file.txt"), []byte("new content"), 0644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - repo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{Dir: repoDir}) - if err != nil { - t.Fatalf("Failed to create test repo: %v", err) + remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{ + Name: "origin", + URLs: []string{"https://github.com/googleapis/librarian.git"}, + }) + return &MockRepository{ + Dir: t.TempDir(), + RemotesValue: []*git.Remote{remote}, } - return repo }, setupMockClient: func(t *testing.T) GitHubClient { return &mockGitHubClient{ @@ -325,19 +317,6 @@ func TestCommitAndPush(t *testing.T) { } }, push: true, - validatePostTest: func(t *testing.T, repo gitrepo.Repository) { - localRepo, ok := repo.(*gitrepo.LocalRepository) - if !ok { - t.Fatalf("Expected *gitrepo.LocalRepository, got %T", repo) - } - isClean, err := localRepo.IsClean() - if err != nil { - t.Fatalf("Failed to check repo status: %v", err) - } - if !isClean { - t.Errorf("Expected repository to be clean after commit, but it's dirty") - } - }, }, { name: "No GitHub Remote", @@ -374,6 +353,30 @@ func TestCommitAndPush(t *testing.T) { wantErr: true, expectedErrMsg: "mock add all error", }, + { + name: "Create branch error", + setupMockRepo: func(t *testing.T) gitrepo.Repository { + remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{ + Name: "origin", + URLs: []string{"https://github.com/googleapis/librarian.git"}, + }) + + status := make(git.Status) + status["file.txt"] = &git.FileStatus{Worktree: git.Modified} + return &MockRepository{ + Dir: t.TempDir(), + AddAllStatus: status, + RemotesValue: []*git.Remote{remote}, + CreateBranchAndCheckoutError: errors.New("create branch error"), + } + }, + setupMockClient: func(t *testing.T) GitHubClient { + return nil + }, + push: true, + wantErr: true, + expectedErrMsg: "create branch error", + }, { name: "Commit error", setupMockRepo: func(t *testing.T) gitrepo.Repository { @@ -398,6 +401,30 @@ func TestCommitAndPush(t *testing.T) { wantErr: true, expectedErrMsg: "commit error", }, + { + name: "Push error", + setupMockRepo: func(t *testing.T) gitrepo.Repository { + remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{ + Name: "origin", + URLs: []string{"https://github.com/googleapis/librarian.git"}, + }) + + status := make(git.Status) + status["file.txt"] = &git.FileStatus{Worktree: git.Modified} + return &MockRepository{ + Dir: t.TempDir(), + AddAllStatus: status, + RemotesValue: []*git.Remote{remote}, + PushError: errors.New("push error"), + } + }, + setupMockClient: func(t *testing.T) GitHubClient { + return nil + }, + push: true, + wantErr: true, + expectedErrMsg: "push error", + }, { name: "Create pull request error", setupMockRepo: func(t *testing.T) gitrepo.Repository { diff --git a/internal/librarian/generate.go b/internal/librarian/generate.go index aa73535ca5..b901e1a11c 100644 --- a/internal/librarian/generate.go +++ b/internal/librarian/generate.go @@ -135,6 +135,7 @@ func (r *generateRunner) run(ctx context.Context) error { if err := r.generateSingleLibrary(ctx, libraryID, outputDir); err != nil { return err } + prBody += fmt.Sprintf("feat: generated %s\n", libraryID) } else { for _, library := range r.state.Libraries { if err := r.generateSingleLibrary(ctx, library.ID, outputDir); err != nil { diff --git a/internal/librarian/mocks_test.go b/internal/librarian/mocks_test.go index 22fc35c43a..aed09587d4 100644 --- a/internal/librarian/mocks_test.go +++ b/internal/librarian/mocks_test.go @@ -202,6 +202,8 @@ type MockRepository struct { GetCommitsForPathsSinceTagError error ChangedFilesInCommitValue []string ChangedFilesInCommitError error + CreateBranchAndCheckoutError error + PushError error } func (m *MockRepository) IsClean() (bool, error) { @@ -247,3 +249,17 @@ func (m *MockRepository) ChangedFilesInCommit(hash string) ([]string, error) { } return m.ChangedFilesInCommitValue, nil } + +func (m *MockRepository) CreateBranchAndCheckout(name string) error { + if m.CreateBranchAndCheckoutError != nil { + return m.CreateBranchAndCheckoutError + } + return nil +} + +func (m *MockRepository) Push(name string) error { + if m.PushError != nil { + return m.PushError + } + return nil +}