finn/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Remove SavePatch and generate patches on the fly (#9302)

Browse source 
* Save patches to temporary files

* Remove SavePatch and generate patches on the fly

* Use ioutil.TempDir

* fixup! Use ioutil.TempDir

* fixup! fixup! Use ioutil.TempDir

* RemoveAll LocalCopyPath() in initIntergrationTest

* Default to status checking on PR creation

* Remove unnecessary set to StatusChecking

* Protect against unable to load repo

* Handle conflicts

* Restore original conflict setting

* In TestPullRequests update status to StatusChecking before running TestPatch
This commit is contained in:
zeripath 2019-12-13 22:21:06 +00:00 • committed by Antoine GIRARD
parent 8f16a2c37b
commit 74179d1b5e
16 changed files with 432 additions and 406 deletions
Download patch file Download diff file Expand all files Collapse all files

18
models/helper_directory.go
View file

@ -6,15 +6,13 @@ package models
import (
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"time"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"github.com/unknwon/com"
)
// LocalCopyPath returns the local repository temporary copy path.
@ -27,11 +25,15 @@ func LocalCopyPath() string {
// CreateTemporaryPath creates a temporary path
func CreateTemporaryPath(prefix string) (string, error) {
timeStr := com.ToStr(time.Now().Nanosecond()) // SHOULD USE SOMETHING UNIQUE
basePath := path.Join(LocalCopyPath(), prefix+"-"+timeStr+".git")
if err := os.MkdirAll(filepath.Dir(basePath), os.ModePerm); err != nil {
log.Error("Unable to create temporary directory: %s (%v)", basePath, err)
return "", fmt.Errorf("Failed to create dir %s: %v", basePath, err)
if err := os.MkdirAll(LocalCopyPath(), os.ModePerm); err != nil {
log.Error("Unable to create localcopypath directory: %s (%v)", LocalCopyPath(), err)
return "", fmt.Errorf("Failed to create localcopypath directory %s: %v", LocalCopyPath(), err)
}
basePath, err := ioutil.TempDir(LocalCopyPath(), prefix+".git")
if err != nil {
log.Error("Unable to create temporary directory: %s-*.git (%v)", prefix, err)
return "", fmt.Errorf("Failed to create dir %s-*.git: %v", prefix, err)
}
return basePath, nil
}

4
models/issue_xref_test.go
View file

@ -142,8 +142,8 @@ func testCreatePR(t *testing.T, repo, doer int64, title, content string) *PullRe
r := AssertExistsAndLoadBean(t, &Repository{ID: repo}).(*Repository)
d := AssertExistsAndLoadBean(t, &User{ID: doer}).(*User)
i := &Issue{RepoID: r.ID, PosterID: d.ID, Poster: d, Title: title, Content: content, IsPull: true}
pr := &PullRequest{HeadRepoID: repo, BaseRepoID: repo, HeadBranch: "head", BaseBranch: "base"}
assert.NoError(t, NewPullRequest(r, i, nil, nil, pr, nil))
pr := &PullRequest{HeadRepoID: repo, BaseRepoID: repo, HeadBranch: "head", BaseBranch: "base", Status: PullRequestStatusMergeable}
assert.NoError(t, NewPullRequest(r, i, nil, nil, pr))
pr.Issue = i
return pr
}

187
models/pull.go
View file

@ -6,22 +6,16 @@
package models
import (
"bufio"
"fmt"
"os"
"path"
"path/filepath"
"strconv"
"strings"
"time"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
"github.com/unknwon/com"
)
// PullRequestType defines pull request type
@ -481,125 +475,12 @@ func (pr *PullRequest) SetMerged() (err error) {
return nil
}
// patchConflicts is a list of conflict description from Git.
var patchConflicts = []string{
"patch does not apply",
"already exists in working directory",
"unrecognized input",
"error:",
}
// TestPatch checks if patch can be merged to base repository without conflict.
func (pr *PullRequest) TestPatch() error {
return pr.testPatch(x)
}
// testPatch checks if patch can be merged to base repository without conflict.
func (pr *PullRequest) testPatch(e Engine) (err error) {
if pr.BaseRepo == nil {
pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID)
if err != nil {
return fmt.Errorf("GetRepositoryByID: %v", err)
}
}
patchPath, err := pr.BaseRepo.patchPath(e, pr.Index)
if err != nil {
return fmt.Errorf("BaseRepo.PatchPath: %v", err)
}
// Fast fail if patch does not exist, this assumes data is corrupted.
if !com.IsFile(patchPath) {
log.Trace("PullRequest[%d].testPatch: ignored corrupted data", pr.ID)
return nil
}
RepoWorkingPool.CheckIn(com.ToStr(pr.BaseRepoID))
defer RepoWorkingPool.CheckOut(com.ToStr(pr.BaseRepoID))
log.Trace("PullRequest[%d].testPatch (patchPath): %s", pr.ID, patchPath)
pr.Status = PullRequestStatusChecking
indexTmpPath := filepath.Join(os.TempDir(), "gitea-"+pr.BaseRepo.Name+"-"+strconv.Itoa(time.Now().Nanosecond()))
defer os.Remove(indexTmpPath)
_, err = git.NewCommand("read-tree", pr.BaseBranch).RunInDirWithEnv("", []string{"GIT_DIR=" + pr.BaseRepo.RepoPath(), "GIT_INDEX_FILE=" + indexTmpPath})
if err != nil {
return fmt.Errorf("git read-tree --index-output=%s %s: %v", indexTmpPath, pr.BaseBranch, err)
}
prUnit, err := pr.BaseRepo.getUnit(e, UnitTypePullRequests)
if err != nil {
return err
}
prConfig := prUnit.PullRequestsConfig()
args := []string{"apply", "--check", "--cached"}
if prConfig.IgnoreWhitespaceConflicts {
args = append(args, "--ignore-whitespace")
}
args = append(args, patchPath)
pr.ConflictedFiles = []string{}
stderrBuilder := new(strings.Builder)
err = git.NewCommand(args...).RunInDirTimeoutEnvPipeline(
[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
-1,
"",
nil,
stderrBuilder)
stderr := stderrBuilder.String()
if err != nil {
for i := range patchConflicts {
if strings.Contains(stderr, patchConflicts[i]) {
log.Trace("PullRequest[%d].testPatch (apply): has conflict: %s", pr.ID, stderr)
const prefix = "error: patch failed:"
pr.Status = PullRequestStatusConflict
pr.ConflictedFiles = make([]string, 0, 5)
scanner := bufio.NewScanner(strings.NewReader(stderr))
for scanner.Scan() {
line := scanner.Text()
if strings.HasPrefix(line, prefix) {
var found bool
var filepath = strings.TrimSpace(strings.Split(line[len(prefix):], ":")[0])
for _, f := range pr.ConflictedFiles {
if f == filepath {
found = true
break
}
}
if !found {
pr.ConflictedFiles = append(pr.ConflictedFiles, filepath)
}
}
// only list 10 conflicted files
if len(pr.ConflictedFiles) >= 10 {
break
}
}
if len(pr.ConflictedFiles) > 0 {
log.Trace("Found %d files conflicted: %v", len(pr.ConflictedFiles), pr.ConflictedFiles)
}
return nil
}
}
return fmt.Errorf("git apply --check: %v - %s", err, stderr)
}
return nil
}
// NewPullRequest creates new pull request with labels for repository.
func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte) (err error) {
func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) {
// Retry several times in case INSERT fails due to duplicate key for (repo_id, index); see #7887
i := 0
for {
if err = newPullRequestAttempt(repo, pull, labelIDs, uuids, pr, patch); err == nil {
if err = newPullRequestAttempt(repo, pull, labelIDs, uuids, pr); err == nil {
return nil
}
if !IsErrNewIssueInsert(err) {
@ -613,7 +494,7 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
return fmt.Errorf("NewPullRequest: too many errors attempting to insert the new issue. Last error was: %v", err)
}
func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte) (err error) {
func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) {
sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
@ -635,20 +516,6 @@ func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuid
pr.Index = pull.Index
pr.BaseRepo = repo
pr.Status = PullRequestStatusChecking
if len(patch) > 0 {
if err = repo.savePatch(sess, pr.Index, patch); err != nil {
return fmt.Errorf("SavePatch: %v", err)
}
if err = pr.testPatch(sess); err != nil {
return fmt.Errorf("testPatch: %v", err)
}
}
// No conflict appears after test means mergeable.
if pr.Status == PullRequestStatusChecking {
pr.Status = PullRequestStatusMergeable
}
pr.IssueID = pull.ID
if _, err = sess.Insert(pr); err != nil {
@ -764,54 +631,6 @@ func (pr *PullRequest) UpdateCols(cols ...string) error {
return err
}
// UpdatePatch generates and saves a new patch.
func (pr *PullRequest) UpdatePatch() (err error) {
if err = pr.GetHeadRepo(); err != nil {
return fmt.Errorf("GetHeadRepo: %v", err)
} else if pr.HeadRepo == nil {
log.Trace("PullRequest[%d].UpdatePatch: ignored corrupted data", pr.ID)
return nil
}
if err = pr.GetBaseRepo(); err != nil {
return fmt.Errorf("GetBaseRepo: %v", err)
}
headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
return fmt.Errorf("OpenRepository: %v", err)
}
defer headGitRepo.Close()
// Add a temporary remote.
tmpRemote := com.ToStr(time.Now().UnixNano())
if err = headGitRepo.AddRemote(tmpRemote, RepoPath(pr.BaseRepo.MustOwner().Name, pr.BaseRepo.Name), true); err != nil {
return fmt.Errorf("AddRemote: %v", err)
}
defer func() {
if err := headGitRepo.RemoveRemote(tmpRemote); err != nil {
log.Error("UpdatePatch: RemoveRemote: %s", err)
}
}()
pr.MergeBase, _, err = headGitRepo.GetMergeBase(tmpRemote, pr.BaseBranch, pr.HeadBranch)
if err != nil {
return fmt.Errorf("GetMergeBase: %v", err)
} else if err = pr.Update(); err != nil {
return fmt.Errorf("Update: %v", err)
}
patch, err := headGitRepo.GetPatch(pr.MergeBase, pr.HeadBranch)
if err != nil {
return fmt.Errorf("GetPatch: %v", err)
}
if err = pr.BaseRepo.SavePatch(pr.Index, patch); err != nil {
return fmt.Errorf("BaseRepo.SavePatch: %v", err)
}
return nil
}
// PushToBaseRepo pushes commits from branches of head repository to
// corresponding branches of base repository.
// FIXME: Only push branches that are actually updates?

2
models/pull_test.go
View file

@ -190,8 +190,6 @@ func TestPullRequest_UpdateCols(t *testing.T) {
CheckConsistencyFor(t, pr)
}
// TODO TestPullRequest_UpdatePatch
// TODO TestPullRequest_PushToBaseRepo
func TestPullRequestList_LoadAttributes(t *testing.T) {

36
models/repo.go
View file

@ -887,42 +887,6 @@ func (repo *Repository) DescriptionHTML() template.HTML {
return template.HTML(markup.Sanitize(string(desc)))
}
// PatchPath returns corresponding patch file path of repository by given issue ID.
func (repo *Repository) PatchPath(index int64) (string, error) {
return repo.patchPath(x, index)
}
func (repo *Repository) patchPath(e Engine, index int64) (string, error) {
if err := repo.getOwner(e); err != nil {
return "", err
}
return filepath.Join(RepoPath(repo.Owner.Name, repo.Name), "pulls", com.ToStr(index)+".patch"), nil
}
// SavePatch saves patch data to corresponding location by given issue ID.
func (repo *Repository) SavePatch(index int64, patch []byte) error {
return repo.savePatch(x, index, patch)
}
func (repo *Repository) savePatch(e Engine, index int64, patch []byte) error {
patchPath, err := repo.patchPath(e, index)
if err != nil {
return fmt.Errorf("PatchPath: %v", err)
}
dir := filepath.Dir(patchPath)
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return fmt.Errorf("Failed to create dir %s: %v", dir, err)
}
if err = ioutil.WriteFile(patchPath, patch, 0644); err != nil {
return fmt.Errorf("WriteFile: %v", err)
}
return nil
}
func isRepositoryExist(e Engine, u *User, repoName string) (bool, error) {
has, err := e.Get(&Repository{
OwnerID: u.ID,