Provide Default messages for merges (#9393)

Co-Authored-By: guillep2k <[email protected]>

custom/conf/app.ini.sample

@@ -76,6 +76,16 @@ WORK_IN_PROGRESS_PREFIXES=WIP:,[WIP]
 CLOSE_KEYWORDS=close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved
 ; List of keywords used in Pull Request comments to automatically reopen a related issue
 REOPEN_KEYWORDS=reopen,reopens,reopened
+; In the default merge message for squash commits include at most this many commits
+DEFAULT_MERGE_MESSAGE_COMMITS_LIMIT=50
+; In the default merge message for squash commits limit the size of the commit messages to this
+DEFAULT_MERGE_MESSAGE_SIZE=5120
+; In the default merge message for squash commits walk all commits to include all authors in the Co-authored-by otherwise just use those in the limited list
+DEFAULT_MERGE_MESSAGE_ALL_AUTHORS=false
+; In default merge messages limit the number of approvers listed as Reviewed-by: to this many
+DEFAULT_MERGE_MESSAGE_MAX_APPROVERS=10
+; In default merge messages only include approvers who are official
+DEFAULT_MERGE_MESSAGE_OFFICIAL_APPROVERS_ONLY=true
 
 [repository.issue]
 ; List of reasons why a Pull Request or Issue can be locked

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -77,6 +77,11 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
  keywords used in Pull Request comments to automatically close a related issue
 - `REOPEN_KEYWORDS`: **reopen**, **reopens**, **reopened**: List of keywords used in Pull Request comments to automatically reopen
  a related issue
+- `DEFAULT_MERGE_MESSAGE_COMMITS_LIMIT`: **50**: In the default merge message for squash commits include at most this many commits. Set to `-1` to include all commits
+- `DEFAULT_MERGE_MESSAGE_SIZE`: **5120**: In the default merge message for squash commits limit the size of the commit messages. Set to `-1` to have no limit.
+- `DEFAULT_MERGE_MESSAGE_ALL_AUTHORS`: **false**: In the default merge message for squash commits walk all commits to include all authors in the Co-authored-by otherwise just use those in the limited list
+- `DEFAULT_MERGE_MESSAGE_MAX_APPROVERS`: **10**: In default merge messages limit the number of approvers listed as `Reviewed-by:`. Set to `-1` to include all.
+- `DEFAULT_MERGE_MESSAGE_OFFICIAL_APPROVERS_ONLY`: **true**: In default merge messages only include approvers who are officially allowed to review.
 
 ### Repository - Issue (`repository.issue`)
 

models/pull.go

@@ -7,6 +7,7 @@ package models
 
 import (
 	"fmt"
+	"io"
 	"strings"
 
 	"code.gitea.io/gitea/modules/git"
@@ -177,6 +178,206 @@ func (pr *PullRequest) GetDefaultMergeMessage() string {
 	return fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.MustHeadUserName(), pr.HeadRepo.Name, pr.BaseBranch)
 }
 
+// GetCommitMessages returns the commit messages between head and merge base (if there is one)
+func (pr *PullRequest) GetCommitMessages() string {
+	if err := pr.LoadIssue(); err != nil {
+		log.Error("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err)
+		return ""
+	}
+
+	if err := pr.Issue.LoadPoster(); err != nil {
+		log.Error("Cannot load poster %d for pr id %d, index %d Error: %v", pr.Issue.PosterID, pr.ID, pr.Index, err)
+		return ""
+	}
+
+	if pr.HeadRepo == nil {
+		var err error
+		pr.HeadRepo, err = GetRepositoryByID(pr.HeadRepoID)
+		if err != nil {
+			log.Error("GetRepositoryById[%d]: %v", pr.HeadRepoID, err)
+			return ""
+		}
+	}
+
+	gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
+	if err != nil {
+		log.Error("Unable to open head repository: Error: %v", err)
+		return ""
+	}
+	defer gitRepo.Close()
+
+	headCommit, err := gitRepo.GetBranchCommit(pr.HeadBranch)
+	if err != nil {
+		log.Error("Unable to get head commit: %s Error: %v", pr.HeadBranch, err)
+		return ""
+	}
+
+	mergeBase, err := gitRepo.GetCommit(pr.MergeBase)
+	if err != nil {
+		log.Error("Unable to get merge base commit: %s Error: %v", pr.MergeBase, err)
+		return ""
+	}
+
+	limit := setting.Repository.PullRequest.DefaultMergeMessageCommitsLimit
+
+	list, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, 0)
+	if err != nil {
+		log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err)
+		return ""
+	}
+
+	maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize
+
+	posterSig := pr.Issue.Poster.NewGitSig().String()
+
+	authorsMap := map[string]bool{}
+	authors := make([]string, 0, list.Len())
+	stringBuilder := strings.Builder{}
+	element := list.Front()
+	for element != nil {
+		commit := element.Value.(*git.Commit)
+
+		if maxSize < 0 || stringBuilder.Len() < maxSize {
+			toWrite := []byte(commit.CommitMessage)
+			if len(toWrite) > maxSize-stringBuilder.Len() && maxSize > -1 {
+				toWrite = append(toWrite[:maxSize-stringBuilder.Len()], "..."...)
+			}
+			if _, err := stringBuilder.Write(toWrite); err != nil {
+				log.Error("Unable to write commit message Error: %v", err)
+				return ""
+			}
+
+			if _, err := stringBuilder.WriteRune('\n'); err != nil {
+				log.Error("Unable to write commit message Error: %v", err)
+				return ""
+			}
+		}
+
+		authorString := commit.Author.String()
+		if !authorsMap[authorString] && authorString != posterSig {
+			authors = append(authors, authorString)
+			authorsMap[authorString] = true
+		}
+		element = element.Next()
+	}
+
+	// Consider collecting the remaining authors
+	if limit >= 0 && setting.Repository.PullRequest.DefaultMergeMessageAllAuthors {
+		skip := limit
+		limit = 30
+		for {
+			list, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip)
+			if err != nil {
+				log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err)
+				return ""
+
+			}
+			if list.Len() == 0 {
+				break
+			}
+			element := list.Front()
+			for element != nil {
+				commit := element.Value.(*git.Commit)
+
+				authorString := commit.Author.String()
+				if !authorsMap[authorString] && authorString != posterSig {
+					authors = append(authors, authorString)
+					authorsMap[authorString] = true
+				}
+				element = element.Next()
+			}
+
+		}
+	}
+
+	if len(authors) > 0 {
+		if _, err := stringBuilder.WriteRune('\n'); err != nil {
+			log.Error("Unable to write to string builder Error: %v", err)
+			return ""
+		}
+	}
+
+	for _, author := range authors {
+		if _, err := stringBuilder.Write([]byte("Co-authored-by: ")); err != nil {
+			log.Error("Unable to write to string builder Error: %v", err)
+			return ""
+		}
+		if _, err := stringBuilder.Write([]byte(author)); err != nil {
+			log.Error("Unable to write to string builder Error: %v", err)
+			return ""
+		}
+		if _, err := stringBuilder.WriteRune('\n'); err != nil {
+			log.Error("Unable to write to string builder Error: %v", err)
+			return ""
+		}
+	}
+
+	return stringBuilder.String()
+}
+
+// GetApprovers returns the approvers of the pull request
+func (pr *PullRequest) GetApprovers() string {
+
+	stringBuilder := strings.Builder{}
+	if err := pr.getReviewedByLines(&stringBuilder); err != nil {
+		log.Error("Unable to getReviewedByLines: Error: %v", err)
+		return ""
+	}
+
+	return stringBuilder.String()
+}
+
+func (pr *PullRequest) getReviewedByLines(writer io.Writer) error {
+	maxReviewers := setting.Repository.PullRequest.DefaultMergeMessageMaxApprovers
+
+	if maxReviewers == 0 {
+		return nil
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	// Note: This doesn't page as we only expect a very limited number of reviews
+	reviews, err := findReviews(sess, FindReviewOptions{
+		Type:         ReviewTypeApprove,
+		IssueID:      pr.IssueID,
+		OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
+	})
+	if err != nil {
+		log.Error("Unable to FindReviews for PR ID %d: %v", pr.ID, err)
+		return err
+	}
+
+	reviewersWritten := 0
+
+	for _, review := range reviews {
+		if maxReviewers > 0 && reviewersWritten > maxReviewers {
+			break
+		}
+
+		if err := review.loadReviewer(sess); err != nil && !IsErrUserNotExist(err) {
+			log.Error("Unable to LoadReviewer[%d] for PR ID %d : %v", review.ReviewerID, pr.ID, err)
+			return err
+		} else if review.Reviewer == nil {
+			continue
+		}
+		if _, err := writer.Write([]byte("Reviewed-by: ")); err != nil {
+			return err
+		}
+		if _, err := writer.Write([]byte(review.Reviewer.NewGitSig().String())); err != nil {
+			return err
+		}
+		if _, err := writer.Write([]byte{'\n'}); err != nil {
+			return err
+		}
+		reviewersWritten++
+	}
+	return sess.Commit()
+}
+
 // GetDefaultSquashMessage returns default message used when squash and merging pull request
 func (pr *PullRequest) GetDefaultSquashMessage() string {
 	if err := pr.LoadIssue(); err != nil {

models/review.go

@@ -125,9 +125,10 @@ func GetReviewByID(id int64) (*Review, error) {
 
 // FindReviewOptions represent possible filters to find reviews
 type FindReviewOptions struct {
-	Type       ReviewType
-	IssueID    int64
-	ReviewerID int64
+	Type         ReviewType
+	IssueID      int64
+	ReviewerID   int64
+	OfficialOnly bool
 }
 
 func (opts *FindReviewOptions) toCond() builder.Cond {
@@ -141,6 +142,9 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
 	if opts.Type != ReviewTypeUnknown {
 		cond = cond.And(builder.Eq{"type": opts.Type})
 	}
+	if opts.OfficialOnly {
+		cond = cond.And(builder.Eq{"official": true})
+	}
 	return cond
 }
 

modules/git/repo_commit.go

@@ -315,7 +315,28 @@ func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (in
 
 // CommitsBetween returns a list that contains commits between [last, before).
 func (repo *Repository) CommitsBetween(last *Commit, before *Commit) (*list.List, error) {
-	stdout, err := NewCommand("rev-list", before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
+	var stdout []byte
+	var err error
+	if before == nil {
+		stdout, err = NewCommand("rev-list", before.ID.String()).RunInDirBytes(repo.Path)
+	} else {
+		stdout, err = NewCommand("rev-list", before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
+	}
+	if err != nil {
+		return nil, err
+	}
+	return repo.parsePrettyFormatLogToList(bytes.TrimSpace(stdout))
+}
+
+// CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [last, before)
+func (repo *Repository) CommitsBetweenLimit(last *Commit, before *Commit, limit, skip int) (*list.List, error) {
+	var stdout []byte
+	var err error
+	if before == nil {
+		stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), last.ID.String()).RunInDirBytes(repo.Path)
+	} else {
+		stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
+	}
 	if err != nil {
 		return nil, err
 	}
@@ -328,6 +349,9 @@ func (repo *Repository) CommitsBetweenIDs(last, before string) (*list.List, erro
 	if err != nil {
 		return nil, err
 	}
+	if before == "" {
+		return repo.CommitsBetween(lastCommit, nil)
+	}
 	beforeCommit, err := repo.GetCommit(before)
 	if err != nil {
 		return nil, err

modules/setting/repository.go

@@ -60,9 +60,14 @@ var (
 
 		// Pull request settings
 		PullRequest struct {
-			WorkInProgressPrefixes []string
-			CloseKeywords          []string
-			ReopenKeywords         []string
+			WorkInProgressPrefixes                   []string
+			CloseKeywords                            []string
+			ReopenKeywords                           []string
+			DefaultMergeMessageCommitsLimit          int
+			DefaultMergeMessageSize                  int
+			DefaultMergeMessageAllAuthors            bool
+			DefaultMergeMessageMaxApprovers          int
+			DefaultMergeMessageOfficialApproversOnly bool
 		} `ini:"repository.pull-request"`
 
 		// Issue Setting
@@ -127,15 +132,25 @@ var (
 
 		// Pull request settings
 		PullRequest: struct {
-			WorkInProgressPrefixes []string
-			CloseKeywords          []string
-			ReopenKeywords         []string
+			WorkInProgressPrefixes                   []string
+			CloseKeywords                            []string
+			ReopenKeywords                           []string
+			DefaultMergeMessageCommitsLimit          int
+			DefaultMergeMessageSize                  int
+			DefaultMergeMessageAllAuthors            bool
+			DefaultMergeMessageMaxApprovers          int
+			DefaultMergeMessageOfficialApproversOnly bool
 		}{
 			WorkInProgressPrefixes: []string{"WIP:", "[WIP]"},
 			// Same as GitHub. See
 			// https://help.github.com/articles/closing-issues-via-commit-messages
-			CloseKeywords:  strings.Split("close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved", ","),
-			ReopenKeywords: strings.Split("reopen,reopens,reopened", ","),
+			CloseKeywords:                            strings.Split("close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved", ","),
+			ReopenKeywords:                           strings.Split("reopen,reopens,reopened", ","),
+			DefaultMergeMessageCommitsLimit:          50,
+			DefaultMergeMessageSize:                  5 * 1024,
+			DefaultMergeMessageAllAuthors:            false,
+			DefaultMergeMessageMaxApprovers:          10,
+			DefaultMergeMessageOfficialApproversOnly: true,
 		},
 
 		// Issue settings

templates/repo/issue/view_content/pull.tmpl

@@ -131,6 +131,7 @@
 					{{end}}
 					{{if .AllowMerge}}
 						{{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}}
+						{{$approvers := .Issue.PullRequest.GetApprovers}}
 						{{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowRebaseMerge $prUnit.PullRequestsConfig.AllowSquash}}
 							<div class="ui divider"></div>
 							{{if $prUnit.PullRequestsConfig.AllowMerge}}
@@ -141,7 +142,7 @@
 										<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
 									</div>
 									<div class="field">
-										<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}"></textarea>
+										<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{$approvers}}</textarea>
 									</div>
 									<button class="ui green button" type="submit" name="do" value="merge">
 										{{$.i18n.Tr "repo.pulls.merge_pull_request"}}
@@ -173,7 +174,7 @@
 										<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
 									</div>
 									<div class="field">
-										<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}"></textarea>
+										<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{$approvers}}</textarea>
 									</div>
 									<button class="ui green button" type="submit" name="do" value="rebase-merge">
 										{{$.i18n.Tr "repo.pulls.rebase_merge_commit_pull_request"}}
@@ -185,14 +186,15 @@
 							</div>
 							{{end}}
 							{{if $prUnit.PullRequestsConfig.AllowSquash}}
+							{{$commitMessages := .Issue.PullRequest.GetCommitMessages}}
 							<div class="ui form squash-fields" style="display: none">
 								<form action="{{.Link}}/merge" method="post">
 									{{.CsrfTokenHtml}}
 									<div class="field">
 										<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultSquashMessage}}">
 									</div>
 									<div class="field">
-										<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}"></textarea>
+										<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{$commitMessages}}{{$approvers}}</textarea>
 									</div>
 									<button class="ui green button" type="submit" name="do" value="squash">
 										{{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}}