From c72beb2dcfd7060d028ef8464ef9dc46f36c37e1 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Thu, 22 Apr 2021 20:09:17 +0100
Subject: [PATCH 1/7] Defer closing the gitrepo until the end of the wrapped
 context functions

There was a mistake in #15372 where deferral of gitrepo close occurs before it should.

This PR fixes this.

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/context/repo.go | 19 +++++++++++--------
 modules/web/route.go    | 30 ++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/modules/context/repo.go b/modules/context/repo.go
index 5ce31e9e3504b..8fc948b50995d 100644
--- a/modules/context/repo.go
+++ b/modules/context/repo.go
@@ -6,6 +6,7 @@
 package context
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"net/url"
@@ -393,7 +394,7 @@ func RepoIDAssignment() func(ctx *Context) {
 }
 
 // RepoAssignment returns a middleware to handle repository assignment
-func RepoAssignment(ctx *Context) {
+func RepoAssignment(ctx *Context) (cancel context.CancelFunc) {
 	var (
 		owner *models.User
 		err   error
@@ -529,12 +530,12 @@ func RepoAssignment(ctx *Context) {
 	ctx.Repo.GitRepo = gitRepo
 
 	// We opened it, we should close it
-	defer func() {
+	cancel = func() {
 		// If it's been set to nil then assume someone else has closed it.
 		if ctx.Repo.GitRepo != nil {
 			ctx.Repo.GitRepo.Close()
 		}
-	}()
+	}
 
 	// Stop at this point when the repo is empty.
 	if ctx.Repo.Repository.IsEmpty {
@@ -619,6 +620,7 @@ func RepoAssignment(ctx *Context) {
 		ctx.Data["GoDocDirectory"] = prefix + "{/dir}"
 		ctx.Data["GoDocFile"] = prefix + "{/dir}/{file}#L{line}"
 	}
+	return
 }
 
 // RepoRefType type of repo reference
@@ -643,7 +645,7 @@ const (
 
 // RepoRef handles repository reference names when the ref name is not
 // explicitly given
-func RepoRef() func(*Context) {
+func RepoRef() func(*Context) context.CancelFunc {
 	// since no ref name is explicitly specified, ok to just use branch
 	return RepoRefByType(RepoRefBranch)
 }
@@ -722,8 +724,8 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
 
 // RepoRefByType handles repository reference name for a specific type
 // of repository reference
-func RepoRefByType(refType RepoRefType) func(*Context) {
-	return func(ctx *Context) {
+func RepoRefByType(refType RepoRefType) func(*Context) context.CancelFunc {
+	return func(ctx *Context) (cancel context.CancelFunc) {
 		// Empty repository does not have reference information.
 		if ctx.Repo.Repository.IsEmpty {
 			return
@@ -742,12 +744,12 @@ func RepoRefByType(refType RepoRefType) func(*Context) {
 				return
 			}
 			// We opened it, we should close it
-			defer func() {
+			cancel = func() {
 				// If it's been set to nil then assume someone else has closed it.
 				if ctx.Repo.GitRepo != nil {
 					ctx.Repo.GitRepo.Close()
 				}
-			}()
+			}
 		}
 
 		// Get default branch.
@@ -841,6 +843,7 @@ func RepoRefByType(refType RepoRefType) func(*Context) {
 			return
 		}
 		ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount
+		return
 	}
 }
 
diff --git a/modules/web/route.go b/modules/web/route.go
index 6f9e76bdf3890..319d08f5981c1 100644
--- a/modules/web/route.go
+++ b/modules/web/route.go
@@ -5,6 +5,7 @@
 package web
 
 import (
+	goctx "context"
 	"fmt"
 	"net/http"
 	"reflect"
@@ -27,6 +28,7 @@ func Wrap(handlers ...interface{}) http.HandlerFunc {
 		switch t := handler.(type) {
 		case http.HandlerFunc, func(http.ResponseWriter, *http.Request),
 			func(ctx *context.Context),
+			func(ctx *context.Context) goctx.CancelFunc,
 			func(*context.APIContext),
 			func(*context.PrivateContext),
 			func(http.Handler) http.Handler:
@@ -48,6 +50,15 @@ func Wrap(handlers ...interface{}) http.HandlerFunc {
 				if r, ok := resp.(context.ResponseWriter); ok && r.Status() > 0 {
 					return
 				}
+			case func(ctx *context.Context) goctx.CancelFunc:
+				ctx := context.GetContext(req)
+				cancel := t(ctx)
+				if cancel != nil {
+					defer cancel()
+				}
+				if ctx.Written() {
+					return
+				}
 			case func(ctx *context.Context):
 				ctx := context.GetContext(req)
 				t(ctx)
@@ -94,6 +105,23 @@ func Middle(f func(ctx *context.Context)) func(netx http.Handler) http.Handler {
 	}
 }
 
+// MiddleCancel wrap a context function as a chi middleware
+func MiddleCancel(f func(ctx *context.Context) goctx.CancelFunc) func(netx http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+			ctx := context.GetContext(req)
+			cancel := f(ctx)
+			if cancel != nil {
+				defer cancel()
+			}
+			if ctx.Written() {
+				return
+			}
+			next.ServeHTTP(ctx.Resp, ctx.Req)
+		})
+	}
+}
+
 // MiddleAPI wrap a context function as a chi middleware
 func MiddleAPI(f func(ctx *context.APIContext)) func(netx http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
@@ -163,6 +191,8 @@ func (r *Route) Use(middlewares ...interface{}) {
 				r.R.Use(t)
 			case func(*context.Context):
 				r.R.Use(Middle(t))
+			case func(*context.Context) goctx.CancelFunc:
+				r.R.Use(MiddleCancel(t))
 			case func(*context.APIContext):
 				r.R.Use(MiddleAPI(t))
 			default:

From adfb35f289c26ad832f0e05b172757e644ffdb93 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Sat, 17 Apr 2021 13:55:32 +0100
Subject: [PATCH 2/7] Use common git cat-files

Use common git cat-file --batch and git cat-file --batch-check to
significantly reduce calls to git.

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/context/repo.go                    |   8 +-
 modules/git/batch_reader.go                |  54 ++++++++--
 modules/git/blob_nogogit.go                | 114 +++++++++++++++------
 modules/git/blob_test.go                   |   5 +-
 modules/git/commit_info_nogogit.go         |  12 ++-
 modules/git/notes_nogogit.go               |   9 +-
 modules/git/parse_nogogit.go               |  48 +++++++++
 modules/git/pipeline/lfs_nogogit.go        |   9 +-
 modules/git/repo_base_nogogit.go           |  49 ++++++++-
 modules/git/repo_blob_nogogit.go           |   4 +-
 modules/git/repo_blob_test.go              |   2 +-
 modules/git/repo_branch_nogogit.go         |  20 +++-
 modules/git/repo_commit.go                 |  10 +-
 modules/git/repo_commit_nogogit.go         |  62 +++++------
 modules/git/repo_language_stats_nogogit.go |   2 +-
 modules/git/repo_tag_nogogit.go            |   6 +-
 modules/git/repo_tree_nogogit.go           |  44 +++-----
 modules/git/tree_blob_nogogit.go           |   1 +
 modules/git/tree_entry.go                  |   9 +-
 modules/git/tree_entry_nogogit.go          |  10 +-
 modules/git/tree_nogogit.go                |  48 +++++++++
 modules/indexer/code/bleve.go              |   2 +-
 modules/indexer/code/elastic_search.go     |   2 +-
 routers/repo/download.go                   |  12 +++
 24 files changed, 414 insertions(+), 128 deletions(-)

diff --git a/modules/context/repo.go b/modules/context/repo.go
index 8fc948b50995d..2518d2cd9a17d 100644
--- a/modules/context/repo.go
+++ b/modules/context/repo.go
@@ -902,12 +902,18 @@ func (ctx *Context) IssueTemplatesFromDefaultBranch() []api.IssueTemplate {
 					log.Debug("DataAsync: %v", err)
 					continue
 				}
-				defer r.Close()
+				closed := false
+				defer func() {
+					if !closed {
+						_ = r.Close()
+					}
+				}()
 				data, err := ioutil.ReadAll(r)
 				if err != nil {
 					log.Debug("ReadAll: %v", err)
 					continue
 				}
+				_ = r.Close()
 				var it api.IssueTemplate
 				content, err := markdown.ExtractMetadata(string(data), &it)
 				if err != nil {
diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go
index 1905067cb4a33..841f38d92a674 100644
--- a/modules/git/batch_reader.go
+++ b/modules/git/batch_reader.go
@@ -13,9 +13,44 @@ import (
 	"strings"
 )
 
+// WriteCloserError wraps an io.WriteCloser with an additional CloseWithError function
+type WriteCloserError interface {
+	io.WriteCloser
+	CloseWithError(err error) error
+}
+
+// CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function
+func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
+	batchStdinReader, batchStdinWriter := io.Pipe()
+	batchStdoutReader, batchStdoutWriter := io.Pipe()
+	cancel := func() {
+		_ = batchStdinReader.Close()
+		_ = batchStdinWriter.Close()
+		_ = batchStdoutReader.Close()
+		_ = batchStdoutWriter.Close()
+	}
+
+	go func() {
+		stderr := strings.Builder{}
+		err := NewCommand("cat-file", "--batch-check").RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
+		if err != nil {
+			_ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
+			_ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String()))
+		} else {
+			_ = batchStdoutWriter.Close()
+			_ = batchStdinReader.Close()
+		}
+	}()
+
+	// For simplicities sake we'll us a buffered reader to read from the cat-file --batch
+	batchReader := bufio.NewReader(batchStdoutReader)
+
+	return batchStdinWriter, batchReader, cancel
+}
+
 // CatFileBatch opens git cat-file --batch in the provided repo and returns a stdin pipe, a stdout reader and cancel function
-func CatFileBatch(repoPath string) (*io.PipeWriter, *bufio.Reader, func()) {
-	// Next feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
+func CatFileBatch(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
+	// We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
 	// so let's create a batch stdin and stdout
 	batchStdinReader, batchStdinWriter := io.Pipe()
 	batchStdoutReader, batchStdoutWriter := io.Pipe()
@@ -54,19 +89,20 @@ func ReadBatchLine(rd *bufio.Reader) (sha []byte, typ string, size int64, err er
 	}
 	sha = sha[:len(sha)-1]
 
-	typ, err = rd.ReadString(' ')
+	typ, err = rd.ReadString('\n')
 	if err != nil {
 		return
 	}
-	typ = typ[:len(typ)-1]
 
-	var sizeStr string
-	sizeStr, err = rd.ReadString('\n')
-	if err != nil {
+	idx := strings.Index(typ, " ")
+	if idx < 0 {
+		err = ErrNotExist{ID: string(sha)}
 		return
 	}
+	sizeStr := typ[idx+1 : len(typ)-1]
+	typ = typ[:idx]
 
-	size, err = strconv.ParseInt(sizeStr[:len(sizeStr)-1], 10, 64)
+	size, err = strconv.ParseInt(sizeStr, 10, 64)
 	return
 }
 
@@ -128,7 +164,7 @@ headerLoop:
 	}
 
 	// Discard the rest of the commit
-	discard := size - n
+	discard := size - n + 1
 	for discard > math.MaxInt32 {
 		_, err := rd.Discard(math.MaxInt32)
 		if err != nil {
diff --git a/modules/git/blob_nogogit.go b/modules/git/blob_nogogit.go
index e917a316195d1..89f574d2b8cb1 100644
--- a/modules/git/blob_nogogit.go
+++ b/modules/git/blob_nogogit.go
@@ -8,48 +8,54 @@ package git
 
 import (
 	"bufio"
+	"bytes"
 	"io"
-	"strconv"
-	"strings"
+	"io/ioutil"
+	"math"
 )
 
 // Blob represents a Git object.
 type Blob struct {
 	ID SHA1
 
-	gotSize  bool
-	size     int64
-	repoPath string
-	name     string
+	gotSize bool
+	size    int64
+	name    string
+	repo    *Repository
 }
 
 // DataAsync gets a ReadCloser for the contents of a blob without reading it all.
 // Calling the Close function on the result will discard all unread output.
 func (b *Blob) DataAsync() (io.ReadCloser, error) {
-	stdoutReader, stdoutWriter := io.Pipe()
+	wr, rd, cancel := b.repo.CatFileBatch()
 
-	go func() {
-		stderr := &strings.Builder{}
-		err := NewCommand("cat-file", "--batch").RunInDirFullPipeline(b.repoPath, stdoutWriter, stderr, strings.NewReader(b.ID.String()+"\n"))
-		if err != nil {
-			err = ConcatenateError(err, stderr.String())
-			_ = stdoutWriter.CloseWithError(err)
-		} else {
-			_ = stdoutWriter.Close()
-		}
-	}()
-
-	bufReader := bufio.NewReader(stdoutReader)
-	_, _, size, err := ReadBatchLine(bufReader)
+	_, err := wr.Write([]byte(b.ID.String() + "\n"))
 	if err != nil {
-		stdoutReader.Close()
+		cancel()
 		return nil, err
 	}
+	_, _, size, err := ReadBatchLine(rd)
+	if err != nil {
+		cancel()
+		return nil, err
+	}
+	b.gotSize = true
+	b.size = size
 
-	return &LimitedReaderCloser{
-		R: bufReader,
-		C: stdoutReader,
-		N: size,
+	if size < 4096 {
+		bs, err := ioutil.ReadAll(io.LimitReader(rd, size))
+		if err != nil {
+			cancel()
+			return nil, err
+		}
+		_, err = rd.Discard(1)
+		return io.NopCloser(bytes.NewReader(bs)), err
+	}
+
+	return &blobReader{
+		rd:     rd,
+		n:      size,
+		cancel: cancel,
 	}, nil
 }
 
@@ -59,18 +65,66 @@ func (b *Blob) Size() int64 {
 		return b.size
 	}
 
-	size, err := NewCommand("cat-file", "-s", b.ID.String()).RunInDir(b.repoPath)
+	wr, rd, cancel := b.repo.CatFileBatchCheck()
+	defer cancel()
+	_, err := wr.Write([]byte(b.ID.String() + "\n"))
 	if err != nil {
-		log("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repoPath, err)
+		log("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err)
 		return 0
 	}
-
-	b.size, err = strconv.ParseInt(size[:len(size)-1], 10, 64)
+	_, _, b.size, err = ReadBatchLine(rd)
 	if err != nil {
-		log("error whilst parsing size %s for %s in %s. Error: %v", size, b.ID.String(), b.repoPath, err)
+		log("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err)
 		return 0
 	}
+
 	b.gotSize = true
 
 	return b.size
 }
+
+type blobReader struct {
+	rd     *bufio.Reader
+	n      int64
+	cancel func()
+}
+
+func (b *blobReader) Read(p []byte) (n int, err error) {
+	if b.n <= 0 {
+		return 0, io.EOF
+	}
+	if int64(len(p)) > b.n {
+		p = p[0:b.n]
+	}
+	n, err = b.rd.Read(p)
+	b.n -= int64(n)
+	return
+}
+
+// Close implements io.Closer
+func (b *blobReader) Close() error {
+	if b.n > 0 {
+		for b.n > math.MaxInt32 {
+			n, err := b.rd.Discard(math.MaxInt32)
+			b.n -= int64(n)
+			if err != nil {
+				b.cancel()
+				return err
+			}
+			b.n -= math.MaxInt32
+		}
+		n, err := b.rd.Discard(int(b.n))
+		b.n -= int64(n)
+		if err != nil {
+			b.cancel()
+			return err
+		}
+	}
+	if b.n == 0 {
+		_, err := b.rd.Discard(1)
+		b.n--
+		b.cancel()
+		return err
+	}
+	return nil
+}
diff --git a/modules/git/blob_test.go b/modules/git/blob_test.go
index d02251ed90244..2ceda6c4ef17d 100644
--- a/modules/git/blob_test.go
+++ b/modules/git/blob_test.go
@@ -29,9 +29,10 @@ func TestBlob_Data(t *testing.T) {
 	r, err := testBlob.DataAsync()
 	assert.NoError(t, err)
 	require.NotNil(t, r)
-	defer r.Close()
 
 	data, err := ioutil.ReadAll(r)
+	assert.NoError(t, r.Close())
+
 	assert.NoError(t, err)
 	assert.Equal(t, output, string(data))
 }
@@ -54,7 +55,7 @@ func Benchmark_Blob_Data(b *testing.B) {
 		if err != nil {
 			b.Fatal(err)
 		}
-		defer r.Close()
 		ioutil.ReadAll(r)
+		_ = r.Close()
 	}
 }
diff --git a/modules/git/commit_info_nogogit.go b/modules/git/commit_info_nogogit.go
index f14b7b82910fc..2d50a1dcdd7fa 100644
--- a/modules/git/commit_info_nogogit.go
+++ b/modules/git/commit_info_nogogit.go
@@ -141,7 +141,7 @@ func GetLastCommitForPaths(commit *Commit, treePath string, paths []string) ([]*
 		}
 	}()
 
-	batchStdinWriter, batchReader, cancel := CatFileBatch(commit.repo.Path)
+	batchStdinWriter, batchReader, cancel := commit.repo.CatFileBatch()
 	defer cancel()
 
 	mapsize := 4096
@@ -234,6 +234,10 @@ revListLoop:
 					// FIXME: is there any order to the way strings are emitted from cat-file?
 					// if there is - then we could skip once we've passed all of our data
 				}
+				if _, err := batchReader.Discard(1); err != nil {
+					return nil, err
+				}
+
 				break treeReadingLoop
 			}
 
@@ -278,6 +282,9 @@ revListLoop:
 					return nil, err
 				}
 			}
+			if _, err := batchReader.Discard(1); err != nil {
+				return nil, err
+			}
 
 			// if we haven't found a treeID for the target directory our search is over
 			if len(treeID) == 0 {
@@ -342,6 +349,9 @@ revListLoop:
 		if err != nil {
 			return nil, err
 		}
+		if _, err := batchReader.Discard(1); err != nil {
+			return nil, err
+		}
 		commitCommits[i] = c
 	}
 
diff --git a/modules/git/notes_nogogit.go b/modules/git/notes_nogogit.go
index 1379e50853656..d5d194b23f1b0 100644
--- a/modules/git/notes_nogogit.go
+++ b/modules/git/notes_nogogit.go
@@ -43,11 +43,18 @@ func GetNote(repo *Repository, commitID string, note *Note) error {
 	if err != nil {
 		return err
 	}
-	defer dataRc.Close()
+	closed := false
+	defer func() {
+		if !closed {
+			_ = dataRc.Close()
+		}
+	}()
 	d, err := ioutil.ReadAll(dataRc)
 	if err != nil {
 		return err
 	}
+	_ = dataRc.Close()
+	closed = true
 	note.Message = d
 
 	treePath := ""
diff --git a/modules/git/parse_nogogit.go b/modules/git/parse_nogogit.go
index e9e93f66fdc11..b45b31f239455 100644
--- a/modules/git/parse_nogogit.go
+++ b/modules/git/parse_nogogit.go
@@ -7,8 +7,10 @@
 package git
 
 import (
+	"bufio"
 	"bytes"
 	"fmt"
+	"io"
 	"strconv"
 	"strings"
 )
@@ -86,3 +88,49 @@ func parseTreeEntries(data []byte, ptree *Tree) ([]*TreeEntry, error) {
 	}
 	return entries, nil
 }
+
+func catBatchParseTreeEntries(ptree *Tree, rd *bufio.Reader, sz int64) ([]*TreeEntry, error) {
+	fnameBuf := make([]byte, 4096)
+	modeBuf := make([]byte, 40)
+	shaBuf := make([]byte, 40)
+	entries := make([]*TreeEntry, 0, 10)
+
+loop:
+	for sz > 0 {
+		mode, fname, sha, count, err := ParseTreeLine(rd, modeBuf, fnameBuf, shaBuf)
+		if err != nil {
+			if err == io.EOF {
+				break loop
+			}
+			return nil, err
+		}
+		sz -= int64(count)
+		entry := new(TreeEntry)
+		entry.ptree = ptree
+
+		switch string(mode) {
+		case "100644":
+			entry.entryMode = EntryModeBlob
+		case "100755":
+			entry.entryMode = EntryModeExec
+		case "120000":
+			entry.entryMode = EntryModeSymlink
+		case "160000":
+			entry.entryMode = EntryModeCommit
+		case "40000":
+			entry.entryMode = EntryModeTree
+		default:
+			log("Unknown mode: %v", string(mode))
+			return nil, fmt.Errorf("unknown mode: %v", string(mode))
+		}
+
+		entry.ID = MustID(sha)
+		entry.name = string(fname)
+		entries = append(entries, entry)
+	}
+	if _, err := rd.Discard(1); err != nil {
+		return entries, err
+	}
+
+	return entries, nil
+}
diff --git a/modules/git/pipeline/lfs_nogogit.go b/modules/git/pipeline/lfs_nogogit.go
index 79f7528d3335e..6113bb301df1a 100644
--- a/modules/git/pipeline/lfs_nogogit.go
+++ b/modules/git/pipeline/lfs_nogogit.go
@@ -43,8 +43,6 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 
 	basePath := repo.Path
 
-	hashStr := hash.String()
-
 	// Use rev-list to provide us with all commits in order
 	revListReader, revListWriter := io.Pipe()
 	defer func() {
@@ -64,7 +62,7 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 
 	// Next feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
 	// so let's create a batch stdin and stdout
-	batchStdinWriter, batchReader, cancel := git.CatFileBatch(repo.Path)
+	batchStdinWriter, batchReader, cancel := repo.CatFileBatch()
 	defer cancel()
 
 	// We'll use a scanner for the revList because it's simpler than a bufio.Reader
@@ -132,8 +130,7 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 						return nil, err
 					}
 					n += int64(count)
-					sha := git.To40ByteSHA(sha20byte)
-					if bytes.Equal(sha, []byte(hashStr)) {
+					if bytes.Equal(sha20byte, hash[:]) {
 						result := LFSResult{
 							Name:         curPath + string(fname),
 							SHA:          curCommit.ID.String(),
@@ -143,7 +140,7 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 						}
 						resultsMap[curCommit.ID.String()+":"+curPath+string(fname)] = &result
 					} else if string(mode) == git.EntryModeTree.String() {
-						trees = append(trees, sha)
+						trees = append(trees, git.To40ByteSHA(sha20byte))
 						paths = append(paths, curPath+string(fname)+"/")
 					}
 				}
diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go
index e05219a4e705e..53826319bf9ae 100644
--- a/modules/git/repo_base_nogogit.go
+++ b/modules/git/repo_base_nogogit.go
@@ -8,6 +8,8 @@
 package git
 
 import (
+	"bufio"
+	"context"
 	"errors"
 	"path/filepath"
 )
@@ -19,6 +21,14 @@ type Repository struct {
 	tagCache *ObjectCache
 
 	gpgSettings *GPGSettings
+
+	batchCancel context.CancelFunc
+	batchReader *bufio.Reader
+	batchWriter WriteCloserError
+
+	checkCancel context.CancelFunc
+	checkReader *bufio.Reader
+	checkWriter WriteCloserError
 }
 
 // OpenRepository opens the repository at the given path.
@@ -29,12 +39,47 @@ func OpenRepository(repoPath string) (*Repository, error) {
 	} else if !isDir(repoPath) {
 		return nil, errors.New("no such file or directory")
 	}
-	return &Repository{
+
+	repo := &Repository{
 		Path:     repoPath,
 		tagCache: newObjectCache(),
-	}, nil
+	}
+
+	repo.batchWriter, repo.batchReader, repo.batchCancel = CatFileBatch(repoPath)
+	repo.checkWriter, repo.checkReader, repo.checkCancel = CatFileBatchCheck(repo.Path)
+
+	return repo, nil
+}
+
+// CatFileBatch obtains a CatFileBatch for this repository
+func (repo *Repository) CatFileBatch() (WriteCloserError, *bufio.Reader, func()) {
+	if repo.batchCancel == nil || repo.batchReader.Buffered() > 0 {
+		log("Opening temporary cat file batch")
+		return CatFileBatch(repo.Path)
+	}
+	return repo.batchWriter, repo.batchReader, func() {}
+}
+
+func (repo *Repository) CatFileBatchCheck() (WriteCloserError, *bufio.Reader, func()) {
+	if repo.checkCancel == nil || repo.checkReader.Buffered() > 0 {
+		log("Opening temporary cat file batch-check")
+		return CatFileBatchCheck(repo.Path)
+	}
+	return repo.checkWriter, repo.checkReader, func() {}
 }
 
 // Close this repository, in particular close the underlying gogitStorage if this is not nil
 func (repo *Repository) Close() {
+	if repo.batchCancel != nil {
+		repo.batchCancel()
+		repo.batchReader = nil
+		repo.batchWriter = nil
+		repo.batchCancel = nil
+	}
+	if repo.checkCancel != nil {
+		repo.checkCancel()
+		repo.checkCancel = nil
+		repo.checkReader = nil
+		repo.checkWriter = nil
+	}
 }
diff --git a/modules/git/repo_blob_nogogit.go b/modules/git/repo_blob_nogogit.go
index 9959420df4e98..afb08d29cb982 100644
--- a/modules/git/repo_blob_nogogit.go
+++ b/modules/git/repo_blob_nogogit.go
@@ -11,7 +11,7 @@ func (repo *Repository) getBlob(id SHA1) (*Blob, error) {
 		return nil, ErrNotExist{id.String(), ""}
 	}
 	return &Blob{
-		ID:       id,
-		repoPath: repo.Path,
+		ID:   id,
+		repo: repo,
 	}, nil
 }
diff --git a/modules/git/repo_blob_test.go b/modules/git/repo_blob_test.go
index 52a124db2a750..ccf418b3059f8 100644
--- a/modules/git/repo_blob_test.go
+++ b/modules/git/repo_blob_test.go
@@ -33,9 +33,9 @@ func TestRepository_GetBlob_Found(t *testing.T) {
 
 		dataReader, err := blob.DataAsync()
 		assert.NoError(t, err)
-		defer dataReader.Close()
 
 		data, err := ioutil.ReadAll(dataReader)
+		assert.NoError(t, dataReader.Close())
 		assert.NoError(t, err)
 		assert.Equal(t, testCase.Data, data)
 	}
diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go
index 0628a572859c1..13ddcf06cf65e 100644
--- a/modules/git/repo_branch_nogogit.go
+++ b/modules/git/repo_branch_nogogit.go
@@ -13,12 +13,30 @@ import (
 	"strings"
 )
 
+// IsReferenceExist returns true if given reference exists in the repository.
+func (repo *Repository) IsReferenceExist(name string) bool {
+	if name == "" {
+		return false
+	}
+
+	wr, rd, cancel := repo.CatFileBatchCheck()
+	defer cancel()
+	_, err := wr.Write([]byte(name + "\n"))
+	if err != nil {
+		log("Error writing to CatFileBatchCheck %v", err)
+		return false
+	}
+	_, _, _, err = ReadBatchLine(rd)
+	return err == nil
+}
+
 // IsBranchExist returns true if given branch exists in current repository.
 func (repo *Repository) IsBranchExist(name string) bool {
 	if name == "" {
 		return false
 	}
-	return IsReferenceExist(repo.Path, BranchPrefix+name)
+
+	return repo.IsReferenceExist(BranchPrefix + name)
 }
 
 // GetBranches returns branches from the repository, skipping skip initial branches and
diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index 5e2db34fd18e3..6eabfc0730a93 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -33,16 +33,18 @@ func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
 		}
 	}
 
-	actualCommitID, err := NewCommand("rev-parse", "--verify", commitID).RunInDir(repo.Path)
+	wr, rd, cancel := repo.CatFileBatchCheck()
+	defer cancel()
+	wr.Write([]byte(commitID + "\n"))
+	sha, _, _, err := ReadBatchLine(rd)
 	if err != nil {
-		if strings.Contains(err.Error(), "unknown revision or path") ||
-			strings.Contains(err.Error(), "fatal: Needed a single revision") {
+		if IsErrNotExist(err) {
 			return SHA1{}, ErrNotExist{commitID, ""}
 		}
 		return SHA1{}, err
 	}
 
-	return NewIDFromString(actualCommitID)
+	return MustID(sha), nil
 }
 
 // GetCommit returns commit object of by ID string.
diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go
index 0a92de17037ef..f6d3e6ff9cded 100644
--- a/modules/git/repo_commit_nogogit.go
+++ b/modules/git/repo_commit_nogogit.go
@@ -7,9 +7,7 @@
 package git
 
 import (
-	"bufio"
 	"errors"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"strings"
@@ -34,15 +32,15 @@ func (repo *Repository) ResolveReference(name string) (string, error) {
 
 // GetRefCommitID returns the last commit ID string of given reference (branch or tag).
 func (repo *Repository) GetRefCommitID(name string) (string, error) {
-	stdout, err := NewCommand("show-ref", "--verify", "--hash", name).RunInDir(repo.Path)
-	if err != nil {
-		if strings.Contains(err.Error(), "not a valid ref") {
-			return "", ErrNotExist{name, ""}
-		}
-		return "", err
+	wr, rd, cancel := repo.CatFileBatchCheck()
+	defer cancel()
+	_, _ = wr.Write([]byte(name + "\n"))
+	shaBs, _, _, err := ReadBatchLine(rd)
+	if IsErrNotExist(err) {
+		return "", ErrNotExist{name, ""}
 	}
 
-	return strings.TrimSpace(stdout), nil
+	return string(shaBs), nil
 }
 
 // IsCommitExist returns true if given commit exists in current repository.
@@ -52,26 +50,14 @@ func (repo *Repository) IsCommitExist(name string) bool {
 }
 
 func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
-	stdoutReader, stdoutWriter := io.Pipe()
-	defer func() {
-		_ = stdoutReader.Close()
-		_ = stdoutWriter.Close()
-	}()
-
-	go func() {
-		stderr := strings.Builder{}
-		err := NewCommand("cat-file", "--batch").RunInDirFullPipeline(repo.Path, stdoutWriter, &stderr, strings.NewReader(id.String()+"\n"))
-		if err != nil {
-			_ = stdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
-		} else {
-			_ = stdoutWriter.Close()
-		}
-	}()
+	wr, rd, cancel := repo.CatFileBatch()
+	defer cancel()
+
+	_, _ = wr.Write([]byte(id.String() + "\n"))
 
-	bufReader := bufio.NewReader(stdoutReader)
-	_, typ, size, err := ReadBatchLine(bufReader)
+	_, typ, size, err := ReadBatchLine(rd)
 	if err != nil {
-		if errors.Is(err, io.EOF) {
+		if errors.Is(err, io.EOF) || IsErrNotExist(err) {
 			return nil, ErrNotExist{ID: id.String()}
 		}
 		return nil, err
@@ -83,7 +69,11 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
 	case "tag":
 		// then we need to parse the tag
 		// and load the commit
-		data, err := ioutil.ReadAll(io.LimitReader(bufReader, size))
+		data, err := ioutil.ReadAll(io.LimitReader(rd, size))
+		if err != nil {
+			return nil, err
+		}
+		_, err = rd.Discard(1)
 		if err != nil {
 			return nil, err
 		}
@@ -104,10 +94,22 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
 
 		return commit, nil
 	case "commit":
-		return CommitFromReader(repo, id, io.LimitReader(bufReader, size))
+		commit, err := CommitFromReader(repo, id, io.LimitReader(rd, size))
+		if err != nil {
+			return nil, err
+		}
+		_, err = rd.Discard(1)
+		if err != nil {
+			return nil, err
+		}
+
+		return commit, nil
 	default:
-		_ = stdoutReader.CloseWithError(fmt.Errorf("unknown typ: %s", typ))
 		log("Unknown typ: %s", typ)
+		_, err = rd.Discard(int(size) + 1)
+		if err != nil {
+			return nil, err
+		}
 		return nil, ErrNotExist{
 			ID: id.String(),
 		}
diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go
index 3f197f8d74e9d..0130d0a300871 100644
--- a/modules/git/repo_language_stats_nogogit.go
+++ b/modules/git/repo_language_stats_nogogit.go
@@ -21,7 +21,7 @@ import (
 func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, error) {
 	// We will feed the commit IDs in order into cat-file --batch, followed by blobs as necessary.
 	// so let's create a batch stdin and stdout
-	batchStdinWriter, batchReader, cancel := CatFileBatch(repo.Path)
+	batchStdinWriter, batchReader, cancel := repo.CatFileBatch()
 	defer cancel()
 
 	writeID := func(id string) error {
diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go
index b3fa5d6dc4077..a9e122aeaa7d6 100644
--- a/modules/git/repo_tag_nogogit.go
+++ b/modules/git/repo_tag_nogogit.go
@@ -9,7 +9,11 @@ package git
 
 // IsTagExist returns true if given tag exists in the repository.
 func (repo *Repository) IsTagExist(name string) bool {
-	return IsReferenceExist(repo.Path, TagPrefix+name)
+	if name == "" {
+		return false
+	}
+
+	return repo.IsReferenceExist(TagPrefix + name)
 }
 
 // GetTags returns all tags of the repository.
diff --git a/modules/git/repo_tree_nogogit.go b/modules/git/repo_tree_nogogit.go
index 867c3fa5aae0f..967f8aea3f44d 100644
--- a/modules/git/repo_tree_nogogit.go
+++ b/modules/git/repo_tree_nogogit.go
@@ -7,33 +7,18 @@
 package git
 
 import (
-	"bufio"
-	"fmt"
 	"io"
 	"io/ioutil"
-	"strings"
 )
 
 func (repo *Repository) getTree(id SHA1) (*Tree, error) {
-	stdoutReader, stdoutWriter := io.Pipe()
-	defer func() {
-		_ = stdoutReader.Close()
-		_ = stdoutWriter.Close()
-	}()
+	wr, rd, cancel := repo.CatFileBatch()
+	defer cancel()
 
-	go func() {
-		stderr := &strings.Builder{}
-		err := NewCommand("cat-file", "--batch").RunInDirFullPipeline(repo.Path, stdoutWriter, stderr, strings.NewReader(id.String()+"\n"))
-		if err != nil {
-			_ = stdoutWriter.CloseWithError(ConcatenateError(err, stderr.String()))
-		} else {
-			_ = stdoutWriter.Close()
-		}
-	}()
+	_, _ = wr.Write([]byte(id.String() + "\n"))
 
-	bufReader := bufio.NewReader(stdoutReader)
 	// ignore the SHA
-	_, typ, size, err := ReadBatchLine(bufReader)
+	_, typ, size, err := ReadBatchLine(rd)
 	if err != nil {
 		return nil, err
 	}
@@ -41,7 +26,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
 	switch typ {
 	case "tag":
 		resolvedID := id
-		data, err := ioutil.ReadAll(io.LimitReader(bufReader, size))
+		data, err := ioutil.ReadAll(io.LimitReader(rd, size))
 		if err != nil {
 			return nil, err
 		}
@@ -54,24 +39,27 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
 			return nil, err
 		}
 		commit.Tree.ResolvedID = resolvedID
-		log("tag.commit.Tree: %s %v", commit.Tree.ID.String(), commit.Tree.repo)
 		return &commit.Tree, nil
 	case "commit":
-		commit, err := CommitFromReader(repo, id, io.LimitReader(bufReader, size))
+		commit, err := CommitFromReader(repo, id, io.LimitReader(rd, size))
 		if err != nil {
-			_ = stdoutReader.CloseWithError(err)
+			return nil, err
+		}
+		if _, err := rd.Discard(1); err != nil {
 			return nil, err
 		}
 		commit.Tree.ResolvedID = commit.ID
-		log("commit.Tree: %s %v", commit.Tree.ID.String(), commit.Tree.repo)
 		return &commit.Tree, nil
 	case "tree":
-		stdoutReader.Close()
 		tree := NewTree(repo, id)
 		tree.ResolvedID = id
+		tree.entries, err = catBatchParseTreeEntries(tree, rd, size)
+		if err != nil {
+			return nil, err
+		}
+		tree.entriesParsed = true
 		return tree, nil
 	default:
-		_ = stdoutReader.CloseWithError(fmt.Errorf("unknown typ: %s", typ))
 		return nil, ErrNotExist{
 			ID: id.String(),
 		}
@@ -81,12 +69,12 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
 // GetTree find the tree object in the repository.
 func (repo *Repository) GetTree(idStr string) (*Tree, error) {
 	if len(idStr) != 40 {
-		res, err := NewCommand("rev-parse", "--verify", idStr).RunInDir(repo.Path)
+		res, err := repo.GetRefCommitID(idStr)
 		if err != nil {
 			return nil, err
 		}
 		if len(res) > 0 {
-			idStr = res[:len(res)-1]
+			idStr = res
 		}
 	}
 	id, err := NewIDFromString(idStr)
diff --git a/modules/git/tree_blob_nogogit.go b/modules/git/tree_blob_nogogit.go
index 6da0ccfe8e56e..fdd8d79c8b632 100644
--- a/modules/git/tree_blob_nogogit.go
+++ b/modules/git/tree_blob_nogogit.go
@@ -15,6 +15,7 @@ import (
 func (t *Tree) GetTreeEntryByPath(relpath string) (*TreeEntry, error) {
 	if len(relpath) == 0 {
 		return &TreeEntry{
+			ptree:     t,
 			ID:        t.ID,
 			name:      "",
 			fullName:  "",
diff --git a/modules/git/tree_entry.go b/modules/git/tree_entry.go
index 498767a63eb0f..3644d00f36e2a 100644
--- a/modules/git/tree_entry.go
+++ b/modules/git/tree_entry.go
@@ -34,12 +34,19 @@ func (te *TreeEntry) FollowLink() (*TreeEntry, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer r.Close()
+	closed := false
+	defer func() {
+		if !closed {
+			_ = r.Close()
+		}
+	}()
 	buf := make([]byte, te.Size())
 	_, err = io.ReadFull(r, buf)
 	if err != nil {
 		return nil, err
 	}
+	_ = r.Close()
+	closed = true
 
 	lnk := string(buf)
 	t := te.ptree
diff --git a/modules/git/tree_entry_nogogit.go b/modules/git/tree_entry_nogogit.go
index fd60de36f528d..41356ceba2326 100644
--- a/modules/git/tree_entry_nogogit.go
+++ b/modules/git/tree_entry_nogogit.go
@@ -84,10 +84,10 @@ func (te *TreeEntry) IsExecutable() bool {
 // Blob returns the blob object the entry
 func (te *TreeEntry) Blob() *Blob {
 	return &Blob{
-		ID:       te.ID,
-		repoPath: te.ptree.repo.Path,
-		name:     te.Name(),
-		size:     te.size,
-		gotSize:  te.sized,
+		ID:      te.ID,
+		name:    te.Name(),
+		size:    te.size,
+		gotSize: te.sized,
+		repo:    te.ptree.repo,
 	}
 }
diff --git a/modules/git/tree_nogogit.go b/modules/git/tree_nogogit.go
index 3ebdf10631dbc..9661d8faea158 100644
--- a/modules/git/tree_nogogit.go
+++ b/modules/git/tree_nogogit.go
@@ -7,6 +7,8 @@
 package git
 
 import (
+	"io"
+	"math"
 	"strings"
 )
 
@@ -32,6 +34,52 @@ func (t *Tree) ListEntries() (Entries, error) {
 		return t.entries, nil
 	}
 
+	if t.repo != nil {
+		wr, rd, cancel := t.repo.CatFileBatch()
+		defer cancel()
+
+		_, _ = wr.Write([]byte(t.ID.String() + "\n"))
+		_, typ, sz, err := ReadBatchLine(rd)
+		if err != nil {
+			return nil, err
+		}
+		if typ == "commit" {
+			treeID, err := ReadTreeID(rd, sz)
+			if err != nil && err != io.EOF {
+				return nil, err
+			}
+			_, _ = wr.Write([]byte(treeID + "\n"))
+			_, typ, sz, err = ReadBatchLine(rd)
+			if err != nil {
+				return nil, err
+			}
+		}
+		if typ == "tree" {
+			t.entries, err = catBatchParseTreeEntries(t, rd, sz)
+			if err != nil {
+				return nil, err
+			}
+			t.entriesParsed = true
+			return t.entries, nil
+		}
+
+		// Not a tree just use ls-tree instead
+		for sz > math.MaxInt32 {
+			discarded, err := rd.Discard(math.MaxInt32)
+			sz -= int64(discarded)
+			if err != nil {
+				return nil, err
+			}
+		}
+		for sz > 0 {
+			discarded, err := rd.Discard(int(sz))
+			sz -= int64(discarded)
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+
 	stdout, err := NewCommand("ls-tree", "-l", t.ID.String()).RunInDirBytes(t.repo.Path)
 	if err != nil {
 		if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "fatal: not a tree object") {
diff --git a/modules/indexer/code/bleve.go b/modules/indexer/code/bleve.go
index 416adeea74f2c..1d6aa51bc2213 100644
--- a/modules/indexer/code/bleve.go
+++ b/modules/indexer/code/bleve.go
@@ -176,7 +176,7 @@ func NewBleveIndexer(indexDir string) (*BleveIndexer, bool, error) {
 	return indexer, created, err
 }
 
-func (b *BleveIndexer) addUpdate(batchWriter *io.PipeWriter, batchReader *bufio.Reader, commitSha string, update fileUpdate, repo *models.Repository, batch rupture.FlushingBatch) error {
+func (b *BleveIndexer) addUpdate(batchWriter git.WriteCloserError, batchReader *bufio.Reader, commitSha string, update fileUpdate, repo *models.Repository, batch rupture.FlushingBatch) error {
 	// Ignore vendored files in code search
 	if setting.Indexer.ExcludeVendored && analyze.IsVendor(update.Filename) {
 		return nil
diff --git a/modules/indexer/code/elastic_search.go b/modules/indexer/code/elastic_search.go
index ebb7910fdcb40..982b36e8df404 100644
--- a/modules/indexer/code/elastic_search.go
+++ b/modules/indexer/code/elastic_search.go
@@ -175,7 +175,7 @@ func (b *ElasticSearchIndexer) init() (bool, error) {
 	return exists, nil
 }
 
-func (b *ElasticSearchIndexer) addUpdate(batchWriter *io.PipeWriter, batchReader *bufio.Reader, sha string, update fileUpdate, repo *models.Repository) ([]elastic.BulkableRequest, error) {
+func (b *ElasticSearchIndexer) addUpdate(batchWriter git.WriteCloserError, batchReader *bufio.Reader, sha string, update fileUpdate, repo *models.Repository) ([]elastic.BulkableRequest, error) {
 	// Ignore vendored files in code search
 	if setting.Indexer.ExcludeVendored && analyze.IsVendor(update.Filename) {
 		return nil, nil
diff --git a/routers/repo/download.go b/routers/repo/download.go
index 1eedec8cb1775..dafa62d0d9b8d 100644
--- a/routers/repo/download.go
+++ b/routers/repo/download.go
@@ -100,7 +100,11 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob) error {
 	if err != nil {
 		return err
 	}
+	closed := false
 	defer func() {
+		if closed {
+			return
+		}
 		if err = dataRc.Close(); err != nil {
 			log.Error("ServeBlobOrLFS: Close: %v", err)
 		}
@@ -110,6 +114,10 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob) error {
 	if pointer.IsValid() {
 		meta, _ := ctx.Repo.Repository.GetLFSMetaObjectByOid(pointer.Oid)
 		if meta == nil {
+			if err = dataRc.Close(); err != nil {
+				log.Error("ServeBlobOrLFS: Close: %v", err)
+			}
+			closed = true
 			return ServeBlob(ctx, blob)
 		}
 		if httpcache.HandleGenericETagCache(ctx.Req, ctx.Resp, `"`+pointer.Oid+`"`) {
@@ -126,6 +134,10 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob) error {
 		}()
 		return ServeData(ctx, ctx.Repo.TreePath, meta.Size, lfsDataRc)
 	}
+	if err = dataRc.Close(); err != nil {
+		log.Error("ServeBlobOrLFS: Close: %v", err)
+	}
+	closed = true
 
 	return ServeBlob(ctx, blob)
 }

From 7775733da0571bf5c948e9ad1fca3ecd3f8dcc7f Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Fri, 30 Apr 2021 09:43:43 +0100
Subject: [PATCH 3/7] placate lint

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/git/repo_base_nogogit.go | 5 +++--
 modules/git/repo_commit.go       | 5 ++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go
index 53826319bf9ae..772ec34e906e5 100644
--- a/modules/git/repo_base_nogogit.go
+++ b/modules/git/repo_base_nogogit.go
@@ -54,15 +54,16 @@ func OpenRepository(repoPath string) (*Repository, error) {
 // CatFileBatch obtains a CatFileBatch for this repository
 func (repo *Repository) CatFileBatch() (WriteCloserError, *bufio.Reader, func()) {
 	if repo.batchCancel == nil || repo.batchReader.Buffered() > 0 {
-		log("Opening temporary cat file batch")
+		log("Opening temporary cat file batch for: %s", repo.Path)
 		return CatFileBatch(repo.Path)
 	}
 	return repo.batchWriter, repo.batchReader, func() {}
 }
 
+// CatFileBatchCheck obtains a CatFileBatchCheck for this repository
 func (repo *Repository) CatFileBatchCheck() (WriteCloserError, *bufio.Reader, func()) {
 	if repo.checkCancel == nil || repo.checkReader.Buffered() > 0 {
-		log("Opening temporary cat file batch-check")
+		log("Opening temporary cat file batch-check: %s", repo.Path)
 		return CatFileBatchCheck(repo.Path)
 	}
 	return repo.checkWriter, repo.checkReader, func() {}
diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index 6eabfc0730a93..b4f299ff27c20 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -35,7 +35,10 @@ func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
 
 	wr, rd, cancel := repo.CatFileBatchCheck()
 	defer cancel()
-	wr.Write([]byte(commitID + "\n"))
+	_, err := wr.Write([]byte(commitID + "\n"))
+	if err != nil {
+		return SHA1{}, err
+	}
 	sha, _, _, err := ReadBatchLine(rd)
 	if err != nil {
 		if IsErrNotExist(err) {

From 729b73563ac4febfa2c9fad8cb9f36765d23fa96 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Fri, 30 Apr 2021 09:55:39 +0100
Subject: [PATCH 4/7] ioutil.NopCloser not io.NopCloser

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/git/blob_nogogit.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/git/blob_nogogit.go b/modules/git/blob_nogogit.go
index 89f574d2b8cb1..cdaeb636a944f 100644
--- a/modules/git/blob_nogogit.go
+++ b/modules/git/blob_nogogit.go
@@ -49,7 +49,7 @@ func (b *Blob) DataAsync() (io.ReadCloser, error) {
 			return nil, err
 		}
 		_, err = rd.Discard(1)
-		return io.NopCloser(bytes.NewReader(bs)), err
+		return ioutil.NopCloser(bytes.NewReader(bs)), err
 	}
 
 	return &blobReader{

From 44e6741495dc93c677128129e9deacd82b301d58 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Fri, 30 Apr 2021 18:28:22 +0100
Subject: [PATCH 5/7] Fix go-git making

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/git/repo_commit.go         | 26 --------------------------
 modules/git/repo_commit_gogit.go   | 21 +++++++++++++++++++++
 modules/git/repo_commit_nogogit.go | 26 ++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index b4f299ff27c20..664a7445dd9dc 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -24,32 +24,6 @@ func (repo *Repository) GetTagCommitID(name string) (string, error) {
 	return repo.GetRefCommitID(TagPrefix + name)
 }
 
-// ConvertToSHA1 returns a Hash object from a potential ID string
-func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
-	if len(commitID) == 40 {
-		sha1, err := NewIDFromString(commitID)
-		if err == nil {
-			return sha1, nil
-		}
-	}
-
-	wr, rd, cancel := repo.CatFileBatchCheck()
-	defer cancel()
-	_, err := wr.Write([]byte(commitID + "\n"))
-	if err != nil {
-		return SHA1{}, err
-	}
-	sha, _, _, err := ReadBatchLine(rd)
-	if err != nil {
-		if IsErrNotExist(err) {
-			return SHA1{}, ErrNotExist{commitID, ""}
-		}
-		return SHA1{}, err
-	}
-
-	return MustID(sha), nil
-}
-
 // GetCommit returns commit object of by ID string.
 func (repo *Repository) GetCommit(commitID string) (*Commit, error) {
 	id, err := repo.ConvertToSHA1(commitID)
diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go
index 48b0cfe19d063..2f9b1c4206f42 100644
--- a/modules/git/repo_commit_gogit.go
+++ b/modules/git/repo_commit_gogit.go
@@ -30,6 +30,27 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
 	return ref.Hash().String(), nil
 }
 
+// ConvertToSHA1 returns a Hash object from a potential ID string
+func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
+	if len(commitID) == 40 {
+		sha1, err := NewIDFromString(commitID)
+		if err == nil {
+			return sha1, nil
+		}
+	}
+
+	actualCommitID, err := NewCommand("rev-parse", "--verify", commitID).RunInDir(repo.Path)
+	if err != nil {
+		if strings.Contains(err.Error(), "unknown revision or path") ||
+			strings.Contains(err.Error(), "fatal: Needed a single revision") {
+			return SHA1{}, ErrNotExist{commitID, ""}
+		}
+		return SHA1{}, err
+	}
+
+	return NewIDFromString(actualCommitID)
+}
+
 // IsCommitExist returns true if given commit exists in current repository.
 func (repo *Repository) IsCommitExist(name string) bool {
 	hash := plumbing.NewHash(name)
diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go
index f6d3e6ff9cded..7b9a6c5c31d26 100644
--- a/modules/git/repo_commit_nogogit.go
+++ b/modules/git/repo_commit_nogogit.go
@@ -115,3 +115,29 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
 		}
 	}
 }
+
+// ConvertToSHA1 returns a Hash object from a potential ID string
+func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
+	if len(commitID) == 40 {
+		sha1, err := NewIDFromString(commitID)
+		if err == nil {
+			return sha1, nil
+		}
+	}
+
+	wr, rd, cancel := repo.CatFileBatchCheck()
+	defer cancel()
+	_, err := wr.Write([]byte(commitID + "\n"))
+	if err != nil {
+		return SHA1{}, err
+	}
+	sha, _, _, err := ReadBatchLine(rd)
+	if err != nil {
+		if IsErrNotExist(err) {
+			return SHA1{}, ErrNotExist{commitID, ""}
+		}
+		return SHA1{}, err
+	}
+
+	return MustID(sha), nil
+}

From 7de0354128f36464758e0329518143c0db98c4d2 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Fri, 30 Apr 2021 19:55:58 +0100
Subject: [PATCH 6/7] agh 40byte not 20byte sha

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/git/batch_reader.go        | 1 +
 modules/git/repo_commit_nogogit.go | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go
index 841f38d92a674..3d3a6916f5355 100644
--- a/modules/git/batch_reader.go
+++ b/modules/git/batch_reader.go
@@ -82,6 +82,7 @@ func CatFileBatch(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
 // ReadBatchLine reads the header line from cat-file --batch
 // We expect:
 // <sha> SP <type> SP <size> LF
+// sha is a 40byte not 20byte here
 func ReadBatchLine(rd *bufio.Reader) (sha []byte, typ string, size int64, err error) {
 	sha, err = rd.ReadBytes(' ')
 	if err != nil {
diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go
index 7b9a6c5c31d26..1abddf31b5fc9 100644
--- a/modules/git/repo_commit_nogogit.go
+++ b/modules/git/repo_commit_nogogit.go
@@ -118,7 +118,7 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
 
 // ConvertToSHA1 returns a Hash object from a potential ID string
 func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
-	if len(commitID) == 40 {
+	if len(commitID) == 40 && SHAPattern.MatchString(commitID) {
 		sha1, err := NewIDFromString(commitID)
 		if err == nil {
 			return sha1, nil
@@ -139,5 +139,5 @@ func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
 		return SHA1{}, err
 	}
 
-	return MustID(sha), nil
+	return MustIDFromString(string(sha)), nil
 }

From 5317d34fd004f4e2607926a3aa5008448f945dd1 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Fri, 30 Apr 2021 20:15:47 +0100
Subject: [PATCH 7/7] protect against nil girepo close

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/git/repo_base_nogogit.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go
index 772ec34e906e5..c7d6019d77314 100644
--- a/modules/git/repo_base_nogogit.go
+++ b/modules/git/repo_base_nogogit.go
@@ -71,6 +71,9 @@ func (repo *Repository) CatFileBatchCheck() (WriteCloserError, *bufio.Reader, fu
 
 // Close this repository, in particular close the underlying gogitStorage if this is not nil
 func (repo *Repository) Close() {
+	if repo == nil {
+		return
+	}
 	if repo.batchCancel != nil {
 		repo.batchCancel()
 		repo.batchReader = nil