Merge branch 'pks-git-lstree-sha256' into 'master'

lstree: Support SHA256 object hash

See merge request gitlab-org/gitaly!4789

14 files changed, 61 insertions, 80 deletions

diff --git a/cmd/gitaly-git2go/submodule_test.go b/cmd/gitaly-git2go/submodule_test.go
index 7e6c2a5bd..c05bc0af5 100644
--- a/cmd/gitaly-git2go/submodule_test.go
+++ b/cmd/gitaly-git2go/submodule_test.go
@@ -119,7 +119,7 @@ func TestSubmodule(t *testing.T) {
 				fmt.Sprintf("%s^{tree}:", response.CommitID),
 				tc.command.Submodule,
 			)
-			parser := lstree.NewParser(bytes.NewReader(entry))
+			parser := lstree.NewParser(bytes.NewReader(entry), git.ObjectHashSHA1)
 			parsedEntry, err := parser.NextEntry()
 			require.NoError(t, err)
 			require.Equal(t, tc.command.Submodule, parsedEntry.Path)
diff --git a/internal/git/gitpipe/ls_tree.go b/internal/git/gitpipe/ls_tree.go
index 10c6237c1..5d2ad775f 100644
--- a/internal/git/gitpipe/ls_tree.go
+++ b/internal/git/gitpipe/ls_tree.go
@@ -80,7 +80,7 @@ func LsTree(
 			return
 		}
 
-		parser := lstree.NewParser(cmd)
+		parser := lstree.NewParser(cmd, git.ObjectHashSHA1)
 		for {
 			entry, err := parser.NextEntry()
 			if err != nil {
diff --git a/internal/git/gittest/tree.go b/internal/git/gittest/tree.go
index 41194ed72..361767b49 100644
--- a/internal/git/gittest/tree.go
+++ b/internal/git/gittest/tree.go
@@ -88,6 +88,8 @@ func WriteTree(t testing.TB, cfg config.Cfg, repoPath string, entries []TreeEntr
 			entryType = "blob"
 		case "040000":
 			entryType = "tree"
+		case "160000":
+			entryType = "commit"
 		default:
 			t.Fatalf("invalid entry type %q", entry.Mode)
 		}
diff --git a/internal/git/lstree/list_entries.go b/internal/git/lstree/list_entries.go
index e68a5055a..24156c7a7 100644
--- a/internal/git/lstree/list_entries.go
+++ b/internal/git/lstree/list_entries.go
@@ -60,7 +60,12 @@ func ListEntries(
 		return nil, fmt.Errorf("spawning git-ls-tree: %w", err)
 	}
 
-	parser := NewParser(cmd)
+	objectHash, err := repo.ObjectHash(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("detecting object hash: %w", err)
+	}
+
+	parser := NewParser(cmd, objectHash)
 	var entries []*Entry
 	for {
 		entry, err := parser.NextEntry()
diff --git a/internal/git/lstree/list_entries_test.go b/internal/git/lstree/list_entries_test.go
index 939c72b81..9a3d3cba7 100644
--- a/internal/git/lstree/list_entries_test.go
+++ b/internal/git/lstree/list_entries_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package lstree
 
 import (
@@ -14,10 +12,12 @@ import (
 )
 
 func TestListEntries(t *testing.T) {
+	t.Parallel()
+
 	cfg := testcfg.Build(t)
 	ctx := testhelper.Context(t)
 
-	repoProto, repoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
+	repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 	blobID := gittest.WriteBlob(t, cfg, repoPath, []byte("blob contents"))
@@ -151,8 +151,8 @@ func TestListEntries(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			results, err := ListEntries(ctx, repo, tc.treeish, tc.cfg)
-			require.Equal(t, tc.expectedResults, results)
 			require.Equal(t, tc.expectedErr, err)
+			require.Equal(t, tc.expectedResults, results)
 		})
 	}
 }
diff --git a/internal/git/lstree/parser.go b/internal/git/lstree/parser.go
index 2a35df472..7c1a58adb 100644
--- a/internal/git/lstree/parser.go
+++ b/internal/git/lstree/parser.go
@@ -14,13 +14,15 @@ var ErrParse = errors.New("failed to parse git ls-tree response")
 
 // Parser holds the necessary state for parsing the ls-tree output
 type Parser struct {
-	reader *bufio.Reader
+	reader     *bufio.Reader
+	objectHash git.ObjectHash
 }
 
 // NewParser returns a new Parser
-func NewParser(src io.Reader) *Parser {
+func NewParser(src io.Reader, objectHash git.ObjectHash) *Parser {
 	return &Parser{
-		reader: bufio.NewReader(src),
+		reader:     bufio.NewReader(src),
+		objectHash: objectHash,
 	}
 }
 
@@ -61,7 +63,7 @@ func (p *Parser) NextEntry() (*Entry, error) {
 		return nil, err
 	}
 
-	objectID, err := git.ObjectHashSHA1.FromHex(string(treeEntryID))
+	objectID, err := p.objectHash.FromHex(string(treeEntryID))
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/git/lstree/parser_test.go b/internal/git/lstree/parser_test.go
index 6f8bb1912..4fc4d09c1 100644
--- a/internal/git/lstree/parser_test.go
+++ b/internal/git/lstree/parser_test.go
@@ -1,93 +1,74 @@
-//go:build !gitaly_test_sha256
-
 package lstree
 
 import (
+	"bytes"
 	"io"
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 )
 
 func TestParser(t *testing.T) {
-	testCases := []struct {
-		desc     string
-		filename string
-		entries  Entries
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
+	_, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
+
+	gitignoreBlobID := gittest.WriteBlob(t, cfg, repoPath, []byte("gitignore"))
+	gitmodulesBlobID := gittest.WriteBlob(t, cfg, repoPath, []byte("gitmodules"))
+	submoduleCommitID := gittest.WriteCommit(t, cfg, repoPath)
+
+	regularEntriesTreeID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+		{Path: ".gitignore", Mode: "100644", OID: gitignoreBlobID},
+		{Path: ".gitmodules", Mode: "100644", OID: gitmodulesBlobID},
+		{Path: "entry with space", Mode: "040000", OID: gittest.DefaultObjectHash.EmptyTreeOID},
+		{Path: "gitlab-shell", Mode: "160000", OID: submoduleCommitID},
+	})
+
+	for _, tc := range []struct {
+		desc            string
+		treeID          git.ObjectID
+		expectedEntries Entries
 	}{
 		{
-			desc:     "regular entries",
-			filename: "testdata/z-lstree.txt",
-			entries: Entries{
-				{
-					Mode:     []byte("100644"),
-					Type:     Blob,
-					ObjectID: "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
-					Path:     ".gitignore",
-				},
-				{
-					Mode:     []byte("100644"),
-					Type:     Blob,
-					ObjectID: "0792c58905eff3432b721f8c4a64363d8e28d9ae",
-					Path:     ".gitmodules",
-				},
-				{
-					Mode:     []byte("040000"),
-					Type:     Tree,
-					ObjectID: "3c122d2b7830eca25235131070602575cf8b41a1",
-					Path:     "encoding",
-				},
-				{
-					Mode:     []byte("160000"),
-					Type:     Submodule,
-					ObjectID: "79bceae69cb5750d6567b223597999bfa91cb3b9",
-					Path:     "gitlab-shell",
-				},
-			},
-		},
-		{
-			desc:     "irregular path",
-			filename: "testdata/z-lstree-irregular.txt",
-			entries: Entries{
+			desc:   "regular entries",
+			treeID: regularEntriesTreeID,
+			expectedEntries: Entries{
 				{
 					Mode:     []byte("100644"),
 					Type:     Blob,
-					ObjectID: "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
+					ObjectID: gitignoreBlobID,
 					Path:     ".gitignore",
 				},
 				{
 					Mode:     []byte("100644"),
 					Type:     Blob,
-					ObjectID: "0792c58905eff3432b721f8c4a64363d8e28d9ae",
+					ObjectID: gitmodulesBlobID,
 					Path:     ".gitmodules",
 				},
 				{
 					Mode:     []byte("040000"),
 					Type:     Tree,
-					ObjectID: "3c122d2b7830eca25235131070602575cf8b41a1",
-					Path:     "some encoding",
+					ObjectID: gittest.DefaultObjectHash.EmptyTreeOID,
+					Path:     "entry with space",
 				},
 				{
 					Mode:     []byte("160000"),
 					Type:     Submodule,
-					ObjectID: "79bceae69cb5750d6567b223597999bfa91cb3b9",
+					ObjectID: submoduleCommitID,
 					Path:     "gitlab-shell",
 				},
 			},
 		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			file, err := os.Open(testCase.filename)
-
-			require.NoError(t, err)
-			defer file.Close()
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			treeData := gittest.Exec(t, cfg, "-C", repoPath, "ls-tree", "-z", tc.treeID.String())
 
+			parser := NewParser(bytes.NewReader(treeData), gittest.DefaultObjectHash)
 			parsedEntries := Entries{}
-
-			parser := NewParser(file)
 			for {
 				entry, err := parser.NextEntry()
 				if err == io.EOF {
@@ -98,14 +79,7 @@ func TestParser(t *testing.T) {
 				parsedEntries = append(parsedEntries, *entry)
 			}
 
-			expectedEntries := testCase.entries
-			require.Len(t, expectedEntries, len(parsedEntries))
-
-			for index, parsedEntry := range parsedEntries {
-				expectedEntry := expectedEntries[index]
-
-				require.Equal(t, expectedEntry, parsedEntry)
-			}
+			require.Equal(t, tc.expectedEntries, parsedEntries)
 		})
 	}
 }
diff --git a/internal/git/lstree/testdata/z-lstree-irregular.txt b/internal/git/lstree/testdata/z-lstree-irregular.txt
deleted file mode 100644
index ed55df3a4..000000000
--- a/internal/git/lstree/testdata/z-lstree-irregular.txt
+++ /dev/null
diff --git a/internal/git/lstree/testdata/z-lstree.txt b/internal/git/lstree/testdata/z-lstree.txt
deleted file mode 100644
index 653a2f8b1..000000000
--- a/internal/git/lstree/testdata/z-lstree.txt
+++ /dev/null
diff --git a/internal/git/lstree/testhelper_test.go b/internal/git/lstree/testhelper_test.go
index 9136297e2..619a98800 100644
--- a/internal/git/lstree/testhelper_test.go
+++ b/internal/git/lstree/testhelper_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package lstree
 
 import (
diff --git a/internal/gitaly/service/commit/list_files.go b/internal/gitaly/service/commit/list_files.go
index e252688cd..9a1da2197 100644
--- a/internal/gitaly/service/commit/list_files.go
+++ b/internal/gitaly/service/commit/list_files.go
@@ -81,7 +81,7 @@ func (s *server) listFiles(repo git.RepositoryExecutor, revision string, stream
 
 	sender := chunk.New(&listFilesSender{stream: stream})
 
-	for parser := lstree.NewParser(cmd); ; {
+	for parser := lstree.NewParser(cmd, git.ObjectHashSHA1); ; {
 		entry, err := parser.NextEntry()
 		if err == io.EOF {
 			break
diff --git a/internal/gitaly/service/commit/list_last_commits_for_tree.go b/internal/gitaly/service/commit/list_last_commits_for_tree.go
index 4a7300de1..7e1c35b83 100644
--- a/internal/gitaly/service/commit/list_last_commits_for_tree.go
+++ b/internal/gitaly/service/commit/list_last_commits_for_tree.go
@@ -123,7 +123,7 @@ func (s *server) newLSTreeParser(in *gitalypb.ListLastCommitsForTreeRequest, str
 		return nil, nil, err
 	}
 
-	return cmd, lstree.NewParser(cmd), nil
+	return cmd, lstree.NewParser(cmd, git.ObjectHashSHA1), nil
 }
 
 func sendCommitsForTree(batch []*gitalypb.ListLastCommitsForTreeResponse_CommitForTree, stream gitalypb.CommitService_ListLastCommitsForTreeServer) error {
diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go
index 442cc2964..43ab9856b 100644
--- a/internal/gitaly/service/operations/submodules_test.go
+++ b/internal/gitaly/service/operations/submodules_test.go
@@ -93,7 +93,7 @@ func TestSuccessfulUserUpdateSubmoduleRequest(t *testing.T) {
 			require.Equal(t, commitMessage, commit.Subject)
 
 			entry := gittest.Exec(t, cfg, "-C", repoPath, "ls-tree", "-z", fmt.Sprintf("%s^{tree}:", response.BranchUpdate.CommitId), testCase.submodule)
-			parser := lstree.NewParser(bytes.NewReader(entry))
+			parser := lstree.NewParser(bytes.NewReader(entry), git.ObjectHashSHA1)
 			parsedEntry, err := parser.NextEntry()
 			require.NoError(t, err)
 			require.Equal(t, testCase.submodule, parsedEntry.Path)
diff --git a/internal/gitaly/service/repository/license.go b/internal/gitaly/service/repository/license.go
index 55b28c4b0..8fc277ca6 100644
--- a/internal/gitaly/service/repository/license.go
+++ b/internal/gitaly/service/repository/license.go
@@ -123,7 +123,7 @@ func (f *gitFiler) ReadDir(string) ([]filer.File, error) {
 		return nil, err
 	}
 
-	tree := lstree.NewParser(cmd)
+	tree := lstree.NewParser(cmd, git.ObjectHashSHA1)
 
 	var files []filer.File
 	for {