aboutsummaryrefslogtreecommitdiff
path: root/services
diff options
context:
space:
mode:
authorBrecht Van Lommel2023-03-10 04:17:04 +0100
committerGitHub2023-03-09 22:17:04 -0500
commitd647e74502fdf734c89b3e6592a9ad88c3005971 (patch)
tree5c7c2fbd84bb95c7fffd277ccbddc4382617a597 /services
parent0141d1667d4d0e98124c11e179928a42ffe6ab1a (diff)
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.
Diffstat (limited to 'services')
-rw-r--r--services/pull/merge_squash.go48
-rw-r--r--services/pull/pull.go12
2 files changed, 53 insertions, 7 deletions
diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go
index d6e731498..f52a2301d 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 d8923d0d5..e40e59a2c 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