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

@@ -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)
 }

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())
+}

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)
 				}

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)
+		}
+	})
+}