Fix tag/release deletion

[jolheiser]

Currently you cannot delete a light tag from a repo because deleting a tag is never a "new tag".

1. Create a repo
2. Push a new light tag git tag test && git push --tags
3. Delete the tag and update it in remote git tag -d test && git push origin :test
4. Tag still shows up in release tab

[codecov-io]

Codecov Report

Merging #10663 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #10663      +/-   ##
==========================================
- Coverage    43.6%   43.58%   -0.02%     
==========================================
  Files         588      588              
  Lines       82485    82485              
==========================================
- Hits        35965    35954      -11     
- Misses      42059    42066       +7     
- Partials     4461     4465       +4

Impacted Files	Coverage Δ
modules/repofiles/update.go	38.69% <0%> (-0.39%)	⬇️
modules/indexer/stats/queue.go	62.5% <0%> (-18.75%)	⬇️
modules/indexer/stats/db.go	40.62% <0%> (-18.75%)	⬇️
modules/git/utils.go	65.67% <0%> (-4.48%)	⬇️
modules/queue/unique_queue_disk_channel.go	53.84% <0%> (-1.93%)	⬇️
services/pull/pull.go	34.7% <0%> (-1.18%)	⬇️
modules/queue/workerpool.go	56.93% <0%> (-1.07%)	⬇️
modules/log/event.go	64.61% <0%> (-1.03%)	⬇️
modules/git/repo.go	46.78% <0%> (-0.92%)	⬇️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4901096...78728a6. Read the comment docs.

[guillep2k]

Maybe this also needs a revision:

gitea/modules/repofiles/update.go

Lines 702 to 709 in 78728a6

	// If is tag reference
	tagName := opts.TagName()
	if opts.IsDelRef() {
	delTags = append(delTags, tagName)
	} else {
	cache.Remove(repo.GetCommitsCountCacheKey(tagName, true))
	addTags = append(addTags, tagName)
	}

Looks like the cache.Remove() call should be inside the if opts.IsDelRef() block rather than the else.

[jolheiser]

Maybe? I'm not sure, as back in the 1.11 branch before this was changed it was still in the latter block.

gitea/modules/repofiles/update.go

Lines 573 to 580 in 7e0a5b1

	if strings.HasPrefix(opts.RefFullName, git.TagPrefix) {
	// If is tag reference
	tagName := opts.RefFullName[len(git.TagPrefix):]
	if isDelRef {
	delTags = append(delTags, tagName)
	} else {
	cache.Remove(repo.GetCommitsCountCacheKey(tagName, true))
	addTags = append(addTags, tagName)

Same in 1.10 branch

gitea/modules/repofiles/update.go

Lines 439 to 454 in 81072af

	if strings.HasPrefix(opts.RefFullName, git.TagPrefix) {
	// If is tag reference
	tagName := opts.RefFullName[len(git.TagPrefix):]
	if isDelRef {
	err = models.PushUpdateDeleteTag(repo, tagName)
	if err != nil {
	return fmt.Errorf("PushUpdateDeleteTag: %v", err)
	}
	} else {
	// Clear cache for tag commit count
	cache.Remove(repo.GetCommitsCountCacheKey(tagName, true))
	err = models.PushUpdateAddTag(repo, gitRepo, tagName)
	if err != nil {
	return fmt.Errorf("PushUpdateAddTag: %v", err)
	}
	}

Of course, that's not to say it isn't a bug that's been around for a while. 😅

[guillep2k]

Maybe? I'm not sure, as back in the 1.11 branch before this was changed it was still in the latter block.

Yeah, but what could we be having cached about a ref that didn't exist before? Notice it's using tagName as the key. And why aren't we clearing the cache on the deleted ref?

I can't follow the code ATM, but it's truly suspicious. 🤔

[lunny]

This bug is introduced by #10105. See https://github.com/go-gitea/gitea/pull/10105/files#r389336929