Merge pull request 'fix: make syncronize tags to database handle annoted tags' (#5641) from gusted/forgejo-sync-tags into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5641
Reviewed-by: Otto <otto@codeberg.org>

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