Use git read-tree -m for better cherry-picking and reversion first

Signed-off-by: Andrew Thornton <[email protected]>

routers/web/repo/cherry_pick.go

@@ -129,43 +129,9 @@ func CherryPickPost(ctx *context.Context) {
 		Message:      message,
 	}
 
-	buf := &bytes.Buffer{}
-	if form.Revert {
-		if err := git.GetReverseRawDiff(
-			ctx,
-			ctx.Repo.Repository.RepoPath(),
-			sha,
-			buf,
-		); err != nil {
-			if git.IsErrNotExist(err) {
-				ctx.NotFound("GetRawDiff",
-					errors.New("commit "+ctx.Params(":sha")+" does not exist."))
-				return
-			}
-			ctx.ServerError("GetRawDiff", err)
-			return
-		}
-	} else {
-		if err := git.GetRawDiff(
-			ctx,
-			ctx.Repo.Repository.RepoPath(),
-			sha,
-			git.RawDiffType("patch"),
-			buf,
-		); err != nil {
-			if git.IsErrNotExist(err) {
-				ctx.NotFound("GetRawDiff",
-					errors.New("commit "+ctx.Params(":sha")+" does not exist."))
-				return
-			}
-			ctx.ServerError("GetRawDiff", err)
-			return
-		}
-	}
-	opts.Content = buf.String()
-	ctx.Data["FileContent"] = opts.Content
-
-	if _, err := files.ApplyDiffPatch(ctx.Repo.Repository, ctx.User, opts); err != nil {
+	// First lets try the simple plain read-tree -m approach
+	opts.Content = sha
+	if _, err := files.CherryPick(ctx.Repo.Repository, ctx.User, form.Revert, opts); err != nil {
 		if models.IsErrBranchAlreadyExists(err) {
 			// For when a user specifies a new branch that already exists
 			ctx.Data["Err_NewBranchName"] = true
@@ -176,8 +142,59 @@ func CherryPickPost(ctx *context.Context) {
 			}
 		} else if models.IsErrCommitIDDoesNotMatch(err) {
 			ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplPatchFile, &form)
+		}
+		// Drop through to the apply technique
+
+		buf := &bytes.Buffer{}
+		if form.Revert {
+			if err := git.GetReverseRawDiff(
+				ctx,
+				ctx.Repo.Repository.RepoPath(),
+				sha,
+				buf,
+			); err != nil {
+				if git.IsErrNotExist(err) {
+					ctx.NotFound("GetRawDiff",
+						errors.New("commit "+ctx.Params(":sha")+" does not exist."))
+					return
+				}
+				ctx.ServerError("GetRawDiff", err)
+				return
+			}
 		} else {
-			ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_apply_patch", err), tplPatchFile, &form)
+			if err := git.GetRawDiff(
+				ctx,
+				ctx.Repo.Repository.RepoPath(),
+				sha,
+				git.RawDiffType("patch"),
+				buf,
+			); err != nil {
+				if git.IsErrNotExist(err) {
+					ctx.NotFound("GetRawDiff",
+						errors.New("commit "+ctx.Params(":sha")+" does not exist."))
+					return
+				}
+				ctx.ServerError("GetRawDiff", err)
+				return
+			}
+		}
+		opts.Content = buf.String()
+		ctx.Data["FileContent"] = opts.Content
+
+		if _, err := files.ApplyDiffPatch(ctx.Repo.Repository, ctx.User, opts); err != nil {
+			if models.IsErrBranchAlreadyExists(err) {
+				// For when a user specifies a new branch that already exists
+				ctx.Data["Err_NewBranchName"] = true
+				if branchErr, ok := err.(models.ErrBranchAlreadyExists); ok {
+					ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchErr.BranchName), tplCherryPick, &form)
+				} else {
+					ctx.Error(500, err.Error())
+				}
+			} else if models.IsErrCommitIDDoesNotMatch(err) {
+				ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplPatchFile, &form)
+			} else {
+				ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_apply_patch", err), tplPatchFile, &form)
+			}
 		}
 	}
 

services/forms/repo_form.go

@@ -752,7 +752,7 @@ type CherryPickForm struct {
 	CommitChoice  string `binding:"Required;MaxSize(50)"`
 	NewBranchName string `binding:"GitRefName;MaxSize(100)"`
 	LastCommit    string
-	Revert        bool `binding:"Required"`
+	Revert        bool
 	Signoff       bool
 }
 

services/repository/files/cherry_pick.go

@@ -0,0 +1,121 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package files
+
+import (
+	"fmt"
+	"strings"
+
+	"code.gitea.io/gitea/models"
+	repo_model "code.gitea.io/gitea/models/repo"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/structs"
+)
+
+// CherryPick cherrypicks or reverts a commit to the given repository
+func CherryPick(repo *repo_model.Repository, doer *user_model.User, revert bool, opts *ApplyDiffPatchOptions) (*structs.FileResponse, error) {
+	if err := opts.Validate(repo, doer); err != nil {
+		return nil, err
+	}
+	message := strings.TrimSpace(opts.Message)
+
+	author, committer := GetAuthorAndCommitterUsers(opts.Author, opts.Committer, doer)
+
+	t, err := NewTemporaryUploadRepository(repo)
+	if err != nil {
+		log.Error("%v", err)
+	}
+	defer t.Close()
+	if err := t.Clone(opts.OldBranch); err != nil {
+		return nil, err
+	}
+	if err := t.SetDefaultIndex(); err != nil {
+		return nil, err
+	}
+
+	// Get the commit of the original branch
+	commit, err := t.GetBranchCommit(opts.OldBranch)
+	if err != nil {
+		return nil, err // Couldn't get a commit for the branch
+	}
+
+	// Assigned LastCommitID in opts if it hasn't been set
+	if opts.LastCommitID == "" {
+		opts.LastCommitID = commit.ID.String()
+	} else {
+		lastCommitID, err := t.gitRepo.ConvertToSHA1(opts.LastCommitID)
+		if err != nil {
+			return nil, fmt.Errorf("CherryPick: Invalid last commit ID: %v", err)
+		}
+		opts.LastCommitID = lastCommitID.String()
+		if commit.ID.String() != opts.LastCommitID {
+			return nil, models.ErrCommitIDDoesNotMatch{
+				GivenCommitID:   opts.LastCommitID,
+				CurrentCommitID: opts.LastCommitID,
+			}
+		}
+	}
+
+	commit, err = t.GetCommit(strings.TrimSpace(opts.Content))
+	if err != nil {
+		return nil, err
+	}
+	parent, err := commit.ParentID(0)
+	if err != nil {
+		parent = git.MustIDFromString(git.EmptyTreeSHA)
+	}
+
+	base, right := parent.String(), commit.ID.String()
+
+	if revert {
+		right, base = base, right
+	}
+
+	stdout := &strings.Builder{}
+	stderr := &strings.Builder{}
+
+	err = git.NewCommand("read-tree", "-m", base, opts.LastCommitID, right).RunInDirFullPipeline(t.basePath, stdout, stderr, nil)
+	if err != nil {
+		return nil, fmt.Errorf("Error: Stdout: %s\nStderr: %s\nErr: %v", stdout.String(), stderr.String(), err)
+	}
+
+	treeHash, err := t.WriteTree()
+	if err != nil {
+		// likely non-sensical tree due to merge conflicts...
+		return nil, err
+	}
+
+	// Now commit the tree
+	var commitHash string
+	if opts.Dates != nil {
+		commitHash, err = t.CommitTreeWithDate(author, committer, treeHash, message, opts.Signoff, opts.Dates.Author, opts.Dates.Committer)
+	} else {
+		commitHash, err = t.CommitTree(author, committer, treeHash, message, opts.Signoff)
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	// Then push this tree to NewBranch
+	if err := t.Push(doer, commitHash, opts.NewBranch); err != nil {
+		return nil, err
+	}
+
+	commit, err = t.GetCommit(commitHash)
+	if err != nil {
+		return nil, err
+	}
+
+	fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil
+	verification := GetPayloadCommitVerification(commit)
+	fileResponse := &structs.FileResponse{
+		Commit:       fileCommitResponse,
+		Verification: verification,
+	}
+
+	return fileResponse, nil
+}