Skip to content

Commit 144648a

Browse files
GiteaBotwolfogre
andauthored
Fix IsObjectExist with gogit (#31790) (#31806)
Backport #31790 by @wolfogre Fix #31271. When gogit is enabled, `IsObjectExist` calls `repo.gogitRepo.ResolveRevision`, which is not correct. It's for checking references not objects, it could work with commit hash since it's both a valid reference and a commit object, but it doesn't work with blob objects. So it causes #31271 because it reports that all blob objects do not exist. Co-authored-by: Jason Song <[email protected]>
1 parent 8d11946 commit 144648a

File tree

4 files changed

+121
-10
lines changed

4 files changed

+121
-10
lines changed

modules/git/repo_branch_gogit.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,33 @@ import (
1414
"github.com/go-git/go-git/v5/plumbing/storer"
1515
)
1616

17-
// IsObjectExist returns true if given reference exists in the repository.
17+
// IsObjectExist returns true if the given object exists in the repository.
18+
// FIXME: Inconsistent behavior with nogogit edition
19+
// Unlike the implementation of IsObjectExist in nogogit edition, it does not support short hashes here.
20+
// For example, IsObjectExist("153f451") will return false, but it will return true in nogogit edition.
21+
// To fix this, the solution could be adding support for short hashes in gogit edition if it's really needed.
1822
func (repo *Repository) IsObjectExist(name string) bool {
1923
if name == "" {
2024
return false
2125
}
2226

23-
_, err := repo.gogitRepo.ResolveRevision(plumbing.Revision(name))
24-
27+
_, err := repo.gogitRepo.Object(plumbing.AnyObject, plumbing.NewHash(name))
2528
return err == nil
2629
}
2730

2831
// IsReferenceExist returns true if given reference exists in the repository.
32+
// FIXME: Inconsistent behavior with nogogit edition
33+
// Unlike the implementation of IsObjectExist in nogogit edition, it does not support blob hashes here.
34+
// For example, IsObjectExist([existing_blob_hash]) will return false, but it will return true in nogogit edition.
35+
// To fix this, the solution could be refusing to support blob hashes in nogogit edition since a blob hash is not a reference.
2936
func (repo *Repository) IsReferenceExist(name string) bool {
3037
if name == "" {
3138
return false
3239
}
3340

34-
reference, err := repo.gogitRepo.Reference(plumbing.ReferenceName(name), true)
35-
if err != nil {
36-
return false
37-
}
38-
return reference.Type() != plumbing.InvalidReference
41+
_, err := repo.gogitRepo.ResolveRevision(plumbing.Revision(name))
42+
43+
return err == nil
3944
}
4045

4146
// IsBranchExist returns true if given branch exists in current repository.

modules/git/repo_branch_nogogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
"code.gitea.io/gitea/modules/log"
1717
)
1818

19-
// IsObjectExist returns true if given reference exists in the repository.
19+
// IsObjectExist returns true if the given object exists in the repository.
2020
func (repo *Repository) IsObjectExist(name string) bool {
2121
if name == "" {
2222
return false

modules/git/repo_branch_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
1112
)
1213

1314
func TestRepository_GetBranches(t *testing.T) {
@@ -94,3 +95,107 @@ func BenchmarkGetRefsBySha(b *testing.B) {
9495
_, _ = bareRepo5.GetRefsBySha("c83380d7056593c51a699d12b9c00627bd5743e9", "")
9596
_, _ = bareRepo5.GetRefsBySha("58a4bcc53ac13e7ff76127e0fb518b5262bf09af", "")
9697
}
98+
99+
func TestRepository_IsObjectExist(t *testing.T) {
100+
repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare"))
101+
require.NoError(t, err)
102+
defer repo.Close()
103+
104+
// FIXME: Inconsistent behavior between gogit and nogogit editions
105+
// See the comment of IsObjectExist in gogit edition for more details.
106+
supportShortHash := !isGogit
107+
108+
tests := []struct {
109+
name string
110+
arg string
111+
want bool
112+
}{
113+
{
114+
name: "empty",
115+
arg: "",
116+
want: false,
117+
},
118+
{
119+
name: "branch",
120+
arg: "master",
121+
want: false,
122+
},
123+
{
124+
name: "commit hash",
125+
arg: "ce064814f4a0d337b333e646ece456cd39fab612",
126+
want: true,
127+
},
128+
{
129+
name: "short commit hash",
130+
arg: "ce06481",
131+
want: supportShortHash,
132+
},
133+
{
134+
name: "blob hash",
135+
arg: "153f451b9ee7fa1da317ab17a127e9fd9d384310",
136+
want: true,
137+
},
138+
{
139+
name: "short blob hash",
140+
arg: "153f451",
141+
want: supportShortHash,
142+
},
143+
}
144+
for _, tt := range tests {
145+
t.Run(tt.name, func(t *testing.T) {
146+
assert.Equal(t, tt.want, repo.IsObjectExist(tt.arg))
147+
})
148+
}
149+
}
150+
151+
func TestRepository_IsReferenceExist(t *testing.T) {
152+
repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare"))
153+
require.NoError(t, err)
154+
defer repo.Close()
155+
156+
// FIXME: Inconsistent behavior between gogit and nogogit editions
157+
// See the comment of IsReferenceExist in gogit edition for more details.
158+
supportBlobHash := !isGogit
159+
160+
tests := []struct {
161+
name string
162+
arg string
163+
want bool
164+
}{
165+
{
166+
name: "empty",
167+
arg: "",
168+
want: false,
169+
},
170+
{
171+
name: "branch",
172+
arg: "master",
173+
want: true,
174+
},
175+
{
176+
name: "commit hash",
177+
arg: "ce064814f4a0d337b333e646ece456cd39fab612",
178+
want: true,
179+
},
180+
{
181+
name: "short commit hash",
182+
arg: "ce06481",
183+
want: true,
184+
},
185+
{
186+
name: "blob hash",
187+
arg: "153f451b9ee7fa1da317ab17a127e9fd9d384310",
188+
want: supportBlobHash,
189+
},
190+
{
191+
name: "short blob hash",
192+
arg: "153f451",
193+
want: supportBlobHash,
194+
},
195+
}
196+
for _, tt := range tests {
197+
t.Run(tt.name, func(t *testing.T) {
198+
assert.Equal(t, tt.want, repo.IsReferenceExist(tt.arg))
199+
})
200+
}
201+
}

modules/markup/html.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1159,7 +1159,8 @@ func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) {
11591159
})
11601160
}
11611161

1162-
exist = ctx.GitRepo.IsObjectExist(hash)
1162+
// Don't use IsObjectExist since it doesn't support short hashs with gogit edition.
1163+
exist = ctx.GitRepo.IsReferenceExist(hash)
11631164
ctx.ShaExistCache[hash] = exist
11641165
}
11651166

0 commit comments

Comments
 (0)