Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SavePatch and generate patches on the fly #9302

Merged
merged 17 commits into from
Dec 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integrations/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func initIntegrationTest() {

setting.SetCustomPathAndConf("", "", "")
setting.NewContext()
os.RemoveAll(models.LocalCopyPath())
setting.CheckLFSVersion()
setting.InitDBConfig()

Expand Down Expand Up @@ -182,7 +183,6 @@ func prepareTestEnv(t testing.TB, skip ...int) func() {
deferFn := PrintCurrentTest(t, ourSkip)
assert.NoError(t, models.LoadFixtures())
assert.NoError(t, os.RemoveAll(setting.RepoRootPath))
assert.NoError(t, os.RemoveAll(models.LocalCopyPath()))
zeripath marked this conversation as resolved.
Show resolved Hide resolved

assert.NoError(t, com.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"),
setting.RepoRootPath))
Expand Down
18 changes: 10 additions & 8 deletions models/helper_directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions models/issue_xref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
187 changes: 3 additions & 184 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
lafriks marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand All @@ -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 {
Expand All @@ -635,20 +516,6 @@ func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuid

pr.Index = pull.Index
pr.BaseRepo = repo
pr.Status = PullRequestStatusChecking
zeripath marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -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?
Expand Down
2 changes: 0 additions & 2 deletions models/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
36 changes: 0 additions & 36 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 15 additions & 13 deletions modules/git/repo_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package git

import (
"bytes"
"container/list"
"fmt"
"io"
Expand Down Expand Up @@ -94,19 +93,22 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string)
return compareInfo, nil
}

// GetPatch generates and returns patch data between given revisions.
func (repo *Repository) GetPatch(base, head string) ([]byte, error) {
return NewCommand("diff", "-p", "--binary", base, head).RunInDirBytes(repo.Path)
// GetDiffOrPatch generates either diff or formatted patch data between given revisions
func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, formatted bool) error {
if formatted {
return repo.GetPatch(base, head, w)
}
return repo.GetDiff(base, head, w)
}

// GetFormatPatch generates and returns format-patch data between given revisions.
func (repo *Repository) GetFormatPatch(base, head string) (io.Reader, error) {
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)
// GetDiff generates and returns patch data between given revisions.
func (repo *Repository) GetDiff(base, head string, w io.Writer) error {
return NewCommand("diff", "-p", "--binary", base, head).
RunInDirPipeline(repo.Path, w, nil)
}

if err := NewCommand("format-patch", "--binary", "--stdout", base+"..."+head).
RunInDirPipeline(repo.Path, stdout, stderr); err != nil {
return nil, concatenateError(err, stderr.String())
}
return stdout, nil
// GetPatch generates and returns format-patch data between given revisions.
func (repo *Repository) GetPatch(base, head string, w io.Writer) error {
return NewCommand("format-patch", "--binary", "--stdout", base+"..."+head).
RunInDirPipeline(repo.Path, w, nil)
}
Loading