diff --git a/go.mod b/go.mod index e8038f4..4a0f0c4 100644 --- a/go.mod +++ b/go.mod @@ -6,8 +6,7 @@ toolchain go1.24.4 require ( github.com/blang/semver/v4 v4.0.0 - github.com/go-git/go-billy/v5 v5.6.2 - github.com/go-git/go-git/v5 v5.16.2 + github.com/go-git/go-git/v5 v5.16.1 github.com/google/go-github/v72 v72.0.0 github.com/leodido/go-conventionalcommits v0.12.0 github.com/spf13/cobra v1.9.1 @@ -26,6 +25,7 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/emirpasic/gods v1.18.1 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect + github.com/go-git/go-billy/v5 v5.6.2 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect diff --git a/go.sum b/go.sum index 3c37570..12e5551 100644 --- a/go.sum +++ b/go.sum @@ -33,8 +33,8 @@ github.com/go-git/go-billy/v5 v5.6.2 h1:6Q86EsPXMa7c3YZ3aLAQsMA0VlWmy43r6FHqa/UN github.com/go-git/go-billy/v5 v5.6.2/go.mod h1:rcFC2rAsp/erv7CMz9GczHcuD0D32fWzH+MJAU+jaUU= github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4= github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII= -github.com/go-git/go-git/v5 v5.16.2 h1:fT6ZIOjE5iEnkzKyxTHK1W4HGAsPhqEqiSAssSO77hM= -github.com/go-git/go-git/v5 v5.16.2/go.mod h1:4Ge4alE/5gPs30F2H1esi2gPd69R0C39lolkucHBOp8= +github.com/go-git/go-git/v5 v5.16.1 h1:TuxMBWNL7R05tXsUGi0kh1vi4tq0WfXNLlIrAkXG1k8= +github.com/go-git/go-git/v5 v5.16.1/go.mod h1:4Ge4alE/5gPs30F2H1esi2gPd69R0C39lolkucHBOp8= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 h1:f+oWsMOmNPc8JmEHVZIycC7hBoQxHH9pNKQORJNozsQ= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8/go.mod h1:wcDNUvekVysuuOpQKo3191zZyTpiI6se1N1ULghS0sw= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= diff --git a/internal/forge/forge.go b/internal/forge/forge.go index 3e4c25e..0bd119a 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -17,9 +17,6 @@ type Forge interface { GitAuth() transport.AuthMethod - // CommitAuthor returns the git author used for the release commit. It should be the user whose token is used to talk to the API. - CommitAuthor(context.Context) (git.Author, error) - // LatestTags returns the last stable tag created on the main branch. If there is a more recent pre-release tag, // that is also returned. If no tag is found, it returns nil. LatestTags(context.Context) (git.Releases, error) diff --git a/internal/forge/github/github.go b/internal/forge/github/github.go index 3bff3e6..a296c2a 100644 --- a/internal/forge/github/github.go +++ b/internal/forge/github/github.go @@ -29,13 +29,6 @@ const ( EnvRepository = "GITHUB_REPOSITORY" ) -var ( - gitHubActionsBotAuthor = git.Author{ - Name: "github-actions[bot]", - Email: "41898282+github-actions[bot]@users.noreply.github.com", - } -) - var _ forge.Forge = &GitHub{} type GitHub struct { @@ -68,22 +61,6 @@ func (g *GitHub) GitAuth() transport.AuthMethod { } } -func (g *GitHub) CommitAuthor(ctx context.Context) (git.Author, error) { - g.log.DebugContext(ctx, "getting commit author from current token user") - - user, _, err := g.client.Users.Get(ctx, "") - if err != nil { - g.log.WarnContext(ctx, "failed to get commit author from API, using default github-actions[bot] user", "error", err) - - return gitHubActionsBotAuthor, nil - } - - return git.Author{ - Name: user.GetName(), - Email: user.GetEmail(), - }, nil -} - func (g *GitHub) LatestTags(ctx context.Context) (git.Releases, error) { g.log.DebugContext(ctx, "listing all tags in github repository") diff --git a/internal/forge/gitlab/gitlab.go b/internal/forge/gitlab/gitlab.go index 06de7fd..f53777a 100644 --- a/internal/forge/gitlab/gitlab.go +++ b/internal/forge/gitlab/gitlab.go @@ -69,22 +69,6 @@ func (g *GitLab) GitAuth() transport.AuthMethod { } } -func (g *GitLab) CommitAuthor(ctx context.Context) (git.Author, error) { - g.log.DebugContext(ctx, "getting commit author from current token user") - - user, _, err := g.client.Users.CurrentUser(gitlab.WithContext(ctx)) - if err != nil { - return git.Author{}, err - } - - // TODO: Return bot when nothing is returned? - - return git.Author{ - Name: user.Name, - Email: user.Email, - }, nil -} - func (g *GitLab) LatestTags(ctx context.Context) (git.Releases, error) { g.log.DebugContext(ctx, "listing all tags in gitlab repository") diff --git a/internal/git/git.go b/internal/git/git.go index 136a828..87d94d9 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -45,27 +45,6 @@ type Releases struct { Stable *Tag } -type Author struct { - Name string - Email string -} - -func (a Author) signature(when time.Time) *object.Signature { - return &object.Signature{ - Name: a.Name, - Email: a.Email, - When: when, - } -} - -func (a Author) String() string { - return fmt.Sprintf("%s <%s>", a.Name, a.Email) -} - -var ( - committer = Author{Name: "releaser-pleaser", Email: ""} -) - func CloneRepo(ctx context.Context, logger *slog.Logger, cloneURL, branch string, auth transport.AuthMethod) (*Repository, error) { dir, err := os.MkdirTemp("", "releaser-pleaser.*") if err != nil { @@ -171,17 +150,15 @@ func (r *Repository) UpdateFile(_ context.Context, path string, create bool, upd return nil } -func (r *Repository) Commit(_ context.Context, message string, author Author) (Commit, error) { +func (r *Repository) Commit(_ context.Context, message string) (Commit, error) { worktree, err := r.r.Worktree() if err != nil { return Commit{}, err } - now := time.Now() - releaseCommitHash, err := worktree.Commit(message, &git.CommitOptions{ - Author: author.signature(now), - Committer: committer.signature(now), + Author: signature(), + Committer: signature(), }) if err != nil { return Commit{}, fmt.Errorf("failed to commit changes: %w", err) @@ -193,27 +170,8 @@ func (r *Repository) Commit(_ context.Context, message string, author Author) (C }, nil } -// HasChangesWithRemote checks if the following two diffs are equal: -// -// - **Local**: remote/main..branch -// - **Remote**: (git merge-base remote/main remote/branch)..remote/branch -// -// This is done to avoid pushing when the only change would be a rebase of remote/branch onto the current remote/main. -func (r *Repository) HasChangesWithRemote(ctx context.Context, mainBranch, prBranch string) (bool, error) { - return r.hasChangesWithRemote(ctx, - plumbing.NewRemoteReferenceName(remoteName, mainBranch), - plumbing.NewBranchReferenceName(prBranch), - plumbing.NewRemoteReferenceName(remoteName, prBranch), - ) -} - -func (r *Repository) hasChangesWithRemote(ctx context.Context, mainBranchRef, localPRBranchRef, remotePRBranchRef plumbing.ReferenceName) (bool, error) { - commitOnRemoteMain, err := r.commitFromRef(mainBranchRef) - if err != nil { - return false, err - } - - commitOnRemotePRBranch, err := r.commitFromRef(remotePRBranchRef) +func (r *Repository) HasChangesWithRemote(ctx context.Context, branch string) (bool, error) { + remoteRef, err := r.r.Reference(plumbing.NewRemoteReferenceName(remoteName, branch), false) if err != nil { if err.Error() == "reference not found" { // No remote branch means that there are changes @@ -223,60 +181,29 @@ func (r *Repository) hasChangesWithRemote(ctx context.Context, mainBranchRef, lo return false, err } - currentRemotePRMergeBase, err := r.mergeBase(commitOnRemoteMain, commitOnRemotePRBranch) - if err != nil { - return false, err - } - if currentRemotePRMergeBase == nil { - // If there is no merge base something weird has happened with the - // remote main branch, and we should definitely push updates. - return false, nil - } - - remoteDiff, err := commitOnRemotePRBranch.PatchContext(ctx, currentRemotePRMergeBase) + remoteCommit, err := r.r.CommitObject(remoteRef.Hash()) if err != nil { return false, err } - commitOnLocalPRBranch, err := r.commitFromRef(localPRBranchRef) + localRef, err := r.r.Reference(plumbing.NewBranchReferenceName(branch), false) if err != nil { return false, err } - localDiff, err := commitOnRemoteMain.PatchContext(ctx, commitOnLocalPRBranch) + localCommit, err := r.r.CommitObject(localRef.Hash()) if err != nil { return false, err } - return remoteDiff.String() == localDiff.String(), nil -} - -func (r *Repository) commitFromRef(refName plumbing.ReferenceName) (*object.Commit, error) { - ref, err := r.r.Reference(refName, false) + diff, err := localCommit.PatchContext(ctx, remoteCommit) if err != nil { - return nil, err + return false, err } - commit, err := r.r.CommitObject(ref.Hash()) - if err != nil { - return nil, err - } + hasChanges := len(diff.FilePatches()) > 0 - return commit, nil -} - -func (r *Repository) mergeBase(a, b *object.Commit) (*object.Commit, error) { - mergeBases, err := a.MergeBase(b) - if err != nil { - return nil, err - } - - if len(mergeBases) == 0 { - return nil, nil - } - - // :shrug: We dont really care which commit we pick, at worst we do an unnecessary push. - return mergeBases[0], nil + return hasChanges, nil } func (r *Repository) ForcePush(ctx context.Context, branch string) error { @@ -296,3 +223,11 @@ func (r *Repository) ForcePush(ctx context.Context, branch string) error { Auth: r.auth, }) } + +func signature() *object.Signature { + return &object.Signature{ + Name: "releaser-pleaser", + Email: "", + When: time.Now(), + } +} diff --git a/internal/git/git_test.go b/internal/git/git_test.go deleted file mode 100644 index 3d05538..0000000 --- a/internal/git/git_test.go +++ /dev/null @@ -1,172 +0,0 @@ -package git - -import ( - "context" - "reflect" - "strconv" - "testing" - "time" - - "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" - "github.com/stretchr/testify/assert" -) - -func TestAuthor_signature(t *testing.T) { - now := time.Now() - - tests := []struct { - author Author - want *object.Signature - }{ - {author: Author{Name: "foo", Email: "bar@example.com"}, want: &object.Signature{Name: "foo", Email: "bar@example.com", When: now}}, - {author: Author{Name: "bar", Email: "foo@example.com"}, want: &object.Signature{Name: "bar", Email: "foo@example.com", When: now}}, - } - for i, tt := range tests { - t.Run(strconv.FormatInt(int64(i), 10), func(t *testing.T) { - if got := tt.author.signature(now); !reflect.DeepEqual(got, tt.want) { - t.Errorf("signature() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestAuthor_String(t *testing.T) { - tests := []struct { - author Author - want string - }{ - {author: Author{Name: "foo", Email: "bar@example.com"}, want: "foo "}, - {author: Author{Name: "bar", Email: "foo@example.com"}, want: "bar "}, - } - for i, tt := range tests { - t.Run(strconv.FormatInt(int64(i), 10), func(t *testing.T) { - if got := tt.author.String(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("String() = %v, want %v", got, tt.want) - } - }) - } -} - -const testMainBranch = "main" -const testPRBranch = "releaser-pleaser" - -func TestRepository_HasChangesWithRemote(t *testing.T) { - // go-git/v5 has a bug where it tries to delete the repo root dir (".") multiple times if there is no file left in it. - // this happens while switching branches in worktree.go rmFileAndDirsIfEmpty. - // TODO: Fix bug upstream - // For now I just make sure that there is always at least one file left in the dir by adding an empty "README.md" in the test util. - - mainBranchRef := plumbing.NewBranchReferenceName(testMainBranch) - localPRBranchRef := plumbing.NewBranchReferenceName(testPRBranch) - remotePRBranchRef := plumbing.NewBranchReferenceName("remote/" + testPRBranch) - - tests := []struct { - name string - repo TestRepo - want bool - wantErr assert.ErrorAssertionFunc - }{ - { - name: "no remote pr branch", - repo: WithTestRepo( - WithCommit( - "chore: release v1.0.0", - WithFile("VERSION", "v1.0.0"), - ), - WithCommit( - "chore: release v1.1.0", - OnBranch(mainBranchRef), - AsNewBranch(localPRBranchRef), - WithFile("VERSION", "v1.1.0"), - ), - ), - want: true, - wantErr: assert.NoError, - }, - { - name: "remote pr branch matches local", - repo: WithTestRepo( - WithCommit( - "chore: release v1.0.0", - WithFile("VERSION", "v1.0.0"), - ), - WithCommit( - "chore: release v1.1.0", - OnBranch(mainBranchRef), - AsNewBranch(remotePRBranchRef), - WithFile("VERSION", "v1.1.0"), - ), - WithCommit( - "chore: release v1.1.0", - OnBranch(mainBranchRef), - AsNewBranch(localPRBranchRef), - WithFile("VERSION", "v1.1.0"), - ), - ), - want: false, - wantErr: assert.NoError, - }, - { - name: "remote pr only needs rebase", - repo: WithTestRepo( - WithCommit( - "chore: release v1.0.0", - WithFile("VERSION", "v1.0.0"), - ), - WithCommit( - "chore: release v1.1.0", - OnBranch(mainBranchRef), - AsNewBranch(remotePRBranchRef), - WithFile("VERSION", "v1.1.0"), - ), - WithCommit( - "feat: new feature on remote", - OnBranch(mainBranchRef), - WithFile("feature", "yes"), - ), - WithCommit( - "chore: release v1.1.0", - OnBranch(mainBranchRef), - AsNewBranch(localPRBranchRef), - WithFile("VERSION", "v1.1.0"), - ), - ), - want: false, - wantErr: assert.NoError, - }, - { - name: "needs update", - repo: WithTestRepo( - WithCommit( - "chore: release v1.0.0", - WithFile("VERSION", "v1.0.0"), - ), - WithCommit( - "chore: release v1.1.0", - OnBranch(mainBranchRef), - AsNewBranch(remotePRBranchRef), - WithFile("VERSION", "v1.1.0"), - ), - WithCommit( - "chore: release v1.2.0", - OnBranch(mainBranchRef), - AsNewBranch(localPRBranchRef), - WithFile("VERSION", "v1.2.0"), - ), - ), - want: false, - wantErr: assert.NoError, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - repo := tt.repo(t) - got, err := repo.hasChangesWithRemote(context.Background(), mainBranchRef, localPRBranchRef, remotePRBranchRef) - if !tt.wantErr(t, err) { - return - } - assert.Equal(t, tt.want, got) - }) - } -} diff --git a/internal/git/util_test.go b/internal/git/util_test.go deleted file mode 100644 index eff947f..0000000 --- a/internal/git/util_test.go +++ /dev/null @@ -1,189 +0,0 @@ -package git - -import ( - "fmt" - "io" - "log/slog" - "os" - "testing" - "time" - - "github.com/go-git/go-billy/v5/memfs" - "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" - "github.com/go-git/go-git/v5/storage/memory" - "github.com/stretchr/testify/require" -) - -var ( - author = &object.Signature{ - Name: "releaser-pleaser", - When: time.Date(2020, 01, 01, 01, 01, 01, 01, time.UTC), - } -) - -type CommitOption func(*commitOptions) -type commitOptions struct { - cleanFiles bool - files []commitFile - tags []string - newRef plumbing.ReferenceName - parentRef plumbing.ReferenceName -} -type commitFile struct { - path string - content string -} - -type TestCommit func(*testing.T, *Repository) error -type TestRepo func(*testing.T) *Repository - -func WithCommit(message string, options ...CommitOption) TestCommit { - return func(t *testing.T, repo *Repository) error { - t.Helper() - - require.NotEmpty(t, message, "commit message is required") - - opts := &commitOptions{} - for _, opt := range options { - opt(opts) - } - - wt, err := repo.r.Worktree() - require.NoError(t, err) - - if opts.parentRef != "" { - checkoutOptions := &git.CheckoutOptions{} - - if opts.newRef != "" { - parentRef, err := repo.r.Reference(opts.parentRef, false) - require.NoError(t, err) - - checkoutOptions.Create = true - checkoutOptions.Hash = parentRef.Hash() - checkoutOptions.Branch = opts.newRef - } else { - checkoutOptions.Branch = opts.parentRef - } - - err = wt.Checkout(checkoutOptions) - require.NoError(t, err) - } - - // Yeet all files - if opts.cleanFiles { - files, err := wt.Filesystem.ReadDir(".") - require.NoError(t, err, "failed to get current files") - - for _, fileInfo := range files { - err = wt.Filesystem.Remove(fileInfo.Name()) - require.NoError(t, err, "failed to remove file %q", fileInfo.Name()) - } - } - - // Create new files - for _, fileInfo := range opts.files { - file, err := wt.Filesystem.Create(fileInfo.path) - require.NoError(t, err, "failed to create file %q", fileInfo.path) - - _, err = file.Write([]byte(fileInfo.content)) - _ = file.Close() - require.NoError(t, err, "failed to write content to file %q", fileInfo.path) - - _, err = wt.Add(fileInfo.path) - require.NoError(t, err, "failed to stage changes to file %q", fileInfo.path) - - } - - // Commit - commitHash, err := wt.Commit(message, &git.CommitOptions{ - All: true, - AllowEmptyCommits: true, - Author: author, - Committer: author, - }) - require.NoError(t, err, "failed to commit") - - // Create tags - for _, tagName := range opts.tags { - _, err = repo.r.CreateTag(tagName, commitHash, nil) - require.NoError(t, err, "failed to create tag %q", tagName) - } - - return nil - } -} - -func WithFile(path, content string) CommitOption { - return func(opts *commitOptions) { - opts.files = append(opts.files, commitFile{path: path, content: content}) - } -} - -// WithCleanFiles removes all previous files from the repo. Make sure to leave at least one file in the root -// directory when switching branches! -func WithCleanFiles() CommitOption { - return func(opts *commitOptions) { - opts.cleanFiles = true - } -} - -func AsNewBranch(ref plumbing.ReferenceName) CommitOption { - return func(opts *commitOptions) { - opts.newRef = ref - } -} - -func OnBranch(ref plumbing.ReferenceName) CommitOption { - return func(opts *commitOptions) { - opts.parentRef = ref - } -} - -func WithTag(name string) CommitOption { - return func(opts *commitOptions) { - opts.tags = append(opts.tags, name) - } -} - -// Can be useful to debug git issues by using it in a terminal -const useOnDiskTestRepository = false - -func WithTestRepo(commits ...TestCommit) TestRepo { - return func(t *testing.T) *Repository { - t.Helper() - - repo := &Repository{ - logger: slog.New(slog.NewTextHandler(io.Discard, nil)), - } - - var err error - - initOptions := git.InitOptions{DefaultBranch: plumbing.Main} - - if useOnDiskTestRepository { - dir, err := os.MkdirTemp(os.TempDir(), "rp-test-repo-") - require.NoError(t, err, "failed to create temp directory") - - repo.r, err = git.PlainInitWithOptions(dir, &git.PlainInitOptions{InitOptions: initOptions}) - require.NoError(t, err, "failed to create fs repository") - - fmt.Printf("using temp directory: %s", dir) - } else { - repo.r, err = git.InitWithOptions(memory.NewStorage(), memfs.New(), initOptions) - require.NoError(t, err, "failed to create in-memory repository") - } - - // Make initial commit - err = WithCommit("chore: init", WithFile("README.md", "# git test util"))(t, repo) - require.NoError(t, err, "failed to create init commit") - - for i, commit := range commits { - err = commit(t, repo) - require.NoError(t, err, "failed to create commit %d", i) - } - - return repo - } -} diff --git a/releaserpleaser.go b/releaserpleaser.go index 409d8bb..f2f4064 100644 --- a/releaserpleaser.go +++ b/releaserpleaser.go @@ -255,21 +255,16 @@ func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error { } } - releaseCommitAuthor, err := rp.forge.CommitAuthor(ctx) - if err != nil { - return fmt.Errorf("failed to get commit author: %w", err) - } - releaseCommitMessage := fmt.Sprintf("chore(%s): release %s", rp.targetBranch, nextVersion) - releaseCommit, err := repo.Commit(ctx, releaseCommitMessage, releaseCommitAuthor) + releaseCommit, err := repo.Commit(ctx, releaseCommitMessage) if err != nil { return fmt.Errorf("failed to commit changes: %w", err) } - logger.InfoContext(ctx, "created release commit", "commit.hash", releaseCommit.Hash, "commit.message", releaseCommit.Message, "commit.author", releaseCommitAuthor) + logger.InfoContext(ctx, "created release commit", "commit.hash", releaseCommit.Hash, "commit.message", releaseCommit.Message) // Check if anything changed in comparison to the remote branch (if exists) - newReleasePRChanges, err := repo.HasChangesWithRemote(ctx, rp.targetBranch, rpBranch) + newReleasePRChanges, err := repo.HasChangesWithRemote(ctx, rpBranch) if err != nil { return err } @@ -327,11 +322,7 @@ func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error { } func (rp *ReleaserPleaser) analyzedCommitsSince(ctx context.Context, since *git.Tag) ([]commitparser.AnalyzedCommit, error) { - logger := rp.logger.With("method", "analyzedCommitsSince") - - if since != nil { - logger = rp.logger.With("tag.hash", since.Hash, "tag.name", since.Name) - } + logger := rp.logger.With("method", "analyzedCommitsSince", "tag.hash", since.Hash, "tag.name", since.Name) commits, err := rp.forge.CommitsSince(ctx, since) if err != nil {