git/commit: re-implement submodules file reader (#8438)

Reimplement the submodules parser to not depend on the go-git dependency.

See #8222 for the full refactor context.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8438
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>

.golangci.yml

@@ -42,6 +42,8 @@ linters:
               desc: do not use the ini package, use gitea's config system instead
             - pkg: github.com/minio/sha256-simd
               desc: use crypto/sha256 instead, see https://codeberg.org/forgejo/forgejo/pulls/1528
+            - pkg: github.com/go-git/go-git
+              desc: use forgejo.org/modules/git instead, see https://codeberg.org/forgejo/forgejo/pulls/4941
     gocritic:
       disabled-checks:
         - ifElseChain

go.mod

@@ -45,7 +45,6 @@ require (
 	github.com/go-chi/cors v1.2.2
 	github.com/go-co-op/gocron v1.37.0
 	github.com/go-enry/go-enry/v2 v2.9.2
-	github.com/go-git/go-git/v5 v5.13.2
 	github.com/go-ldap/ldap/v3 v3.4.6
 	github.com/go-openapi/spec v0.21.0
 	github.com/go-sql-driver/mysql v1.9.3
@@ -166,6 +165,7 @@ require (
 	github.com/go-fed/httpsig v1.1.0 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
 	github.com/go-git/go-billy/v5 v5.6.2 // indirect
+	github.com/go-git/go-git/v5 v5.13.2 // indirect
 	github.com/go-ini/ini v1.67.0 // indirect
 	github.com/go-openapi/jsonpointer v0.21.1 // indirect
 	github.com/go-openapi/jsonreference v0.21.0 // indirect

modules/base/tool.go

@@ -114,7 +114,7 @@ func EntryIcon(entry *git.TreeEntry) string {
 		return "file-symlink-file"
 	case entry.IsDir():
 		return "file-directory-fill"
-	case entry.IsSubModule():
+	case entry.IsSubmodule():
 		return "file-submodule"
 	}
 

modules/git/commit.go

@@ -16,8 +16,6 @@ import (
 
 	"forgejo.org/modules/log"
 	"forgejo.org/modules/util"
-
-	"github.com/go-git/go-git/v5/config"
 )
 
 // Commit represents a git commit.
@@ -30,7 +28,7 @@ type Commit struct {
 	Signature     *ObjectSignature
 
 	Parents    []ObjectID           // ID strings
-	submoduleCache *ObjectCache
+	submodules map[string]Submodule // submodule indexed by path
 }
 
 // Message returns the commit message. Same as retrieving CommitMessage directly.
@@ -352,64 +350,6 @@ func (c *Commit) GetFileContent(filename string, limit int) (string, error) {
 	return string(bytes), nil
 }
 
-// GetSubModules get all the sub modules of current revision git tree
-func (c *Commit) GetSubModules() (*ObjectCache, error) {
-	if c.submoduleCache != nil {
-		return c.submoduleCache, nil
-	}
-
-	entry, err := c.GetTreeEntryByPath(".gitmodules")
-	if err != nil {
-		if _, ok := err.(ErrNotExist); ok {
-			return nil, nil
-		}
-		return nil, err
-	}
-
-	content, err := entry.Blob().GetBlobContent(10 * 1024)
-	if err != nil {
-		return nil, err
-	}
-
-	c.submoduleCache, err = parseSubmoduleContent([]byte(content))
-	if err != nil {
-		return nil, err
-	}
-	return c.submoduleCache, nil
-}
-
-func parseSubmoduleContent(bs []byte) (*ObjectCache, error) {
-	cfg := config.NewModules()
-	if err := cfg.Unmarshal(bs); err != nil {
-		return nil, err
-	}
-	submoduleCache := newObjectCache()
-	if len(cfg.Submodules) == 0 {
-		return nil, errors.New("no submodules found")
-	}
-	for _, subModule := range cfg.Submodules {
-		submoduleCache.Set(subModule.Path, subModule.URL)
-	}
-
-	return submoduleCache, nil
-}
-
-// GetSubModule returns the URL to the submodule according entryname
-func (c *Commit) GetSubModule(entryname string) (string, error) {
-	modules, err := c.GetSubModules()
-	if err != nil {
-		return "", err
-	}
-
-	if modules != nil {
-		module, has := modules.Get(entryname)
-		if has {
-			return module.(string), nil
-		}
-	}
-	return "", nil
-}
-
 // GetBranchName gets the closest branch name (as returned by 'git name-rev --name-only')
 func (c *Commit) GetBranchName() (string, error) {
 	cmd := NewCommand(c.repo.Ctx, "name-rev", "--exclude", "refs/tags/*", "--name-only", "--no-undefined").AddDynamicArguments(c.ID.String())

modules/git/commit_info.go

@@ -17,7 +17,7 @@ import (
 type CommitInfo struct {
 	Entry     *TreeEntry
 	Commit    *Commit
-	SubModuleFile *SubModuleFile
+	Submodule Submodule
 }
 
 // GetCommitsInfo gets information of all commits that are corresponding to these entries
@@ -71,19 +71,18 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 		}
 
 		// If the entry if a submodule add a submodule file for this
-		if entry.IsSubModule() {
+		if entry.IsSubmodule() {
 			var fullPath string
 			if len(treePath) > 0 {
 				fullPath = treePath + "/" + entry.Name()
 			} else {
 				fullPath = entry.Name()
 			}
-			subModuleURL, err := commit.GetSubModule(fullPath)
+			submodule, err := commit.GetSubmodule(fullPath, entry)
 			if err != nil {
 				return nil, nil, err
 			}
-			subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String())
-			commitsInfo[i].SubModuleFile = subModuleFile
+			commitsInfo[i].Submodule = submodule
 		}
 	}
 

modules/git/commit_test.go

@@ -436,33 +436,3 @@ func TestGetAllBranches(t *testing.T) {
 
 	assert.Equal(t, []string{"branch1", "branch2", "master"}, branches)
 }
-
-func Test_parseSubmoduleContent(t *testing.T) {
-	submoduleFiles := []struct {
-		fileContent  string
-		expectedPath string
-		expectedURL  string
-	}{
-		{
-			fileContent: `[submodule "jakarta-servlet"]
-url = ../../ALP-pool/jakarta-servlet
-path = jakarta-servlet`,
-			expectedPath: "jakarta-servlet",
-			expectedURL:  "../../ALP-pool/jakarta-servlet",
-		},
-		{
-			fileContent: `[submodule "jakarta-servlet"]
-path = jakarta-servlet
-url = ../../ALP-pool/jakarta-servlet`,
-			expectedPath: "jakarta-servlet",
-			expectedURL:  "../../ALP-pool/jakarta-servlet",
-		},
-	}
-	for _, kase := range submoduleFiles {
-		submodule, err := parseSubmoduleContent([]byte(kase.fileContent))
-		require.NoError(t, err)
-		v, ok := submodule.Get(kase.expectedPath)
-		assert.True(t, ok)
-		assert.Equal(t, kase.expectedURL, v)
-	}
-}

modules/git/submodule.go

@@ -6,38 +6,124 @@ package git
 
 import (
 	"fmt"
+	"io"
 	"net"
 	"net/url"
 	"path"
 	"regexp"
 	"strings"
+
+	"forgejo.org/modules/setting"
+	"forgejo.org/modules/util"
+
+	"gopkg.in/ini.v1" //nolint:depguard // used to read .gitmodules
 )
 
-var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+@)?([a-zA-Z0-9._-]+):(.*)$`)
-
-// SubModule submodule is a reference on git repository
-type SubModule struct {
-	Name string
-	URL  string
-}
-
-// SubModuleFile represents a file with submodule type.
-type SubModuleFile struct {
-	*Commit
-
-	refURL string
-	refID  string
-}
-
-// NewSubModuleFile create a new submodule file
-func NewSubModuleFile(c *Commit, refURL, refID string) *SubModuleFile {
-	return &SubModuleFile{
-		Commit: c,
-		refURL: refURL,
-		refID:  refID,
+// GetSubmodule returns the Submodule of a given path
+func (c *Commit) GetSubmodule(path string, entry *TreeEntry) (Submodule, error) {
+	err := c.readSubmodules()
+	if err != nil {
+		// the .gitmodules file exists but could not be read or parsed
+		return Submodule{}, err
 	}
+
+	sm, ok := c.submodules[path]
+	if !ok {
+		// no info found in .gitmodules: fallback to what we can provide
+		return Submodule{
+			Path:   path,
+			Commit: entry.ID,
+		}, nil
+	}
+
+	sm.Commit = entry.ID
+	return sm, nil
 }
 
+// readSubmodules populates the submodules field by reading the .gitmodules file
+func (c *Commit) readSubmodules() error {
+	if c.submodules != nil {
+		return nil
+	}
+
+	entry, err := c.GetTreeEntryByPath(".gitmodules")
+	if err != nil {
+		if IsErrNotExist(err) {
+			c.submodules = make(map[string]Submodule)
+			return nil
+		}
+		return err
+	}
+
+	rc, _, err := entry.Blob().NewTruncatedReader(10 * 1024)
+	if err != nil {
+		return err
+	}
+	defer rc.Close()
+
+	c.submodules, err = parseSubmoduleContent(rc)
+	return err
+}
+
+func parseSubmoduleContent(r io.Reader) (map[string]Submodule, error) {
+	// https://git-scm.com/docs/gitmodules#_description
+	// The .gitmodules file, located in the top-level directory of a Git working tree
+	// is a text file with a syntax matching the requirements of git-config[1].
+	// https://git-scm.com/docs/git-config#_configuration_file
+
+	cfg := ini.Empty(ini.LoadOptions{
+		InsensitiveKeys: true, // "The variable names are case-insensitive", but "Subsection names are case sensitive"
+	})
+	err := cfg.Append(r)
+	if err != nil {
+		return nil, err
+	}
+
+	sections := cfg.Sections()
+	submodule := make(map[string]Submodule, len(sections))
+
+	for _, s := range sections {
+		sm := parseSubmoduleSection(s)
+		if sm.Path == "" || sm.URL == "" {
+			continue
+		}
+		submodule[sm.Path] = sm
+	}
+	return submodule, nil
+}
+
+func parseSubmoduleSection(s *ini.Section) Submodule {
+	section, name, _ := strings.Cut(s.Name(), " ")
+	if !util.ASCIIEqualFold("submodule", section) { // See https://codeberg.org/forgejo/forgejo/pulls/8438#issuecomment-5805251
+		return Submodule{}
+	}
+	_ = name
+
+	sm := Submodule{}
+	if key, _ := s.GetKey("path"); key != nil {
+		sm.Path = key.Value()
+	}
+	if key, _ := s.GetKey("url"); key != nil {
+		sm.URL = key.Value()
+	}
+	return sm
+}
+
+// Submodule represents a parsed git submodule reference.
+type Submodule struct {
+	Path   string   // path property
+	URL    string   // upstream URL
+	Commit ObjectID // upstream Commit-ID
+}
+
+// ResolveUpstreamURL resolves the upstream URL relative to the repo URL.
+func (sm Submodule) ResolveUpstreamURL(repoURL string) string {
+	repoFullName := strings.TrimPrefix(repoURL, setting.AppURL) // currently hacky, but can be dropped when refactoring getRefURL
+	return getRefURL(sm.URL, setting.AppURL, repoFullName, setting.SSH.Domain)
+}
+
+var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+@)?([a-zA-Z0-9._-]+):(.*)$`)
+
 func getRefURL(refURL, urlPrefix, repoFullName, sshDomain string) string {
 	if refURL == "" {
 		return ""
@@ -53,7 +139,7 @@ func getRefURL(refURL, urlPrefix, repoFullName, sshDomain string) string {
 
 	urlPrefix = strings.TrimSuffix(urlPrefix, "/")
 
-	// FIXME: Need to consider branch - which will require changes in modules/git/commit.go:GetSubModules
+	// FIXME: Need to consider branch - which will require changes in parseSubmoduleSection
 	// Relative url prefix check (according to git submodule documentation)
 	if strings.HasPrefix(refURI, "./") || strings.HasPrefix(refURI, "../") {
 		return urlPrefix + path.Clean(path.Join("/", repoFullName, refURI))
@@ -107,13 +193,3 @@ func getRefURL(refURL, urlPrefix, repoFullName, sshDomain string) string {
 
 	return ""
 }
-
-// RefURL guesses and returns reference URL.
-func (sf *SubModuleFile) RefURL(urlPrefix, repoFullName, sshDomain string) string {
-	return getRefURL(sf.refURL, urlPrefix, repoFullName, sshDomain)
-}
-
-// RefID returns reference ID.
-func (sf *SubModuleFile) RefID() string {
-	return sf.refID
-}

modules/git/submodule_test.go

@@ -4,9 +4,11 @@
 package git
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestGetRefURL(t *testing.T) {
@@ -40,3 +42,74 @@ func TestGetRefURL(t *testing.T) {
 		assert.Equal(t, kase.expect, getRefURL(kase.refURL, kase.prefixURL, kase.parentPath, kase.SSHDomain))
 	}
 }
+
+func Test_parseSubmoduleContent(t *testing.T) {
+	submoduleFiles := []struct {
+		fileContent  string
+		expectedPath string
+		expected     Submodule
+	}{
+		{
+			fileContent: `[submodule "jakarta-servlet"]
+url = ../../ALP-pool/jakarta-servlet
+path = jakarta-servlet`,
+			expectedPath: "jakarta-servlet",
+			expected: Submodule{
+				Path: "jakarta-servlet",
+				URL:  "../../ALP-pool/jakarta-servlet",
+			},
+		},
+		{
+			fileContent: `[submodule "jakarta-servlet"]
+path = jakarta-servlet
+url = ../../ALP-pool/jakarta-servlet`,
+			expectedPath: "jakarta-servlet",
+			expected: Submodule{
+				Path: "jakarta-servlet",
+				URL:  "../../ALP-pool/jakarta-servlet",
+			},
+		},
+		{
+			fileContent: `[submodule "about/documents"]
+  path = about/documents
+  url = git@github.com:example/documents.git
+  branch = gh-pages
+[submodule "custom-name"]
+  path = manifesto
+  url = https://github.com/example/manifesto.git
+[submodule]
+  path = relative/url
+  url = ../such-relative.git
+`,
+			expectedPath: "relative/url",
+			expected: Submodule{
+				Path: "relative/url",
+				URL:  "../such-relative.git",
+			},
+		},
+		{
+			fileContent: `# .gitmodules
+# Subsection names are case sensitive
+[submodule "Seanpm2001/Degoogle-your-life"]
+	path = Its-time-to-cut-WideVine-DRM/DeGoogle-Your-Life/submodule.gitmodules
+	url = https://github.com/seanpm2001/Degoogle-your-life/
+
+[submodule "seanpm2001/degoogle-your-life"]
+	url = https://github.com/seanpm2001/degoogle-your-life/
+# This second section should not be merged with the first, because of casing
+`,
+			expectedPath: "Its-time-to-cut-WideVine-DRM/DeGoogle-Your-Life/submodule.gitmodules",
+			expected: Submodule{
+				Path: "Its-time-to-cut-WideVine-DRM/DeGoogle-Your-Life/submodule.gitmodules",
+				URL:  "https://github.com/seanpm2001/Degoogle-your-life/",
+			},
+		},
+	}
+	for _, kase := range submoduleFiles {
+		submodule, err := parseSubmoduleContent(strings.NewReader(kase.fileContent))
+		require.NoError(t, err)
+		v, ok := submodule[kase.expectedPath]
+		assert.True(t, ok)
+		assert.Equal(t, kase.expected, v)
+	}
+}

modules/git/tree_blob.go

@@ -17,7 +17,6 @@ func (t *Tree) GetTreeEntryByPath(relpath string) (*TreeEntry, error) {
 			ptree:     t,
 			ID:        t.ID,
 			name:      "",
-			fullName:  "",
 			entryMode: EntryModeTree,
 		}, nil
 	}
@@ -55,7 +54,7 @@ func (t *Tree) GetBlobByPath(relpath string) (*Blob, error) {
 		return nil, err
 	}
 
-	if !entry.IsDir() && !entry.IsSubModule() {
+	if !entry.IsDir() && !entry.IsSubmodule() {
 		return entry.Blob(), nil
 	}
 

modules/git/tree_entry.go

@@ -23,14 +23,10 @@ type TreeEntry struct {
 
 	size  int64
 	sized bool
-	fullName string
 }
 
 // Name returns the name of the entry
 func (te *TreeEntry) Name() string {
-	if te.fullName != "" {
-		return te.fullName
-	}
 	return te.name
 }
 
@@ -68,8 +64,8 @@ func (te *TreeEntry) Size() int64 {
 	return te.size
 }
 
-// IsSubModule if the entry is a sub module
-func (te *TreeEntry) IsSubModule() bool {
+// IsSubmodule if the entry is a submodule
+func (te *TreeEntry) IsSubmodule() bool {
 	return te.entryMode == EntryModeCommit
 }
 
@@ -214,7 +210,7 @@ func (te *TreeEntry) Tree() *Tree {
 
 // GetSubJumpablePathName return the full path of subdirectory jumpable ( contains only one directory )
 func (te *TreeEntry) GetSubJumpablePathName() string {
-	if te.IsSubModule() || !te.IsDir() {
+	if te.IsSubmodule() || !te.IsDir() {
 		return ""
 	}
 	tree, err := te.ptree.SubTree(te.Name())
@@ -241,7 +237,7 @@ type customSortableEntries struct {
 
 var sorter = []func(t1, t2 *TreeEntry, cmp func(s1, s2 string) bool) bool{
 	func(t1, t2 *TreeEntry, cmp func(s1, s2 string) bool) bool {
-		return (t1.IsDir() || t1.IsSubModule()) && !t2.IsDir() && !t2.IsSubModule()
+		return (t1.IsDir() || t1.IsSubmodule()) && !t2.IsDir() && !t2.IsSubmodule()
 	},
 	func(t1, t2 *TreeEntry, cmp func(s1, s2 string) bool) bool {
 		return cmp(t1.Name(), t2.Name())

routers/api/v1/repo/file.go

@@ -241,7 +241,7 @@ func getBlobForEntry(ctx *context.APIContext) (blob *git.Blob, entry *git.TreeEn
 		return nil, nil, nil
 	}
 
-	if entry.IsDir() || entry.IsSubModule() {
+	if entry.IsDir() || entry.IsSubmodule() {
 		ctx.NotFound("getBlobForEntry", nil)
 		return nil, nil, nil
 	}

routers/web/repo/download.go

@@ -92,7 +92,7 @@ func getBlobForEntry(ctx *context.Context) (*git.Blob, *time.Time) {
 		return nil, nil
 	}
 
-	if entry.IsDir() || entry.IsSubModule() {
+	if entry.IsDir() || entry.IsSubmodule() {
 		ctx.NotFound("getBlobForEntry", nil)
 		return nil, nil
 	}

routers/web/repo/treelist.go

@@ -42,7 +42,7 @@ func isExcludedEntry(entry *git.TreeEntry) bool {
 		return true
 	}
 
-	if entry.IsSubModule() {
+	if entry.IsSubmodule() {
 		return true
 	}
 

routers/web/repo/view.go

@@ -1057,14 +1057,13 @@ func renderHomeCode(ctx *context.Context) {
 		return
 	}
 
-	if entry.IsSubModule() {
-		subModuleURL, err := ctx.Repo.Commit.GetSubModule(entry.Name())
+	if entry.IsSubmodule() {
+		submodule, err := ctx.Repo.Commit.GetSubmodule(ctx.Repo.TreePath, entry)
 		if err != nil {
-			HandleGitError(ctx, "Repo.Commit.GetSubModule", err)
+			HandleGitError(ctx, "Repo.Commit.GetSubmodule", err)
 			return
 		}
-		subModuleFile := git.NewSubModuleFile(ctx.Repo.Commit, subModuleURL, entry.ID.String())
-		ctx.Redirect(subModuleFile.RefURL(setting.AppURL, ctx.Repo.Repository.FullName(), setting.SSH.Domain))
+		ctx.Redirect(submodule.ResolveUpstreamURL(ctx.Repo.Repository.HTMLURL()))
 	} else if entry.IsDir() {
 		renderDirectory(ctx)
 	} else {

services/repository/files/content.go

@@ -108,7 +108,7 @@ func GetObjectTypeFromTreeEntry(entry *git.TreeEntry) ContentType {
 	switch {
 	case entry.IsDir():
 		return ContentTypeDir
-	case entry.IsSubModule():
+	case entry.IsSubmodule():
 		return ContentTypeSubmodule
 	case entry.IsExecutable(), entry.IsRegular():
 		return ContentTypeRegular
@@ -211,14 +211,14 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref
 			return nil, err
 		}
 		contentsResponse.Target = &targetFromContent
-	} else if entry.IsSubModule() {
+	} else if entry.IsSubmodule() {
 		contentsResponse.Type = string(ContentTypeSubmodule)
-		submoduleURL, err := commit.GetSubModule(treePath)
+		submodule, err := commit.GetSubmodule(treePath, entry)
 		if err != nil {
 			return nil, err
 		}
-		if submoduleURL != "" {
-			contentsResponse.SubmoduleGitURL = &submoduleURL
+		if submodule.URL != "" {
+			contentsResponse.SubmoduleGitURL = &submodule.URL
 		}
 	}
 	// Handle links
@@ -230,7 +230,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref
 		downloadURLString := downloadURL.String()
 		contentsResponse.DownloadURL = &downloadURLString
 	}
-	if !entry.IsSubModule() {
+	if !entry.IsSubmodule() {
 		htmlURL, err := url.Parse(repo.HTMLURL() + "/src/" + url.PathEscape(string(refType)) + "/" + util.PathEscapeSegments(ref) + "/" + util.PathEscapeSegments(treePath))
 		if err != nil {
 			return nil, err

services/repository/files/tree.go

@@ -87,7 +87,7 @@ func GetTreeBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git
 		if entries[e].IsDir() {
 			copy(treeURL[copyPos:], entries[e].ID.String())
 			tree.Entries[i].URL = string(treeURL)
-		} else if entries[e].IsSubModule() {
+		} else if entries[e].IsSubmodule() {
 			// In Github Rest API Version=2022-11-28, if a tree entry is a submodule,
 			// its url will be returned as an empty string.
 			// So the URL will be set to "" here.

templates/repo/view_list.tmpl

@@ -20,17 +20,17 @@
 		{{range $item := .Files}}
 			{{$entry := $item.Entry}}
 			{{$commit := $item.Commit}}
-			{{$subModuleFile := $item.SubModuleFile}}
 			<tr data-entryname="{{$entry.Name}}" data-ready="{{if $commit}}true{{else}}false{{end}}" class="{{if not $commit}}not{{end}}ready entry">
 				<td class="name four wide">
 					<span class="truncate">
-						{{if $entry.IsSubModule}}
-							{{$refURL := $subModuleFile.RefURL AppUrl $.Repository.FullName $.SSHDomain}} {{/* FIXME: the usage of AppUrl seems incorrect, it would be fixed in the future, use AppSubUrl instead */}}
+						{{if $entry.IsSubmodule}}
+							{{$submodule := $item.Submodule}}
+							{{$refURL := $submodule.ResolveUpstreamURL $.Repository.HTMLURL}}
 							{{$icon := (svg "octicon-file-submodule" 16 "tw-mr-2")}}
 							{{if $refURL}}
-								<a class="muted" href="{{$refURL}}">{{$icon}}{{$entry.Name}}</a><span class="at">@</span><a href="{{$refURL}}/commit/{{PathEscape $subModuleFile.RefID}}">{{ShortSha $subModuleFile.RefID}}</a>
+								<a class="muted" href="{{$refURL}}">{{$icon}}{{$entry.Name}}</a><span class="at">@</span><a href="{{$refURL}}/commit/{{PathEscape $submodule.Commit.String}}">{{ShortSha $submodule.Commit.String}}</a>
 							{{else}}
-								{{$icon}}{{$entry.Name}}<span class="at">@</span>{{ShortSha $subModuleFile.RefID}}
+								{{$icon}}{{$entry.Name}}<span class="at">@</span>{{ShortSha $submodule.Commit.String}}
 							{{end}}
 						{{else}}
 							{{if $entry.IsDir}}