Merge branch 'jc/revert-remove-cherry-pick-ff' into 'master'

Revert "Merge branch 'jc/remove-cherry-pick-ff' into 'master'"

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6237

Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>

7 files changed, 636 insertions, 70 deletions

diff --git a/cmd/gitaly-git2go/cherry_pick.go b/cmd/gitaly-git2go/cherry_pick.go
new file mode 100644
index 000000000..0de2ada5f
--- /dev/null
+++ b/cmd/gitaly-git2go/cherry_pick.go
@@ -0,0 +1,127 @@
+//go:build static && system_libgit2
+
+package main
+
+import (
+	"context"
+	"encoding/gob"
+	"errors"
+	"flag"
+	"fmt"
+
+	git "github.com/libgit2/git2go/v34"
+	"gitlab.com/gitlab-org/gitaly/v16/cmd/gitaly-git2go/git2goutil"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git2go"
+)
+
+type cherryPickSubcommand struct{}
+
+func (cmd *cherryPickSubcommand) Flags() *flag.FlagSet {
+	return flag.NewFlagSet("cherry-pick", flag.ExitOnError)
+}
+
+func (cmd *cherryPickSubcommand) Run(ctx context.Context, decoder *gob.Decoder, encoder *gob.Encoder) error {
+	var request git2go.CherryPickCommand
+	if err := decoder.Decode(&request); err != nil {
+		return err
+	}
+
+	commitID, err := cmd.cherryPick(ctx, &request)
+	return encoder.Encode(git2go.Result{
+		CommitID: commitID,
+		Err:      git2go.SerializableError(err),
+	})
+}
+
+func (cmd *cherryPickSubcommand) verify(ctx context.Context, r *git2go.CherryPickCommand) error {
+	if r.Repository == "" {
+		return errors.New("missing repository")
+	}
+	if r.CommitterName == "" {
+		return errors.New("missing committer name")
+	}
+	if r.CommitterMail == "" {
+		return errors.New("missing committer mail")
+	}
+	if r.CommitterDate.IsZero() {
+		return errors.New("missing committer date")
+	}
+	if r.Message == "" {
+		return errors.New("missing message")
+	}
+	if r.Ours == "" {
+		return errors.New("missing ours")
+	}
+	if r.Commit == "" {
+		return errors.New("missing commit")
+	}
+
+	return nil
+}
+
+func (cmd *cherryPickSubcommand) cherryPick(ctx context.Context, r *git2go.CherryPickCommand) (string, error) {
+	if err := cmd.verify(ctx, r); err != nil {
+		return "", err
+	}
+
+	repo, err := git2goutil.OpenRepository(r.Repository)
+	if err != nil {
+		return "", fmt.Errorf("could not open repository: %w", err)
+	}
+	defer repo.Free()
+
+	ours, err := lookupCommit(repo, r.Ours)
+	if err != nil {
+		return "", fmt.Errorf("ours commit lookup: %w", err)
+	}
+
+	pick, err := lookupCommit(repo, r.Commit)
+	if err != nil {
+		return "", fmt.Errorf("commit lookup: %w", err)
+	}
+
+	opts, err := git.DefaultCherrypickOptions()
+	if err != nil {
+		return "", fmt.Errorf("could not get default cherry-pick options: %w", err)
+	}
+	opts.Mainline = r.Mainline
+
+	index, err := repo.CherrypickCommit(pick, ours, opts)
+	if err != nil {
+		return "", fmt.Errorf("could not cherry-pick commit: %w", err)
+	}
+
+	if index.HasConflicts() {
+		conflictingFiles, err := getConflictingFiles(index)
+		if err != nil {
+			return "", fmt.Errorf("getting conflicting files: %w", err)
+		}
+
+		return "", git2go.ConflictingFilesError{
+			ConflictingFiles: conflictingFiles,
+		}
+	}
+
+	treeOID, err := index.WriteTreeTo(repo)
+	if err != nil {
+		return "", fmt.Errorf("could not write tree: %w", err)
+	}
+	tree, err := repo.LookupTree(treeOID)
+	if err != nil {
+		return "", fmt.Errorf("lookup tree: %w", err)
+	}
+
+	if treeOID.Equal(ours.TreeId()) {
+		return "", git2go.EmptyError{}
+	}
+
+	committer := git.Signature(git2go.NewSignature(r.CommitterName, r.CommitterMail, r.CommitterDate))
+
+	commitID, err := git2goutil.NewCommitSubmitter(repo, r.SigningKey).
+		Commit(pick.Author(), &committer, git.MessageEncodingUTF8, r.Message, tree, ours)
+	if err != nil {
+		return "", fmt.Errorf("create not create cherry-pick commit: %w", err)
+	}
+
+	return commitID.String(), nil
+}
diff --git a/cmd/gitaly-git2go/cherry_pick_test.go b/cmd/gitaly-git2go/cherry_pick_test.go
new file mode 100644
index 000000000..d9bfc2e39
--- /dev/null
+++ b/cmd/gitaly-git2go/cherry_pick_test.go
@@ -0,0 +1,290 @@
+//go:build static && system_libgit2 && !gitaly_test_sha256
+
+package main
+
+import (
+	"testing"
+	"time"
+
+	git "github.com/libgit2/git2go/v34"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/cmd/gitaly-git2go/git2goutil"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git2go"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
+)
+
+func TestCherryPick_validation(t *testing.T) {
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+	testcfg.BuildGitalyGit2Go(t, cfg)
+	executor := buildExecutor(t, cfg)
+
+	testcases := []struct {
+		desc        string
+		request     git2go.CherryPickCommand
+		expectedErr string
+	}{
+		{
+			desc:        "no arguments",
+			expectedErr: "cherry-pick: missing repository",
+		},
+		{
+			desc:        "missing repository",
+			request:     git2go.CherryPickCommand{CommitterName: "Foo", CommitterMail: "foo@example.com", CommitterDate: time.Now(), Message: "Foo", Ours: "HEAD", Commit: "HEAD"},
+			expectedErr: "cherry-pick: missing repository",
+		},
+		{
+			desc:        "missing committer name",
+			request:     git2go.CherryPickCommand{Repository: repoPath, CommitterMail: "foo@example.com", CommitterDate: time.Now(), Message: "Foo", Ours: "HEAD", Commit: "HEAD"},
+			expectedErr: "cherry-pick: missing committer name",
+		},
+		{
+			desc:        "missing committer mail",
+			request:     git2go.CherryPickCommand{Repository: repoPath, CommitterName: "Foo", CommitterDate: time.Now(), Message: "Foo", Ours: "HEAD", Commit: "HEAD"},
+			expectedErr: "cherry-pick: missing committer mail",
+		},
+		{
+			desc:        "missing committer date",
+			request:     git2go.CherryPickCommand{Repository: repoPath, CommitterName: "Foo", CommitterMail: "foo@example.com", Message: "Foo", Ours: "HEAD", Commit: "HEAD"},
+			expectedErr: "cherry-pick: missing committer date",
+		},
+		{
+			desc:        "missing message",
+			request:     git2go.CherryPickCommand{Repository: repoPath, CommitterName: "Foo", CommitterMail: "foo@example.com", CommitterDate: time.Now(), Ours: "HEAD", Commit: "HEAD"},
+			expectedErr: "cherry-pick: missing message",
+		},
+		{
+			desc:        "missing ours",
+			request:     git2go.CherryPickCommand{Repository: repoPath, CommitterName: "Foo", CommitterMail: "foo@example.com", CommitterDate: time.Now(), Message: "Foo", Commit: "HEAD"},
+			expectedErr: "cherry-pick: missing ours",
+		},
+		{
+			desc:        "missing commit",
+			request:     git2go.CherryPickCommand{Repository: repoPath, CommitterName: "Foo", CommitterMail: "foo@example.com", CommitterDate: time.Now(), Message: "Foo", Ours: "HEAD"},
+			expectedErr: "cherry-pick: missing commit",
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.desc, func(t *testing.T) {
+			_, err := executor.CherryPick(ctx, repo, tc.request)
+			require.EqualError(t, err, tc.expectedErr)
+		})
+	}
+}
+
+func TestCherryPick(t *testing.T) {
+	testcases := []struct {
+		desc             string
+		base             []gittest.TreeEntry
+		ours             []gittest.TreeEntry
+		commit           []gittest.TreeEntry
+		expected         map[string]string
+		expectedCommitID string
+		expectedErrAs    interface{}
+		expectedErrMsg   string
+	}{
+		{
+			desc: "trivial cherry-pick succeeds",
+			base: []gittest.TreeEntry{
+				{Path: "file", Content: "foo", Mode: "100644"},
+			},
+			ours: []gittest.TreeEntry{
+				{Path: "file", Content: "foo", Mode: "100644"},
+			},
+			commit: []gittest.TreeEntry{
+				{Path: "file", Content: "foobar", Mode: "100644"},
+			},
+			expected: map[string]string{
+				"file": "foobar",
+			},
+			expectedCommitID: "a54ea83118c363c34cc605a6e61fd7abc4795cc4",
+		},
+		{
+			desc: "conflicting cherry-pick fails",
+			base: []gittest.TreeEntry{
+				{Path: "file", Content: "foo", Mode: "100644"},
+			},
+			ours: []gittest.TreeEntry{
+				{Path: "file", Content: "fooqux", Mode: "100644"},
+			},
+			commit: []gittest.TreeEntry{
+				{Path: "file", Content: "foobar", Mode: "100644"},
+			},
+			expectedErrAs:  &git2go.ConflictingFilesError{},
+			expectedErrMsg: "cherry-pick: there are conflicting files",
+		},
+		{
+			desc: "empty cherry-pick fails",
+			base: []gittest.TreeEntry{
+				{Path: "file", Content: "foo", Mode: "100644"},
+			},
+			ours: []gittest.TreeEntry{
+				{Path: "file", Content: "fooqux", Mode: "100644"},
+			},
+			commit: []gittest.TreeEntry{
+				{Path: "file", Content: "fooqux", Mode: "100644"},
+			},
+			expectedErrAs:  &git2go.EmptyError{},
+			expectedErrMsg: "cherry-pick: could not apply because the result was empty",
+		},
+		{
+			desc:           "fails on nonexistent ours commit",
+			expectedErrAs:  &git2go.CommitNotFoundError{},
+			expectedErrMsg: "cherry-pick: ours commit lookup: commit not found: \"nonexistent\"",
+		},
+		{
+			desc: "fails on nonexistent cherry-pick commit",
+			ours: []gittest.TreeEntry{
+				{Path: "file", Content: "fooqux", Mode: "100644"},
+			},
+			expectedErrAs:  &git2go.CommitNotFoundError{},
+			expectedErrMsg: "cherry-pick: commit lookup: commit not found: \"nonexistent\"",
+		},
+	}
+	for _, tc := range testcases {
+		ctx := testhelper.Context(t)
+		cfg := testcfg.Build(t)
+
+		repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+			SkipCreationViaService: true,
+			Seed:                   gittest.SeedGitLabTest,
+		})
+		testcfg.BuildGitalyGit2Go(t, cfg)
+		executor := buildExecutor(t, cfg)
+
+		base := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries(tc.base...))
+
+		ours, commit := "nonexistent", "nonexistent"
+		if len(tc.ours) > 0 {
+			ours = gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(base), gittest.WithTreeEntries(tc.ours...)).String()
+		}
+		if len(tc.commit) > 0 {
+			commit = gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(base), gittest.WithTreeEntries(tc.commit...)).String()
+		}
+
+		t.Run(tc.desc, func(t *testing.T) {
+			committer := git.Signature{
+				Name:  "Baz",
+				Email: "baz@example.com",
+				When:  time.Date(2021, 1, 17, 14, 45, 51, 0, time.FixedZone("", +2*60*60)),
+			}
+
+			response, err := executor.CherryPick(ctx, repo, git2go.CherryPickCommand{
+				Repository:    repoPath,
+				CommitterName: committer.Name,
+				CommitterMail: committer.Email,
+				CommitterDate: committer.When,
+				Message:       "Foo",
+				Ours:          ours,
+				Commit:        commit,
+			})
+
+			if tc.expectedErrMsg != "" {
+				require.EqualError(t, err, tc.expectedErrMsg)
+
+				if tc.expectedErrAs != nil {
+					require.ErrorAs(t, err, tc.expectedErrAs)
+				}
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tc.expectedCommitID, response.String())
+
+			repo, err := git2goutil.OpenRepository(repoPath)
+			require.NoError(t, err)
+			defer repo.Free()
+
+			commitOid, err := git.NewOid(response.String())
+			require.NoError(t, err)
+
+			commit, err := repo.LookupCommit(commitOid)
+			require.NoError(t, err)
+
+			// The computed author does not include the timezone string "UTC+1".
+			expectedAuthor := DefaultAuthor
+			expectedAuthor.When = expectedAuthor.When.In(time.FixedZone("", 1*60*60))
+			require.Equal(t, &expectedAuthor, commit.Author())
+			require.Equal(t, &committer, commit.Committer())
+
+			tree, err := commit.Tree()
+			require.NoError(t, err)
+			require.Len(t, tc.expected, int(tree.EntryCount()))
+
+			for name, contents := range tc.expected {
+				entry := tree.EntryByName(name)
+				require.NotNil(t, entry)
+
+				blob, err := repo.LookupBlob(entry.Id)
+				require.NoError(t, err)
+				require.Equal(t, []byte(contents), blob.Contents())
+			}
+		})
+	}
+}
+
+func TestCherryPickStructuredErrors(t *testing.T) {
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
+	testcfg.BuildGitalyGit2Go(t, cfg)
+	executor := buildExecutor(t, cfg)
+
+	base := gittest.WriteCommit(
+		t,
+		cfg,
+		repoPath,
+		gittest.WithTreeEntries(gittest.TreeEntry{
+			Path: "file", Content: "foo", Mode: "100644",
+		}))
+
+	ours := gittest.WriteCommit(
+		t,
+		cfg,
+		repoPath,
+		gittest.WithParents(base),
+		gittest.WithTreeEntries(
+			gittest.TreeEntry{Path: "file", Content: "fooqux", Mode: "100644"},
+		)).String()
+
+	commit := gittest.WriteCommit(
+		t,
+		cfg,
+		repoPath,
+		gittest.WithParents(base),
+		gittest.WithTreeEntries(
+			gittest.TreeEntry{Path: "file", Content: "foobar", Mode: "100644"},
+		)).String()
+
+	committer := git.Signature{
+		Name:  "Baz",
+		Email: "baz@example.com",
+		When:  time.Date(2021, 1, 17, 14, 45, 51, 0, time.FixedZone("", +2*60*60)),
+	}
+
+	_, err := executor.CherryPick(ctx, repo, git2go.CherryPickCommand{
+		Repository:    repoPath,
+		CommitterName: committer.Name,
+		CommitterMail: committer.Email,
+		CommitterDate: committer.When,
+		Message:       "Foo",
+		Ours:          ours,
+		Commit:        commit,
+	})
+
+	require.EqualError(t, err, "cherry-pick: there are conflicting files")
+	require.ErrorAs(t, err, &git2go.ConflictingFilesError{})
+}
diff --git a/cmd/gitaly-git2go/main.go b/cmd/gitaly-git2go/main.go
index 33b00b52b..17b4d046c 100644
--- a/cmd/gitaly-git2go/main.go
+++ b/cmd/gitaly-git2go/main.go
@@ -25,7 +25,8 @@ type subcmd interface {
 }
 
 var subcommands = map[string]subcmd{
-	"rebase": &rebaseSubcommand{},
+	"cherry-pick": &cherryPickSubcommand{},
+	"rebase":      &rebaseSubcommand{},
 }
 
 func fatalf(logger logrus.FieldLogger, encoder *gob.Encoder, format string, args ...interface{}) {
diff --git a/internal/featureflag/ff_cherry_pick_pure_git.go b/internal/featureflag/ff_cherry_pick_pure_git.go
new file mode 100644
index 000000000..311e56c05
--- /dev/null
+++ b/internal/featureflag/ff_cherry_pick_pure_git.go
@@ -0,0 +1,10 @@
+package featureflag
+
+// CherryPickPureGit will enable the UserCherryPick RPC to use
+// git-merge-tree instead of git2go
+var CherryPickPureGit = NewFeatureFlag(
+	"cherry_pick_pure_git",
+	"v16.2.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/5421",
+	false,
+)
diff --git a/internal/git2go/cherry_pick.go b/internal/git2go/cherry_pick.go
new file mode 100644
index 000000000..435d85cbf
--- /dev/null
+++ b/internal/git2go/cherry_pick.go
@@ -0,0 +1,38 @@
+package git2go
+
+import (
+	"context"
+	"time"
+
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
+)
+
+// CherryPickCommand contains parameters to perform a cherry-pick.
+type CherryPickCommand struct {
+	// Repository is the path where to execute the cherry-pick.
+	Repository string
+	// CommitterName is the committer name for the resulting commit.
+	CommitterName string
+	// CommitterMail is the committer mail for the resulting commit.
+	CommitterMail string
+	// CommitterDate is the committer date of revert commit.
+	CommitterDate time.Time
+	// Message is the message to be used for the resulting commit.
+	Message string
+	// Ours is the commit that the revert is applied to.
+	Ours string
+	// Commit is the commit that is to be picked.
+	Commit string
+	// Mainline is the parent to be considered the mainline
+	Mainline uint
+	// SigningKey is a path to the key to sign commit using OpenPGP
+	SigningKey string
+}
+
+// CherryPick performs a cherry-pick via gitaly-git2go.
+func (b *Executor) CherryPick(ctx context.Context, repo storage.Repository, m CherryPickCommand) (git.ObjectID, error) {
+	m.SigningKey = b.signingKey
+
+	return b.runWithGob(ctx, repo, "cherry-pick", m)
+}
diff --git a/internal/gitaly/service/operations/cherry_pick.go b/internal/gitaly/service/operations/cherry_pick.go
index 137f2ec11..d939827ec 100644
--- a/internal/gitaly/service/operations/cherry_pick.go
+++ b/internal/gitaly/service/operations/cherry_pick.go
@@ -7,8 +7,10 @@ import (
 	"strings"
 	"time"
 
+	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook/updateref"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
@@ -36,6 +38,16 @@ func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPic
 		return nil, structerr.NewInternal("has branches: %w", err)
 	}
 
+	repoPath, err := quarantineRepo.Path()
+	if err != nil {
+		return nil, err
+	}
+
+	var mainline uint
+	if len(req.Commit.ParentIds) > 1 {
+		mainline = 1
+	}
+
 	committerDate := time.Now()
 	if req.Timestamp != nil {
 		committerDate = req.Timestamp.AsTime()
@@ -43,84 +55,131 @@ func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPic
 
 	var newrev git.ObjectID
 
-	cherryCommit, err := quarantineRepo.ReadCommit(ctx, git.Revision(req.Commit.Id))
-	if err != nil {
-		if errors.Is(err, localrepo.ErrObjectNotFound) {
-			return nil, structerr.NewNotFound("cherry-pick: commit lookup: commit not found: %q", req.Commit.Id)
-		}
-		return nil, fmt.Errorf("cherry pick: %w", err)
-	}
-	cherryDate := cherryCommit.Author.GetDate().AsTime()
-	loc, err := time.Parse("-0700", string(cherryCommit.Author.GetTimezone()))
-	if err != nil {
-		return nil, fmt.Errorf("get cherry commit location: %w", err)
-	}
-	cherryDate = cherryDate.In(loc.Location())
-
-	// Cherry-pick is implemented using git-merge-tree(1). We
-	// "merge" in the changes from the commit that is cherry-picked,
-	// compared to it's parent commit (specified as merge base).
-	treeOID, err := quarantineRepo.MergeTree(
-		ctx,
-		startRevision.String(),
-		req.Commit.Id,
-		localrepo.WithMergeBase(git.Revision(req.Commit.Id+"^")),
-		localrepo.WithConflictingFileNamesOnly(),
-	)
-	if err != nil {
-		var conflictErr *localrepo.MergeTreeConflictError
-		if errors.As(err, &conflictErr) {
-			conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFileInfo))
-			for _, conflictingFileInfo := range conflictErr.ConflictingFileInfo {
-				conflictingFiles = append(conflictingFiles, []byte(conflictingFileInfo.FileName))
+	if featureflag.CherryPickPureGit.IsEnabled(ctx) {
+		cherryCommit, err := quarantineRepo.ReadCommit(ctx, git.Revision(req.Commit.Id))
+		if err != nil {
+			if errors.Is(err, localrepo.ErrObjectNotFound) {
+				return nil, structerr.NewNotFound("cherry-pick: commit lookup: commit not found: %q", req.Commit.Id)
 			}
+			return nil, fmt.Errorf("cherry pick: %w", err)
+		}
+		cherryDate := cherryCommit.Author.GetDate().AsTime()
+		loc, err := time.Parse("-0700", string(cherryCommit.Author.GetTimezone()))
+		if err != nil {
+			return nil, fmt.Errorf("get cherry commit location: %w", err)
+		}
+		cherryDate = cherryDate.In(loc.Location())
 
-			return nil, structerr.NewFailedPrecondition("cherry pick: %w", err).WithDetail(
-				&gitalypb.UserCherryPickError{
-					Error: &gitalypb.UserCherryPickError_CherryPickConflict{
-						CherryPickConflict: &gitalypb.MergeConflictError{
-							ConflictingFiles: conflictingFiles,
+		// Cherry-pick is implemented using git-merge-tree(1). We
+		// "merge" in the changes from the commit that is cherry-picked,
+		// compared to it's parent commit (specified as merge base).
+		treeOID, err := quarantineRepo.MergeTree(
+			ctx,
+			startRevision.String(),
+			req.Commit.Id,
+			localrepo.WithMergeBase(git.Revision(req.Commit.Id+"^")),
+			localrepo.WithConflictingFileNamesOnly(),
+		)
+		if err != nil {
+			var conflictErr *localrepo.MergeTreeConflictError
+			if errors.As(err, &conflictErr) {
+				conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFileInfo))
+				for _, conflictingFileInfo := range conflictErr.ConflictingFileInfo {
+					conflictingFiles = append(conflictingFiles, []byte(conflictingFileInfo.FileName))
+				}
+
+				return nil, structerr.NewFailedPrecondition("cherry pick: %w", err).WithDetail(
+					&gitalypb.UserCherryPickError{
+						Error: &gitalypb.UserCherryPickError_CherryPickConflict{
+							CherryPickConflict: &gitalypb.MergeConflictError{
+								ConflictingFiles: conflictingFiles,
+							},
 						},
 					},
+				)
+			}
+
+			return nil, fmt.Errorf("cherry-pick command: %w", err)
+		}
+
+		oldTree, err := quarantineRepo.ResolveRevision(
+			ctx,
+			git.Revision(fmt.Sprintf("%s^{tree}", startRevision.String())),
+		)
+		if err != nil {
+			return nil, fmt.Errorf("resolve old tree: %w", err)
+		}
+		if oldTree == treeOID {
+			return nil, structerr.NewFailedPrecondition("cherry-pick: could not apply because the result was empty").WithDetail(
+				&gitalypb.UserCherryPickError{
+					Error: &gitalypb.UserCherryPickError_ChangesAlreadyApplied{},
 				},
 			)
 		}
 
-		return nil, fmt.Errorf("cherry-pick command: %w", err)
-	}
-
-	oldTree, err := quarantineRepo.ResolveRevision(
-		ctx,
-		git.Revision(fmt.Sprintf("%s^{tree}", startRevision.String())),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("resolve old tree: %w", err)
-	}
-	if oldTree == treeOID {
-		return nil, structerr.NewFailedPrecondition("cherry-pick: could not apply because the result was empty").WithDetail(
-			&gitalypb.UserCherryPickError{
-				Error: &gitalypb.UserCherryPickError_ChangesAlreadyApplied{},
+		newrev, err = quarantineRepo.WriteCommit(
+			ctx,
+			localrepo.WriteCommitConfig{
+				TreeID:         treeOID,
+				Message:        string(req.Message),
+				Parents:        []git.ObjectID{startRevision},
+				AuthorName:     string(cherryCommit.Author.Name),
+				AuthorEmail:    string(cherryCommit.Author.Email),
+				AuthorDate:     cherryDate,
+				CommitterName:  string(req.User.Name),
+				CommitterEmail: string(req.User.Email),
+				CommitterDate:  committerDate,
+				SigningKey:     s.signingKey,
 			},
 		)
-	}
+		if err != nil {
+			return nil, fmt.Errorf("write commit: %w", err)
+		}
+	} else {
+		newrev, err = s.git2goExecutor.CherryPick(ctx, quarantineRepo, git2go.CherryPickCommand{
+			Repository:    repoPath,
+			CommitterName: string(req.User.Name),
+			CommitterMail: string(req.User.Email),
+			CommitterDate: committerDate,
+			Message:       string(req.Message),
+			Commit:        req.Commit.Id,
+			Ours:          startRevision.String(),
+			Mainline:      mainline,
+		})
+		if err != nil {
+			var conflictErr git2go.ConflictingFilesError
+			var emptyErr git2go.EmptyError
 
-	newrev, err = quarantineRepo.WriteCommit(
-		ctx,
-		localrepo.WriteCommitConfig{
-			TreeID:         treeOID,
-			Message:        string(req.Message),
-			Parents:        []git.ObjectID{startRevision},
-			AuthorName:     string(cherryCommit.Author.Name),
-			AuthorEmail:    string(cherryCommit.Author.Email),
-			AuthorDate:     cherryDate,
-			CommitterName:  string(req.User.Name),
-			CommitterEmail: string(req.User.Email),
-			CommitterDate:  committerDate,
-			SigningKey:     s.signingKey,
-		},
-	)
-	if err != nil {
-		return nil, fmt.Errorf("write commit: %w", err)
+			switch {
+			case errors.As(err, &conflictErr):
+				conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFiles))
+				for _, conflictingFile := range conflictErr.ConflictingFiles {
+					conflictingFiles = append(conflictingFiles, []byte(conflictingFile))
+				}
+
+				return nil, structerr.NewFailedPrecondition("cherry pick: %w", err).WithDetail(
+					&gitalypb.UserCherryPickError{
+						Error: &gitalypb.UserCherryPickError_CherryPickConflict{
+							CherryPickConflict: &gitalypb.MergeConflictError{
+								ConflictingFiles: conflictingFiles,
+							},
+						},
+					},
+				)
+			case errors.As(err, &emptyErr):
+				return nil, structerr.NewFailedPrecondition("%w", err).WithDetail(
+					&gitalypb.UserCherryPickError{
+						Error: &gitalypb.UserCherryPickError_ChangesAlreadyApplied{},
+					},
+				)
+			case errors.As(err, &git2go.CommitNotFoundError{}):
+				return nil, structerr.NewNotFound("%w", err)
+			case errors.Is(err, git2go.ErrInvalidArgument):
+				return nil, structerr.NewInvalidArgument("%w", err)
+			default:
+				return nil, fmt.Errorf("cherry-pick command: %w", err)
+			}
+		}
 	}
 
 	referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName))
diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go
index 54d9237f3..66ed82a88 100644
--- a/internal/gitaly/service/operations/cherry_pick_test.go
+++ b/internal/gitaly/service/operations/cherry_pick_test.go
@@ -30,6 +30,7 @@ func TestUserCherryPick(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testUserCherryPick)
 }
@@ -37,6 +38,8 @@ func TestUserCherryPick(t *testing.T) {
 func testUserCherryPick(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
 	destinationBranch := "dst-branch"
@@ -360,6 +363,7 @@ func TestServer_UserCherryPick_successfulGitHooks(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickSuccessfulGitHooks)
 }
@@ -367,6 +371,8 @@ func TestServer_UserCherryPick_successfulGitHooks(t *testing.T) {
 func testServerUserCherryPickSuccessfulGitHooks(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -416,6 +422,7 @@ func TestServer_UserCherryPick_mergeCommit(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickMergeCommit)
 }
@@ -434,6 +441,8 @@ func testServerUserCherryPickMergeCommit(t *testing.T, ctx context.Context) {
 		testcfg.BuildGitalyGPG(t, cfg)
 	}
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
@@ -509,7 +518,7 @@ func testServerUserCherryPickMergeCommit(t *testing.T, ctx context.Context) {
 			{Mode: "100644", Path: "z", Content: "zucchini"},
 		})
 
-	if featureflag.GPGSigning.IsEnabled(ctx) {
+	if featureflag.GPGSigning.IsEnabled(ctx) && featureflag.CherryPickPureGit.IsEnabled(ctx) {
 		data, err := repo.ReadObject(ctx, git.ObjectID(response.BranchUpdate.CommitId))
 		require.NoError(t, err)
 
@@ -526,6 +535,7 @@ func TestServer_UserCherryPick_stableID(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickStableID)
 }
@@ -533,6 +543,8 @@ func TestServer_UserCherryPick_stableID(t *testing.T) {
 func testServerUserCherryPickStableID(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -614,7 +626,7 @@ func TestServer_UserCherryPick_failedValidations(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickFailedValidations)
 }
@@ -622,6 +634,8 @@ func TestServer_UserCherryPick_failedValidations(t *testing.T) {
 func testServerUserCherryPickFailedValidations(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -725,6 +739,7 @@ func TestServer_UserCherryPick_failedWithPreReceiveError(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickFailedWithPreReceiveError)
 }
@@ -732,6 +747,8 @@ func TestServer_UserCherryPick_failedWithPreReceiveError(t *testing.T) {
 func testServerUserCherryPickFailedWithPreReceiveError(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -787,6 +804,7 @@ func TestServer_UserCherryPick_failedWithCreateTreeError(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickFailedWithCreateTreeError)
 }
@@ -794,6 +812,8 @@ func TestServer_UserCherryPick_failedWithCreateTreeError(t *testing.T) {
 func testServerUserCherryPickFailedWithCreateTreeError(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -838,6 +858,7 @@ func TestServer_UserCherryPick_failedWithCommitError(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickFailedWithCommitError)
 }
@@ -845,6 +866,8 @@ func TestServer_UserCherryPick_failedWithCommitError(t *testing.T) {
 func testServerUserCherryPickFailedWithCommitError(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -898,6 +921,7 @@ func TestServer_UserCherryPick_failedWithConflict(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickFailedWithConflict)
 }
@@ -905,6 +929,8 @@ func TestServer_UserCherryPick_failedWithConflict(t *testing.T) {
 func testServerUserCherryPickFailedWithConflict(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -962,6 +988,7 @@ func TestServer_UserCherryPick_successfulWithGivenCommits(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickSuccessfulWithGivenCommits)
 }
@@ -969,6 +996,8 @@ func TestServer_UserCherryPick_successfulWithGivenCommits(t *testing.T) {
 func testServerUserCherryPickSuccessfulWithGivenCommits(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -1037,6 +1066,7 @@ func TestServer_UserCherryPick_quarantine(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickQuarantine)
 }
@@ -1044,6 +1074,8 @@ func TestServer_UserCherryPick_quarantine(t *testing.T) {
 func testServerUserCherryPickQuarantine(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -1103,6 +1135,7 @@ func TestServer_UserCherryPick_reverse(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
+		featureflag.CherryPickPureGit,
 		featureflag.GPGSigning,
 	).Run(t, testServerUserCherryPickReverse)
 }
@@ -1110,6 +1143,8 @@ func TestServer_UserCherryPick_reverse(t *testing.T) {
 func testServerUserCherryPickReverse(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	skipSHA256WithGit2goCherryPick(t, ctx)
+
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -1177,3 +1212,9 @@ func testServerUserCherryPickReverse(t *testing.T, ctx context.Context) {
 		)
 	}
 }
+
+func skipSHA256WithGit2goCherryPick(t *testing.T, ctx context.Context) {
+	if gittest.DefaultObjectHash.Format == git.ObjectHashSHA256.Format && featureflag.CherryPickPureGit.IsDisabled(ctx) {
+		t.Skip("SHA256 repositories are only supported when using the pure Git implementation")
+	}
+}