git2go: Use gob for submodule sub-command invocation

So far we used JSON serialization to invoke submodule
sub-command of the git2go executable. We are moving
towards usage of the gob serialization to do the same
and use stdin and stdout of the process to get an input
and return the result of the execution. gob doesn't
require us to do mapping of the fields and allows
passing typed errors between the processes.

In order to support smooth migration we should support
both serialization formats. We can drop support of
the JSON in the next release.

Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/3232

Changelog: changed

2 files changed, 104 insertions, 19 deletions

diff --git a/cmd/gitaly-git2go/submodule.go b/cmd/gitaly-git2go/submodule.go
index fbf3e13df..1cdbd69da 100644
--- a/cmd/gitaly-git2go/submodule.go
+++ b/cmd/gitaly-git2go/submodule.go
@@ -5,6 +5,7 @@ package main
 
 import (
 	"context"
+	"encoding/gob"
 	"flag"
 	"fmt"
 	"io"
@@ -25,54 +26,75 @@ func (cmd *submoduleSubcommand) Flags() *flag.FlagSet {
 	return flags
 }
 
-func (cmd *submoduleSubcommand) Run(_ context.Context, _ io.Reader, w io.Writer) error {
-	request, err := git2go.SubmoduleCommandFromSerialized(cmd.request)
+func (cmd *submoduleSubcommand) Run(_ context.Context, r io.Reader, w io.Writer) error {
+	var request git2go.SubmoduleCommand
+	if cmd.request == "" {
+		if err := gob.NewDecoder(r).Decode(&request); err != nil {
+			return fmt.Errorf("deserializing submodule command request: %w", err)
+		}
+	} else {
+		var err error
+		request, err = git2go.SubmoduleCommandFromSerialized(cmd.request)
+		if err != nil {
+			return fmt.Errorf("deserializing submodule command request: %w", err)
+		}
+	}
+
+	res, err := cmd.run(request)
 	if err != nil {
-		return fmt.Errorf("deserializing submodule command request: %w", err)
+		return err
 	}
 
+	if cmd.request == "" {
+		return gob.NewEncoder(w).Encode(res)
+	}
+
+	return res.SerializeTo(w)
+}
+
+func (cmd *submoduleSubcommand) run(request git2go.SubmoduleCommand) (*git2go.SubmoduleResult, error) {
 	if request.AuthorDate.IsZero() {
 		request.AuthorDate = time.Now()
 	}
 
 	smCommitOID, err := git.NewOid(request.CommitSHA)
 	if err != nil {
-		return fmt.Errorf("converting %s to OID: %w", request.CommitSHA, err)
+		return nil, fmt.Errorf("converting %s to OID: %w", request.CommitSHA, err)
 	}
 
 	repo, err := git2goutil.OpenRepository(request.Repository)
 	if err != nil {
-		return fmt.Errorf("open repository: %w", err)
+		return nil, fmt.Errorf("open repository: %w", err)
 	}
 
 	fullBranchRefName := "refs/heads/" + request.Branch
 	o, err := repo.RevparseSingle(fullBranchRefName)
 	if err != nil {
-		return fmt.Errorf("%s: %w", git2go.LegacyErrPrefixInvalidBranch, err)
+		return nil, fmt.Errorf("%s: %w", git2go.LegacyErrPrefixInvalidBranch, err)
 	}
 
 	startCommit, err := o.AsCommit()
 	if err != nil {
-		return fmt.Errorf("peeling %s as a commit: %w", o.Id(), err)
+		return nil, fmt.Errorf("peeling %s as a commit: %w", o.Id(), err)
 	}
 
 	rootTree, err := startCommit.Tree()
 	if err != nil {
-		return fmt.Errorf("root tree from starting commit: %w", err)
+		return nil, fmt.Errorf("root tree from starting commit: %w", err)
 	}
 
 	index, err := git.NewIndex()
 	if err != nil {
-		return fmt.Errorf("creating new index: %w", err)
+		return nil, fmt.Errorf("creating new index: %w", err)
 	}
 
 	if err := index.ReadTree(rootTree); err != nil {
-		return fmt.Errorf("reading root tree into index: %w", err)
+		return nil, fmt.Errorf("reading root tree into index: %w", err)
 	}
 
 	smEntry, err := index.EntryByPath(request.Submodule, 0)
 	if err != nil {
-		return fmt.Errorf(
+		return nil, fmt.Errorf(
 			"%s: %w",
 			git2go.LegacyErrPrefixInvalidSubmodulePath, err,
 		)
@@ -80,14 +102,14 @@ func (cmd *submoduleSubcommand) Run(_ context.Context, _ io.Reader, w io.Writer)
 
 	if smEntry.Id.Cmp(smCommitOID) == 0 {
 		//nolint
-		return fmt.Errorf(
+		return nil, fmt.Errorf(
 			"The submodule %s is already at %s",
 			request.Submodule, request.CommitSHA,
 		)
 	}
 
 	if smEntry.Mode != git.FilemodeCommit {
-		return fmt.Errorf(
+		return nil, fmt.Errorf(
 			"%s: %w",
 			git2go.LegacyErrPrefixInvalidSubmodulePath, err,
 		)
@@ -96,17 +118,17 @@ func (cmd *submoduleSubcommand) Run(_ context.Context, _ io.Reader, w io.Writer)
 	newEntry := *smEntry      // copy by value
 	newEntry.Id = smCommitOID // assign new commit SHA
 	if err := index.Add(&newEntry); err != nil {
-		return fmt.Errorf("add new submodule entry to index: %w", err)
+		return nil, fmt.Errorf("add new submodule entry to index: %w", err)
 	}
 
 	newRootTreeOID, err := index.WriteTreeTo(repo)
 	if err != nil {
-		return fmt.Errorf("write index to repo: %w", err)
+		return nil, fmt.Errorf("write index to repo: %w", err)
 	}
 
 	newTree, err := repo.LookupTree(newRootTreeOID)
 	if err != nil {
-		return fmt.Errorf("looking up new submodule entry root tree: %w", err)
+		return nil, fmt.Errorf("looking up new submodule entry root tree: %w", err)
 	}
 
 	committer := git.Signature(
@@ -125,13 +147,13 @@ func (cmd *submoduleSubcommand) Run(_ context.Context, _ io.Reader, w io.Writer)
 		startCommit,
 	)
 	if err != nil {
-		return fmt.Errorf(
+		return nil, fmt.Errorf(
 			"%s: %w",
 			git2go.LegacyErrPrefixFailedCommit, err,
 		)
 	}
 
-	return git2go.SubmoduleResult{
+	return &git2go.SubmoduleResult{
 		CommitID: newCommitOID.String(),
-	}.SerializeTo(w)
+	}, nil
 }
diff --git a/cmd/gitaly-git2go/submodule_test.go b/cmd/gitaly-git2go/submodule_test.go
index d83bc9767..1ddf0903f 100644
--- a/cmd/gitaly-git2go/submodule_test.go
+++ b/cmd/gitaly-git2go/submodule_test.go
@@ -5,7 +5,11 @@ package main
 
 import (
 	"bytes"
+	"encoding/base64"
+	"encoding/json"
 	"fmt"
+	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -131,3 +135,62 @@ func TestSubmodule(t *testing.T) {
 		})
 	}
 }
+
+// TestSubmodule_backwardsCompatibility check that submodule sub-command able to process both:
+// - JSON encoded flag as input
+// - gob encoded data from stdin
+// The support of first option should be dropped in the next release (14.7) together with this test.
+func TestSubmodule_backwardsCompatibility(t *testing.T) {
+	const commitMessage = "Update Submodule message"
+	const commitSHA = "41fa1bc9e0f0630ced6a8a211d60c2af425ecc2d"
+	const submodule = "gitlab-grack"
+
+	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+	testcfg.BuildGitalyGit2Go(t, cfg)
+	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	marshalled, err := json.Marshal(git2go.SubmoduleCommand{
+		Repository: repoPath,
+		AuthorName: string(gittest.TestUser.Name),
+		AuthorMail: string(gittest.TestUser.Email),
+		Message:    commitMessage,
+		CommitSHA:  commitSHA,
+		Submodule:  submodule,
+		Branch:     "master",
+	})
+	require.NoError(t, err)
+	encoded := base64.StdEncoding.EncodeToString(marshalled)
+
+	cmd := exec.CommandContext(ctx, filepath.Join(cfg.BinDir, "gitaly-git2go"), "submodule", "-request", encoded)
+	output, err := cmd.CombinedOutput()
+	require.NoError(t, err, string(output))
+	decoded, err := base64.StdEncoding.DecodeString(string(output))
+	require.NoError(t, err)
+	result := git2go.SubmoduleResult{}
+	require.NoError(t, json.Unmarshal(decoded, &result))
+
+	commit, err := repo.ReadCommit(ctx, git.Revision(result.CommitID))
+	require.NoError(t, err)
+	require.Equal(t, commit.Author.Email, gittest.TestUser.Email)
+	require.Equal(t, commit.Committer.Email, gittest.TestUser.Email)
+	require.Equal(t, string(commit.Subject), commitMessage)
+
+	entry := gittest.Exec(
+		t,
+		cfg,
+		"-C",
+		repoPath,
+		"ls-tree",
+		"-z",
+		fmt.Sprintf("%s^{tree}:", result.CommitID),
+		submodule,
+	)
+	parser := lstree.NewParser(bytes.NewReader(entry))
+	parsedEntry, err := parser.NextEntry()
+	require.NoError(t, err)
+	require.Equal(t, submodule, parsedEntry.Path)
+	require.Equal(t, commitSHA, parsedEntry.ObjectID.String())
+}