Merge branch 'go-gitea:main' into main

Author: badhezi

models/git/commit_status.go

@@ -230,18 +230,24 @@ func (status *CommitStatus) HideActionsURL(ctx context.Context) {
 
 // CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc
 func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus {
+	// This function is widely used, but it is not quite right.
+	// Ideally it should return something like "CommitStatusSummary" with properly aggregated state.
+	// GitHub's behavior: if all statuses are "skipped", GitHub will return "success" as the combined status.
 	var lastStatus *CommitStatus
 	state := api.CommitStatusSuccess
 	for _, status := range statuses {
-		if status.State.NoBetterThan(state) {
+		if state == status.State || status.State.HasHigherPriorityThan(state) {
 			state = status.State
 			lastStatus = status
 		}
 	}
 	if lastStatus == nil {
 		if len(statuses) > 0 {
+			// FIXME: a bad case: Gitea just returns the first commit status, its status is "skipped" in this case.
 			lastStatus = statuses[0]
 		} else {
+			// FIXME: another bad case: if the "statuses" slice is empty, the returned value is an invalid CommitStatus, all its fields are empty.
+			// Frontend code (tmpl&vue) sometimes depend on the empty fields to skip rendering commit status elements (need to double check in the future)
 			lastStatus = &CommitStatus{}
 		}
 	}

models/git/commit_status_summary.go

@@ -59,7 +59,11 @@ func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) er
 	if err != nil {
 		return err
 	}
-	state := CalcCommitStatus(commitStatuses)
+	// it guarantees that commitStatuses is not empty because this function is always called after a commit status is created
+	if len(commitStatuses) == 0 {
+		setting.PanicInDevOrTesting("no commit statuses found for repo %d and sha %s", repoID, sha)
+	}
+	state := CalcCommitStatus(commitStatuses) // non-empty commitStatuses is guaranteed
 	// mysql will return 0 when update a record which state hasn't been changed which behaviour is different from other database,
 	// so we need to use insert in on duplicate
 	if setting.Database.Type.IsMySQL() {

models/issues/issue_update.go

@@ -12,9 +12,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
 	access_model "code.gitea.io/gitea/models/perm/access"
-	project_model "code.gitea.io/gitea/models/project"
 	repo_model "code.gitea.io/gitea/models/repo"
-	system_model "code.gitea.io/gitea/models/system"
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
@@ -715,138 +713,13 @@ func UpdateReactionsMigrationsByType(ctx context.Context, gitServiceType api.Git
 	return err
 }
 
-// DeleteIssuesByRepoID deletes issues by repositories id
-func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []string, err error) {
-	// MariaDB has a performance bug: https://jira.mariadb.org/browse/MDEV-16289
-	// so here it uses "DELETE ... WHERE IN" with pre-queried IDs.
-	sess := db.GetEngine(ctx)
-
-	for {
-		issueIDs := make([]int64, 0, db.DefaultMaxInSize)
-
-		err := sess.Table(&Issue{}).Where("repo_id = ?", repoID).OrderBy("id").Limit(db.DefaultMaxInSize).Cols("id").Find(&issueIDs)
-		if err != nil {
-			return nil, err
-		}
-
-		if len(issueIDs) == 0 {
-			break
-		}
-
-		// Delete content histories
-		_, err = sess.In("issue_id", issueIDs).Delete(&ContentHistory{})
-		if err != nil {
-			return nil, err
-		}
-
-		// Delete comments and attachments
-		_, err = sess.In("issue_id", issueIDs).Delete(&Comment{})
-		if err != nil {
-			return nil, err
-		}
-
-		// Dependencies for issues in this repository
-		_, err = sess.In("issue_id", issueIDs).Delete(&IssueDependency{})
-		if err != nil {
-			return nil, err
-		}
-
-		// Delete dependencies for issues in other repositories
-		_, err = sess.In("dependency_id", issueIDs).Delete(&IssueDependency{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&IssueUser{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&Reaction{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&IssueWatch{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&Stopwatch{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&TrackedTime{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&project_model.ProjectIssue{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("dependent_issue_id", issueIDs).Delete(&Comment{})
-		if err != nil {
-			return nil, err
-		}
-
-		var attachments []*repo_model.Attachment
-		err = sess.In("issue_id", issueIDs).Find(&attachments)
-		if err != nil {
-			return nil, err
-		}
-
-		for j := range attachments {
-			attachmentPaths = append(attachmentPaths, attachments[j].RelativePath())
-		}
-
-		_, err = sess.In("issue_id", issueIDs).Delete(&repo_model.Attachment{})
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = sess.In("id", issueIDs).Delete(&Issue{})
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return attachmentPaths, err
-}
-
-// DeleteOrphanedIssues delete issues without a repo
-func DeleteOrphanedIssues(ctx context.Context) error {
-	var attachmentPaths []string
-	err := db.WithTx(ctx, func(ctx context.Context) error {
-		var ids []int64
-
-		if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id").
-			Join("LEFT", "repository", "issue.repo_id=repository.id").
-			Where(builder.IsNull{"repository.id"}).GroupBy("issue.repo_id").
-			Find(&ids); err != nil {
-			return err
-		}
-
-		for i := range ids {
-			paths, err := DeleteIssuesByRepoID(ctx, ids[i])
-			if err != nil {
-				return err
-			}
-			attachmentPaths = append(attachmentPaths, paths...)
-		}
-
-		return nil
-	})
-	if err != nil {
-		return err
-	}
-
-	// Remove issue attachment files.
-	for i := range attachmentPaths {
-		// FIXME: it's not right, because the attachment might not be on local filesystem
-		system_model.RemoveAllWithNotice(ctx, "Delete issue attachment", attachmentPaths[i])
+func GetOrphanedIssueRepoIDs(ctx context.Context) ([]int64, error) {
+	var repoIDs []int64
+	if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id").
+		Join("LEFT", "repository", "issue.repo_id=repository.id").
+		Where(builder.IsNull{"repository.id"}).
+		Find(&repoIDs); err != nil {
+		return nil, err
 	}
-	return nil
+	return repoIDs, nil
 }

modules/actions/workflows.go

@@ -43,21 +43,23 @@ func IsWorkflow(path string) bool {
 	return strings.HasPrefix(path, ".gitea/workflows") || strings.HasPrefix(path, ".github/workflows")
 }
 
-func ListWorkflows(commit *git.Commit) (git.Entries, error) {
-	tree, err := commit.SubTree(".gitea/workflows")
+func ListWorkflows(commit *git.Commit) (string, git.Entries, error) {
+	rpath := ".gitea/workflows"
+	tree, err := commit.SubTree(rpath)
 	if _, ok := err.(git.ErrNotExist); ok {
-		tree, err = commit.SubTree(".github/workflows")
+		rpath = ".github/workflows"
+		tree, err = commit.SubTree(rpath)
 	}
 	if _, ok := err.(git.ErrNotExist); ok {
-		return nil, nil
+		return "", nil, nil
 	}
 	if err != nil {
-		return nil, err
+		return "", nil, err
 	}
 
 	entries, err := tree.ListEntriesRecursiveFast()
 	if err != nil {
-		return nil, err
+		return "", nil, err
 	}
 
 	ret := make(git.Entries, 0, len(entries))
@@ -66,7 +68,7 @@ func ListWorkflows(commit *git.Commit) (git.Entries, error) {
 			ret = append(ret, entry)
 		}
 	}
-	return ret, nil
+	return rpath, ret, nil
 }
 
 func GetContentFromEntry(entry *git.TreeEntry) ([]byte, error) {
@@ -102,7 +104,7 @@ func DetectWorkflows(
 	payload api.Payloader,
 	detectSchedule bool,
 ) ([]*DetectedWorkflow, []*DetectedWorkflow, error) {
-	entries, err := ListWorkflows(commit)
+	_, entries, err := ListWorkflows(commit)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -147,7 +149,7 @@ func DetectWorkflows(
 }
 
 func DetectScheduledWorkflows(gitRepo *git.Repository, commit *git.Commit) ([]*DetectedWorkflow, error) {
-	entries, err := ListWorkflows(commit)
+	_, entries, err := ListWorkflows(commit)
 	if err != nil {
 		return nil, err
 	}

modules/structs/commit_status.go

@@ -18,6 +18,8 @@ const (
 	CommitStatusFailure CommitStatusState = "failure"
 	// CommitStatusWarning is for when the CommitStatus is Warning
 	CommitStatusWarning CommitStatusState = "warning"
+	// CommitStatusSkipped is for when CommitStatus is Skipped
+	CommitStatusSkipped CommitStatusState = "skipped"
 )
 
 var commitStatusPriorities = map[CommitStatusState]int{
@@ -26,25 +28,17 @@ var commitStatusPriorities = map[CommitStatusState]int{
 	CommitStatusWarning: 2,
 	CommitStatusPending: 3,
 	CommitStatusSuccess: 4,
+	CommitStatusSkipped: 5,
 }
 
 func (css CommitStatusState) String() string {
 	return string(css)
 }
 
-// NoBetterThan returns true if this State is no better than the given State
-// This function only handles the states defined in CommitStatusPriorities
-func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool {
-	// NoBetterThan only handles the 5 states above
-	if _, exist := commitStatusPriorities[css]; !exist {
-		return false
-	}
-
-	if _, exist := commitStatusPriorities[css2]; !exist {
-		return false
-	}
-
-	return commitStatusPriorities[css] <= commitStatusPriorities[css2]
+// HasHigherPriorityThan returns true if this state has higher priority than the other
+// Undefined states are considered to have the highest priority like CommitStatusError(0)
+func (css CommitStatusState) HasHigherPriorityThan(other CommitStatusState) bool {
+	return commitStatusPriorities[css] < commitStatusPriorities[other]
 }
 
 // IsPending represents if commit status state is pending