From fc4cb07beb93d24bdf94b6dfa1cac519f54154d9 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 16 Jul 2025 20:07:38 +0800 Subject: [PATCH] Fix submodule nil check (#35096) Fix #35095 --- modules/git/commit_info.go | 2 +- modules/git/commit_info_test.go | 11 +++++++++++ modules/git/commit_submodule.go | 3 ++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/modules/git/commit_info.go b/modules/git/commit_info.go index a796f4a204..9079e36677 100644 --- a/modules/git/commit_info.go +++ b/modules/git/commit_info.go @@ -15,7 +15,7 @@ type CommitInfo struct { 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 { + if submodule == nil || err != nil { return nil, err } return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, entry.ID.String()), nil diff --git a/modules/git/commit_info_test.go b/modules/git/commit_info_test.go index caaa6a502b..f8060bc33d 100644 --- a/modules/git/commit_info_test.go +++ b/modules/git/commit_info_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -120,6 +121,16 @@ func TestEntries_GetCommitsInfo(t *testing.T) { defer clonedRepo1.Close() testGetCommitsInfo(t, clonedRepo1) + + t.Run("NonExistingSubmoduleAsNil", func(t *testing.T) { + commit, err := bareRepo1.GetCommit("HEAD") + require.NoError(t, err) + tree, err := commit.GetTreeEntryByPath("file1.txt") + require.NoError(t, err) + cisf, err := getCommitInfoSubmoduleFile("/any/repo-link", tree, commit, "") + require.NoError(t, err) + assert.Nil(t, cisf) + }) } func BenchmarkEntries_GetCommitsInfo(b *testing.B) { diff --git a/modules/git/commit_submodule.go b/modules/git/commit_submodule.go index 031fd4e5d0..ff253b7eca 100644 --- a/modules/git/commit_submodule.go +++ b/modules/git/commit_submodule.go @@ -35,7 +35,8 @@ func (c *Commit) GetSubModules() (*ObjectCache[*SubModule], error) { return c.submoduleCache, nil } -// GetSubModule get the submodule according entry name +// GetSubModule gets the submodule by the entry name. +// It returns "nil, nil" if the submodule does not exist, caller should always remember to check the "nil" func (c *Commit) GetSubModule(entryName string) (*SubModule, error) { modules, err := c.GetSubModules() if err != nil {