diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index c9ca82128..b082bc40f 100644 --- a/custom/conf/app.ini.sample +++ b/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 diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index e71fb1b3b..08aca1edf 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/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`) diff --git a/models/pull.go b/models/pull.go index ba9c57577..9a8777aca 100644 --- a/models/pull.go +++ b/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 { diff --git a/models/review.go b/models/review.go index 493959e78..bc7dfbcd1 100644 --- a/models/review.go +++ b/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 } diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 5808c7600..8762b63e2 100644 --- a/modules/git/repo_commit.go +++ b/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 diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 3e7393efb..06797e891 100644 --- a/modules/setting/repository.go +++ b/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 diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 04f8a86ca..3503a5174 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/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}}
{{if $prUnit.PullRequestsConfig.AllowMerge}} @@ -141,7 +142,7 @@
- +