Merge branch 'main' into lunny/remove_repo_ref_in_commit

Author: lunny

models/issues/pull.go

@@ -417,10 +417,6 @@ func (pr *PullRequest) GetGitHeadRefName() string {
 	return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index)
 }
 
-func (pr *PullRequest) GetGitHeadBranchRefName() string {
-	return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch)
-}
-
 // GetReviewCommentsCount returns the number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR)
 func (pr *PullRequest) GetReviewCommentsCount(ctx context.Context) int {
 	opts := FindCommentsOptions{
@@ -646,9 +642,8 @@ func (pr *PullRequest) UpdateCols(ctx context.Context, cols ...string) error {
 }
 
 // UpdateColsIfNotMerged updates specific fields of a pull request if it has not been merged
-func (pr *PullRequest) UpdateColsIfNotMerged(ctx context.Context, cols ...string) error {
-	_, err := db.GetEngine(ctx).Where("id = ? AND has_merged = ?", pr.ID, false).Cols(cols...).Update(pr)
-	return err
+func (pr *PullRequest) UpdateColsIfNotMerged(ctx context.Context, cols ...string) (int64, error) {
+	return db.GetEngine(ctx).Where("id = ? AND has_merged = ?", pr.ID, false).Cols(cols...).Update(pr)
 }
 
 // IsWorkInProgress determine if the Pull Request is a Work In Progress by its title

models/migrations/v1_12/v136.go

@@ -6,11 +6,10 @@ package v1_12
 import (
 	"fmt"
 	"math"
-	"path/filepath"
-	"strings"
 	"time"
 
-	"code.gitea.io/gitea/modules/git"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -85,12 +84,9 @@ func AddCommitDivergenceToPulls(x *xorm.Engine) error {
 				log.Error("Missing base repo with id %d for PR ID %d", pr.BaseRepoID, pr.ID)
 				continue
 			}
-			userPath := filepath.Join(setting.RepoRootPath, strings.ToLower(baseRepo.OwnerName))
-			repoPath := filepath.Join(userPath, strings.ToLower(baseRepo.Name)+".git")
-
+			repoStore := repo_model.StorageRepo(repo_model.RelativePath(baseRepo.OwnerName, baseRepo.Name))
 			gitRefName := fmt.Sprintf("refs/pull/%d/head", pr.Index)
-
-			divergence, err := git.GetDivergingCommits(graceful.GetManager().HammerContext(), repoPath, pr.BaseBranch, gitRefName)
+			divergence, err := gitrepo.GetDivergingCommits(graceful.GetManager().HammerContext(), repoStore, pr.BaseBranch, gitRefName)
 			if err != nil {
 				log.Warn("Could not recalculate Divergence for pull: %d", pr.ID)
 				pr.CommitsAhead = 0

modules/actions/workflows.go

@@ -377,20 +377,28 @@ func matchIssuesEvent(issuePayload *api.IssuePayload, evt *jobparser.Event) bool
 			// Actions with the same name:
 			// opened, edited, closed, reopened, assigned, unassigned, milestoned, demilestoned
 			// Actions need to be converted:
-			// label_updated -> labeled
+			// label_updated -> labeled (when adding) or unlabeled (when removing)
 			// label_cleared -> unlabeled
 			// Unsupported activity types:
 			// deleted, transferred, pinned, unpinned, locked, unlocked
 
-			action := issuePayload.Action
-			switch action {
+			actions := []string{}
+			switch issuePayload.Action {
 			case api.HookIssueLabelUpdated:
-				action = "labeled"
+				if len(issuePayload.Changes.AddedLabels) > 0 {
+					actions = append(actions, "labeled")
+				}
+				if len(issuePayload.Changes.RemovedLabels) > 0 {
+					actions = append(actions, "unlabeled")
+				}
 			case api.HookIssueLabelCleared:
-				action = "unlabeled"
+				actions = append(actions, "unlabeled")
+			default:
+				actions = append(actions, string(issuePayload.Action))
 			}
+
 			for _, val := range vals {
-				if glob.MustCompile(val, '/').Match(string(action)) {
+				if slices.ContainsFunc(actions, glob.MustCompile(val, '/').Match) {
 					matchTimes++
 					break
 				}

modules/actions/workflows_test.go

@@ -154,3 +154,184 @@ func TestDetectMatched(t *testing.T) {
 		})
 	}
 }
+
+func TestMatchIssuesEvent(t *testing.T) {
+	testCases := []struct {
+		desc      string
+		payload   *api.IssuePayload
+		yamlOn    string
+		expected  bool
+		eventType string
+	}{
+		{
+			desc: "Label deletion should trigger unlabeled event",
+			payload: &api.IssuePayload{
+				Action: api.HookIssueLabelUpdated,
+				Issue: &api.Issue{
+					Labels: []*api.Label{},
+				},
+				Changes: &api.ChangesPayload{
+					RemovedLabels: []*api.Label{
+						{ID: 123, Name: "deleted-label"},
+					},
+				},
+			},
+			yamlOn:    "on:\n  issues:\n    types: [unlabeled]",
+			expected:  true,
+			eventType: "unlabeled",
+		},
+		{
+			desc: "Label deletion with existing labels should trigger unlabeled event",
+			payload: &api.IssuePayload{
+				Action: api.HookIssueLabelUpdated,
+				Issue: &api.Issue{
+					Labels: []*api.Label{
+						{ID: 456, Name: "existing-label"},
+					},
+				},
+				Changes: &api.ChangesPayload{
+					AddedLabels: nil,
+					RemovedLabels: []*api.Label{
+						{ID: 123, Name: "deleted-label"},
+					},
+				},
+			},
+			yamlOn:    "on:\n  issues:\n    types: [unlabeled]",
+			expected:  true,
+			eventType: "unlabeled",
+		},
+		{
+			desc: "Label addition should trigger labeled event",
+			payload: &api.IssuePayload{
+				Action: api.HookIssueLabelUpdated,
+				Issue: &api.Issue{
+					Labels: []*api.Label{
+						{ID: 123, Name: "new-label"},
+					},
+				},
+				Changes: &api.ChangesPayload{
+					AddedLabels: []*api.Label{
+						{ID: 123, Name: "new-label"},
+					},
+					RemovedLabels: []*api.Label{}, // Empty array, no labels removed
+				},
+			},
+			yamlOn:    "on:\n  issues:\n    types: [labeled]",
+			expected:  true,
+			eventType: "labeled",
+		},
+		{
+			desc: "Label clear should trigger unlabeled event",
+			payload: &api.IssuePayload{
+				Action: api.HookIssueLabelCleared,
+				Issue: &api.Issue{
+					Labels: []*api.Label{},
+				},
+			},
+			yamlOn:    "on:\n  issues:\n    types: [unlabeled]",
+			expected:  true,
+			eventType: "unlabeled",
+		},
+		{
+			desc: "Both adding and removing labels should trigger labeled event",
+			payload: &api.IssuePayload{
+				Action: api.HookIssueLabelUpdated,
+				Issue: &api.Issue{
+					Labels: []*api.Label{
+						{ID: 789, Name: "new-label"},
+					},
+				},
+				Changes: &api.ChangesPayload{
+					AddedLabels: []*api.Label{
+						{ID: 789, Name: "new-label"},
+					},
+					RemovedLabels: []*api.Label{
+						{ID: 123, Name: "deleted-label"},
+					},
+				},
+			},
+			yamlOn:    "on:\n  issues:\n    types: [labeled]",
+			expected:  true,
+			eventType: "labeled",
+		},
+		{
+			desc: "Both adding and removing labels should trigger unlabeled event",
+			payload: &api.IssuePayload{
+				Action: api.HookIssueLabelUpdated,
+				Issue: &api.Issue{
+					Labels: []*api.Label{
+						{ID: 789, Name: "new-label"},
+					},
+				},
+				Changes: &api.ChangesPayload{
+					AddedLabels: []*api.Label{
+						{ID: 789, Name: "new-label"},
+					},
+					RemovedLabels: []*api.Label{
+						{ID: 123, Name: "deleted-label"},
+					},
+				},
+			},
+			yamlOn:    "on:\n  issues:\n    types: [unlabeled]",
+			expected:  true,
+			eventType: "unlabeled",
+		},
+		{
+			desc: "Both adding and removing labels should trigger both events",
+			payload: &api.IssuePayload{
+				Action: api.HookIssueLabelUpdated,
+				Issue: &api.Issue{
+					Labels: []*api.Label{
+						{ID: 789, Name: "new-label"},
+					},
+				},
+				Changes: &api.ChangesPayload{
+					AddedLabels: []*api.Label{
+						{ID: 789, Name: "new-label"},
+					},
+					RemovedLabels: []*api.Label{
+						{ID: 123, Name: "deleted-label"},
+					},
+				},
+			},
+			yamlOn:    "on:\n  issues:\n    types: [labeled, unlabeled]",
+			expected:  true,
+			eventType: "multiple",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			evts, err := GetEventsFromContent([]byte(tc.yamlOn))
+			assert.NoError(t, err)
+			assert.Len(t, evts, 1)
+
+			// Test if the event matches as expected
+			assert.Equal(t, tc.expected, matchIssuesEvent(tc.payload, evts[0]))
+
+			// For extra validation, check that action mapping works correctly
+			if tc.eventType == "multiple" {
+				// Skip direct action mapping validation for multiple events case
+				// as one action can map to multiple event types
+				return
+			}
+
+			// Determine expected action for single event case
+			var expectedAction string
+			switch tc.payload.Action {
+			case api.HookIssueLabelUpdated:
+				if tc.eventType == "labeled" {
+					expectedAction = "labeled"
+				} else if tc.eventType == "unlabeled" {
+					expectedAction = "unlabeled"
+				}
+			case api.HookIssueLabelCleared:
+				expectedAction = "unlabeled"
+			default:
+				expectedAction = string(tc.payload.Action)
+			}
+
+			assert.Equal(t, expectedAction, tc.eventType, "Event type should match expected")
+		})
+	}
+}

modules/git/repo.go

@@ -243,36 +243,6 @@ func GetLatestCommitTime(ctx context.Context, repoPath string) (time.Time, error
 	return time.Parse("Mon Jan _2 15:04:05 2006 -0700", commitTime)
 }
 
-// DivergeObject represents commit count diverging commits
-type DivergeObject struct {
-	Ahead  int
-	Behind int
-}
-
-// GetDivergingCommits returns the number of commits a targetBranch is ahead or behind a baseBranch
-func GetDivergingCommits(ctx context.Context, repoPath, baseBranch, targetBranch string) (do DivergeObject, err error) {
-	cmd := gitcmd.NewCommand("rev-list", "--count", "--left-right").
-		AddDynamicArguments(baseBranch + "..." + targetBranch).AddArguments("--")
-	stdout, _, err := cmd.RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath})
-	if err != nil {
-		return do, err
-	}
-	left, right, found := strings.Cut(strings.Trim(stdout, "\n"), "\t")
-	if !found {
-		return do, fmt.Errorf("git rev-list output is missing a tab: %q", stdout)
-	}
-
-	do.Behind, err = strconv.Atoi(left)
-	if err != nil {
-		return do, err
-	}
-	do.Ahead, err = strconv.Atoi(right)
-	if err != nil {
-		return do, err
-	}
-	return do, nil
-}
-
 // CreateBundle create bundle content to the target path
 func (repo *Repository) CreateBundle(ctx context.Context, commit string, out io.Writer) error {
 	tmp, cleanup, err := setting.AppDataTempDir("git-repo-content").MkdirTempRandom("gitea-bundle")

modules/git/repo_commit_gogit.go

@@ -38,16 +38,6 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
 	return ref.Hash().String(), nil
 }
 
-// SetReference sets the commit ID string of given reference (e.g. branch or tag).
-func (repo *Repository) SetReference(name, commitID string) error {
-	return repo.gogitRepo.Storer.SetReference(plumbing.NewReferenceFromStrings(name, commitID))
-}
-
-// RemoveReference removes the given reference (e.g. branch or tag).
-func (repo *Repository) RemoveReference(name string) error {
-	return repo.gogitRepo.Storer.RemoveReference(plumbing.ReferenceName(name))
-}
-
 // ConvertToHash returns a Hash object from a potential ID string
 func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
 	objectFormat, err := repo.GetObjectFormat()

modules/git/repo_commit_nogogit.go

@@ -51,18 +51,6 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
 	return string(shaBs), nil
 }
 
-// SetReference sets the commit ID string of given reference (e.g. branch or tag).
-func (repo *Repository) SetReference(name, commitID string) error {
-	_, _, err := gitcmd.NewCommand("update-ref").AddDynamicArguments(name, commitID).RunStdString(repo.Ctx, &gitcmd.RunOpts{Dir: repo.Path})
-	return err
-}
-
-// RemoveReference removes the given reference (e.g. branch or tag).
-func (repo *Repository) RemoveReference(name string) error {
-	_, _, err := gitcmd.NewCommand("update-ref", "--no-deref", "-d").AddDynamicArguments(name).RunStdString(repo.Ctx, &gitcmd.RunOpts{Dir: repo.Path})
-	return err
-}
-
 // IsCommitExist returns true if given commit exists in current repository.
 func (repo *Repository) IsCommitExist(name string) bool {
 	if err := ensureValidGitRepository(repo.Ctx, repo.Path); err != nil {

modules/git/repo_compare_test.go

@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"code.gitea.io/gitea/modules/git/gitcmd"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -99,7 +101,9 @@ func TestReadWritePullHead(t *testing.T) {
 
 	// Write a fake sha1 with only 40 zeros
 	newCommit := "feaf4ba6bc635fec442f46ddd4512416ec43c2c2"
-	err = repo.SetReference(PullPrefix+"1/head", newCommit)
+	_, _, err = gitcmd.NewCommand("update-ref").
+		AddDynamicArguments(PullPrefix+"1/head", newCommit).
+		RunStdString(t.Context(), &gitcmd.RunOpts{Dir: repo.Path})
 	if err != nil {
 		assert.NoError(t, err)
 		return
@@ -116,7 +120,9 @@ func TestReadWritePullHead(t *testing.T) {
 	assert.Equal(t, headContents, newCommit)
 
 	// Remove file after the test
-	err = repo.RemoveReference(PullPrefix + "1/head")
+	_, _, err = gitcmd.NewCommand("update-ref", "--no-deref", "-d").
+		AddDynamicArguments(PullPrefix+"1/head").
+		RunStdString(t.Context(), &gitcmd.RunOpts{Dir: repo.Path})
 	assert.NoError(t, err)
 }