diff options
author | Jason Song <i@wolfogre.com> | 2024-08-09 10:40:45 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-09 02:40:45 +0000 |
commit | f4d3120f9d1de6a260a5e625b3ffa6b35a069e9b (patch) | |
tree | 9bc743cbc952a62cb4570e300cea3bfd88eebd39 | |
parent | 33cc5837a655ad544b936d4d040ca36d74092588 (diff) | |
download | gitea-f4d3120f9d1de6a260a5e625b3ffa6b35a069e9b.tar.gz gitea-f4d3120f9d1de6a260a5e625b3ffa6b35a069e9b.zip |
Fix `IsObjectExist` with gogit (#31790)
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.
-rw-r--r-- | modules/git/repo_branch_gogit.go | 21 | ||||
-rw-r--r-- | modules/git/repo_branch_nogogit.go | 2 | ||||
-rw-r--r-- | modules/git/repo_branch_test.go | 105 | ||||
-rw-r--r-- | modules/markup/html.go | 3 |
4 files changed, 121 insertions, 10 deletions
diff --git a/modules/git/repo_branch_gogit.go b/modules/git/repo_branch_gogit.go index d1ec14d811..dbc4a5fedc 100644 --- a/modules/git/repo_branch_gogit.go +++ b/modules/git/repo_branch_gogit.go @@ -14,28 +14,33 @@ import ( "github.com/go-git/go-git/v5/plumbing/storer" ) -// IsObjectExist returns true if given reference exists in the repository. +// IsObjectExist returns true if the given object exists in the repository. +// FIXME: Inconsistent behavior with nogogit edition +// Unlike the implementation of IsObjectExist in nogogit edition, it does not support short hashes here. +// For example, IsObjectExist("153f451") will return false, but it will return true in nogogit edition. +// To fix this, the solution could be adding support for short hashes in gogit edition if it's really needed. func (repo *Repository) IsObjectExist(name string) bool { if name == "" { return false } - _, err := repo.gogitRepo.ResolveRevision(plumbing.Revision(name)) - + _, err := repo.gogitRepo.Object(plumbing.AnyObject, plumbing.NewHash(name)) return err == nil } // IsReferenceExist returns true if given reference exists in the repository. +// FIXME: Inconsistent behavior with nogogit edition +// Unlike the implementation of IsObjectExist in nogogit edition, it does not support blob hashes here. +// For example, IsObjectExist([existing_blob_hash]) will return false, but it will return true in nogogit edition. +// To fix this, the solution could be refusing to support blob hashes in nogogit edition since a blob hash is not a reference. func (repo *Repository) IsReferenceExist(name string) bool { if name == "" { return false } - reference, err := repo.gogitRepo.Reference(plumbing.ReferenceName(name), true) - if err != nil { - return false - } - return reference.Type() != plumbing.InvalidReference + _, err := repo.gogitRepo.ResolveRevision(plumbing.Revision(name)) + + return err == nil } // IsBranchExist returns true if given branch exists in current repository. diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index 470faebe25..63d0f7268a 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -16,7 +16,7 @@ import ( "code.gitea.io/gitea/modules/log" ) -// IsObjectExist returns true if given reference exists in the repository. +// IsObjectExist returns true if the given object exists in the repository. func (repo *Repository) IsObjectExist(name string) bool { if name == "" { return false diff --git a/modules/git/repo_branch_test.go b/modules/git/repo_branch_test.go index fe788946e5..009c545832 100644 --- a/modules/git/repo_branch_test.go +++ b/modules/git/repo_branch_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestRepository_GetBranches(t *testing.T) { @@ -94,3 +95,107 @@ func BenchmarkGetRefsBySha(b *testing.B) { _, _ = bareRepo5.GetRefsBySha("c83380d7056593c51a699d12b9c00627bd5743e9", "") _, _ = bareRepo5.GetRefsBySha("58a4bcc53ac13e7ff76127e0fb518b5262bf09af", "") } + +func TestRepository_IsObjectExist(t *testing.T) { + repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare")) + require.NoError(t, err) + defer repo.Close() + + // FIXME: Inconsistent behavior between gogit and nogogit editions + // See the comment of IsObjectExist in gogit edition for more details. + supportShortHash := !isGogit + + tests := []struct { + name string + arg string + want bool + }{ + { + name: "empty", + arg: "", + want: false, + }, + { + name: "branch", + arg: "master", + want: false, + }, + { + name: "commit hash", + arg: "ce064814f4a0d337b333e646ece456cd39fab612", + want: true, + }, + { + name: "short commit hash", + arg: "ce06481", + want: supportShortHash, + }, + { + name: "blob hash", + arg: "153f451b9ee7fa1da317ab17a127e9fd9d384310", + want: true, + }, + { + name: "short blob hash", + arg: "153f451", + want: supportShortHash, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, repo.IsObjectExist(tt.arg)) + }) + } +} + +func TestRepository_IsReferenceExist(t *testing.T) { + repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare")) + require.NoError(t, err) + defer repo.Close() + + // FIXME: Inconsistent behavior between gogit and nogogit editions + // See the comment of IsReferenceExist in gogit edition for more details. + supportBlobHash := !isGogit + + tests := []struct { + name string + arg string + want bool + }{ + { + name: "empty", + arg: "", + want: false, + }, + { + name: "branch", + arg: "master", + want: true, + }, + { + name: "commit hash", + arg: "ce064814f4a0d337b333e646ece456cd39fab612", + want: true, + }, + { + name: "short commit hash", + arg: "ce06481", + want: true, + }, + { + name: "blob hash", + arg: "153f451b9ee7fa1da317ab17a127e9fd9d384310", + want: supportBlobHash, + }, + { + name: "short blob hash", + arg: "153f451", + want: supportBlobHash, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, repo.IsReferenceExist(tt.arg)) + }) + } +} diff --git a/modules/markup/html.go b/modules/markup/html.go index b8069d459a..8d3327c49e 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -1144,7 +1144,8 @@ func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { }) } - exist = ctx.GitRepo.IsObjectExist(hash) + // Don't use IsObjectExist since it doesn't support short hashs with gogit edition. + exist = ctx.GitRepo.IsReferenceExist(hash) ctx.ShaExistCache[hash] = exist } |