refactor: simplify ParseCatFileTreeLine and catBatchParseTreeEntries (#37210)

Simplify ParseCatFileTreeLine: it is faster without the preset buffers,
and easier to read and maintain.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

modules/git/catfile_batch_reader.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"io"
 	"math"
+	"slices"
 	"strconv"
 	"strings"
 	"sync/atomic"
@@ -172,77 +173,46 @@ headerLoop:
 	return id, DiscardFull(rd, size-n+1)
 }
 
-// git tree files are a list:
-// <mode-in-ascii> SP <fname> NUL <binary Hash>
-//
-// Unfortunately this 20-byte notation is somewhat in conflict to all other git tools
-// Therefore we need some method to convert these binary hashes to hex hashes
-
 // ParseCatFileTreeLine reads an entry from a tree in a cat-file --batch stream
-// This carefully avoids allocations - except where fnameBuf is too small.
+// Each entry is composed of:
-// It is recommended therefore to pass in an fnameBuf large enough to avoid almost all allocations
+// <mode-in-ascii-dropping-initial-zeros> SP <name> NUL <binary-hash>
-//
+func ParseCatFileTreeLine(objectFormat ObjectFormat, rd BufferedReader) (mode EntryMode, name string, objID ObjectID, n int, err error) {
-// Each line is composed of:
+	// use the in-buffer memory as much as possible to avoid extra allocations
-// <mode-in-ascii-dropping-initial-zeros> SP <fname> NUL <binary HASH>
+	bufBytes, err := rd.ReadSlice('\x00')
-//
+	const maxEntryInfoBytes = 1024 * 1024
-// We don't attempt to convert the raw HASH to save a lot of time
+	if errors.Is(err, bufio.ErrBufferFull) {
-func ParseCatFileTreeLine(objectFormat ObjectFormat, rd BufferedReader, modeBuf, fnameBuf, shaBuf []byte) (mode, fname, sha []byte, n int, err error) {
+		bufBytes = slices.Clone(bufBytes)
-	var readBytes []byte
+		for len(bufBytes) < maxEntryInfoBytes && errors.Is(err, bufio.ErrBufferFull) {
-
+			var tmp []byte
-	// Read the Mode & fname
+			tmp, err = rd.ReadSlice('\x00')
-	readBytes, err = rd.ReadSlice('\x00')
+			bufBytes = append(bufBytes, tmp...)
-	if err != nil {
-		return mode, fname, sha, n, err
-	}
-	idx := bytes.IndexByte(readBytes, ' ')
-	if idx < 0 {
-		log.Debug("missing space in readBytes ParseCatFileTreeLine: %s", readBytes)
-		return mode, fname, sha, n, &ErrNotExist{}
-	}
-
-	n += idx + 1
-	copy(modeBuf, readBytes[:idx])
-	if len(modeBuf) >= idx {
-		modeBuf = modeBuf[:idx]
-	} else {
-		modeBuf = append(modeBuf, readBytes[len(modeBuf):idx]...)
-	}
-	mode = modeBuf
-
-	readBytes = readBytes[idx+1:]
-
-	// Deal with the fname
-	copy(fnameBuf, readBytes)
-	if len(fnameBuf) > len(readBytes) {
-		fnameBuf = fnameBuf[:len(readBytes)]
-	} else {
-		fnameBuf = append(fnameBuf, readBytes[len(fnameBuf):]...)
-	}
-	for err == bufio.ErrBufferFull {
-		readBytes, err = rd.ReadSlice('\x00')
-		fnameBuf = append(fnameBuf, readBytes...)
-	}
-	n += len(fnameBuf)
-	if err != nil {
-		return mode, fname, sha, n, err
-	}
-	fnameBuf = fnameBuf[:len(fnameBuf)-1]
-	fname = fnameBuf
-
-	// Deal with the binary hash
-	idx = 0
-	length := objectFormat.FullLength() / 2
-	for idx < length {
-		var read int
-		read, err = rd.Read(shaBuf[idx:length])
-		n += read
-		if err != nil {
-			return mode, fname, sha, n, err
 		}
-		idx += read
 	}
-	sha = shaBuf
+	if err != nil {
-	return mode, fname, sha, n, err
+		return mode, name, objID, len(bufBytes), err
+	}
+
+	idx := bytes.IndexByte(bufBytes, ' ')
+	if idx < 0 {
+		return mode, name, objID, len(bufBytes), errors.New("invalid CatFileTreeLine output")
+	}
+
+	mode = ParseEntryMode(util.UnsafeBytesToString(bufBytes[:idx]))
+	name = string(bufBytes[idx+1 : len(bufBytes)-1]) // trim the NUL terminator, it needs a copy because the bufBytes will be reused by the reader
+	if mode == EntryModeNoEntry {
+		return mode, name, objID, len(bufBytes), errors.New("invalid entry mode: " + string(bufBytes[:idx]))
+	}
+
+	switch objectFormat {
+	case Sha1ObjectFormat:
+		objID = &Sha1Hash{}
+	case Sha256ObjectFormat:
+		objID = &Sha256Hash{}
+	default:
+		panic("unsupported object format: " + objectFormat.Name())
+	}
+	readIDLen, err := io.ReadFull(rd, objID.RawValue())
+	return mode, name, objID, len(bufBytes) + readIDLen, err
 }
 
 func DiscardFull(rd BufferedReader, discard int64) error {

modules/git/parse_treeentry.go

@@ -5,10 +5,7 @@ package git
 
 import (
 	"bytes"
-	"fmt"
 	"io"
-
-	"code.gitea.io/gitea/modules/log"
 )
 
 // ParseTreeEntries parses the output of a `git ls-tree -l` command.
@@ -47,14 +44,11 @@ func parseTreeEntries(data []byte, ptree *Tree) ([]*TreeEntry, error) {
 }
 
 func catBatchParseTreeEntries(objectFormat ObjectFormat, ptree *Tree, rd BufferedReader, sz int64) ([]*TreeEntry, error) {
-	fnameBuf := make([]byte, 4096)
-	modeBuf := make([]byte, 40)
-	shaBuf := make([]byte, objectFormat.FullLength())
 	entries := make([]*TreeEntry, 0, 10)
 
 loop:
 	for sz > 0 {
-		mode, fname, sha, count, err := ParseCatFileTreeLine(objectFormat, rd, modeBuf, fnameBuf, shaBuf)
+		mode, fname, objID, count, err := ParseCatFileTreeLine(objectFormat, rd)
 		if err != nil {
 			if err == io.EOF {
 				break loop
@@ -64,25 +58,9 @@ loop:
 		sz -= int64(count)
 		entry := new(TreeEntry)
 		entry.ptree = ptree
-
+		entry.entryMode = mode
-		switch string(mode) {
+		entry.ID = objID
-		case "100644":
+		entry.name = fname
-			entry.entryMode = EntryModeBlob
-		case "100755":
-			entry.entryMode = EntryModeExec
-		case "120000":
-			entry.entryMode = EntryModeSymlink
-		case "160000":
-			entry.entryMode = EntryModeCommit
-		case "40000", "40755": // git uses 40000 for tree object, but some users may get 40755 for unknown reasons
-			entry.entryMode = EntryModeTree
-		default:
-			log.Debug("Unknown mode: %v", string(mode))
-			return nil, fmt.Errorf("unknown mode: %v", string(mode))
-		}
-
-		entry.ID = objectFormat.MustID(sha)
-		entry.name = string(fname)
 		entries = append(entries, entry)
 	}
 	if _, err := rd.Discard(1); err != nil {

modules/git/parse_treeentry_test.go

@@ -4,6 +4,9 @@
 package git
 
 import (
+	"bufio"
+	"io"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -100,3 +103,31 @@ func TestParseTreeEntriesInvalid(t *testing.T) {
 	assert.Error(t, err)
 	assert.Empty(t, entries)
 }
+
+func TestParseCatFileTreeLine(t *testing.T) {
+	input := "100644 looooooooooooooooooooooooong-file-name.txt\x0012345678901234567890"
+	input += "40755 some-directory\x00abcdefg123abcdefg123"
+
+	var readCount int
+
+	buf := bufio.NewReaderSize(strings.NewReader(input), 20) // NewReaderSize has a limit: min buffer size = 16
+	mode, name, objID, n, err := ParseCatFileTreeLine(Sha1ObjectFormat, buf)
+	readCount += n
+	assert.NoError(t, err)
+	assert.Equal(t, EntryModeBlob, mode)
+	assert.Equal(t, "looooooooooooooooooooooooong-file-name.txt", name)
+	assert.Equal(t, "12345678901234567890", string(objID.RawValue()))
+
+	mode, name, objID, n, err = ParseCatFileTreeLine(Sha1ObjectFormat, buf)
+	readCount += n
+	assert.NoError(t, err)
+	assert.Equal(t, EntryModeTree, mode)
+	assert.Equal(t, "some-directory", name)
+	assert.Equal(t, "abcdefg123abcdefg123", string(objID.RawValue()))
+
+	assert.Equal(t, len(input), readCount)
+
+	_, _, _, n, err = ParseCatFileTreeLine(Sha1ObjectFormat, buf)
+	assert.ErrorIs(t, err, io.EOF)
+	assert.Zero(t, n)
+}

modules/git/pipeline/lfs_nogogit.go

@@ -8,7 +8,6 @@ package pipeline
 import (
 	"bufio"
 	"bytes"
-	"encoding/hex"
 	"io"
 	"sort"
 	"strings"
@@ -46,10 +45,6 @@ func findLFSFileFunc(repo *git.Repository, objectID git.ObjectID, revListReader
 	trees := []string{}
 	paths := []string{}
 
-	fnameBuf := make([]byte, 4096)
-	modeBuf := make([]byte, 40)
-	workingShaBuf := make([]byte, objectID.Type().FullLength()/2)
-
 	for scan.Scan() {
 		// Get the next commit ID
 		commitID := scan.Text()
@@ -93,23 +88,23 @@ func findLFSFileFunc(repo *git.Repository, objectID git.ObjectID, revListReader
 			case "tree":
 				var n int64
 				for n < info.Size {
-					mode, fname, binObjectID, count, err := git.ParseCatFileTreeLine(objectID.Type(), batchReader, modeBuf, fnameBuf, workingShaBuf)
+					mode, fname, shaID, count, err := git.ParseCatFileTreeLine(objectID.Type(), batchReader)
 					if err != nil {
 						return nil, err
 					}
 					n += int64(count)
-					if bytes.Equal(binObjectID, objectID.RawValue()) {
+					if bytes.Equal(shaID.RawValue(), objectID.RawValue()) {
 						result := LFSResult{
-							Name:         curPath + string(fname),
+							Name:         curPath + fname,
 							SHA:          curCommit.ID.String(),
 							Summary:      strings.Split(strings.TrimSpace(curCommit.CommitMessage), "\n")[0],
 							When:         curCommit.Author.When,
 							ParentHashes: curCommit.Parents,
 						}
-						resultsMap[curCommit.ID.String()+":"+curPath+string(fname)] = &result
+						resultsMap[curCommit.ID.String()+":"+curPath+fname] = &result
-					} else if string(mode) == git.EntryModeTree.String() {
+					} else if mode == git.EntryModeTree {
-						trees = append(trees, hex.EncodeToString(binObjectID))
+						trees = append(trees, shaID.String())
-						paths = append(paths, curPath+string(fname)+"/")
+						paths = append(paths, curPath+fname+"/")
 					}
 				}
 				if _, err := batchReader.Discard(1); err != nil {

modules/git/tree_entry_mode.go

@@ -66,9 +66,10 @@ func ParseEntryMode(mode string) EntryMode {
 		return EntryModeSymlink
 	case "160000":
 		return EntryModeCommit
-	case "040000":
+	case "040000", "40000": // leading-zero is optional
 		return EntryModeTree
 	default:
+		// if the faster path didn't work, try parsing the mode as an integer and masking off the file type bits
 		// git uses 040000 for tree object, but some users may get 040755 from non-standard git implementations
 		m, _ := strconv.ParseInt(mode, 8, 32)
 		modeInt := EntryMode(m)

modules/git/tree_entry_test.go

@@ -46,7 +46,9 @@ func TestParseEntryMode(t *testing.T) {
 		{"160755", EntryModeCommit},
 
 		{"040000", EntryModeTree},
+		{"40000", EntryModeTree},
 		{"040755", EntryModeTree},
+		{"40755", EntryModeTree},
 
 		{"777777", EntryModeNoEntry}, // invalid mode
 	}