From d9ef43a7126ab83af563c1c9b54cdf0092327b2a Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Mon, 9 Aug 2021 20:08:51 +0200 Subject: [PATCH] Replace `list.List` with slices (#16311) * Replaced list with slice. * Fixed usage of pointer to temporary variable. * Replaced LIFO list with slice. * Lint * Removed type check. * Removed duplicated code. * Lint * Fixed merge. Co-authored-by: 6543 <6543@obermui.de> --- models/commit.go | 20 +++++ models/commit_status.go | 18 ++-- models/gpg_key_commit_verification.go | 18 ++-- models/issue_comment.go | 83 +++++++------------ models/pull_sign.go | 3 +- models/user.go | 19 ++--- modules/git/commit.go | 11 ++- modules/git/repo.go | 11 ++- modules/git/repo_commit.go | 38 ++++----- modules/git/repo_commit_test.go | 2 +- modules/git/repo_compare.go | 5 +- modules/repository/commits.go | 12 ++- modules/repository/commits_test.go | 30 +++---- modules/templates/helper.go | 29 ------- routers/api/v1/repo/commits.go | 10 +-- routers/api/v1/repo/pull.go | 25 ++---- routers/web/repo/blame.go | 17 ++-- routers/web/repo/commit.go | 17 +--- routers/web/repo/compare.go | 12 ++- routers/web/repo/pull.go | 20 ++--- routers/web/repo/wiki.go | 5 +- services/mirror/mirror_pull.go | 2 +- services/pull/pull.go | 32 ++++--- services/repository/push.go | 5 +- templates/mail/issue/default.tmpl | 5 +- templates/repo/commits_list.tmpl | 7 +- templates/repo/commits_list_small.tmpl | 7 +- .../repo/issue/view_content/comments.tmpl | 2 +- templates/user/dashboard/feeds.tmpl | 20 ++--- 29 files changed, 183 insertions(+), 302 deletions(-) create mode 100644 models/commit.go diff --git a/models/commit.go b/models/commit.go new file mode 100644 index 000000000..474825820 --- /dev/null +++ b/models/commit.go @@ -0,0 +1,20 @@ +// Copyright 2021 Gitea. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "code.gitea.io/gitea/modules/git" +) + +// ConvertFromGitCommit converts git commits into SignCommitWithStatuses +func ConvertFromGitCommit(commits []*git.Commit, repo *Repository) []*SignCommitWithStatuses { + return ParseCommitsWithStatus( + ParseCommitsWithSignature( + ValidateCommitsWithEmails(commits), + repo, + ), + repo, + ) +} diff --git a/models/commit_status.go b/models/commit_status.go index 1105c3b17..c27582024 100644 --- a/models/commit_status.go +++ b/models/commit_status.go @@ -5,7 +5,6 @@ package models import ( - "container/list" "crypto/sha1" "fmt" "strings" @@ -257,16 +256,12 @@ type SignCommitWithStatuses struct { } // ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state -func ParseCommitsWithStatus(oldCommits *list.List, repo *Repository) *list.List { - var ( - newCommits = list.New() - e = oldCommits.Front() - ) +func ParseCommitsWithStatus(oldCommits []*SignCommit, repo *Repository) []*SignCommitWithStatuses { + newCommits := make([]*SignCommitWithStatuses, 0, len(oldCommits)) - for e != nil { - c := e.Value.(SignCommit) - commit := SignCommitWithStatuses{ - SignCommit: &c, + for _, c := range oldCommits { + commit := &SignCommitWithStatuses{ + SignCommit: c, } statuses, err := GetLatestCommitStatus(repo.ID, commit.ID.String(), ListOptions{}) if err != nil { @@ -276,8 +271,7 @@ func ParseCommitsWithStatus(oldCommits *list.List, repo *Repository) *list.List commit.Status = CalcCommitStatus(statuses) } - newCommits.PushBack(commit) - e = e.Next() + newCommits = append(newCommits, commit) } return newCommits } diff --git a/models/gpg_key_commit_verification.go b/models/gpg_key_commit_verification.go index f0c27f13a..a4c7d7028 100644 --- a/models/gpg_key_commit_verification.go +++ b/models/gpg_key_commit_verification.go @@ -5,7 +5,6 @@ package models import ( - "container/list" "fmt" "hash" "strings" @@ -68,24 +67,19 @@ const ( ) // ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys. -func ParseCommitsWithSignature(oldCommits *list.List, repository *Repository) *list.List { - var ( - newCommits = list.New() - e = oldCommits.Front() - ) +func ParseCommitsWithSignature(oldCommits []*UserCommit, repository *Repository) []*SignCommit { + newCommits := make([]*SignCommit, 0, len(oldCommits)) keyMap := map[string]bool{} - for e != nil { - c := e.Value.(UserCommit) - signCommit := SignCommit{ - UserCommit: &c, + for _, c := range oldCommits { + signCommit := &SignCommit{ + UserCommit: c, Verification: ParseCommitWithSignature(c.Commit), } _ = CalculateTrustStatus(signCommit.Verification, repository, &keyMap) - newCommits.PushBack(signCommit) - e = e.Next() + newCommits = append(newCommits, signCommit) } return newCommits } diff --git a/models/issue_comment.go b/models/issue_comment.go index ae2dc0811..755041efd 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -7,7 +7,6 @@ package models import ( - "container/list" "fmt" "regexp" "strconv" @@ -191,11 +190,11 @@ type Comment struct { RefIssue *Issue `xorm:"-"` RefComment *Comment `xorm:"-"` - Commits *list.List `xorm:"-"` - OldCommit string `xorm:"-"` - NewCommit string `xorm:"-"` - CommitsNum int64 `xorm:"-"` - IsForcePush bool `xorm:"-"` + Commits []*SignCommitWithStatuses `xorm:"-"` + OldCommit string `xorm:"-"` + NewCommit string `xorm:"-"` + CommitsNum int64 `xorm:"-"` + IsForcePush bool `xorm:"-"` } // PushActionContent is content of push pull comment @@ -675,13 +674,8 @@ func (c *Comment) LoadPushCommits() (err error) { } defer gitRepo.Close() - c.Commits = gitRepo.GetCommitsFromIDs(data.CommitIDs) - c.CommitsNum = int64(c.Commits.Len()) - if c.CommitsNum > 0 { - c.Commits = ValidateCommitsWithEmails(c.Commits) - c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.Repo) - c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) - } + c.Commits = ConvertFromGitCommit(gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo) + c.CommitsNum = int64(len(c.Commits)) } return err @@ -1293,21 +1287,17 @@ func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch return nil, false, err } - var ( - commits *list.List - commitChecks map[string]commitBranchCheckItem - ) - commits, err = newCommit.CommitsBeforeUntil(oldCommitID) + commits, err := newCommit.CommitsBeforeUntil(oldCommitID) if err != nil { return nil, false, err } - commitIDs = make([]string, 0, commits.Len()) - commitChecks = make(map[string]commitBranchCheckItem) + commitIDs = make([]string, 0, len(commits)) + commitChecks := make(map[string]*commitBranchCheckItem) - for e := commits.Front(); e != nil; e = e.Next() { - commitChecks[e.Value.(*git.Commit).ID.String()] = commitBranchCheckItem{ - Commit: e.Value.(*git.Commit), + for _, commit := range commits { + commitChecks[commit.ID.String()] = &commitBranchCheckItem{ + Commit: commit, Checked: false, } } @@ -1316,8 +1306,8 @@ func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch return } - for e := commits.Back(); e != nil; e = e.Prev() { - commitID := e.Value.(*git.Commit).ID.String() + for i := len(commits) - 1; i >= 0; i-- { + commitID := commits[i].ID.String() if item, ok := commitChecks[commitID]; ok && item.Checked { commitIDs = append(commitIDs, commitID) } @@ -1331,64 +1321,49 @@ type commitBranchCheckItem struct { Checked bool } -func commitBranchCheck(gitRepo *git.Repository, startCommit *git.Commit, endCommitID, baseBranch string, commitList map[string]commitBranchCheckItem) (err error) { - var ( - item commitBranchCheckItem - ok bool - listItem *list.Element - tmp string - ) - +func commitBranchCheck(gitRepo *git.Repository, startCommit *git.Commit, endCommitID, baseBranch string, commitList map[string]*commitBranchCheckItem) error { if startCommit.ID.String() == endCommitID { - return + return nil } - checkStack := list.New() - checkStack.PushBack(startCommit.ID.String()) - listItem = checkStack.Back() + checkStack := make([]string, 0, 10) + checkStack = append(checkStack, startCommit.ID.String()) - for listItem != nil { - tmp = listItem.Value.(string) - checkStack.Remove(listItem) + for len(checkStack) > 0 { + commitID := checkStack[0] + checkStack = checkStack[1:] - if item, ok = commitList[tmp]; !ok { - listItem = checkStack.Back() + item, ok := commitList[commitID] + if !ok { continue } if item.Commit.ID.String() == endCommitID { - listItem = checkStack.Back() continue } - if err = item.Commit.LoadBranchName(); err != nil { - return + if err := item.Commit.LoadBranchName(); err != nil { + return err } if item.Commit.Branch == baseBranch { - listItem = checkStack.Back() continue } if item.Checked { - listItem = checkStack.Back() continue } item.Checked = true - commitList[tmp] = item parentNum := item.Commit.ParentCount() for i := 0; i < parentNum; i++ { - var parentCommit *git.Commit - parentCommit, err = item.Commit.Parent(i) + parentCommit, err := item.Commit.Parent(i) if err != nil { - return + return err } - checkStack.PushBack(parentCommit.ID.String()) + checkStack = append(checkStack, parentCommit.ID.String()) } - - listItem = checkStack.Back() } return nil } diff --git a/models/pull_sign.go b/models/pull_sign.go index 10a6522eb..e7cf4ab66 100644 --- a/models/pull_sign.go +++ b/models/pull_sign.go @@ -118,8 +118,7 @@ Loop: if err != nil { return false, "", nil, err } - for e := commitList.Front(); e != nil; e = e.Next() { - commit = e.Value.(*git.Commit) + for _, commit := range commitList { verification := ParseCommitWithSignature(commit) if !verification.Verified { return false, "", nil, &ErrWontSign{commitsSigned} diff --git a/models/user.go b/models/user.go index a4f94999e..c68417a2c 100644 --- a/models/user.go +++ b/models/user.go @@ -6,7 +6,6 @@ package models import ( - "container/list" "context" "crypto/sha256" "crypto/subtle" @@ -1509,16 +1508,13 @@ func ValidateCommitWithEmail(c *git.Commit) *User { } // ValidateCommitsWithEmails checks if authors' e-mails of commits are corresponding to users. -func ValidateCommitsWithEmails(oldCommits *list.List) *list.List { +func ValidateCommitsWithEmails(oldCommits []*git.Commit) []*UserCommit { var ( - u *User - emails = map[string]*User{} - newCommits = list.New() - e = oldCommits.Front() + emails = make(map[string]*User) + newCommits = make([]*UserCommit, 0, len(oldCommits)) ) - for e != nil { - c := e.Value.(*git.Commit) - + for _, c := range oldCommits { + var u *User if c.Author != nil { if v, ok := emails[c.Author.Email]; !ok { u, _ = GetUserByEmail(c.Author.Email) @@ -1526,15 +1522,12 @@ func ValidateCommitsWithEmails(oldCommits *list.List) *list.List { } else { u = v } - } else { - u = nil } - newCommits.PushBack(UserCommit{ + newCommits = append(newCommits, &UserCommit{ User: u, Commit: c, }) - e = e.Next() } return newCommits } diff --git a/modules/git/commit.go b/modules/git/commit.go index 3ce2b0388..fe2c2b977 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -8,7 +8,6 @@ package git import ( "bufio" "bytes" - "container/list" "errors" "fmt" "io" @@ -187,12 +186,12 @@ func (c *Commit) CommitsCount() (int64, error) { } // CommitsByRange returns the specific page commits before current revision, every page's number default by CommitsRangeSize -func (c *Commit) CommitsByRange(page, pageSize int) (*list.List, error) { +func (c *Commit) CommitsByRange(page, pageSize int) ([]*Commit, error) { return c.repo.commitsByRange(c.ID, page, pageSize) } // CommitsBefore returns all the commits before current revision -func (c *Commit) CommitsBefore() (*list.List, error) { +func (c *Commit) CommitsBefore() ([]*Commit, error) { return c.repo.getCommitsBefore(c.ID) } @@ -228,12 +227,12 @@ func (c *Commit) HasPreviousCommit(commitHash SHA1) (bool, error) { } // CommitsBeforeLimit returns num commits before current revision -func (c *Commit) CommitsBeforeLimit(num int) (*list.List, error) { +func (c *Commit) CommitsBeforeLimit(num int) ([]*Commit, error) { return c.repo.getCommitsBeforeLimit(c.ID, num) } // CommitsBeforeUntil returns the commits between commitID to current revision -func (c *Commit) CommitsBeforeUntil(commitID string) (*list.List, error) { +func (c *Commit) CommitsBeforeUntil(commitID string) ([]*Commit, error) { endCommit, err := c.repo.GetCommit(commitID) if err != nil { return nil, err @@ -281,7 +280,7 @@ func NewSearchCommitsOptions(searchString string, forAllRefs bool) SearchCommits } // SearchCommits returns the commits match the keyword before current revision -func (c *Commit) SearchCommits(opts SearchCommitsOptions) (*list.List, error) { +func (c *Commit) SearchCommits(opts SearchCommitsOptions) ([]*Commit, error) { return c.repo.searchCommits(c.ID, opts) } diff --git a/modules/git/repo.go b/modules/git/repo.go index 43f329f44..4e6f90c3e 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -7,7 +7,6 @@ package git import ( "bytes" - "container/list" "context" "fmt" "os" @@ -33,10 +32,10 @@ func (repo *Repository) GetAllCommitsCount() (int64, error) { return AllCommitsCount(repo.Path, false) } -func (repo *Repository) parsePrettyFormatLogToList(logs []byte) (*list.List, error) { - l := list.New() +func (repo *Repository) parsePrettyFormatLogToList(logs []byte) ([]*Commit, error) { + var commits []*Commit if len(logs) == 0 { - return l, nil + return commits, nil } parts := bytes.Split(logs, []byte{'\n'}) @@ -46,10 +45,10 @@ func (repo *Repository) parsePrettyFormatLogToList(logs []byte) (*list.List, err if err != nil { return nil, err } - l.PushBack(commit) + commits = append(commits, commit) } - return l, nil + return commits, nil } // IsRepoURLAccessible checks if given repository URL is accessible. diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 16ee5b2fd..e456f04e8 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -7,7 +7,6 @@ package git import ( "bytes" - "container/list" "io" "io/ioutil" "strconv" @@ -84,10 +83,10 @@ func (repo *Repository) GetCommitByPath(relpath string) (*Commit, error) { if err != nil { return nil, err } - return commits.Front().Value.(*Commit), nil + return commits[0], nil } -func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) (*list.List, error) { +func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) ([]*Commit, error) { stdout, err := NewCommand("log", id.String(), "--skip="+strconv.Itoa((page-1)*pageSize), "--max-count="+strconv.Itoa(pageSize), prettyLogFormat).RunInDirBytes(repo.Path) @@ -97,7 +96,7 @@ func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) (*list.List, return repo.parsePrettyFormatLogToList(stdout) } -func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) (*list.List, error) { +func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Commit, error) { // create new git log command with limit of 100 commis cmd := NewCommand("log", id.String(), "-100", prettyLogFormat) // ignore case @@ -201,7 +200,7 @@ func (repo *Repository) FileCommitsCount(revision, file string) (int64, error) { } // CommitsByFileAndRange return the commits according revision file and the page -func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) (*list.List, error) { +func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ([]*Commit, error) { skip := (page - 1) * setting.Git.CommitsRangeSize stdoutReader, stdoutWriter := io.Pipe() @@ -226,7 +225,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( _, err := io.CopyN(ioutil.Discard, stdoutReader, int64(skip*41)) if err != nil { if err == io.EOF { - return list.New(), nil + return []*Commit{}, nil } _ = stdoutReader.CloseWithError(err) return nil, err @@ -241,7 +240,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( } // CommitsByFileAndRangeNoFollow return the commits according revision file and the page -func (repo *Repository) CommitsByFileAndRangeNoFollow(revision, file string, page int) (*list.List, error) { +func (repo *Repository) CommitsByFileAndRangeNoFollow(revision, file string, page int) ([]*Commit, error) { stdout, err := NewCommand("log", revision, "--skip="+strconv.Itoa((page-1)*50), "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize), prettyLogFormat, "--", file).RunInDirBytes(repo.Path) if err != nil { @@ -266,7 +265,7 @@ func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (in // CommitsBetween returns a list that contains commits between [before, last). // If before is detached (removed by reset + push) it is not included. -func (repo *Repository) CommitsBetween(last *Commit, before *Commit) (*list.List, error) { +func (repo *Repository) CommitsBetween(last *Commit, before *Commit) ([]*Commit, error) { var stdout []byte var err error if before == nil { @@ -286,7 +285,7 @@ func (repo *Repository) CommitsBetween(last *Commit, before *Commit) (*list.List } // CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [before, last) -func (repo *Repository) CommitsBetweenLimit(last *Commit, before *Commit, limit, skip int) (*list.List, error) { +func (repo *Repository) CommitsBetweenLimit(last *Commit, before *Commit, limit, skip int) ([]*Commit, error) { var stdout []byte var err error if before == nil { @@ -306,7 +305,7 @@ func (repo *Repository) CommitsBetweenLimit(last *Commit, before *Commit, limit, } // CommitsBetweenIDs return commits between twoe commits -func (repo *Repository) CommitsBetweenIDs(last, before string) (*list.List, error) { +func (repo *Repository) CommitsBetweenIDs(last, before string) ([]*Commit, error) { lastCommit, err := repo.GetCommit(last) if err != nil { return nil, err @@ -334,7 +333,7 @@ func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) { } // commitsBefore the limit is depth, not total number of returned commits. -func (repo *Repository) commitsBefore(id SHA1, limit int) (*list.List, error) { +func (repo *Repository) commitsBefore(id SHA1, limit int) ([]*Commit, error) { cmd := NewCommand("log") if limit > 0 { cmd.AddArguments("-"+strconv.Itoa(limit), prettyLogFormat, id.String()) @@ -352,9 +351,8 @@ func (repo *Repository) commitsBefore(id SHA1, limit int) (*list.List, error) { return nil, err } - commits := list.New() - for logEntry := formattedLog.Front(); logEntry != nil; logEntry = logEntry.Next() { - commit := logEntry.Value.(*Commit) + commits := make([]*Commit, 0, len(formattedLog)) + for _, commit := range formattedLog { branches, err := repo.getBranches(commit, 2) if err != nil { return nil, err @@ -364,17 +362,17 @@ func (repo *Repository) commitsBefore(id SHA1, limit int) (*list.List, error) { break } - commits.PushBack(commit) + commits = append(commits, commit) } return commits, nil } -func (repo *Repository) getCommitsBefore(id SHA1) (*list.List, error) { +func (repo *Repository) getCommitsBefore(id SHA1) ([]*Commit, error) { return repo.commitsBefore(id, 0) } -func (repo *Repository) getCommitsBeforeLimit(id SHA1, num int) (*list.List, error) { +func (repo *Repository) getCommitsBeforeLimit(id SHA1, num int) ([]*Commit, error) { return repo.commitsBefore(id, num) } @@ -413,13 +411,13 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) } // GetCommitsFromIDs get commits from commit IDs -func (repo *Repository) GetCommitsFromIDs(commitIDs []string) (commits *list.List) { - commits = list.New() +func (repo *Repository) GetCommitsFromIDs(commitIDs []string) []*Commit { + commits := make([]*Commit, 0, len(commitIDs)) for _, commitID := range commitIDs { commit, err := repo.GetCommit(commitID) if err == nil && commit != nil { - commits.PushBack(commit) + commits = append(commits, commit) } } diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index a6c27ea4d..594333484 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -97,6 +97,6 @@ func TestRepository_CommitsBetweenIDs(t *testing.T) { for i, c := range cases { commits, err := bareRepo1.CommitsBetweenIDs(c.NewID, c.OldID) assert.NoError(t, err) - assert.Equal(t, c.ExpectedCommits, commits.Len(), "case %d", i) + assert.Equal(t, c.ExpectedCommits, len(commits), "case %d", i) } } diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 3255e6839..5d1208aab 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -7,7 +7,6 @@ package git import ( "bytes" - "container/list" "fmt" "io" "regexp" @@ -23,7 +22,7 @@ type CompareInfo struct { MergeBase string BaseCommitID string HeadCommitID string - Commits *list.List + Commits []*Commit NumFiles int } @@ -90,7 +89,7 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string) return nil, fmt.Errorf("parsePrettyFormatLogToList: %v", err) } } else { - compareInfo.Commits = list.New() + compareInfo.Commits = []*Commit{} compareInfo.MergeBase, err = GetFullCommitID(repo.Path, remoteBranch) if err != nil { compareInfo.MergeBase = remoteBranch diff --git a/modules/repository/commits.go b/modules/repository/commits.go index eaaf3b8b1..7f2210574 100644 --- a/modules/repository/commits.go +++ b/modules/repository/commits.go @@ -5,7 +5,6 @@ package repository import ( - "container/list" "fmt" "time" @@ -175,12 +174,11 @@ func CommitToPushCommit(commit *git.Commit) *PushCommit { } } -// ListToPushCommits transforms a list.List to PushCommits type. -func ListToPushCommits(l *list.List) *PushCommits { - var commits []*PushCommit - for e := l.Front(); e != nil; e = e.Next() { - commit := CommitToPushCommit(e.Value.(*git.Commit)) - commits = append(commits, commit) +// GitToPushCommits transforms a list of git.Commits to PushCommits type. +func GitToPushCommits(gitCommits []*git.Commit) *PushCommits { + commits := make([]*PushCommit, 0, len(gitCommits)) + for _, commit := range gitCommits { + commits = append(commits, CommitToPushCommit(commit)) } return &PushCommits{commits, nil, "", make(map[string]string), make(map[string]*models.User)} } diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index 8e0d8bf90..9f0852525 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -5,7 +5,6 @@ package repository import ( - "container/list" "crypto/md5" "fmt" "testing" @@ -173,21 +172,22 @@ func TestListToPushCommits(t *testing.T) { hash2, err := git.NewIDFromString(hexString2) assert.NoError(t, err) - l := list.New() - l.PushBack(&git.Commit{ - ID: hash1, - Author: sig, - Committer: sig, - CommitMessage: "Message1", - }) - l.PushBack(&git.Commit{ - ID: hash2, - Author: sig, - Committer: sig, - CommitMessage: "Message2", - }) + l := []*git.Commit{ + { + ID: hash1, + Author: sig, + Committer: sig, + CommitMessage: "Message1", + }, + { + ID: hash2, + Author: sig, + Committer: sig, + CommitMessage: "Message2", + }, + } - pushCommits := ListToPushCommits(l) + pushCommits := GitToPushCommits(l) if assert.Len(t, pushCommits.Commits, 2) { assert.Equal(t, "Message1", pushCommits.Commits[0].Message) assert.Equal(t, hexString1, pushCommits.Commits[0].Sha1) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 1e8f00b66..6517127eb 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -7,7 +7,6 @@ package templates import ( "bytes" - "container/list" "errors" "fmt" "html" @@ -126,7 +125,6 @@ func NewFuncMap() []template.FuncMap { }, "SizeFmt": base.FileSize, "CountFmt": base.FormatNumberSI, - "List": List, "SubStr": func(str string, start, length int) string { if len(str) == 0 { return "" @@ -297,18 +295,6 @@ func NewFuncMap() []template.FuncMap { }, "CommentMustAsDiff": gitdiff.CommentMustAsDiff, "MirrorRemoteAddress": mirrorRemoteAddress, - "CommitType": func(commit interface{}) string { - switch commit.(type) { - case models.SignCommitWithStatuses: - return "SignCommitWithStatuses" - case models.SignCommit: - return "SignCommit" - case models.UserCommit: - return "UserCommit" - default: - return "" - } - }, "NotificationSettings": func() map[string]interface{} { return map[string]interface{}{ "MinTimeout": int(setting.UI.Notification.MinTimeout / time.Millisecond), @@ -428,7 +414,6 @@ func NewTextFuncMap() []texttmpl.FuncMap { "DateFmtShort": func(t time.Time) string { return t.Format("Jan 02, 2006") }, - "List": List, "SubStr": func(str string, start, length int) string { if len(str) == 0 { return "" @@ -636,20 +621,6 @@ func JSEscape(raw string) string { return template.JSEscapeString(raw) } -// List traversings the list -func List(l *list.List) chan interface{} { - e := l.Front() - c := make(chan interface{}) - go func() { - for e != nil { - c <- e.Value - e = e.Next() - } - close(c) - }() - return c -} - // Sha1 returns sha1 sum of string func Sha1(str string) string { return base.EncodeSha1(str) diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index e4bea4dee..9950a7d45 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -190,20 +190,14 @@ func GetAllCommits(ctx *context.APIContext) { userCache := make(map[string]*models.User) - apiCommits := make([]*api.Commit, commits.Len()) - - i := 0 - for commitPointer := commits.Front(); commitPointer != nil; commitPointer = commitPointer.Next() { - commit := commitPointer.Value.(*git.Commit) - + apiCommits := make([]*api.Commit, len(commits)) + for i, commit := range commits { // Create json struct apiCommits[i], err = convert.ToCommit(ctx.Repo.Repository, commit, userCache) if err != nil { ctx.Error(http.StatusInternalServerError, "toCommit", err) return } - - i++ } // kept for backwards compatibility diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 3ae7b2683..9be6228bf 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1211,7 +1211,7 @@ func GetPullRequestCommits(ctx *context.APIContext) { listOptions := utils.GetListOptions(ctx) - totalNumberOfCommits := commits.Len() + totalNumberOfCommits := len(commits) totalNumberOfPages := int(math.Ceil(float64(totalNumberOfCommits) / float64(listOptions.PageSize))) userCache := make(map[string]*models.User) @@ -1222,29 +1222,14 @@ func GetPullRequestCommits(ctx *context.APIContext) { end = totalNumberOfCommits } - apiCommits := make([]*api.Commit, end-start) - - i := 0 - addedCommitsCount := 0 - for commitPointer := commits.Front(); commitPointer != nil; commitPointer = commitPointer.Next() { - if i < start { - i++ - continue - } - if i >= end { - break - } - - commit := commitPointer.Value.(*git.Commit) - - // Create json struct - apiCommits[addedCommitsCount], err = convert.ToCommit(ctx.Repo.Repository, commit, userCache) - addedCommitsCount++ + apiCommits := make([]*api.Commit, 0, end-start) + for i := start; i < end; i++ { + apiCommit, err := convert.ToCommit(ctx.Repo.Repository, commits[i], userCache) if err != nil { ctx.ServerError("toCommit", err) return } - i++ + apiCommits = append(apiCommits, apiCommit) } ctx.SetLinkHeader(int(totalNumberOfCommits), listOptions.PageSize) diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index 4ade9e9a9..c2da8e9cd 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -5,7 +5,6 @@ package repo import ( - "container/list" "fmt" "html" gotemplate "html/template" @@ -138,15 +137,15 @@ func RefBlame(ctx *context.Context) { ctx.HTML(http.StatusOK, tplBlame) } -func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) (map[string]models.UserCommit, map[string]string) { +func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) (map[string]*models.UserCommit, map[string]string) { // store commit data by SHA to look up avatar info etc - commitNames := make(map[string]models.UserCommit) + commitNames := make(map[string]*models.UserCommit) // previousCommits contains links from SHA to parent SHA, // if parent also contains the current TreePath. previousCommits := make(map[string]string) // and as blameParts can reference the same commits multiple // times, we cache the lookup work locally - commits := list.New() + commits := make([]*git.Commit, 0, len(blameParts)) commitCache := map[string]*git.Commit{} commitCache[ctx.Repo.Commit.ID.String()] = ctx.Repo.Commit @@ -190,22 +189,18 @@ func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) (map[st } } - commits.PushBack(commit) - - commitNames[commit.ID.String()] = models.UserCommit{} + commits = append(commits, commit) } // populate commit email addresses to later look up avatars. - commits = models.ValidateCommitsWithEmails(commits) - for e := commits.Front(); e != nil; e = e.Next() { - c := e.Value.(models.UserCommit) + for _, c := range models.ValidateCommitsWithEmails(commits) { commitNames[c.ID.String()] = c } return commitNames, previousCommits } -func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames map[string]models.UserCommit, previousCommits map[string]string) { +func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames map[string]*models.UserCommit, previousCommits map[string]string) { repoLink := ctx.Repo.RepoLink var lines = make([]string, 0) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index fcc17a40a..e1d93a243 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -72,10 +72,7 @@ func Commits(ctx *context.Context) { ctx.ServerError("CommitsByRange", err) return } - commits = models.ValidateCommitsWithEmails(commits) - commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository) - commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository) - ctx.Data["Commits"] = commits + ctx.Data["Commits"] = models.ConvertFromGitCommit(commits, ctx.Repo.Repository) ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name @@ -193,10 +190,8 @@ func SearchCommits(ctx *context.Context) { ctx.ServerError("SearchCommits", err) return } - commits = models.ValidateCommitsWithEmails(commits) - commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository) - commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository) - ctx.Data["Commits"] = commits + ctx.Data["CommitCount"] = len(commits) + ctx.Data["Commits"] = models.ConvertFromGitCommit(commits, ctx.Repo.Repository) ctx.Data["Keyword"] = query if all { @@ -204,7 +199,6 @@ func SearchCommits(ctx *context.Context) { } ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name - ctx.Data["CommitCount"] = commits.Len() ctx.Data["Branch"] = ctx.Repo.BranchName ctx.HTML(http.StatusOK, tplCommits) } @@ -239,10 +233,7 @@ func FileHistory(ctx *context.Context) { ctx.ServerError("CommitsByFileAndRange", err) return } - commits = models.ValidateCommitsWithEmails(commits) - commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository) - commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository) - ctx.Data["Commits"] = commits + ctx.Data["Commits"] = models.ConvertFromGitCommit(commits, ctx.Repo.Repository) ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 45d0624ca..511a74cdd 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -551,14 +551,12 @@ func PrepareCompareDiff( return false } - compareInfo.Commits = models.ValidateCommitsWithEmails(compareInfo.Commits) - compareInfo.Commits = models.ParseCommitsWithSignature(compareInfo.Commits, headRepo) - compareInfo.Commits = models.ParseCommitsWithStatus(compareInfo.Commits, headRepo) - ctx.Data["Commits"] = compareInfo.Commits - ctx.Data["CommitCount"] = compareInfo.Commits.Len() + commits := models.ConvertFromGitCommit(compareInfo.Commits, headRepo) + ctx.Data["Commits"] = commits + ctx.Data["CommitCount"] = len(commits) - if compareInfo.Commits.Len() == 1 { - c := compareInfo.Commits.Front().Value.(models.SignCommitWithStatuses) + if len(commits) == 1 { + c := commits[0] title = strings.TrimSpace(c.UserCommit.Summary()) body := strings.Split(strings.TrimSpace(c.UserCommit.Message()), "\n") diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 5c61e0d5f..ccdd670e6 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -7,7 +7,6 @@ package repo import ( - "container/list" "crypto/subtle" "errors" "fmt" @@ -327,11 +326,11 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C ctx.ServerError("GetCompareInfo", err) return nil } - ctx.Data["NumCommits"] = compareInfo.Commits.Len() + ctx.Data["NumCommits"] = len(compareInfo.Commits) ctx.Data["NumFiles"] = compareInfo.NumFiles - if compareInfo.Commits.Len() != 0 { - sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String() + if len(compareInfo.Commits) != 0 { + sha := compareInfo.Commits[0].ID.String() commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, sha, models.ListOptions{}) if err != nil { ctx.ServerError("GetLatestCommitStatus", err) @@ -411,7 +410,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare return nil } - ctx.Data["NumCommits"] = compareInfo.Commits.Len() + ctx.Data["NumCommits"] = len(compareInfo.Commits) ctx.Data["NumFiles"] = compareInfo.NumFiles return compareInfo } @@ -543,7 +542,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare ctx.Data["ConflictedFiles"] = pull.ConflictedFiles } - ctx.Data["NumCommits"] = compareInfo.Commits.Len() + ctx.Data["NumCommits"] = len(compareInfo.Commits) ctx.Data["NumFiles"] = compareInfo.NumFiles return compareInfo } @@ -559,7 +558,6 @@ func ViewPullCommits(ctx *context.Context) { } pull := issue.PullRequest - var commits *list.List var prInfo *git.CompareInfo if pull.HasMerged { prInfo = PrepareMergedViewPullInfo(ctx, issue) @@ -576,12 +574,10 @@ func ViewPullCommits(ctx *context.Context) { ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name - commits = prInfo.Commits - commits = models.ValidateCommitsWithEmails(commits) - commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository) - commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository) + + commits := models.ConvertFromGitCommit(prInfo.Commits, ctx.Repo.Repository) ctx.Data["Commits"] = commits - ctx.Data["CommitCount"] = commits.Len() + ctx.Data["CommitCount"] = len(commits) getBranchData(ctx, issue) ctx.HTML(http.StatusOK, tplPullCommits) diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 7226039e2..2057229bf 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -312,10 +312,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) ctx.ServerError("CommitsByFileAndRangeNoFollow", err) return nil, nil } - commitsHistory = models.ValidateCommitsWithEmails(commitsHistory) - commitsHistory = models.ParseCommitsWithSignature(commitsHistory, ctx.Repo.Repository) - - ctx.Data["Commits"] = commitsHistory + ctx.Data["Commits"] = models.ConvertFromGitCommit(commitsHistory, ctx.Repo.Repository) pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5) pager.SetDefaultParams(ctx) diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index a16724b36..89b5df463 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -354,7 +354,7 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { continue } - theCommits := repo_module.ListToPushCommits(commits) + theCommits := repo_module.GitToPushCommits(commits) if len(theCommits.Commits) > setting.UI.FeedMaxCommitNum { theCommits.Commits = theCommits.Commits[:setting.UI.FeedMaxCommitNum] } diff --git a/services/pull/pull.go b/services/pull/pull.go index b33f64113..23407ea67 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -84,11 +84,11 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 return err } - if compareInfo.Commits.Len() > 0 { + if len(compareInfo.Commits) > 0 { data := models.PushActionContent{IsForcePush: false} - data.CommitIDs = make([]string, 0, compareInfo.Commits.Len()) - for e := compareInfo.Commits.Back(); e != nil; e = e.Prev() { - data.CommitIDs = append(data.CommitIDs, e.Value.(*git.Commit).ID.String()) + data.CommitIDs = make([]string, 0, len(compareInfo.Commits)) + for i := len(compareInfo.Commits) - 1; i >= 0; i-- { + data.CommitIDs = append(data.CommitIDs, compareInfo.Commits[i].ID.String()) } dataJSON, err := json.Marshal(data) @@ -611,7 +611,7 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { limit := setting.Repository.PullRequest.DefaultMergeMessageCommitsLimit - list, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, 0) + commits, 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 "" @@ -620,7 +620,7 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { posterSig := pr.Issue.Poster.NewGitSig().String() authorsMap := map[string]bool{} - authors := make([]string, 0, list.Len()) + authors := make([]string, 0, len(commits)) stringBuilder := strings.Builder{} if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { @@ -635,15 +635,16 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { } // commits list is in reverse chronological order - element := list.Back() - for element != nil { - commit := element.Value.(*git.Commit) + first := true + for i := len(commits) - 1; i >= 0; i-- { + commit := commits[i] if setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize if maxSize < 0 || stringBuilder.Len() < maxSize { var toWrite []byte - if element == list.Back() { + if first { + first = false toWrite = []byte(strings.TrimPrefix(commit.CommitMessage, pr.Issue.Title)) } else { toWrite = []byte(commit.CommitMessage) @@ -669,7 +670,6 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { authors = append(authors, authorString) authorsMap[authorString] = true } - element = element.Prev() } // Consider collecting the remaining authors @@ -677,25 +677,21 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { skip := limit limit = 30 for { - list, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip) + commits, 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 { + if len(commits) == 0 { break } - element := list.Front() - for element != nil { - commit := element.Value.(*git.Commit) - + for _, commit := range commits { authorString := commit.Author.String() if !authorsMap[authorString] && authorString != posterSig { authors = append(authors, authorString) authorsMap[authorString] = true } - element = element.Next() } skip += limit } diff --git a/services/repository/push.go b/services/repository/push.go index 26df6b8e4..cf7060ade 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -5,7 +5,6 @@ package repository import ( - "container/list" "fmt" "time" @@ -147,7 +146,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { refName := opts.RefName() // Push new branch. - var l *list.List + var l []*git.Commit if opts.IsNewRef() { if repo.IsEmpty { // Change default branch and empty status only if pushed ref is non-empty branch. repo.DefaultBranch = refName @@ -191,7 +190,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } } - commits := repo_module.ListToPushCommits(l) + commits := repo_module.GitToPushCommits(l) commits.HeadCommit = repo_module.CommitToPushCommit(newCommit) if err := repofiles.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil { diff --git a/templates/mail/issue/default.tmpl b/templates/mail/issue/default.tmpl index 071acdcbc..6c74f34c2 100644 --- a/templates/mail/issue/default.tmpl +++ b/templates/mail/issue/default.tmpl @@ -30,7 +30,7 @@ {{.i18n.Tr "mail.issue.action.force_push" .Doer.Name .Comment.Issue.PullRequest.HeadBranch $oldCommitLink $newCommitLink | Str2html}} {{else}} - {{.i18n.Tr (TrN .i18n.Lang .Comment.Commits.Len "mail.issue.action.push_1" "mail.issue.action.push_n") .Doer.Name .Comment.Issue.PullRequest.HeadBranch .Comment.Commits.Len | Str2html}} + {{.i18n.Tr (TrN .i18n.Lang (len .Comment.Commits) "mail.issue.action.push_1" "mail.issue.action.push_n") .Doer.Name .Comment.Issue.PullRequest.HeadBranch (len .Comment.Commits) | Str2html}} {{end}}

{{end}} @@ -69,9 +69,8 @@ {{end -}} {{if eq .ActionName "push"}} - {{ $r:= List .Comment.Commits}}