From 6da194fae8ef241b2349d00df28f4e83975e2efa Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 21 Oct 2024 15:48:22 +0200 Subject: [PATCH] fix: make syncronize tags to database handle annoted tags - When an admin wants syncronize tags in the Git data to the database via the admin dashboard all annoted tags loses their title. This was caused because the code didn't correctly handle annoted tags. Annoted tags have their own objectID to store the annoted message, unlike 'normal' tags which point to the commitID. While the function was being run for annoted tags, the code thought it found a mismatch in the objectIDs, because the stored version was actually correct which pointed to the commitID but the code found the objectID of the annoted tag. - Make `SyncReleasesWithTags` corectly handle annoted tags. - Added unit and integration tests. - Resolves #5628 --- modules/git/repo_commit.go | 4 ++- modules/git/repo_commit_test.go | 37 +++++++++++++++++++++ modules/repository/repo.go | 4 +-- tests/integration/repo_tag_test.go | 52 ++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 3 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 1f3d64fe03..16ecb713a0 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -22,7 +22,9 @@ func (repo *Repository) GetBranchCommitID(name string) (string, error) { return repo.GetRefCommitID(BranchPrefix + name) } -// GetTagCommitID returns last commit ID string of given tag. +// GetTagCommitID returns last commit ID string of given tag. If the tag is an +// annoted tag it will return the objectID of that tag instead of the commitID +// the tag is pointing to. `GetTagCommit` handles annoted tags correctly. func (repo *Repository) GetTagCommitID(name string) (string, error) { return repo.GetRefCommitID(TagPrefix + name) } diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index e2a9f97fae..6a8874fd74 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -101,3 +101,40 @@ func TestRepository_CommitsBetweenIDs(t *testing.T) { assert.Len(t, commits, c.ExpectedCommits, "case %d", i) } } + +func TestGetTagCommit(t *testing.T) { + t.Setenv("GIT_COMMITTER_DATE", "2006-01-01 13:37") + bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") + + clonedPath, err := cloneRepo(t, bareRepo1Path) + require.NoError(t, err) + + bareRepo1, err := openRepositoryWithDefaultContext(clonedPath) + require.NoError(t, err) + defer bareRepo1.Close() + + lTagCommitID := "6fbd69e9823458e6c4a2fc5c0f6bc022b2f2acd1" + lTagName := "lightweightTag" + bareRepo1.CreateTag(lTagName, lTagCommitID) + + aTagCommitID := "8006ff9adbf0cb94da7dad9e537e53817f9fa5c0" + aTagName := "annotatedTag" + aTagMessage := "my annotated message" + bareRepo1.CreateAnnotatedTag(aTagName, aTagMessage, aTagCommitID) + + aTagID, err := bareRepo1.GetTagCommitID(aTagName) + require.NoError(t, err) + assert.NotEqualValues(t, aTagCommitID, aTagID) + + lTagID, err := bareRepo1.GetTagCommitID(lTagName) + require.NoError(t, err) + assert.EqualValues(t, lTagCommitID, lTagID) + + aTag, err := bareRepo1.GetTagCommit(aTagName) + require.NoError(t, err) + assert.EqualValues(t, aTagCommitID, aTag.ID.String()) + + lTag, err := bareRepo1.GetTagCommit(lTagName) + require.NoError(t, err) + assert.EqualValues(t, lTagCommitID, lTag.ID.String()) +} diff --git a/modules/repository/repo.go b/modules/repository/repo.go index e08bc376b8..e15526d78d 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -90,11 +90,11 @@ func SyncReleasesWithTags(ctx context.Context, repo *repo_model.Repository, gitR if rel.IsDraft { continue } - commitID, err := gitRepo.GetTagCommitID(rel.TagName) + commit, err := gitRepo.GetTagCommit(rel.TagName) if err != nil && !git.IsErrNotExist(err) { return fmt.Errorf("unable to GetTagCommitID for %q in Repo[%d:%s/%s]: %w", rel.TagName, repo.ID, repo.OwnerName, repo.Name, err) } - if git.IsErrNotExist(err) || commitID != rel.Sha1 { + if git.IsErrNotExist(err) || commit.ID.String() != rel.Sha1 { if err := repo_model.PushUpdateDeleteTag(ctx, repo, rel.TagName); err != nil { return fmt.Errorf("unable to PushUpdateDeleteTag: %q in Repo[%d:%s/%s]: %w", rel.TagName, repo.ID, repo.OwnerName, repo.Name, err) } diff --git a/tests/integration/repo_tag_test.go b/tests/integration/repo_tag_test.go index d5539cb259..808d2e17e6 100644 --- a/tests/integration/repo_tag_test.go +++ b/tests/integration/repo_tag_test.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" + repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/services/release" "code.gitea.io/gitea/tests" @@ -163,3 +164,54 @@ func TestCreateNewTagProtected(t *testing.T) { require.NoError(t, err) } } + +func TestSyncRepoTags(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + t.Run("Git", func(t *testing.T) { + httpContext := NewAPITestContext(t, owner.Name, repo.Name) + + dstPath := t.TempDir() + + u.Path = httpContext.GitPath() + u.User = url.UserPassword(owner.Name, userPassword) + + doGitClone(dstPath, u)(t) + + _, _, err := git.NewCommand(git.DefaultContext, "tag", "v2", "-m", "this is an annoted tag").RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) + + _, _, err = git.NewCommand(git.DefaultContext, "push", "--tags").RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) + + testTag := func(t *testing.T) { + t.Helper() + req := NewRequestf(t, "GET", "/%s/releases/tag/v2", repo.FullName()) + resp := MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + tagsTab := htmlDoc.Find(".release-list-title") + assert.Contains(t, tagsTab.Text(), "this is an annoted tag") + } + + // Make sure `SyncRepoTags` doesn't modify annoted tags. + testTag(t) + require.NoError(t, repo_module.SyncRepoTags(git.DefaultContext, repo.ID)) + testTag(t) + }) + + // Cleanup + releases, err := db.Find[repo_model.Release](db.DefaultContext, repo_model.FindReleasesOptions{ + IncludeTags: true, + TagNames: []string{"v2"}, + RepoID: repo.ID, + }) + require.NoError(t, err) + + for _, release := range releases { + _, err = db.DeleteByID[repo_model.Release](db.DefaultContext, release.ID) + require.NoError(t, err) + } + }) +}