add submodule diff links (#33097)

This adds links to submodules in diffs, similar to the existing link
when viewing a repo at a specific commit. It does this by expanding diff
parsing to recognize changes to submodules, and find the specific refs
that are added, deleted or changed.

Related #25888

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Rowan Bohde 2025-01-07 19:38:30 -06:00 committed by GitHub
parent ec84687df9
commit a8e7caedfa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 688 additions and 339 deletions

View file

@ -360,7 +360,6 @@ type DiffFile struct {
IsLFSFile bool
IsRenamed bool
IsAmbiguous bool
IsSubmodule bool
Sections []*DiffSection
IsIncomplete bool
IsIncompleteLineTooLong bool
@ -372,6 +371,9 @@ type DiffFile struct {
Language string
Mode string
OldMode string
IsSubmodule bool // if IsSubmodule==true, then there must be a SubmoduleDiffInfo
SubmoduleDiffInfo *SubmoduleDiffInfo
}
// GetType returns type of diff file.
@ -609,9 +611,8 @@ parsingLoop:
if strings.HasPrefix(line, "new mode ") {
curFile.Mode = prepareValue(line, "new mode ")
}
if strings.HasSuffix(line, " 160000\n") {
curFile.IsSubmodule = true
curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{}
}
case strings.HasPrefix(line, "rename from "):
curFile.IsRenamed = true
@ -646,17 +647,17 @@ parsingLoop:
curFile.Mode = prepareValue(line, "new file mode ")
}
if strings.HasSuffix(line, " 160000\n") {
curFile.IsSubmodule = true
curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{}
}
case strings.HasPrefix(line, "deleted"):
curFile.Type = DiffFileDel
curFile.IsDeleted = true
if strings.HasSuffix(line, " 160000\n") {
curFile.IsSubmodule = true
curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{}
}
case strings.HasPrefix(line, "index"):
if strings.HasSuffix(line, " 160000\n") {
curFile.IsSubmodule = true
curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{}
}
case strings.HasPrefix(line, "similarity index 100%"):
curFile.Type = DiffFileRename
@ -915,6 +916,13 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
}
}
curSection.Lines = append(curSection.Lines, diffLine)
// Parse submodule additions
if curFile.SubmoduleDiffInfo != nil {
if ref, found := bytes.CutPrefix(lineBytes, []byte("+Subproject commit ")); found {
curFile.SubmoduleDiffInfo.NewRefID = string(bytes.TrimSpace(ref))
}
}
case '-':
curFileLinesCount++
curFile.Deletion++
@ -936,6 +944,13 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
lastLeftIdx = len(curSection.Lines)
}
curSection.Lines = append(curSection.Lines, diffLine)
// Parse submodule deletion
if curFile.SubmoduleDiffInfo != nil {
if ref, found := bytes.CutPrefix(lineBytes, []byte("-Subproject commit ")); found {
curFile.SubmoduleDiffInfo.PreviousRefID = string(bytes.TrimSpace(ref))
}
}
case ' ':
curFileLinesCount++
if maxLines > -1 && curFileLinesCount >= maxLines {
@ -1195,6 +1210,11 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi
}
}
// Populate Submodule URLs
if diffFile.SubmoduleDiffInfo != nil {
diffFile.SubmoduleDiffInfo.PopulateURL(diffFile, beforeCommit, commit)
}
if !isVendored.Has() {
isVendored = optional.Some(analyze.IsVendor(diffFile.Name))
}

View file

@ -0,0 +1,65 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package gitdiff
import (
"context"
"html/template"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/htmlutil"
"code.gitea.io/gitea/modules/log"
)
type SubmoduleDiffInfo struct {
SubmoduleName string
SubmoduleFile *git.CommitSubmoduleFile // it might be nil if the submodule is not found or unable to parse
NewRefID string
PreviousRefID string
}
func (si *SubmoduleDiffInfo) PopulateURL(diffFile *DiffFile, leftCommit, rightCommit *git.Commit) {
si.SubmoduleName = diffFile.Name
submoduleCommit := rightCommit // If the submodule is added or updated, check at the right commit
if diffFile.IsDeleted {
submoduleCommit = leftCommit // If the submodule is deleted, check at the left commit
}
if submoduleCommit == nil {
return
}
submodule, err := submoduleCommit.GetSubModule(diffFile.GetDiffFileName())
if err != nil {
log.Error("Unable to PopulateURL for submodule %q: GetSubModule: %v", diffFile.GetDiffFileName(), err)
return // ignore the error, do not cause 500 errors for end users
}
if submodule != nil {
si.SubmoduleFile = git.NewCommitSubmoduleFile(submodule.URL, submoduleCommit.ID.String())
}
}
func (si *SubmoduleDiffInfo) CommitRefIDLinkHTML(ctx context.Context, commitID string) template.HTML {
webLink := si.SubmoduleFile.SubmoduleWebLink(ctx, commitID)
if webLink == nil {
return htmlutil.HTMLFormat("%s", base.ShortSha(commitID))
}
return htmlutil.HTMLFormat(`<a href="%s">%s</a>`, webLink.CommitWebLink, base.ShortSha(commitID))
}
func (si *SubmoduleDiffInfo) CompareRefIDLinkHTML(ctx context.Context) template.HTML {
webLink := si.SubmoduleFile.SubmoduleWebLink(ctx, si.PreviousRefID, si.NewRefID)
if webLink == nil {
return htmlutil.HTMLFormat("%s...%s", base.ShortSha(si.PreviousRefID), base.ShortSha(si.NewRefID))
}
return htmlutil.HTMLFormat(`<a href="%s">%s...%s</a>`, webLink.CommitWebLink, base.ShortSha(si.PreviousRefID), base.ShortSha(si.NewRefID))
}
func (si *SubmoduleDiffInfo) SubmoduleRepoLinkHTML(ctx context.Context) template.HTML {
webLink := si.SubmoduleFile.SubmoduleWebLink(ctx)
if webLink == nil {
return htmlutil.HTMLFormat("%s", si.SubmoduleName)
}
return htmlutil.HTMLFormat(`<a href="%s">%s</a>`, webLink.RepoWebLink, si.SubmoduleName)
}

View file

@ -0,0 +1,236 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package gitdiff
import (
"context"
"strings"
"testing"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting"
"github.com/stretchr/testify/assert"
)
func TestParseSubmoduleInfo(t *testing.T) {
type testcase struct {
name string
gitdiff string
infos map[int]SubmoduleDiffInfo
}
tests := []testcase{
{
name: "added",
gitdiff: `diff --git a/.gitmodules b/.gitmodules
new file mode 100644
index 0000000..4ac13c1
--- /dev/null
+++ b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "gitea-mirror"]
+ path = gitea-mirror
+ url = https://gitea.com/gitea/gitea-mirror
diff --git a/gitea-mirror b/gitea-mirror
new file mode 160000
index 0000000..68972a9
--- /dev/null
+++ b/gitea-mirror
@@ -0,0 +1 @@
+Subproject commit 68972a994719ae5c74e28d8fa82fa27c23399bc8
`,
infos: map[int]SubmoduleDiffInfo{
1: {NewRefID: "68972a994719ae5c74e28d8fa82fa27c23399bc8"},
},
},
{
name: "updated",
gitdiff: `diff --git a/gitea-mirror b/gitea-mirror
index 68972a9..c8ffe77 160000
--- a/gitea-mirror
+++ b/gitea-mirror
@@ -1 +1 @@
-Subproject commit 68972a994719ae5c74e28d8fa82fa27c23399bc8
+Subproject commit c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d
`,
infos: map[int]SubmoduleDiffInfo{
0: {
PreviousRefID: "68972a994719ae5c74e28d8fa82fa27c23399bc8",
NewRefID: "c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d",
},
},
},
{
name: "rename",
gitdiff: `diff --git a/.gitmodules b/.gitmodules
index 4ac13c1..0510edd 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +1,3 @@
[submodule "gitea-mirror"]
- path = gitea-mirror
+ path = gitea
url = https://gitea.com/gitea/gitea-mirror
diff --git a/gitea-mirror b/gitea
similarity index 100%
rename from gitea-mirror
rename to gitea
`,
},
{
name: "deleted",
gitdiff: `diff --git a/.gitmodules b/.gitmodules
index 0510edd..e69de29 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +0,0 @@
-[submodule "gitea-mirror"]
- path = gitea
- url = https://gitea.com/gitea/gitea-mirror
diff --git a/gitea b/gitea
deleted file mode 160000
index c8ffe77..0000000
--- a/gitea
+++ /dev/null
@@ -1 +0,0 @@
-Subproject commit c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d
`,
infos: map[int]SubmoduleDiffInfo{
1: {
PreviousRefID: "c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d",
},
},
},
{
name: "moved and updated",
gitdiff: `diff --git a/.gitmodules b/.gitmodules
index 0510edd..bced3d8 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +1,3 @@
[submodule "gitea-mirror"]
- path = gitea
+ path = gitea-1.22
url = https://gitea.com/gitea/gitea-mirror
diff --git a/gitea b/gitea
deleted file mode 160000
index c8ffe77..0000000
--- a/gitea
+++ /dev/null
@@ -1 +0,0 @@
-Subproject commit c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d
diff --git a/gitea-1.22 b/gitea-1.22
new file mode 160000
index 0000000..8eefa1f
--- /dev/null
+++ b/gitea-1.22
@@ -0,0 +1 @@
+Subproject commit 8eefa1f6dedf2488db2c9e12c916e8e51f673160
`,
infos: map[int]SubmoduleDiffInfo{
1: {
PreviousRefID: "c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d",
},
2: {
NewRefID: "8eefa1f6dedf2488db2c9e12c916e8e51f673160",
},
},
},
{
name: "converted to file",
gitdiff: `diff --git a/.gitmodules b/.gitmodules
index 0510edd..e69de29 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +0,0 @@
-[submodule "gitea-mirror"]
- path = gitea
- url = https://gitea.com/gitea/gitea-mirror
diff --git a/gitea b/gitea
deleted file mode 160000
index c8ffe77..0000000
--- a/gitea
+++ /dev/null
@@ -1 +0,0 @@
-Subproject commit c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d
diff --git a/gitea b/gitea
new file mode 100644
index 0000000..33a9488
--- /dev/null
+++ b/gitea
@@ -0,0 +1 @@
+example
`,
infos: map[int]SubmoduleDiffInfo{
1: {
PreviousRefID: "c8ffe777cf9c5bb47a38e3e0b3a3b5de6cd8813d",
},
},
},
{
name: "converted to submodule",
gitdiff: `diff --git a/.gitmodules b/.gitmodules
index e69de29..14ee267 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "gitea"]
+ path = gitea
+ url = https://gitea.com/gitea/gitea-mirror
diff --git a/gitea b/gitea
deleted file mode 100644
index 33a9488..0000000
--- a/gitea
+++ /dev/null
@@ -1 +0,0 @@
-example
diff --git a/gitea b/gitea
new file mode 160000
index 0000000..68972a9
--- /dev/null
+++ b/gitea
@@ -0,0 +1 @@
+Subproject commit 68972a994719ae5c74e28d8fa82fa27c23399bc8
`,
infos: map[int]SubmoduleDiffInfo{
2: {
NewRefID: "68972a994719ae5c74e28d8fa82fa27c23399bc8",
},
},
},
}
for _, testcase := range tests {
testcase := testcase
t.Run(testcase.name, func(t *testing.T) {
diff, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "")
assert.NoError(t, err)
for i, expected := range testcase.infos {
actual := diff.Files[i]
assert.NotNil(t, actual)
assert.Equal(t, expected, *actual.SubmoduleDiffInfo)
}
})
}
}
func TestSubmoduleInfo(t *testing.T) {
sdi := &SubmoduleDiffInfo{
SubmoduleName: "name",
PreviousRefID: "aaaa",
NewRefID: "bbbb",
}
ctx := context.Background()
assert.EqualValues(t, "1111", sdi.CommitRefIDLinkHTML(ctx, "1111"))
assert.EqualValues(t, "aaaa...bbbb", sdi.CompareRefIDLinkHTML(ctx))
assert.EqualValues(t, "name", sdi.SubmoduleRepoLinkHTML(ctx))
sdi.SubmoduleFile = git.NewCommitSubmoduleFile("https://github.com/owner/repo", "1234")
assert.EqualValues(t, `<a href="https://github.com/owner/repo/commit/1111">1111</a>`, sdi.CommitRefIDLinkHTML(ctx, "1111"))
assert.EqualValues(t, `<a href="https://github.com/owner/repo/compare/aaaa...bbbb">aaaa...bbbb</a>`, sdi.CompareRefIDLinkHTML(ctx))
assert.EqualValues(t, `<a href="https://github.com/owner/repo">name</a>`, sdi.SubmoduleRepoLinkHTML(ctx))
}