Do not read or write git reference files directly (#18079)

Git will and can pack references into packfiles and therefore if you write/read the
files directly you will get false results. Instead you should use update-ref and
show-ref. To that end I have created three new functions in git/repo_commit.go that
will do this correctly.

Related #17191

Signed-off-by: Andrew Thornton <art27@cantab.net>

4 files changed, 29 insertions, 41 deletions

diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go
index f00b340d15..39be183f30 100644
--- a/modules/git/repo_commit_gogit.go
+++ b/modules/git/repo_commit_gogit.go
@@ -31,6 +31,16 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
 	return ref.Hash().String(), nil
 }
 
+// SetReference sets the commit ID string of given reference (e.g. branch or tag).
+func (repo *Repository) SetReference(name, commitID string) error {
+	return repo.gogitRepo.Storer.SetReference(plumbing.NewReferenceFromStrings(name, commitID))
+}
+
+// RemoveReference removes the given reference (e.g. branch or tag).
+func (repo *Repository) RemoveReference(name string) error {
+	return repo.gogitRepo.Storer.RemoveReference(plumbing.ReferenceName(name))
+}
+
 // ConvertToSHA1 returns a Hash object from a potential ID string
 func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
 	if len(commitID) == 40 {
diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go
index c8cd7ec882..4c1670742c 100644
--- a/modules/git/repo_commit_nogogit.go
+++ b/modules/git/repo_commit_nogogit.go
@@ -49,6 +49,18 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
 	return string(shaBs), nil
 }
 
+// SetReference sets the commit ID string of given reference (e.g. branch or tag).
+func (repo *Repository) SetReference(name, commitID string) error {
+	_, err := NewCommandContext(repo.Ctx, "update-ref", name, commitID).RunInDir(repo.Path)
+	return err
+}
+
+// RemoveReference removes the given reference (e.g. branch or tag).
+func (repo *Repository) RemoveReference(name string) error {
+	_, err := NewCommandContext(repo.Ctx, "update-ref", "--no-deref", "-d", name).RunInDir(repo.Path)
+	return err
+}
+
 // IsCommitExist returns true if given commit exists in current repository.
 func (repo *Repository) IsCommitExist(name string) bool {
 	_, err := NewCommandContext(repo.Ctx, "cat-file", "-e", name).RunInDir(repo.Path)
diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go
index 3690b74532..fb0ba91265 100644
--- a/modules/git/repo_compare.go
+++ b/modules/git/repo_compare.go
@@ -11,7 +11,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -275,25 +274,6 @@ func (repo *Repository) GetDiffFromMergeBase(base, head string, w io.Writer) err
 	return err
 }
 
-// ReadPullHead will fetch a pull ref if possible or return an error
-func (repo *Repository) ReadPullHead(prID int64) (commitSHA string, err error) {
-	headPath := fmt.Sprintf("refs/pull/%d/head", prID)
-	fullHeadPath := filepath.Join(repo.Path, headPath)
-	loadHead, err := os.Open(fullHeadPath)
-	if err != nil {
-		return "", err
-	}
-	defer loadHead.Close()
-	// Read only the first line of the patch - usually it contains the first commit made in patch
-	scanner := bufio.NewScanner(loadHead)
-	scanner.Scan()
-	commitHead := scanner.Text()
-	if len(commitHead) != 40 {
-		return "", errors.New("head file doesn't contain valid commit ID")
-	}
-	return commitHead, nil
-}
-
 // ReadPatchCommit will check if a diff patch exists and return stats
 func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error) {
 	// Migrated repositories download patches to "pulls" location
@@ -315,16 +295,3 @@ func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error
 	}
 	return commitSHA, nil
 }
-
-// WritePullHead will populate a PR head retrieved from patch file
-func (repo *Repository) WritePullHead(prID int64, commitSHA string) error {
-	headPath := fmt.Sprintf("refs/pull/%d", prID)
-	fullHeadPath := filepath.Join(repo.Path, headPath)
-	// Create missing directory just in case
-	if err := os.MkdirAll(fullHeadPath, os.ModePerm); err != nil {
-		return err
-	}
-	commitBytes := []byte(commitSHA)
-	pullPath := filepath.Join(fullHeadPath, "head")
-	return ioutil.WriteFile(pullPath, commitBytes, os.ModePerm)
-}
diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go
index 4790170d10..301e085aae 100644
--- a/modules/git/repo_compare_test.go
+++ b/modules/git/repo_compare_test.go
@@ -8,7 +8,6 @@ import (
 	"bytes"
 	"io"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/modules/util"
@@ -63,18 +62,18 @@ func TestReadWritePullHead(t *testing.T) {
 	assert.NoError(t, err)
 	defer repo.Close()
 	// Try to open non-existing Pull
-	_, err = repo.ReadPullHead(0)
+	_, err = repo.GetRefCommitID(PullPrefix + "0/head")
 	assert.Error(t, err)
 	// Write a fake sha1 with only 40 zeros
-	newCommit := strings.Repeat("0", 40)
-	err = repo.WritePullHead(1, newCommit)
+	newCommit := "feaf4ba6bc635fec442f46ddd4512416ec43c2c2"
+	err = repo.SetReference(PullPrefix+"1/head", newCommit)
 	assert.NoError(t, err)
-	headFile := filepath.Join(repo.Path, "refs/pull/1/head")
 	// Remove file after the test
-	defer util.Remove(headFile)
-	assert.FileExists(t, headFile)
+	defer func() {
+		_ = repo.RemoveReference(PullPrefix + "1/head")
+	}()
 	// Read the file created
-	headContents, err := repo.ReadPullHead(1)
+	headContents, err := repo.GetRefCommitID(PullPrefix + "1/head")
 	assert.NoError(t, err)
 	assert.Len(t, string(headContents), 40)
 	assert.True(t, string(headContents) == newCommit)