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.
This commit is contained in:
parent
0141d1667d
commit
d647e74502
|
@ -7,24 +7,62 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
|
||||||
repo_model "code.gitea.io/gitea/models/repo"
|
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/git"
|
||||||
"code.gitea.io/gitea/modules/log"
|
"code.gitea.io/gitea/modules/log"
|
||||||
"code.gitea.io/gitea/modules/setting"
|
"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)
|
// doMergeStyleSquash squashes the tracking branch on the current HEAD (=base)
|
||||||
func doMergeStyleSquash(ctx *mergeContext, message string) error {
|
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)
|
cmdMerge := git.NewCommand(ctx, "merge", "--squash").AddDynamicArguments(trackingBranch)
|
||||||
if err := runMergeCommand(ctx, repo_model.MergeStyleSquash, cmdMerge); err != nil {
|
if err := runMergeCommand(ctx, repo_model.MergeStyleSquash, cmdMerge); err != nil {
|
||||||
log.Error("%-v Unable to merge --squash tracking into base: %v", ctx.pr, err)
|
log.Error("%-v Unable to merge --squash tracking into base: %v", ctx.pr, err)
|
||||||
return 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() {
|
if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() {
|
||||||
// add trailer
|
// add trailer
|
||||||
message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String())
|
message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String())
|
||||||
|
|
|
@ -669,7 +669,12 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
|
||||||
|
|
||||||
authorString := commit.Author.String()
|
authorString := commit.Author.String()
|
||||||
if uniqueAuthors.Add(authorString) && authorString != posterSig {
|
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 {
|
for _, commit := range commits {
|
||||||
authorString := commit.Author.String()
|
authorString := commit.Author.String()
|
||||||
if uniqueAuthors.Add(authorString) && authorString != posterSig {
|
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
|
skip += limit
|
||||||
|
|
Loading…
Reference in a new issue