From d647e74502fdf734c89b3e6592a9ad88c3005971 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Fri, 10 Mar 2023 04:17:04 +0100 Subject: [PATCH] Improve squash merge commit author and co-author with private emails (#22977) When emails addresses are private, squash merges always use `@noreply.localhost` for the author of the squash commit. And the author is redundantly added as a co-author in the commit message. Also without private mails, the redundant co-author is possible when committing with a signature that's different than the user full name and primary email. Now try to find a commit by the same user in the list of commits, and prefer the signature from that over one constructed from the account settings. --- services/pull/merge_squash.go | 48 +++++++++++++++++++++++++++++++---- services/pull/pull.go | 12 +++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index d6e7314988..f52a2301d9 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -7,24 +7,62 @@ import ( "fmt" repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" ) +// doMergeStyleSquash gets a commit author signature for squash commits +func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { + if err := ctx.pr.Issue.LoadPoster(ctx); err != nil { + log.Error("%-v Issue[%d].LoadPoster: %v", ctx.pr, ctx.pr.Issue.ID, err) + return nil, err + } + + // Try to get an signature from the same user in one of the commits, as the + // poster email might be private or commits might have a different signature + // than the primary email address of the poster. + gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, ctx.tmpBasePath) + if err != nil { + log.Error("%-v Unable to open base repository: %v", ctx.pr, err) + return nil, err + } + defer closer.Close() + + commits, err := gitRepo.CommitsBetweenIDs(trackingBranch, "HEAD") + if err != nil { + log.Error("%-v Unable to get commits between: %s %s: %v", ctx.pr, "HEAD", trackingBranch, err) + return nil, err + } + + uniqueEmails := make(container.Set[string]) + for _, commit := range commits { + if commit.Author != nil && uniqueEmails.Add(commit.Author.Email) { + commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email) + if commitUser != nil && commitUser.ID == ctx.pr.Issue.Poster.ID { + return commit.Author, nil + } + } + } + + return ctx.pr.Issue.Poster.NewGitSig(), nil +} + // doMergeStyleSquash squashes the tracking branch on the current HEAD (=base) func doMergeStyleSquash(ctx *mergeContext, message string) error { + sig, err := getAuthorSignatureSquash(ctx) + if err != nil { + return fmt.Errorf("getAuthorSignatureSquash: %w", err) + } + cmdMerge := git.NewCommand(ctx, "merge", "--squash").AddDynamicArguments(trackingBranch) if err := runMergeCommand(ctx, repo_model.MergeStyleSquash, cmdMerge); err != nil { log.Error("%-v Unable to merge --squash tracking into base: %v", ctx.pr, err) return err } - if err := ctx.pr.Issue.LoadPoster(ctx); err != nil { - log.Error("%-v Issue[%d].LoadPoster: %v", ctx.pr, ctx.pr.Issue.ID, err) - return fmt.Errorf("LoadPoster: %w", err) - } - sig := ctx.pr.Issue.Poster.NewGitSig() if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() { // add trailer message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String()) diff --git a/services/pull/pull.go b/services/pull/pull.go index d8923d0d57..e40e59a2c5 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -669,7 +669,12 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ authorString := commit.Author.String() if uniqueAuthors.Add(authorString) && authorString != posterSig { - authors = append(authors, authorString) + // Compare use account as well to avoid adding the same author multiple times + // times when email addresses are private or multiple emails are used. + commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email) + if commitUser == nil || commitUser.ID != pr.Issue.Poster.ID { + authors = append(authors, authorString) + } } } @@ -690,7 +695,10 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ for _, commit := range commits { authorString := commit.Author.String() if uniqueAuthors.Add(authorString) && authorString != posterSig { - authors = append(authors, authorString) + commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email) + if commitUser == nil || commitUser.ID != pr.Issue.Poster.ID { + authors = append(authors, authorString) + } } } skip += limit