Skip to content

Commit 12bfa9e

Browse files
wxiaoguangwxiaoguang
authored
Improve submodule relative path handling (#35056) (#35075)
Backport #35056
1 parent dd661e9 commit 12bfa9e

File tree

19 files changed

+99
-102
lines changed

19 files changed

+99
-102
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ been added to each release, please refer to the [blog](https://blog.gitea.com).
77
## [1.24.3](https://github.com/go-gitea/gitea/releases/tag/1.24.3) - 2025-07-12
88

99
* BUGFIXES
10+
* Improve submodule relative path handling (#35056) (#35075)
1011
* Fix incorrect comment diff hunk parsing, fix github asset ID nil panic (#35046) (#35055)
1112
* Fix updating user visibility (#35036) (#35044)
1213
* Support base64-encoded agit push options (#35037) (#35041)

‎modules/git/commit_info.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,20 @@
33

44
package git
55

6+
import "path"
7+
68
// CommitInfo describes the first commit with the provided entry
79
type CommitInfo struct {
810
Entry *TreeEntry
911
Commit *Commit
1012
SubmoduleFile *CommitSubmoduleFile
1113
}
14+
15+
func getCommitInfoSubmoduleFile(repoLink string, entry *TreeEntry, commit *Commit, treePathDir string) (*CommitSubmoduleFile, error) {
16+
fullPath := path.Join(treePathDir, entry.Name())
17+
submodule, err := commit.GetSubModule(fullPath)
18+
if err != nil {
19+
return nil, err
20+
}
21+
return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, entry.ID.String()), nil
22+
}

‎modules/git/commit_info_gogit.go‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
)
1717

1818
// GetCommitsInfo gets information of all commits that are corresponding to these entries
19-
func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
19+
func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
2020
entryPaths := make([]string, len(tes)+1)
2121
// Get the commit for the treePath itself
2222
entryPaths[0] = ""
@@ -71,22 +71,12 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
7171
commitsInfo[i].Commit = entryCommit
7272
}
7373

74-
// If the entry is a submodule add a submodule file for this
74+
// If the entry is a submodule, add a submodule file for this
7575
if entry.IsSubModule() {
76-
subModuleURL := ""
77-
var fullPath string
78-
if len(treePath) > 0 {
79-
fullPath = treePath + "/" + entry.Name()
80-
} else {
81-
fullPath = entry.Name()
82-
}
83-
if subModule, err := commit.GetSubModule(fullPath); err != nil {
76+
commitsInfo[i].SubmoduleFile, err = getCommitInfoSubmoduleFile(repoLink, entry, commit, treePath)
77+
if err != nil {
8478
return nil, nil, err
85-
} else if subModule != nil {
86-
subModuleURL = subModule.URL
8779
}
88-
subModuleFile := NewCommitSubmoduleFile(subModuleURL, entry.ID.String())
89-
commitsInfo[i].SubmoduleFile = subModuleFile
9080
}
9181
}
9282

‎modules/git/commit_info_nogogit.go‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
)
1717

1818
// GetCommitsInfo gets information of all commits that are corresponding to these entries
19-
func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
19+
func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
2020
entryPaths := make([]string, len(tes)+1)
2121
// Get the commit for the treePath itself
2222
entryPaths[0] = ""
@@ -65,22 +65,12 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
6565
log.Debug("missing commit for %s", entry.Name())
6666
}
6767

68-
// If the entry is a submodule add a submodule file for this
68+
// If the entry is a submodule, add a submodule file for this
6969
if entry.IsSubModule() {
70-
subModuleURL := ""
71-
var fullPath string
72-
if len(treePath) > 0 {
73-
fullPath = treePath + "/" + entry.Name()
74-
} else {
75-
fullPath = entry.Name()
76-
}
77-
if subModule, err := commit.GetSubModule(fullPath); err != nil {
70+
commitsInfo[i].SubmoduleFile, err = getCommitInfoSubmoduleFile(repoLink, entry, commit, treePath)
71+
if err != nil {
7872
return nil, nil, err
79-
} else if subModule != nil {
80-
subModuleURL = subModule.URL
8173
}
82-
subModuleFile := NewCommitSubmoduleFile(subModuleURL, entry.ID.String())
83-
commitsInfo[i].SubmoduleFile = subModuleFile
8474
}
8575
}
8676

‎modules/git/commit_info_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func testGetCommitsInfo(t *testing.T, repo1 *Repository) {
8282
}
8383

8484
// FIXME: Context.TODO() - if graceful has started we should use its Shutdown context otherwise use install signals in TestMain.
85-
commitsInfo, treeCommit, err := entries.GetCommitsInfo(t.Context(), commit, testCase.Path)
85+
commitsInfo, treeCommit, err := entries.GetCommitsInfo(t.Context(), "/any/repo-link", commit, testCase.Path)
8686
assert.NoError(t, err, "Unable to get commit information for entries of subtree: %s in commit: %s from testcase due to error: %v", testCase.Path, testCase.CommitID, err)
8787
if err != nil {
8888
t.FailNow()
@@ -159,7 +159,7 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) {
159159
b.ResetTimer()
160160
b.Run(benchmark.name, func(b *testing.B) {
161161
for b.Loop() {
162-
_, _, err := entries.GetCommitsInfo(b.Context(), commit, "")
162+
_, _, err := entries.GetCommitsInfo(b.Context(), "/any/repo-link", commit, "")
163163
if err != nil {
164164
b.Fatal(err)
165165
}

‎modules/git/commit_submodule_file.go‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,57 +6,61 @@ package git
66

77
import (
88
"context"
9+
"path"
910
"strings"
1011

1112
giturl "code.gitea.io/gitea/modules/git/url"
13+
"code.gitea.io/gitea/modules/util"
1214
)
1315

1416
// CommitSubmoduleFile represents a file with submodule type.
1517
type CommitSubmoduleFile struct {
16-
refURL string
17-
refID string
18+
repoLink string
19+
fullPath string
20+
refURL string
21+
refID string
1822

19-
parsed bool
20-
targetRepoLink string
23+
parsed bool
24+
parsedTargetLink string
2125
}
2226

2327
// NewCommitSubmoduleFile create a new submodule file
24-
func NewCommitSubmoduleFile(refURL, refID string) *CommitSubmoduleFile {
25-
return &CommitSubmoduleFile{refURL: refURL, refID: refID}
28+
func NewCommitSubmoduleFile(repoLink, fullPath, refURL, refID string) *CommitSubmoduleFile {
29+
return &CommitSubmoduleFile{repoLink: repoLink, fullPath: fullPath, refURL: refURL, refID: refID}
2630
}
2731

2832
func (sf *CommitSubmoduleFile) RefID() string {
29-
return sf.refID // this function is only used in templates
33+
return sf.refID
3034
}
3135

32-
// SubmoduleWebLink tries to make some web links for a submodule, it also works on "nil" receiver
33-
func (sf *CommitSubmoduleFile) SubmoduleWebLink(ctx context.Context, optCommitID ...string) *SubmoduleWebLink {
36+
func (sf *CommitSubmoduleFile) getWebLinkInTargetRepo(ctx context.Context, moreLinkPath string) *SubmoduleWebLink {
3437
if sf == nil {
3538
return nil
3639
}
40+
if strings.HasPrefix(sf.refURL, "../") {
41+
targetLink := path.Join(sf.repoLink, path.Dir(sf.fullPath), sf.refURL)
42+
return &SubmoduleWebLink{RepoWebLink: targetLink, CommitWebLink: targetLink + moreLinkPath}
43+
}
3744
if !sf.parsed {
3845
sf.parsed = true
39-
if strings.HasPrefix(sf.refURL, "../") {
40-
// FIXME: when handling relative path, this logic is not right. It needs to:
41-
// 1. Remember the submodule's full path and its commit's repo home link
42-
// 2. Resolve the relative path: targetRepoLink = path.Join(repoHomeLink, path.Dir(submoduleFullPath), refURL)
43-
// Not an easy task and need to refactor related code a lot.
44-
sf.targetRepoLink = sf.refURL
45-
} else {
46-
parsedURL, err := giturl.ParseRepositoryURL(ctx, sf.refURL)
47-
if err != nil {
48-
return nil
49-
}
50-
sf.targetRepoLink = giturl.MakeRepositoryWebLink(parsedURL)
46+
parsedURL, err := giturl.ParseRepositoryURL(ctx, sf.refURL)
47+
if err != nil {
48+
return nil
5149
}
50+
sf.parsedTargetLink = giturl.MakeRepositoryWebLink(parsedURL)
5251
}
53-
var commitLink string
54-
if len(optCommitID) == 2 {
55-
commitLink = sf.targetRepoLink + "/compare/" + optCommitID[0] + "..." + optCommitID[1]
56-
} else if len(optCommitID) == 1 {
57-
commitLink = sf.targetRepoLink + "/tree/" + optCommitID[0]
58-
} else {
59-
commitLink = sf.targetRepoLink + "/tree/" + sf.refID
52+
return &SubmoduleWebLink{RepoWebLink: sf.parsedTargetLink, CommitWebLink: sf.parsedTargetLink + moreLinkPath}
53+
}
54+
55+
// SubmoduleWebLinkTree tries to make the submodule's tree link in its own repo, it also works on "nil" receiver
56+
func (sf *CommitSubmoduleFile) SubmoduleWebLinkTree(ctx context.Context, optCommitID ...string) *SubmoduleWebLink {
57+
if sf == nil {
58+
return nil
6059
}
61-
return &SubmoduleWebLink{RepoWebLink: sf.targetRepoLink, CommitWebLink: commitLink}
60+
return sf.getWebLinkInTargetRepo(ctx, "/tree/"+util.OptionalArg(optCommitID, sf.refID))
61+
}
62+
63+
// SubmoduleWebLinkCompare tries to make the submodule's compare link in its own repo, it also works on "nil" receiver
64+
func (sf *CommitSubmoduleFile) SubmoduleWebLinkCompare(ctx context.Context, commitID1, commitID2 string) *SubmoduleWebLink {
65+
return sf.getWebLinkInTargetRepo(ctx, "/compare/"+commitID1+"..."+commitID2)
6266
}

‎modules/git/commit_submodule_file_test.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,29 @@ import (
1010
)
1111

1212
func TestCommitSubmoduleLink(t *testing.T) {
13-
wl := (*CommitSubmoduleFile)(nil).SubmoduleWebLink(t.Context())
14-
assert.Nil(t, wl)
13+
assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkTree(t.Context()))
14+
assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkCompare(t.Context(), "", ""))
1515

1616
t.Run("GitHubRepo", func(t *testing.T) {
17-
sf := NewCommitSubmoduleFile("git@github.com:user/repo.git", "aaaa")
18-
19-
wl := sf.SubmoduleWebLink(t.Context())
17+
sf := NewCommitSubmoduleFile("/any/repo-link", "full-path", "git@github.com:user/repo.git", "aaaa")
18+
wl := sf.SubmoduleWebLinkTree(t.Context())
2019
assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
2120
assert.Equal(t, "https://github.com/user/repo/tree/aaaa", wl.CommitWebLink)
2221

23-
wl = sf.SubmoduleWebLink(t.Context(), "1111")
24-
assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
25-
assert.Equal(t, "https://github.com/user/repo/tree/1111", wl.CommitWebLink)
26-
27-
wl = sf.SubmoduleWebLink(t.Context(), "1111", "2222")
22+
wl = sf.SubmoduleWebLinkCompare(t.Context(), "1111", "2222")
2823
assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink)
2924
assert.Equal(t, "https://github.com/user/repo/compare/1111...2222", wl.CommitWebLink)
3025
})
3126

3227
t.Run("RelativePath", func(t *testing.T) {
33-
sf := NewCommitSubmoduleFile("../../user/repo", "aaaa")
34-
wl := sf.SubmoduleWebLink(t.Context())
35-
assert.Equal(t, "../../user/repo", wl.RepoWebLink)
36-
assert.Equal(t, "../../user/repo/tree/aaaa", wl.CommitWebLink)
28+
sf := NewCommitSubmoduleFile("/subpath/any/repo-home-link", "full-path", "../../user/repo", "aaaa")
29+
wl := sf.SubmoduleWebLinkTree(t.Context())
30+
assert.Equal(t, "/subpath/user/repo", wl.RepoWebLink)
31+
assert.Equal(t, "/subpath/user/repo/tree/aaaa", wl.CommitWebLink)
32+
33+
sf = NewCommitSubmoduleFile("/subpath/any/repo-home-link", "dir/submodule", "../../../user/repo", "aaaa")
34+
wl = sf.SubmoduleWebLinkCompare(t.Context(), "1111", "2222")
35+
assert.Equal(t, "/subpath/user/repo", wl.RepoWebLink)
36+
assert.Equal(t, "/subpath/user/repo/compare/1111...2222", wl.CommitWebLink)
3737
})
3838
}

‎routers/web/repo/commit.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ func Diff(ctx *context.Context) {
317317
maxLines, maxFiles = -1, -1
318318
}
319319

320-
diff, err := gitdiff.GetDiffForRender(ctx, gitRepo, &gitdiff.DiffOptions{
320+
diff, err := gitdiff.GetDiffForRender(ctx, ctx.Repo.RepoLink, gitRepo, &gitdiff.DiffOptions{
321321
AfterCommitID: commitID,
322322
SkipTo: ctx.FormString("skip-to"),
323323
MaxLines: maxLines,

‎routers/web/repo/compare.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ func PrepareCompareDiff(
614614

615615
fileOnly := ctx.FormBool("file-only")
616616

617-
diff, err := gitdiff.GetDiffForRender(ctx, ci.HeadGitRepo,
617+
diff, err := gitdiff.GetDiffForRender(ctx, ci.HeadRepo.Link(), ci.HeadGitRepo,
618618
&gitdiff.DiffOptions{
619619
BeforeCommitID: beforeCommitID,
620620
AfterCommitID: headCommitID,

‎routers/web/repo/pull.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
750750
diffOptions.BeforeCommitID = startCommitID
751751
}
752752

753-
diff, err := gitdiff.GetDiffForRender(ctx, gitRepo, diffOptions, files...)
753+
diff, err := gitdiff.GetDiffForRender(ctx, ctx.Repo.RepoLink, gitRepo, diffOptions, files...)
754754
if err != nil {
755755
ctx.ServerError("GetDiff", err)
756756
return

0 commit comments

Comments
 (0)