Merge remote-tracking branch 'origin/main' into test

modules/git/blob.go

@@ -22,17 +22,22 @@ func (b *Blob) Name() string {
 	return b.name
 }
 
-// GetBlobContent Gets the limited content of the blob as raw text
-func (b *Blob) GetBlobContent(limit int64) (string, error) {
+// GetBlobBytes Gets the limited content of the blob
+func (b *Blob) GetBlobBytes(limit int64) ([]byte, error) {
 	if limit <= 0 {
-		return "", nil
+		return nil, nil
 	}
 	dataRc, err := b.DataAsync()
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 	defer dataRc.Close()
-	buf, err := util.ReadWithLimit(dataRc, int(limit))
+	return util.ReadWithLimit(dataRc, int(limit))
+}
+
+// GetBlobContent Gets the limited content of the blob as raw text
+func (b *Blob) GetBlobContent(limit int64) (string, error) {
+	buf, err := b.GetBlobBytes(limit)
 	return string(buf), err
 }
 
@@ -99,11 +104,9 @@ loop:
 
 // GuessContentType guesses the content type of the blob.
 func (b *Blob) GuessContentType() (typesniffer.SniffedType, error) {
-	r, err := b.DataAsync()
+	buf, err := b.GetBlobBytes(typesniffer.SniffContentSize)
 	if err != nil {
 		return typesniffer.SniffedType{}, err
 	}
-	defer r.Close()
-
-	return typesniffer.DetectContentTypeFromReader(r)
+	return typesniffer.DetectContentType(buf), nil
 }

modules/git/commit.go

@@ -20,10 +20,11 @@ import (
 
 // Commit represents a git commit.
 type Commit struct {
-	Tree
-	ID            ObjectID // The ID of this commit object
-	Author        *Signature
-	Committer     *Signature
+	Tree // FIXME: bad design, this field can be nil if the commit is from "last commit cache"
+
+	ID            ObjectID
+	Author        *Signature // never nil
+	Committer     *Signature // never nil
 	CommitMessage string
 	Signature     *CommitSignature
 

modules/git/commit_info.go

@@ -3,9 +3,20 @@
 
 package git
 
+import "path"
+
 // CommitInfo describes the first commit with the provided entry
 type CommitInfo struct {
 	Entry         *TreeEntry
 	Commit        *Commit
 	SubmoduleFile *CommitSubmoduleFile
 }
+
+func getCommitInfoSubmoduleFile(repoLink string, entry *TreeEntry, commit *Commit, treePathDir string) (*CommitSubmoduleFile, error) {
+	fullPath := path.Join(treePathDir, entry.Name())
+	submodule, err := commit.GetSubModule(fullPath)
+	if err != nil {
+		return nil, err
+	}
+	return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, entry.ID.String()), nil
+}

modules/git/commit_info_gogit.go

@@ -16,7 +16,7 @@ import (
 )
 
 // GetCommitsInfo gets information of all commits that are corresponding to these entries
-func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
+func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
 	entryPaths := make([]string, len(tes)+1)
 	// Get the commit for the treePath itself
 	entryPaths[0] = ""
@@ -71,22 +71,12 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 			commitsInfo[i].Commit = entryCommit
 		}
 
-		// If the entry is a submodule add a submodule file for this
+		// If the entry is a submodule, add a submodule file for this
 		if entry.IsSubModule() {
-			subModuleURL := ""
-			var fullPath string
-			if len(treePath) > 0 {
-				fullPath = treePath + "/" + entry.Name()
-			} else {
-				fullPath = entry.Name()
-			}
-			if subModule, err := commit.GetSubModule(fullPath); err != nil {
+			commitsInfo[i].SubmoduleFile, err = getCommitInfoSubmoduleFile(repoLink, entry, commit, treePath)
+			if err != nil {
 				return nil, nil, err
-			} else if subModule != nil {
-				subModuleURL = subModule.URL
 			}
-			subModuleFile := NewCommitSubmoduleFile(subModuleURL, entry.ID.String())
-			commitsInfo[i].SubmoduleFile = subModuleFile
 		}
 	}
 

modules/git/commit_info_nogogit.go

@@ -15,7 +15,7 @@ import (
 )
 
 // GetCommitsInfo gets information of all commits that are corresponding to these entries
-func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
+func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
 	entryPaths := make([]string, len(tes)+1)
 	// Get the commit for the treePath itself
 	entryPaths[0] = ""
@@ -62,22 +62,12 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 			log.Debug("missing commit for %s", entry.Name())
 		}
 
-		// If the entry is a submodule add a submodule file for this
+		// If the entry is a submodule, add a submodule file for this
 		if entry.IsSubModule() {
-			subModuleURL := ""
-			var fullPath string
-			if len(treePath) > 0 {
-				fullPath = treePath + "/" + entry.Name()
-			} else {
-				fullPath = entry.Name()
-			}
-			if subModule, err := commit.GetSubModule(fullPath); err != nil {
+			commitsInfo[i].SubmoduleFile, err = getCommitInfoSubmoduleFile(repoLink, entry, commit, treePath)
+			if err != nil {
 				return nil, nil, err
-			} else if subModule != nil {
-				subModuleURL = subModule.URL
 			}
-			subModuleFile := NewCommitSubmoduleFile(subModuleURL, entry.ID.String())
-			commitsInfo[i].SubmoduleFile = subModuleFile
 		}
 	}
 

modules/git/commit_info_test.go

@@ -82,7 +82,7 @@ func testGetCommitsInfo(t *testing.T, repo1 *Repository) {
 		}
 
 		// FIXME: Context.TODO() - if graceful has started we should use its Shutdown context otherwise use install signals in TestMain.
-		commitsInfo, treeCommit, err := entries.GetCommitsInfo(t.Context(), commit, testCase.Path)
+		commitsInfo, treeCommit, err := entries.GetCommitsInfo(t.Context(), "/any/repo-link", commit, testCase.Path)
 		assert.NoError(t, err, "Unable to get commit information for entries of subtree: %s in commit: %s from testcase due to error: %v", testCase.Path, testCase.CommitID, err)
 		if err != nil {
 			t.FailNow()
@@ -159,7 +159,7 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) {
 		b.ResetTimer()
 		b.Run(benchmark.name, func(b *testing.B) {
 			for b.Loop() {
-				_, _, err := entries.GetCommitsInfo(b.Context(), commit, "")
+				_, _, err := entries.GetCommitsInfo(b.Context(), "/any/repo-link", commit, "")
 				if err != nil {
 					b.Fatal(err)
 				}

modules/git/commit_submodule_file.go

@@ -6,49 +6,61 @@ package git
 
 import (
 	"context"
+	"path"
+	"strings"
 
 	giturl "code.gitea.io/gitea/modules/git/url"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // CommitSubmoduleFile represents a file with submodule type.
 type CommitSubmoduleFile struct {
-	refURL    string
-	parsedURL *giturl.RepositoryURL
-	parsed    bool
-	refID     string
-	repoLink  string
+	repoLink string
+	fullPath string
+	refURL   string
+	refID    string
+
+	parsed           bool
+	parsedTargetLink string
 }
 
 // NewCommitSubmoduleFile create a new submodule file
-func NewCommitSubmoduleFile(refURL, refID string) *CommitSubmoduleFile {
-	return &CommitSubmoduleFile{refURL: refURL, refID: refID}
+func NewCommitSubmoduleFile(repoLink, fullPath, refURL, refID string) *CommitSubmoduleFile {
+	return &CommitSubmoduleFile{repoLink: repoLink, fullPath: fullPath, refURL: refURL, refID: refID}
 }
 
 func (sf *CommitSubmoduleFile) RefID() string {
-	return sf.refID // this function is only used in templates
+	return sf.refID
 }
 
-// SubmoduleWebLink tries to make some web links for a submodule, it also works on "nil" receiver
-func (sf *CommitSubmoduleFile) SubmoduleWebLink(ctx context.Context, optCommitID ...string) *SubmoduleWebLink {
+func (sf *CommitSubmoduleFile) getWebLinkInTargetRepo(ctx context.Context, moreLinkPath string) *SubmoduleWebLink {
 	if sf == nil {
 		return nil
 	}
+	if strings.HasPrefix(sf.refURL, "../") {
+		targetLink := path.Join(sf.repoLink, path.Dir(sf.fullPath), sf.refURL)
+		return &SubmoduleWebLink{RepoWebLink: targetLink, CommitWebLink: targetLink + moreLinkPath}
+	}
 	if !sf.parsed {
 		sf.parsed = true
 		parsedURL, err := giturl.ParseRepositoryURL(ctx, sf.refURL)
 		if err != nil {
 			return nil
 		}
-		sf.parsedURL = parsedURL
-		sf.repoLink = giturl.MakeRepositoryWebLink(sf.parsedURL)
+		sf.parsedTargetLink = giturl.MakeRepositoryWebLink(parsedURL)
 	}
-	var commitLink string
-	if len(optCommitID) == 2 {
-		commitLink = sf.repoLink + "/compare/" + optCommitID[0] + "..." + optCommitID[1]
-	} else if len(optCommitID) == 1 {
-		commitLink = sf.repoLink + "/tree/" + optCommitID[0]
-	} else {
-		commitLink = sf.repoLink + "/tree/" + sf.refID
-	}
-	return &SubmoduleWebLink{RepoWebLink: sf.repoLink, CommitWebLink: commitLink}
+	return &SubmoduleWebLink{RepoWebLink: sf.parsedTargetLink, CommitWebLink: sf.parsedTargetLink + moreLinkPath}
+}
+
+// SubmoduleWebLinkTree tries to make the submodule's tree link in its own repo, it also works on "nil" receiver
+func (sf *CommitSubmoduleFile) SubmoduleWebLinkTree(ctx context.Context, optCommitID ...string) *SubmoduleWebLink {
+	if sf == nil {
+		return nil
+	}
+	return sf.getWebLinkInTargetRepo(ctx, "/tree/"+util.OptionalArg(optCommitID, sf.refID))
+}
+
+// SubmoduleWebLinkCompare tries to make the submodule's compare link in its own repo, it also works on "nil" receiver
+func (sf *CommitSubmoduleFile) SubmoduleWebLinkCompare(ctx context.Context, commitID1, commitID2 string) *SubmoduleWebLink {
+	return sf.getWebLinkInTargetRepo(ctx, "/compare/"+commitID1+"..."+commitID2)
 }

modules/git/commit_submodule_file_test.go

@@ -10,20 +10,29 @@ import (
 )
 
 func TestCommitSubmoduleLink(t *testing.T) {
-	sf := NewCommitSubmoduleFile("git@github.com:user/repo.git", "aaaa")
+	assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkTree(t.Context()))
+	assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkCompare(t.Context(), "", ""))
 
-	wl := sf.SubmoduleWebLink(t.Context())
-	assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
-	assert.Equal(t, "https://github.com/user/repo/tree/aaaa", wl.CommitWebLink)
+	t.Run("GitHubRepo", func(t *testing.T) {
+		sf := NewCommitSubmoduleFile("/any/repo-link", "full-path", "git@github.com:user/repo.git", "aaaa")
+		wl := sf.SubmoduleWebLinkTree(t.Context())
+		assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
+		assert.Equal(t, "https://github.com/user/repo/tree/aaaa", wl.CommitWebLink)
 
-	wl = sf.SubmoduleWebLink(t.Context(), "1111")
-	assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
-	assert.Equal(t, "https://github.com/user/repo/tree/1111", wl.CommitWebLink)
+		wl = sf.SubmoduleWebLinkCompare(t.Context(), "1111", "2222")
+		assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
+		assert.Equal(t, "https://github.com/user/repo/compare/1111...2222", wl.CommitWebLink)
+	})
 
-	wl = sf.SubmoduleWebLink(t.Context(), "1111", "2222")
-	assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
-	assert.Equal(t, "https://github.com/user/repo/compare/1111...2222", wl.CommitWebLink)
+	t.Run("RelativePath", func(t *testing.T) {
+		sf := NewCommitSubmoduleFile("/subpath/any/repo-home-link", "full-path", "../../user/repo", "aaaa")
+		wl := sf.SubmoduleWebLinkTree(t.Context())
+		assert.Equal(t, "/subpath/user/repo", wl.RepoWebLink)
+		assert.Equal(t, "/subpath/user/repo/tree/aaaa", wl.CommitWebLink)
 
-	wl = (*CommitSubmoduleFile)(nil).SubmoduleWebLink(t.Context())
-	assert.Nil(t, wl)
+		sf = NewCommitSubmoduleFile("/subpath/any/repo-home-link", "dir/submodule", "../../../user/repo", "aaaa")
+		wl = sf.SubmoduleWebLinkCompare(t.Context(), "1111", "2222")
+		assert.Equal(t, "/subpath/user/repo", wl.RepoWebLink)
+		assert.Equal(t, "/subpath/user/repo/compare/1111...2222", wl.CommitWebLink)
+	})
 }

modules/git/diff.go

@@ -99,9 +99,9 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
 	return nil
 }
 
-// ParseDiffHunkString parse the diffhunk content and return
-func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHunk int) {
-	ss := strings.Split(diffhunk, "@@")
+// ParseDiffHunkString parse the diff hunk content and return
+func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightHunk int) {
+	ss := strings.Split(diffHunk, "@@")
 	ranges := strings.Split(ss[1][1:], " ")
 	leftRange := strings.Split(ranges[0], ",")
 	leftLine, _ = strconv.Atoi(leftRange[0][1:])
@@ -112,14 +112,19 @@ func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHu
 		rightRange := strings.Split(ranges[1], ",")
 		rightLine, _ = strconv.Atoi(rightRange[0])
 		if len(rightRange) > 1 {
-			righHunk, _ = strconv.Atoi(rightRange[1])
+			rightHunk, _ = strconv.Atoi(rightRange[1])
 		}
 	} else {
-		log.Debug("Parse line number failed: %v", diffhunk)
+		log.Debug("Parse line number failed: %v", diffHunk)
 		rightLine = leftLine
-		righHunk = leftHunk
+		rightHunk = leftHunk
 	}
-	return leftLine, leftHunk, rightLine, righHunk
+	if rightLine == 0 {
+		// FIXME: GIT-DIFF-CUT-BUG search this tag to see details
+		// this is only a hacky patch, the rightLine&rightHunk might still be incorrect in some cases.
+		rightLine++
+	}
+	return leftLine, leftHunk, rightLine, rightHunk
 }
 
 // Example: @@ -1,8 +1,9 @@ => [..., 1, 8, 1, 9]
@@ -270,6 +275,12 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
 			oldNumOfLines++
 		}
 	}
+
+	// "git diff" outputs "@@ -1 +1,3 @@" for "OLD" => "A\nB\nC"
+	// FIXME: GIT-DIFF-CUT-BUG But there is a bug in CutDiffAroundLine, then the "Patch" stored in the comment model becomes "@@ -1,1 +0,4 @@"
+	// It may generate incorrect results for difference cases, for example: delete 2 line add 1 line, delete 2 line add 2 line etc, need to double check.
+	// For example: "L1\nL2" => "A\nB", then the patch shows "L2" as line 1 on the left (deleted part)
+
 	// construct the new hunk header
 	newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@",
 		oldBegin, oldNumOfLines, newBegin, newNumOfLines)

modules/git/error.go

@@ -32,22 +32,6 @@ func (err ErrNotExist) Unwrap() error {
 	return util.ErrNotExist
 }
 
-// ErrSymlinkUnresolved entry.FollowLink error
-type ErrSymlinkUnresolved struct {
-	Name    string
-	Message string
-}
-
-func (err ErrSymlinkUnresolved) Error() string {
-	return fmt.Sprintf("%s: %s", err.Name, err.Message)
-}
-
-// IsErrSymlinkUnresolved if some error is ErrSymlinkUnresolved
-func IsErrSymlinkUnresolved(err error) bool {
-	_, ok := err.(ErrSymlinkUnresolved)
-	return ok
-}
-
 // ErrBranchNotExist represents a "BranchNotExist" kind of error.
 type ErrBranchNotExist struct {
 	Name string

modules/git/tree_blob_nogogit.go

@@ -11,7 +11,7 @@ import (
 )
 
 // GetTreeEntryByPath get the tree entries according the sub dir
-func (t *Tree) GetTreeEntryByPath(relpath string) (*TreeEntry, error) {
+func (t *Tree) GetTreeEntryByPath(relpath string) (_ *TreeEntry, err error) {
 	if len(relpath) == 0 {
 		return &TreeEntry{
 			ptree:     t,
@@ -21,27 +21,25 @@ func (t *Tree) GetTreeEntryByPath(relpath string) (*TreeEntry, error) {
 		}, nil
 	}
 
-	// FIXME: This should probably use git cat-file --batch to be a bit more efficient
 	relpath = path.Clean(relpath)
 	parts := strings.Split(relpath, "/")
-	var err error
+
 	tree := t
-	for i, name := range parts {
-		if i == len(parts)-1 {
-			entries, err := tree.ListEntries()
-			if err != nil {
-				return nil, err
-			}
-			for _, v := range entries {
-				if v.Name() == name {
-					return v, nil
-				}
-			}
-		} else {
-			tree, err = tree.SubTree(name)
-			if err != nil {
-				return nil, err
-			}
+	for _, name := range parts[:len(parts)-1] {
+		tree, err = tree.SubTree(name)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	name := parts[len(parts)-1]
+	entries, err := tree.ListEntries()
+	if err != nil {
+		return nil, err
+	}
+	for _, v := range entries {
+		if v.Name() == name {
+			return v, nil
 		}
 	}
 	return nil, ErrNotExist{"", relpath}

modules/git/tree_entry.go

@@ -5,7 +5,7 @@
 package git
 
 import (
-	"io"
+	"path"
 	"sort"
 	"strings"
 
@@ -24,77 +24,57 @@ func (te *TreeEntry) Type() string {
 	}
 }
 
-// FollowLink returns the entry pointed to by a symlink
-func (te *TreeEntry) FollowLink() (*TreeEntry, error) {
-	if !te.IsLink() {
-		return nil, ErrSymlinkUnresolved{te.Name(), "not a symlink"}
-	}
-
-	// read the link
-	r, err := te.Blob().DataAsync()
-	if err != nil {
-		return nil, err
-	}
-	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
-
-	// traverse up directories
-	for ; t != nil && strings.HasPrefix(lnk, "../"); lnk = lnk[3:] {
-		t = t.ptree
-	}
-
-	if t == nil {
-		return nil, ErrSymlinkUnresolved{te.Name(), "points outside of repo"}
-	}
-
-	target, err := t.GetTreeEntryByPath(lnk)
-	if err != nil {
-		if IsErrNotExist(err) {
-			return nil, ErrSymlinkUnresolved{te.Name(), "broken link"}
-		}
-		return nil, err
-	}
-	return target, nil
+type EntryFollowResult struct {
+	SymlinkContent string
+	TargetFullPath string
+	TargetEntry    *TreeEntry
 }
 
-// FollowLinks returns the entry ultimately pointed to by a symlink
-func (te *TreeEntry) FollowLinks(optLimit ...int) (*TreeEntry, error) {
+func EntryFollowLink(commit *Commit, fullPath string, te *TreeEntry) (*EntryFollowResult, error) {
 	if !te.IsLink() {
-		return nil, ErrSymlinkUnresolved{te.Name(), "not a symlink"}
+		return nil, util.ErrorWrap(util.ErrUnprocessableContent, "%q is not a symlink", fullPath)
 	}
+
+	// git's filename max length is 4096, hopefully a link won't be longer than multiple of that
+	const maxSymlinkSize = 20 * 4096
+	if te.Blob().Size() > maxSymlinkSize {
+		return nil, util.ErrorWrap(util.ErrUnprocessableContent, "%q content exceeds symlink limit", fullPath)
+	}
+
+	link, err := te.Blob().GetBlobContent(maxSymlinkSize)
+	if err != nil {
+		return nil, err
+	}
+	if strings.HasPrefix(link, "/") {
+		// It's said that absolute path will be stored as is in Git
+		return &EntryFollowResult{SymlinkContent: link}, util.ErrorWrap(util.ErrUnprocessableContent, "%q is an absolute symlink", fullPath)
+	}
+
+	targetFullPath := path.Join(path.Dir(fullPath), link)
+	targetEntry, err := commit.GetTreeEntryByPath(targetFullPath)
+	if err != nil {
+		return &EntryFollowResult{SymlinkContent: link}, err
+	}
+	return &EntryFollowResult{SymlinkContent: link, TargetFullPath: targetFullPath, TargetEntry: targetEntry}, nil
+}
+
+func EntryFollowLinks(commit *Commit, firstFullPath string, firstTreeEntry *TreeEntry, optLimit ...int) (res *EntryFollowResult, err error) {
 	limit := util.OptionalArg(optLimit, 10)
-	entry := te
+	treeEntry, fullPath := firstTreeEntry, firstFullPath
 	for range limit {
-		if !entry.IsLink() {
+		res, err = EntryFollowLink(commit, fullPath, treeEntry)
+		if err != nil {
+			return res, err
+		}
+		treeEntry, fullPath = res.TargetEntry, res.TargetFullPath
+		if !treeEntry.IsLink() {
 			break
 		}
-		next, err := entry.FollowLink()
-		if err != nil {
-			return nil, err
-		}
-		if next.ID == entry.ID {
-			return nil, ErrSymlinkUnresolved{entry.Name(), "recursive link"}
-		}
-		entry = next
 	}
-	if entry.IsLink() {
-		return nil, ErrSymlinkUnresolved{te.Name(), "too many levels of symbolic links"}
+	if treeEntry.IsLink() {
+		return res, util.ErrorWrap(util.ErrUnprocessableContent, "%q has too many links", firstFullPath)
 	}
-	return entry, nil
+	return res, nil
 }
 
 // returns the Tree pointed to by this TreeEntry, or nil if this is not a tree

modules/git/tree_entry_common_test.go

@@ -0,0 +1,76 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/modules/util"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFollowLink(t *testing.T) {
+	r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare")
+	require.NoError(t, err)
+	defer r.Close()
+
+	commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123")
+	require.NoError(t, err)
+
+	// get the symlink
+	{
+		lnkFullPath := "foo/bar/link_to_hello"
+		lnk, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello")
+		require.NoError(t, err)
+		assert.True(t, lnk.IsLink())
+
+		// should be able to dereference to target
+		res, err := EntryFollowLink(commit, lnkFullPath, lnk)
+		require.NoError(t, err)
+		assert.Equal(t, "hello", res.TargetEntry.Name())
+		assert.Equal(t, "foo/nar/hello", res.TargetFullPath)
+		assert.False(t, res.TargetEntry.IsLink())
+		assert.Equal(t, "b14df6442ea5a1b382985a6549b85d435376c351", res.TargetEntry.ID.String())
+	}
+
+	{
+		// should error when called on a normal file
+		entry, err := commit.Tree.GetTreeEntryByPath("file1.txt")
+		require.NoError(t, err)
+		res, err := EntryFollowLink(commit, "file1.txt", entry)
+		assert.ErrorIs(t, err, util.ErrUnprocessableContent)
+		assert.Nil(t, res)
+	}
+
+	{
+		// should error for broken links
+		entry, err := commit.Tree.GetTreeEntryByPath("foo/broken_link")
+		require.NoError(t, err)
+		assert.True(t, entry.IsLink())
+		res, err := EntryFollowLink(commit, "foo/broken_link", entry)
+		assert.ErrorIs(t, err, util.ErrNotExist)
+		assert.Equal(t, "nar/broken_link", res.SymlinkContent)
+	}
+
+	{
+		// should error for external links
+		entry, err := commit.Tree.GetTreeEntryByPath("foo/outside_repo")
+		require.NoError(t, err)
+		assert.True(t, entry.IsLink())
+		res, err := EntryFollowLink(commit, "foo/outside_repo", entry)
+		assert.ErrorIs(t, err, util.ErrNotExist)
+		assert.Equal(t, "../../outside_repo", res.SymlinkContent)
+	}
+
+	{
+		// testing fix for short link bug
+		entry, err := commit.Tree.GetTreeEntryByPath("foo/link_short")
+		require.NoError(t, err)
+		res, err := EntryFollowLink(commit, "foo/link_short", entry)
+		assert.ErrorIs(t, err, util.ErrNotExist)
+		assert.Equal(t, "a", res.SymlinkContent)
+	}
+}

modules/git/tree_entry_gogit.go

@@ -19,16 +19,12 @@ type TreeEntry struct {
 	gogitTreeEntry *object.TreeEntry
 	ptree          *Tree
 
-	size     int64
-	sized    bool
-	fullName string
+	size  int64
+	sized bool
 }
 
 // Name returns the name of the entry
 func (te *TreeEntry) Name() string {
-	if te.fullName != "" {
-		return te.fullName
-	}
 	return te.gogitTreeEntry.Name
 }
 
@@ -55,7 +51,7 @@ func (te *TreeEntry) Size() int64 {
 	return te.size
 }
 
-// IsSubModule if the entry is a sub module
+// IsSubModule if the entry is a submodule
 func (te *TreeEntry) IsSubModule() bool {
 	return te.gogitTreeEntry.Mode == filemode.Submodule
 }

modules/git/tree_entry_mode.go

@@ -15,7 +15,7 @@ type EntryMode int
 // one of these.
 const (
 	// EntryModeNoEntry is possible if the file was added or removed in a commit. In the case of
-	// added the base commit will not have the file in its tree so a mode of 0o000000 is used.
+	// when adding the base commit doesn't have the file in its tree, a mode of 0o000000 is used.
 	EntryModeNoEntry EntryMode = 0o000000
 
 	EntryModeBlob    EntryMode = 0o100644
@@ -30,7 +30,7 @@ func (e EntryMode) String() string {
 	return strconv.FormatInt(int64(e), 8)
 }
 
-// IsSubModule if the entry is a sub module
+// IsSubModule if the entry is a submodule
 func (e EntryMode) IsSubModule() bool {
 	return e == EntryModeCommit
 }

modules/git/tree_entry_nogogit.go

@@ -57,7 +57,7 @@ func (te *TreeEntry) Size() int64 {
 	return te.size
 }
 
-// IsSubModule if the entry is a sub module
+// IsSubModule if the entry is a submodule
 func (te *TreeEntry) IsSubModule() bool {
 	return te.entryMode.IsSubModule()
 }

modules/git/tree_entry_test.go

@@ -53,50 +53,3 @@ func TestEntriesCustomSort(t *testing.T) {
 	assert.Equal(t, "bcd", entries[6].Name())
 	assert.Equal(t, "abc", entries[7].Name())
 }
-
-func TestFollowLink(t *testing.T) {
-	r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare")
-	assert.NoError(t, err)
-	defer r.Close()
-
-	commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123")
-	assert.NoError(t, err)
-
-	// get the symlink
-	lnk, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello")
-	assert.NoError(t, err)
-	assert.True(t, lnk.IsLink())
-
-	// should be able to dereference to target
-	target, err := lnk.FollowLink()
-	assert.NoError(t, err)
-	assert.Equal(t, "hello", target.Name())
-	assert.False(t, target.IsLink())
-	assert.Equal(t, "b14df6442ea5a1b382985a6549b85d435376c351", target.ID.String())
-
-	// should error when called on normal file
-	target, err = commit.Tree.GetTreeEntryByPath("file1.txt")
-	assert.NoError(t, err)
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "file1.txt: not a symlink")
-
-	// should error for broken links
-	target, err = commit.Tree.GetTreeEntryByPath("foo/broken_link")
-	assert.NoError(t, err)
-	assert.True(t, target.IsLink())
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "broken_link: broken link")
-
-	// should error for external links
-	target, err = commit.Tree.GetTreeEntryByPath("foo/outside_repo")
-	assert.NoError(t, err)
-	assert.True(t, target.IsLink())
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "outside_repo: points outside of repo")
-
-	// testing fix for short link bug
-	target, err = commit.Tree.GetTreeEntryByPath("foo/link_short")
-	assert.NoError(t, err)
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "link_short: broken link")
-}

modules/git/tree_gogit.go

@@ -69,7 +69,7 @@ func (t *Tree) ListEntriesRecursiveWithSize() (Entries, error) {
 	seen := map[plumbing.Hash]bool{}
 	walker := object.NewTreeWalker(t.gogitTree, true, seen)
 	for {
-		fullName, entry, err := walker.Next()
+		_, entry, err := walker.Next()
 		if err == io.EOF {
 			break
 		}
@@ -84,7 +84,6 @@ func (t *Tree) ListEntriesRecursiveWithSize() (Entries, error) {
 			ID:             ParseGogitHash(entry.Hash),
 			gogitTreeEntry: &entry,
 			ptree:          t,
-			fullName:       fullName,
 		}
 		entries = append(entries, convertedEntry)
 	}