Skip to content

Fix submodule parsing when the gitmodules is missing #35109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion modules/git/commit_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ 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 submodule == nil || err != nil {
if err != nil {
return nil, err
}
if submodule == nil {
// unable to find submodule from ".gitmodules" file
return NewCommitSubmoduleFile(repoLink, fullPath, "", entry.ID.String()), nil
}
return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, entry.ID.String()), nil
}
9 changes: 8 additions & 1 deletion modules/git/commit_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,14 @@ func TestEntries_GetCommitsInfo(t *testing.T) {
require.NoError(t, err)
cisf, err := getCommitInfoSubmoduleFile("/any/repo-link", tree, commit, "")
require.NoError(t, err)
assert.Nil(t, cisf)
assert.Equal(t, &CommitSubmoduleFile{
repoLink: "/any/repo-link",
fullPath: "file1.txt",
refURL: "",
refID: "e2129701f1a4d54dc44f03c93bca0a2aec7c5449",
}, cisf)
// since there is no refURL, it means that the submodule info doesn't exist, so it won't have a web link
assert.Nil(t, cisf.SubmoduleWebLinkTree(t.Context()))
})
}

Expand Down
13 changes: 8 additions & 5 deletions modules/git/commit_submodule_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,16 @@ func NewCommitSubmoduleFile(repoLink, fullPath, refURL, refID string) *CommitSub
return &CommitSubmoduleFile{repoLink: repoLink, fullPath: fullPath, refURL: refURL, refID: refID}
}

// RefID returns the commit ID of the submodule, it returns empty string for nil receiver
func (sf *CommitSubmoduleFile) RefID() string {
if sf == nil {
return ""
}
return sf.refID
}

func (sf *CommitSubmoduleFile) getWebLinkInTargetRepo(ctx context.Context, moreLinkPath string) *SubmoduleWebLink {
if sf == nil {
if sf == nil || sf.refURL == "" {
return nil
}
if strings.HasPrefix(sf.refURL, "../") {
Expand All @@ -53,14 +57,13 @@ func (sf *CommitSubmoduleFile) getWebLinkInTargetRepo(ctx context.Context, moreL
}

// SubmoduleWebLinkTree tries to make the submodule's tree link in its own repo, it also works on "nil" receiver
// It returns nil if the submodule does not have a valid URL or is nil
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))
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
// It returns nil if the submodule does not have a valid URL or is nil
func (sf *CommitSubmoduleFile) SubmoduleWebLinkCompare(ctx context.Context, commitID1, commitID2 string) *SubmoduleWebLink {
return sf.getWebLinkInTargetRepo(ctx, "/compare/"+commitID1+"..."+commitID2)
}
2 changes: 2 additions & 0 deletions modules/git/commit_submodule_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
func TestCommitSubmoduleLink(t *testing.T) {
assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkTree(t.Context()))
assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkCompare(t.Context(), "", ""))
assert.Nil(t, (&CommitSubmoduleFile{}).SubmoduleWebLinkTree(t.Context()))
assert.Nil(t, (&CommitSubmoduleFile{}).SubmoduleWebLinkCompare(t.Context(), "", ""))

t.Run("GitHubRepo", func(t *testing.T) {
sf := NewCommitSubmoduleFile("/any/repo-link", "full-path", "[email protected]:user/repo.git", "aaaa")
Expand Down
3 changes: 2 additions & 1 deletion routers/web/repo/view_home.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,10 @@ func handleRepoEmptyOrBroken(ctx *context.Context) {
}

func handleRepoViewSubmodule(ctx *context.Context, submodule *git.SubModule) {
// TODO: it needs to use git.NewCommitSubmoduleFile and SubmoduleWebLinkTree to correctly handle relative paths
submoduleRepoURL, err := giturl.ParseRepositoryURL(ctx, submodule.URL)
if err != nil {
HandleGitError(ctx, "prepareToRenderDirOrFile: ParseRepositoryURL", err)
HandleGitError(ctx, "handleRepoViewSubmodule: ParseRepositoryURL", err)
return
}
submoduleURL := giturl.MakeRepositoryWebLink(submoduleRepoURL)
Expand Down
4 changes: 3 additions & 1 deletion services/repository/files/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,9 @@ func newTreeViewNodeFromEntry(ctx context.Context, repoLink string, renderedIcon
} else if subModule != nil {
submoduleFile := git.NewCommitSubmoduleFile(repoLink, node.FullPath, subModule.URL, entry.ID.String())
webLink := submoduleFile.SubmoduleWebLinkTree(ctx)
node.SubmoduleURL = webLink.CommitWebLink
if webLink != nil {
node.SubmoduleURL = webLink.CommitWebLink
}
}
}

Expand Down