fix

Author: wxiaoguang

modules/git/commit.go

@@ -28,7 +28,7 @@ type Commit struct {
 	Signature     *CommitSignature
 
 	Parents        []ObjectID // ID strings
-	submoduleCache *ObjectCache
+	submoduleCache *ObjectCache[*SubModule]
 }
 
 // CommitSignature represents a git commit signature part.
@@ -356,50 +356,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
-	}
-
-	rd, err := entry.Blob().DataAsync()
-	if err != nil {
-		return nil, err
-	}
-	defer rd.Close()
-
-	r := io.LimitReader(rd, 100*1024) // limit to 100KB
-	c.submoduleCache, err = parseSubmoduleContent(r)
-	if err != nil {
-		return nil, err
-	}
-	return c.submoduleCache, nil
-}
-
-// GetSubModule get the sub module according entryname
-func (c *Commit) GetSubModule(entryname string) (*SubModule, error) {
-	modules, err := c.GetSubModules()
-	if err != nil {
-		return nil, err
-	}
-
-	if modules != nil {
-		module, has := modules.Get(entryname)
-		if has {
-			return module.(*SubModule), nil
-		}
-	}
-	return nil, 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")

modules/git/commit_info.go

@@ -7,5 +7,5 @@ package git
 type CommitInfo struct {
 	Entry         *TreeEntry
 	Commit        *Commit
-	SubModuleFile *SubModuleFile
+	SubModuleFile *CommitSubModuleFile
 }

modules/git/commit_info_gogit.go

@@ -71,7 +71,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 			commitsInfo[i].Commit = entryCommit
 		}
 
-		// If the entry if 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
@@ -85,7 +85,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 			} else if subModule != nil {
 				subModuleURL = subModule.URL
 			}
-			subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String())
+			subModuleFile := NewCommitSubModuleFile(subModuleURL, entry.ID.String())
 			commitsInfo[i].SubModuleFile = subModuleFile
 		}
 	}

modules/git/commit_info_nogogit.go

@@ -79,7 +79,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 			} else if subModule != nil {
 				subModuleURL = subModule.URL
 			}
-			subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String())
+			subModuleFile := NewCommitSubModuleFile(subModuleURL, entry.ID.String())
 			commitsInfo[i].SubModuleFile = subModuleFile
 		}
 	}

modules/git/commit_submodule.go

@@ -0,0 +1,47 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+// GetSubModules get all the submodules of current revision git tree
+func (c *Commit) GetSubModules() (*ObjectCache[*SubModule], 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
+	}
+
+	rd, err := entry.Blob().DataAsync()
+	if err != nil {
+		return nil, err
+	}
+	defer rd.Close()
+
+	// at the moment we do not strictly limit the size of the .gitmodules file because some users would have huge .gitmodules files (>1MB)
+	c.submoduleCache, err = configParseSubModules(rd)
+	if err != nil {
+		return nil, err
+	}
+	return c.submoduleCache, nil
+}
+
+// GetSubModule get the submodule according entry name
+func (c *Commit) GetSubModule(entryName string) (*SubModule, error) {
+	modules, err := c.GetSubModules()
+	if err != nil {
+		return nil, err
+	}
+
+	if modules != nil {
+		if module, has := modules.Get(entryName); has {
+			return module, nil
+		}
+	}
+	return nil, nil
+}

modules/git/submodule.go renamed to modules/git/commit_submodule_file.go

@@ -5,9 +5,7 @@
 package git
 
 import (
-	"bufio"
 	"fmt"
-	"io"
 	"net"
 	"net/url"
 	"path"
@@ -17,90 +15,15 @@ import (
 
 var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+@)?([a-zA-Z0-9._-]+):(.*)$`)
 
-// SubModule submodule is a reference on git repository
-type SubModule struct {
-	Path   string
-	URL    string
-	Branch string
-}
-
-// parseSubmoduleContent this is not a complete parse for gitmodules file, it only
-// parses the url and path of submodules
-func parseSubmoduleContent(r io.Reader) (*ObjectCache, error) {
-	var path, url, branch string
-	var state int // 0: find section, 1: find path and url
-	subModules := newObjectCache()
-	scanner := bufio.NewScanner(r)
-	for scanner.Scan() {
-		line := strings.TrimSpace(scanner.Text())
-
-		// Skip empty lines and comments
-		if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") {
-			continue
-		}
-
-		// Section header [section]
-		if strings.HasPrefix(line, "[submodule") && strings.HasSuffix(line, "]") {
-			if path != "" && url != "" {
-				subModules.Set(path, &SubModule{
-					Path:   path,
-					URL:    url,
-					Branch: branch,
-				})
-			}
-			state = 1
-			path = ""
-			url = ""
-			branch = ""
-			continue
-		}
-
-		if state != 1 {
-			continue
-		}
-
-		parts := strings.SplitN(line, "=", 2)
-		if len(parts) != 2 {
-			continue
-		}
-		key := strings.TrimSpace(parts[0])
-		value := strings.TrimSpace(parts[1])
-		switch key {
-		case "path":
-			path = value
-		case "url":
-			url = value
-		case "branch":
-			branch = value
-		}
-	}
-
-	if err := scanner.Err(); err != nil {
-		return nil, fmt.Errorf("error reading file: %w", err)
-	}
-	if path != "" && url != "" {
-		subModules.Set(path, &SubModule{
-			Path:   path,
-			URL:    url,
-			Branch: branch,
-		})
-	}
-
-	return subModules, nil
-}
-
-// SubModuleFile represents a file with submodule type.
-type SubModuleFile struct {
-	*Commit
-
+// CommitSubModuleFile represents a file with submodule type.
+type CommitSubModuleFile struct {
 	refURL string
 	refID  string
 }
 
-// NewSubModuleFile create a new submodule file
-func NewSubModuleFile(c *Commit, refURL, refID string) *SubModuleFile {
-	return &SubModuleFile{
-		Commit: c,
+// NewCommitSubModuleFile create a new submodule file
+func NewCommitSubModuleFile(refURL, refID string) *CommitSubModuleFile {
+	return &CommitSubModuleFile{
 		refURL: refURL,
 		refID:  refID,
 	}
@@ -177,11 +100,12 @@ func getRefURL(refURL, urlPrefix, repoFullName, sshDomain string) string {
 }
 
 // RefURL guesses and returns reference URL.
-func (sf *SubModuleFile) RefURL(urlPrefix, repoFullName, sshDomain string) string {
+// FIXME: template passes AppURL as urlPrefix, it needs to figure out the correct approach (no hard-coded AppURL anymore)
+func (sf *CommitSubModuleFile) RefURL(urlPrefix, repoFullName, sshDomain string) string {
 	return getRefURL(sf.refURL, urlPrefix, repoFullName, sshDomain)
 }
 
 // RefID returns reference ID.
-func (sf *SubModuleFile) RefID() string {
+func (sf *CommitSubModuleFile) RefID() string {
 	return sf.refID
 }

modules/git/submodule_test.go renamed to modules/git/commit_submodule_file_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestGetRefURL(t *testing.T) {
+func TestCommitSubModuleFileGetRefURL(t *testing.T) {
 	kases := []struct {
 		refURL     string
 		prefixURL  string

modules/git/commit_test.go

@@ -135,7 +135,7 @@ author KN4CK3R <[email protected]> 1711702962 +0100
 committer KN4CK3R <[email protected]> 1711702962 +0100
 encoding ISO-8859-1
 gpgsig -----BEGIN PGP SIGNATURE-----
- 
+
  iQGzBAABCgAdFiEE9HRrbqvYxPT8PXbefPSEkrowAa8FAmYGg7IACgkQfPSEkrow
  Aa9olwv+P0HhtCM6CRvlUmPaqswRsDPNR4i66xyXGiSxdI9V5oJL7HLiQIM7KrFR
  gizKa2COiGtugv8fE+TKqXKaJx6uJUJEjaBd8E9Af9PrAzjWj+A84lU6/PgPS8hq
@@ -362,48 +362,3 @@ func Test_GetCommitBranchStart(t *testing.T) {
 	assert.NotEmpty(t, startCommitID)
 	assert.EqualValues(t, "9c9aef8dd84e02bc7ec12641deb4c930a7c30185", startCommitID)
 }
-
-func Test_parseSubmoduleContent(t *testing.T) {
-	submoduleFiles := []struct {
-		fileContent    string
-		expectedPath   string
-		expectedURL    string
-		expectedBranch string
-	}{
-		{
-			fileContent: `[submodule "jakarta-servlet"]
-url = ../../ALP-pool/jakarta-servlet
-path = jakarta-servlet`,
-			expectedPath:   "jakarta-servlet",
-			expectedURL:    "../../ALP-pool/jakarta-servlet",
-			expectedBranch: "",
-		},
-		{
-			fileContent: `[submodule "jakarta-servlet"]
-path = jakarta-servlet
-url = ../../ALP-pool/jakarta-servlet`,
-			expectedPath:   "jakarta-servlet",
-			expectedURL:    "../../ALP-pool/jakarta-servlet",
-			expectedBranch: "",
-		},
-		{
-			fileContent: `[submodule "jakarta-servlet"]
-path = jakarta-servlet
-url = ../../ALP-pool/jakarta-servlet
-branch = stable`,
-			expectedPath:   "jakarta-servlet",
-			expectedURL:    "../../ALP-pool/jakarta-servlet",
-			expectedBranch: "stable",
-		},
-	}
-	for _, kase := range submoduleFiles {
-		submodule, err := parseSubmoduleContent(strings.NewReader(kase.fileContent))
-		assert.NoError(t, err)
-		v, ok := submodule.Get(kase.expectedPath)
-		assert.True(t, ok)
-		subModule := v.(*SubModule)
-		assert.Equal(t, kase.expectedPath, subModule.Path)
-		assert.Equal(t, kase.expectedURL, subModule.URL)
-		assert.Equal(t, kase.expectedBranch, subModule.Branch)
-	}
-}