diff --git a/cmd/hook.go b/cmd/hook.go index 26b3b56053..f8e964d0c6 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -151,9 +151,6 @@ func (d *delayWriter) WriteString(s string) (n int, err error) { } func (d *delayWriter) Close() error { - if d == nil { - return nil - } stopped := d.timer.Stop() if stopped || d.buf == nil { return nil @@ -163,16 +160,6 @@ func (d *delayWriter) Close() error { return err } -type nilWriter struct{} - -func (n *nilWriter) Write(p []byte) (int, error) { - return len(p), nil -} - -func (n *nilWriter) WriteString(s string) (int, error) { - return len(s), nil -} - func parseGitHookCommitRefLine(line string) (oldCommitID, newCommitID string, refFullName git.RefName, ok bool) { fields := strings.Split(line, " ") if len(fields) != 3 { @@ -227,8 +214,7 @@ Gitea or set your environment appropriately.`, "") total := 0 lastline := 0 - var out io.Writer - out = &nilWriter{} + out := io.Discard if setting.Git.VerbosePush { if setting.Git.VerbosePushDelay > 0 { dWriter := newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay) @@ -350,12 +336,10 @@ Gitea or set your environment appropriately.`, "") return nil } - var out io.Writer - var dWriter *delayWriter - out = &nilWriter{} + out := io.Discard if setting.Git.VerbosePush { if setting.Git.VerbosePushDelay > 0 { - dWriter = newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay) + dWriter := newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay) defer dWriter.Close() out = dWriter } else { @@ -382,101 +366,62 @@ Gitea or set your environment appropriately.`, "") PushTrigger: repo_module.PushTrigger(os.Getenv(repo_module.EnvPushTrigger)), IsWiki: isWiki, } - oldCommitIDs := make([]string, hookBatchSize) - newCommitIDs := make([]string, hookBatchSize) - refFullNames := make([]git.RefName, hookBatchSize) - count := 0 - total := 0 - wasEmpty := false - masterPushed := false + + oldCommitIDs := make([]string, 0, hookBatchSize) + newCommitIDs := make([]string, 0, hookBatchSize) + refFullNames := make([]git.RefName, 0, hookBatchSize) results := make([]private.HookPostReceiveBranchResult, 0) + defer func() { + hookPrintResults(results) + }() + + processBatch := func() error { + if len(refFullNames) == 0 { + return nil + } + _, _ = fmt.Fprintf(out, " Processing %d references\n", len(refFullNames)) + hookOptions.OldCommitIDs = oldCommitIDs + hookOptions.NewCommitIDs = newCommitIDs + hookOptions.RefFullNames = refFullNames + resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) + if extra.HasError() { + return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error) + } + results = append(results, resp.Results...) + oldCommitIDs = oldCommitIDs[:0] + newCommitIDs = newCommitIDs[:0] + refFullNames = refFullNames[:0] + return nil + } + scanner := bufio.NewScanner(os.Stdin) for scanner.Scan() { - // TODO: support news feeds for wiki + // wiki doesn't need "post-receive" at the moment if isWiki { continue } - var ok bool - oldCommitIDs[count], newCommitIDs[count], refFullNames[count], ok = parseGitHookCommitRefLine(scanner.Text()) + oldCommitID, newCommitID, refFullName, ok := parseGitHookCommitRefLine(scanner.Text()) if !ok { continue } + _, _ = fmt.Fprintf(out, ".") - fmt.Fprintf(out, ".") - commitID, _ := git.NewIDFromString(newCommitIDs[count]) - if refFullNames[count] == git.BranchPrefix+"master" && !commitID.IsZero() && count == total { - masterPushed = true - } - count++ - total++ - - if count >= hookBatchSize { - fmt.Fprintf(out, " Processing %d references\n", count) - hookOptions.OldCommitIDs = oldCommitIDs - hookOptions.NewCommitIDs = newCommitIDs - hookOptions.RefFullNames = refFullNames - resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) - if extra.HasError() { - _ = dWriter.Close() - hookPrintResults(results) - return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error) + oldCommitIDs = append(oldCommitIDs, oldCommitID) + newCommitIDs = append(newCommitIDs, newCommitID) + refFullNames = append(refFullNames, refFullName) + if len(refFullNames) >= hookBatchSize { + // process and start a new batch + if err := processBatch(); err != nil { + return err } - wasEmpty = wasEmpty || resp.RepoWasEmpty - results = append(results, resp.Results...) - count = 0 } } if err := scanner.Err(); err != nil { - _ = dWriter.Close() - hookPrintResults(results) return fail(ctx, "Hook failed: stdin read error", "scanner error: %v", err) } - - if count == 0 { - if wasEmpty && masterPushed { - // We need to tell the repo to reset the default branch to master - extra := private.SetDefaultBranch(ctx, repoUser, repoName, "master") - if extra.HasError() { - return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error) - } - } - fmt.Fprintf(out, "Processed %d references in total\n", total) - - _ = dWriter.Close() - hookPrintResults(results) - return nil - } - - hookOptions.OldCommitIDs = oldCommitIDs[:count] - hookOptions.NewCommitIDs = newCommitIDs[:count] - hookOptions.RefFullNames = refFullNames[:count] - - fmt.Fprintf(out, " Processing %d references\n", count) - - resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) - if resp == nil { - _ = dWriter.Close() - hookPrintResults(results) - return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error) - } - wasEmpty = wasEmpty || resp.RepoWasEmpty - results = append(results, resp.Results...) - - fmt.Fprintf(out, "Processed %d references in total\n", total) - - if wasEmpty && masterPushed { - // We need to tell the repo to reset the default branch to master - extra := private.SetDefaultBranch(ctx, repoUser, repoName, "master") - if extra.HasError() { - return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error) - } - } - _ = dWriter.Close() - hookPrintResults(results) - - return nil + return processBatch() } func hookPrintResults(results []private.HookPostReceiveBranchResult) { diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 7e6db9abee..1a93504d97 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -36,25 +36,17 @@ func (repo *Repository) GetCommit(ref string) (*Commit, error) { // GetBranchCommit returns the last commit of given branch. func (repo *Repository) GetBranchCommit(name string) (*Commit, error) { - commitID, err := repo.GetBranchCommitID(name) - if err != nil { - return nil, err - } - return repo.GetCommit(commitID) + return repo.GetCommit(RefNameFromBranch(name).String()) } // GetTagCommit get the commit of the specific tag via name func (repo *Repository) GetTagCommit(name string) (*Commit, error) { - commitID, err := repo.GetTagCommitID(name) - if err != nil { - return nil, err - } - return repo.GetCommit(commitID) + return repo.GetCommit(RefNameFromTag(name).String()) } func (repo *Repository) getCommitByPathWithID(id ObjectID, relpath string) (*Commit, error) { // File name starts with ':' must be escaped. - if relpath[0] == ':' { + if strings.HasPrefix(relpath, ":") { relpath = `\` + relpath } diff --git a/modules/private/hook.go b/modules/private/hook.go index cb6cc2f0bd..843288e081 100644 --- a/modules/private/hook.go +++ b/modules/private/hook.go @@ -48,9 +48,7 @@ type SSHLogOption struct { // HookPostReceiveResult represents an individual result from PostReceive type HookPostReceiveResult struct { - Results []HookPostReceiveBranchResult - RepoWasEmpty bool - Err string + Results []HookPostReceiveBranchResult } // HookPostReceiveBranchResult represents an individual branch result from PostReceive diff --git a/modules/repository/push.go b/modules/repository/push.go index 433bbee40a..5260597229 100644 --- a/modules/repository/push.go +++ b/modules/repository/push.go @@ -13,9 +13,12 @@ type PushUpdateOptions struct { PusherName string RepoUserName string RepoName string - RefFullName git.RefName // branch, tag or other name to push - OldCommitID string - NewCommitID string + + // FIXME: this struct's design is not right, the changed commits should be in a separate slice + + RefFullName git.RefName // branch, tag or other name to push + OldCommitID string + NewCommitID string } // IsNewRef return true if it's a first-time push to a branch, tag or etc. diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index 7e6e06a5f7..e19bee3e7d 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -5,6 +5,7 @@ package private import ( "context" + "errors" "fmt" "net/http" @@ -29,29 +30,8 @@ import ( repo_service "gitea.dev/services/repository" ) -// HookPostReceive updates services and users -func HookPostReceive(ctx *gitea_context.PrivateContext) { - opts := web.GetForm(ctx).(*private.HookOptions) - - // We don't rely on RepoAssignment here because: - // a) we don't need the git repo in this function - // OUT OF DATE: we do need the git repo to sync the branch to the db now. - // b) our update function will likely change the repository in the db so we will need to refresh it - // c) we don't always need the repo - - ownerName := ctx.PathParam("owner") - repoName := ctx.PathParam("repo") - - // defer getting the repository at this point - as we should only retrieve it if we're going to call update - var ( - repo *repo_model.Repository - gitRepo *git.Repository - ) - defer func() { _ = gitRepo.Close() }() // it's safe to call Close on a nil pointer, but it needs to use the latest value - +func hookPostReceiveCollectPushUpdates(opts *private.HookOptions, repo *repo_model.Repository) []*repo_module.PushUpdateOptions { updates := make([]*repo_module.PushUpdateOptions, 0, len(opts.OldCommitIDs)) - wasEmpty := false - for i := range opts.OldCommitIDs { refFullName := opts.RefFullNames[i] @@ -60,151 +40,124 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { // or other less-standard refs spaces are ignored since there // may be a very large number of them). if refFullName.IsBranch() || refFullName.IsTag() { - if repo == nil { - repo = loadRepository(ctx, ownerName, repoName) - if ctx.Written() { - // Error handled in loadRepository - return - } - wasEmpty = repo.IsEmpty - } - option := &repo_module.PushUpdateOptions{ RefFullName: refFullName, OldCommitID: opts.OldCommitIDs[i], NewCommitID: opts.NewCommitIDs[i], PusherID: opts.UserID, PusherName: opts.UserName, - RepoUserName: ownerName, - RepoName: repoName, + RepoUserName: repo.OwnerName, + RepoName: repo.Name, } updates = append(updates, option) - if repo.IsEmpty && (refFullName.BranchName() == "master" || refFullName.BranchName() == "main") { - // put the master/main branch first - // FIXME: It doesn't always work, since the master/main branch may not be the first batch of updates. - // If the user pushes many branches at once, the Git hook will call the internal API in batches, rather than all at once. - // See https://github.com/go-gitea/gitea/blob/cb52b17f92e2d2293f7c003649743464492bca48/cmd/hook.go#L27 - // If the user executes `git push origin --all` and pushes more than 30 branches, the master/main may not be the default branch. - copy(updates[1:], updates) - updates[0] = option + } + } + return updates +} + +func hookPostReceiveSyncDatabaseBranches(ctx *gitea_context.PrivateContext, opts *private.HookOptions, repo *repo_model.Repository, updates []*repo_module.PushUpdateOptions) bool { + branchesToSync := make([]*repo_module.PushUpdateOptions, 0, len(updates)) + for _, update := range updates { + if !update.RefFullName.IsBranch() { + continue + } + if update.IsDelRef() { + if err := git_model.MarkBranchAsDeleted(ctx, repo.ID, update.RefFullName.BranchName(), update.PusherID); err != nil { + ctx.PrivateError(http.StatusInternalServerError, err, fmt.Sprintf("failed to mark branch %s as deleted", update.RefFullName)) + return false } + } else { + branchesToSync = append(branchesToSync, update) + // TODO: should we return the error and return the error when pushing? Currently it will log the error and not prevent the pushing + pull_service.UpdatePullsRefs(ctx, repo, update) } } - if repo != nil && len(updates) > 0 { - branchesToSync := make([]*repo_module.PushUpdateOptions, 0, len(updates)) - for _, update := range updates { - if !update.RefFullName.IsBranch() { - continue - } - if repo == nil { - repo = loadRepository(ctx, ownerName, repoName) - if ctx.Written() { - return - } - wasEmpty = repo.IsEmpty - } - - if update.IsDelRef() { - if err := git_model.MarkBranchAsDeleted(ctx, repo.ID, update.RefFullName.BranchName(), update.PusherID); err != nil { - log.Error("Failed to mark branch as deleted: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to mark branch as deleted: %s/%s Error: %v", ownerName, repoName, err), - }) - return - } - } else { - branchesToSync = append(branchesToSync, update) - - // TODO: should we return the error and return the error when pushing? Currently it will log the error and not prevent the pushing - pull_service.UpdatePullsRefs(ctx, repo, update) - } - } - if len(branchesToSync) > 0 { - var err error - gitRepo, err = gitrepo.OpenRepository(ctx, repo) - if err != nil { - log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err), - }) - return - } - - var ( - branchNames = make([]string, 0, len(branchesToSync)) - commitIDs = make([]string, 0, len(branchesToSync)) - ) - for _, update := range branchesToSync { - branchNames = append(branchNames, update.RefFullName.BranchName()) - commitIDs = append(commitIDs, update.NewCommitID) - } - - if err := repo_service.SyncBranchesToDB(ctx, repo.ID, opts.UserID, branchNames, commitIDs, gitRepo.GetCommit); err != nil { - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to sync branch to DB in repository: %s/%s Error: %v", ownerName, repoName, err), - }) - return - } - } - - if err := repo_service.PushUpdates(updates); err != nil { - log.Error("Failed to Update: %s/%s Total Updates: %d", ownerName, repoName, len(updates)) - for i, update := range updates { - log.Error("Failed to Update: %s/%s Update: %d/%d: Branch: %s", ownerName, repoName, i, len(updates), update.RefFullName.BranchName()) - } - log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) - - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err), - }) - return - } + if len(branchesToSync) == 0 { + return true } + gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) + if err != nil { + ctx.PrivateError(http.StatusInternalServerError, err, "failed to open repository") + return false + } + + branchNames := make([]string, 0, len(branchesToSync)) + commitIDs := make([]string, 0, len(branchesToSync)) + for _, update := range branchesToSync { + branchNames = append(branchNames, update.RefFullName.BranchName()) + commitIDs = append(commitIDs, update.NewCommitID) + } + + if err = repo_service.SyncBranchesToDB(ctx, repo.ID, opts.UserID, branchNames, commitIDs, gitRepo.GetCommit); err != nil { + ctx.PrivateError(http.StatusInternalServerError, err, "failed to sync branch to DB") + return false + } + return true +} + +// HookPostReceive updates services and users +func HookPostReceive(ctx *gitea_context.PrivateContext) { + opts := web.GetForm(ctx).(*private.HookOptions) + if opts.IsWiki { + setting.PanicInDevOrTesting("wiki hook-post-receive is not supported") + return + } + + ownerName := ctx.PathParam("owner") + repoName := ctx.PathParam("repo") + repo := loadRepository(ctx, ownerName, repoName) + if ctx.Written() { + return + } + // now, repo can't be nil + + // first, collect updates and sync branches + updates := hookPostReceiveCollectPushUpdates(opts, repo) + if !hookPostReceiveSyncDatabaseBranches(ctx, opts, repo, updates) { + return + } + hookPostReceiveSyncRepoDefaultBranch(ctx, opts, repo) + // handle pull request merging, a pull request action should push at least 1 commit if opts.PushTrigger == repo_module.PushTriggerPRMergeToBase { - handlePullRequestMerging(ctx, opts, ownerName, repoName, updates) - if ctx.Written() { + if !hookPostReceiveHandlePullRequestMerging(ctx, opts, updates) { return } } + if !hookPostReceiveUpdateRepoByOptions(ctx, opts, repo) { + return + } + + // push async updates + if err := repo_service.PushUpdates(updates...); err != nil { + ctx.PrivateError(http.StatusInternalServerError, err, "failed to push updates") + return + } + + hookPostReceiveRespondWithTrailer(ctx, opts, repo) +} + +func hookPostReceiveUpdateRepoByOptions(ctx *gitea_context.PrivateContext, opts *private.HookOptions, repo *repo_model.Repository) bool { isPrivate := opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate) isTemplate := opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate) // Handle Push Options if isPrivate.Has() || isTemplate.Has() { - // load the repository - if repo == nil { - repo = loadRepository(ctx, ownerName, repoName) - if ctx.Written() { - // Error handled in loadRepository - return - } - wasEmpty = repo.IsEmpty - } - pusher, err := loadContextCacheUser(ctx, opts.UserID) if err != nil { - log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err), - }) - return + ctx.PrivateError(http.StatusInternalServerError, err, "failed to load pusher user") + return false } perm, err := access_model.GetDoerRepoPermission(ctx, repo, pusher) if err != nil { - log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err), - }) - return + ctx.PrivateError(http.StatusInternalServerError, err, "failed to load doer repo permission") + return false } if !perm.IsOwner() && !perm.IsAdmin() { - ctx.JSON(http.StatusNotFound, private.HookPostReceiveResult{ - Err: "Permissions denied", - }) - return + ctx.PrivateError(http.StatusNotFound, nil, "permission denied") + return false } // FIXME: these options are not quite right, for example: changing visibility should do more works than just setting the is_private flag @@ -213,22 +166,37 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { // TODO: it needs to do more work repo.IsPrivate = isPrivate.Value() if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_private"); err != nil { - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to change visibility"}) + log.Error("failed to update repo is_private: %v", err) } } if isTemplate.Has() && repo.IsTemplate != isTemplate.Value() { repo.IsTemplate = isTemplate.Value() if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_template"); err != nil { - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to change template status"}) + log.Error("failed to update repo is_template: %v", err) } } } + return true +} +func hookPostReceiveRespondWithTrailer(ctx *gitea_context.PrivateContext, opts *private.HookOptions, repo *repo_model.Repository) { results := make([]private.HookPostReceiveBranchResult, 0, len(opts.OldCommitIDs)) + baseRepo := repo + if repo.IsFork { + if err := repo.GetBaseRepo(ctx); err != nil { + ctx.PrivateError(http.StatusInternalServerError, err, "failed to load base repo") + return + } + if repo.BaseRepo.AllowsPulls(ctx) { + baseRepo = repo.BaseRepo + } + } - // We have to reload the repo in case its state is changed above - repo = nil - var baseRepo *repo_model.Repository + if !baseRepo.AllowsPulls(ctx) { + // We can stop there's no need to go any further + ctx.JSON(http.StatusOK, private.HookPostReceiveResult{}) + return + } // Now handle the pull request notification trailers for i := range opts.OldCommitIDs { @@ -237,66 +205,19 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { // If we've pushed a branch (and not deleted it) if !git.IsEmptyCommitID(newCommitID) && refFullName.IsBranch() { - // First ensure we have the repository loaded, we're allowed pulls requests and we can get the base repo - if repo == nil { - repo = loadRepository(ctx, ownerName, repoName) - if ctx.Written() { - return - } - - baseRepo = repo - - if repo.IsFork { - if err := repo.GetBaseRepo(ctx); err != nil { - log.Error("Failed to get Base Repository of Forked repository: %-v Error: %v", repo, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to get Base Repository of Forked repository: %-v Error: %v", repo, err), - RepoWasEmpty: wasEmpty, - }) - return - } - if repo.BaseRepo.AllowsPulls(ctx) { - baseRepo = repo.BaseRepo - } - } - - if !baseRepo.AllowsPulls(ctx) { - // We can stop there's no need to go any further - ctx.JSON(http.StatusOK, private.HookPostReceiveResult{ - RepoWasEmpty: wasEmpty, - }) - return - } - } - branch := refFullName.BranchName() - if branch == baseRepo.DefaultBranch { - if err := repo_service.AddRepoToLicenseUpdaterQueue(&repo_service.LicenseUpdaterOptions{ - RepoID: repo.ID, - }); err != nil { - ctx.JSON(http.StatusInternalServerError, private.Response{Err: err.Error()}) - return - } - + if branch == baseRepo.DefaultBranch && !repo.IsFork { // If our branch is the default branch of an unforked repo - there's no PR to create or refer to - if !repo.IsFork { - results = append(results, private.HookPostReceiveBranchResult{}) - continue - } + results = append(results, private.HookPostReceiveBranchResult{}) + continue } pr, err := issues_model.GetUnmergedPullRequest(ctx, repo.ID, baseRepo.ID, branch, baseRepo.DefaultBranch, issues_model.PullRequestFlowGithub) - if err != nil && !issues_model.IsErrPullRequestNotExist(err) { - log.Error("Failed to get active PR in: %-v Branch: %s to: %-v Branch: %s Error: %v", repo, branch, baseRepo, baseRepo.DefaultBranch, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf( - "Failed to get active PR in: %-v Branch: %s to: %-v Branch: %s Error: %v", repo, branch, baseRepo, baseRepo.DefaultBranch, err), - RepoWasEmpty: wasEmpty, - }) + if err != nil && !errors.Is(err, util.ErrNotExist) { + ctx.PrivateError(http.StatusInternalServerError, err, "failed to get active PR for branch "+branch) return } - if pr == nil { results = append(results, private.HookPostReceiveBranchResult{ Message: setting.Git.PullRequestPushMessage && baseRepo.AllowsPulls(ctx), @@ -314,43 +235,79 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { } } } - ctx.JSON(http.StatusOK, private.HookPostReceiveResult{ - Results: results, - RepoWasEmpty: wasEmpty, - }) + ctx.JSON(http.StatusOK, private.HookPostReceiveResult{Results: results}) } func loadContextCacheUser(ctx context.Context, id int64) (*user_model.User, error) { return cache.GetWithContextCache(ctx, cachegroup.User, id, user_model.GetUserByID) } -// handlePullRequestMerging handle pull request merging, a pull request action should push at least 1 commit -func handlePullRequestMerging(ctx *gitea_context.PrivateContext, opts *private.HookOptions, ownerName, repoName string, updates []*repo_module.PushUpdateOptions) { +// hookPostReceiveHandlePullRequestMerging handle pull request merging, a pull request action should push at least 1 commit +func hookPostReceiveHandlePullRequestMerging(ctx *gitea_context.PrivateContext, opts *private.HookOptions, updates []*repo_module.PushUpdateOptions) bool { if len(updates) == 0 { - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Pushing a merged PR (pr:%d) no commits pushed ", opts.PullRequestID), - }) - return + err := fmt.Errorf("Pushing a merged PR (pr:%d) no commits pushed ", opts.PullRequestID) + ctx.PrivateError(http.StatusInternalServerError, err, "no push update") + return false } pr, err := issues_model.GetPullRequestByID(ctx, opts.PullRequestID) if err != nil { - log.Error("GetPullRequestByID[%d]: %v", opts.PullRequestID, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "GetPullRequestByID failed"}) - return + ctx.PrivateError(http.StatusInternalServerError, err, "failed to load pull request") + return false } pusher, err := loadContextCacheUser(ctx, opts.UserID) if err != nil { - log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Load pusher user failed"}) - return + ctx.PrivateError(http.StatusInternalServerError, err, "failed to load pusher user") + return false } // FIXME: Maybe we need a `PullRequestStatusMerged` status for PRs that are merged, currently we use the previous status // here to keep it as before, that maybe PullRequestStatusMergeable - if _, err := pull_service.SetMerged(ctx, pr, updates[len(updates)-1].NewCommitID, timeutil.TimeStampNow(), pusher, pr.Status); err != nil { - log.Error("Failed to update PR to merged: %v", err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to update PR to merged"}) + _, err = pull_service.SetMerged(ctx, pr, updates[len(updates)-1].NewCommitID, timeutil.TimeStampNow(), pusher, pr.Status) + if err != nil { + ctx.PrivateError(http.StatusInternalServerError, err, "failed to set pr to merged") + return false + } + return true +} + +func hookPostReceiveSyncRepoDefaultBranch(ctx *gitea_context.PrivateContext, opts *private.HookOptions, repo *repo_model.Repository) { + hasBranch := false + for _, refFullName := range opts.RefFullNames { + if hasBranch = refFullName.IsBranch(); hasBranch { + break + } + } + if !hasBranch { + return + } + gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) + if err != nil { + log.Error("failed to open git repo: %v", err) + return + } + + // if default branch doesn't exist, try to guess one from existing git repo + _, err = gitRepo.GetBranchCommitID(repo.DefaultBranch) + if errors.Is(err, util.ErrNotExist) { + for _, guessBranchName := range []string{"main", "master"} { + if _, err = gitRepo.GetBranchCommitID(guessBranchName); err == nil { + repo.DefaultBranch = guessBranchName + err = repo_model.UpdateDefaultBranch(ctx, repo) + if err != nil { + log.Error("failed to update default branch: %v", err) + return + } + break + } + } + } + + // if default branch was pushed, always keep the HEAD ref in sync + for _, refFullName := range opts.RefFullNames { + if refFullName.IsBranch() && refFullName.BranchName() == repo.DefaultBranch { + _ = gitrepo.SetDefaultBranch(ctx, repo, repo.DefaultBranch) + } } } diff --git a/routers/private/hook_post_receive_test.go b/routers/private/hook_post_receive_test.go index b18d9842e8..b465c7f6e8 100644 --- a/routers/private/hook_post_receive_test.go +++ b/routers/private/hook_post_receive_test.go @@ -32,10 +32,10 @@ func TestHandlePullRequestMerging(t *testing.T) { autoMerge := unittest.AssertExistsAndLoadBean(t, &pull_model.AutoMerge{PullID: pr.ID}) ctx, resp := contexttest.MockPrivateContext(t, "/") - handlePullRequestMerging(ctx, &private.HookOptions{ + hookPostReceiveHandlePullRequestMerging(ctx, &private.HookOptions{ PullRequestID: pr.ID, UserID: 2, - }, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, []*repo_module.PushUpdateOptions{ + }, []*repo_module.PushUpdateOptions{ {NewCommitID: "01234567"}, }) assert.Empty(t, resp.Body.String()) diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index aa3ad614c6..f5972c8db0 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -52,13 +52,16 @@ func Branches(ctx *context.Context) { kw := ctx.FormString("q") - defaultBranch, branches, branchesCount, err := repo_service.LoadBranches(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, optional.None[bool](), kw, page, pageSize) + defaultBranchOptional, branches, branchesCount, err := repo_service.LoadBranches(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, optional.None[bool](), kw, page, pageSize) if err != nil { ctx.ServerError("LoadBranches", err) return } - commitIDs := []string{defaultBranch.DBBranch.CommitID} + commitIDs := make([]string, 0, len(branches)+1) + if defaultBranchOptional != nil { + commitIDs = append(commitIDs, defaultBranchOptional.DBBranch.CommitID) + } for _, branch := range branches { commitIDs = append(commitIDs, branch.DBBranch.CommitID) } @@ -83,7 +86,7 @@ func Branches(ctx *context.Context) { ctx.Data["Branches"] = branches ctx.Data["CommitStatus"] = commitStatus ctx.Data["CommitStatuses"] = commitStatuses - ctx.Data["DefaultBranchBranch"] = defaultBranch + ctx.Data["DefaultBranchBranch"] = defaultBranchOptional pager := context.NewPagination(branchesCount, pageSize, page, 5) pager.AddParamFromRequest(ctx.Req) ctx.Data["Page"] = pager @@ -152,7 +155,7 @@ func RestoreBranchPost(ctx *context.Context) { objectFormat := git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName) // Don't return error below this - if err := repo_service.PushUpdate( + if err := repo_service.PushUpdates( &repo_module.PushUpdateOptions{ RefFullName: git.RefNameFromBranch(deletedBranch.Name), OldCommitID: objectFormat.EmptyObjectID().String(), diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index ba4828faf8..d23cca7fa5 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -18,7 +18,6 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" user_model "gitea.dev/models/user" - "gitea.dev/modules/cache" "gitea.dev/modules/git" "gitea.dev/modules/log" "gitea.dev/modules/optional" @@ -63,22 +62,6 @@ func MustBeAbleToUpload(ctx *context.Context) { } } -func CommitInfoCache(ctx *context.Context) { - var err error - ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) - if err != nil { - ctx.ServerError("GetBranchCommit", err) - return - } - ctx.Repo.CommitsCount, err = ctx.Repo.GetCommitsCount(ctx) - if err != nil { - ctx.ServerError("GetCommitsCount", err) - return - } - ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount - ctx.Repo.GitRepo.LastCommitCache = git.NewLastCommitCache(ctx.Repo.CommitsCount, ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, cache.GetCache()) -} - func checkContextUser(ctx *context.Context, uid int64) *user_model.User { orgs, err := organization.GetOrgsCanCreateRepoByUserID(ctx, ctx.Doer.ID) if err != nil { diff --git a/routers/web/web.go b/routers/web/web.go index 49a83c1fae..ee30b614c8 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1476,15 +1476,13 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Group("/releases", func() { m.Get("/new", repo.NewRelease) m.Post("/new", web.Bind(forms.NewReleaseForm{}), repo.NewReleasePost) + m.Get("/edit/*", repo.EditRelease) + m.Post("/edit/*", web.Bind(forms.EditReleaseForm{}), repo.EditReleasePost) m.Post("/generate-notes", web.Bind(forms.GenerateReleaseNotesForm{}), repo.GenerateReleaseNotes) m.Post("/delete", repo.DeleteRelease) m.Post("/attachments", repo.UploadReleaseAttachment) m.Post("/attachments/remove", repo.DeleteAttachment) }, reqSignIn, context.RepoMustNotBeArchived(), reqRepoReleaseWriter) - m.Group("/releases", func() { - m.Get("/edit/*", repo.EditRelease) - m.Post("/edit/*", web.Bind(forms.EditReleaseForm{}), repo.EditReleasePost) - }, reqSignIn, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache) }, optSignIn, context.RepoAssignment, repo.MustBeNotEmpty, reqRepoReleaseReader) // end "/{username}/{reponame}": repo releases diff --git a/services/context/private.go b/services/context/private.go index 2c0d21102a..e687821b07 100644 --- a/services/context/private.go +++ b/services/context/private.go @@ -9,6 +9,7 @@ import ( "time" "gitea.dev/modules/graceful" + "gitea.dev/modules/private" "gitea.dev/modules/process" "gitea.dev/modules/web" web_types "gitea.dev/modules/web/types" @@ -49,6 +50,14 @@ func (ctx *PrivateContext) Err() error { return ctx.Base.Err() } +func (ctx *PrivateContext) PrivateError(status int, err error, userMsg string) { + errMsg := "" + if err != nil { + errMsg = err.Error() + } + ctx.JSON(status, private.Response{Err: errMsg, UserMsg: userMsg}) +} + type privateContextKeyType struct{} var privateContextKey privateContextKeyType diff --git a/services/context/repo.go b/services/context/repo.go index fa816ba6ad..81df87a67f 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -972,12 +972,9 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) { ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refShortName) if err == nil { ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if strings.Contains(err.Error(), "fatal: not a git repository") || strings.Contains(err.Error(), "object does not exist") { + } else { // if the repository is broken, we can continue to the handler code, to show "Settings -> Delete Repository" for end users log.Error("GetBranchCommit: %v", err) - } else { - ctx.ServerError("GetBranchCommit", err) - return } } else { // there is a path in request guessLegacyPath := refType == "" diff --git a/services/repository/branch.go b/services/repository/branch.go index 9ab485ca71..0869750db4 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -59,9 +59,9 @@ type Branch struct { } // LoadBranches loads branches from the repository limited by page & pageSize. -func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, isDeletedBranch optional.Option[bool], keyword string, page, pageSize int) (*Branch, []*Branch, int64, error) { - defaultDBBranch, err := git_model.GetBranch(ctx, repo.ID, repo.DefaultBranch) - if err != nil { +func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, isDeletedBranch optional.Option[bool], keyword string, page, pageSize int) (defaultBranchOptional *Branch, _ []*Branch, _ int64, _ error) { + defaultDBBranchOptional, err := git_model.GetBranch(ctx, repo.ID, repo.DefaultBranch) + if err != nil && !errors.Is(err, util.ErrNotExist) { return nil, nil, 0, err } @@ -108,13 +108,14 @@ func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git branches = append(branches, branch) } - // Always add the default branch - log.Debug("loadOneBranch: load default: '%s'", defaultDBBranch.Name) - defaultBranch, err := loadOneBranch(ctx, repo, defaultDBBranch, &rules, repoIDToRepo, repoIDToGitRepo) - if err != nil { - return nil, nil, 0, fmt.Errorf("loadOneBranch: %v", err) + if defaultDBBranchOptional != nil { + // Always add the default branch + defaultBranchOptional, err = loadOneBranch(ctx, repo, defaultDBBranchOptional, &rules, repoIDToRepo, repoIDToGitRepo) + if err != nil { + return nil, nil, 0, fmt.Errorf("loadOneBranch: %v", err) + } } - return defaultBranch, branches, totalNumOfBranches, nil + return defaultBranchOptional, branches, totalNumOfBranches, nil } func getDivergenceCacheKey(repoID int64, branchName string) string { @@ -640,7 +641,7 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R func deleteBranchSuccessPostProcess(doer *user_model.User, repo *repo_model.Repository, branchName string, branchCommit *git.Commit) { objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) - if err := PushUpdate( + if err := PushUpdates( &repo_module.PushUpdateOptions{ RefFullName: git.RefNameFromBranch(branchName), OldCommitID: branchCommit.ID.String(), diff --git a/services/repository/push.go b/services/repository/push.go index 895e3de020..7666ecf377 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -32,10 +32,26 @@ import ( // pushQueue represents a queue to handle update pull request tests var pushQueue *queue.WorkerPoolQueue[[]*repo_module.PushUpdateOptions] -// handle passed PR IDs and test the PRs -func handler(items ...[]*repo_module.PushUpdateOptions) [][]*repo_module.PushUpdateOptions { +func initPushQueue() error { + pushQueue = queue.CreateSimpleQueue(graceful.GetManager().ShutdownContext(), "push_update", pushQueueHandler) + if pushQueue == nil { + return errors.New("unable to create push_update queue") + } + go graceful.GetManager().RunWithCancel(pushQueue) + return nil +} + +// PushUpdates adds a push update to push queue, each call must pass the same repo updates +func PushUpdates(opts ...*repo_module.PushUpdateOptions) error { + if len(opts) == 0 { + return nil + } + return pushQueue.Push(opts) +} + +func pushQueueHandler(items ...[]*repo_module.PushUpdateOptions) [][]*repo_module.PushUpdateOptions { for _, opts := range items { - if err := pushUpdates(opts); err != nil { + if err := pushQueueHandleUpdates(opts); err != nil { // Username and repository stays the same between items in opts. pushUpdate := opts[0] log.Error("pushUpdate[%s/%s] failed: %v", pushUpdate.RepoUserName, pushUpdate.RepoName, err) @@ -44,37 +60,8 @@ func handler(items ...[]*repo_module.PushUpdateOptions) [][]*repo_module.PushUpd return nil } -func initPushQueue() error { - pushQueue = queue.CreateSimpleQueue(graceful.GetManager().ShutdownContext(), "push_update", handler) - if pushQueue == nil { - return errors.New("unable to create push_update queue") - } - go graceful.GetManager().RunWithCancel(pushQueue) - return nil -} - -// PushUpdate is an alias of PushUpdates for single push update options -func PushUpdate(opts *repo_module.PushUpdateOptions) error { - return PushUpdates([]*repo_module.PushUpdateOptions{opts}) -} - -// PushUpdates adds a push update to push queue -func PushUpdates(opts []*repo_module.PushUpdateOptions) error { - if len(opts) == 0 { - return nil - } - - for _, opt := range opts { - if opt.IsNewRef() && opt.IsDelRef() { - return errors.New("Old and new revisions are both NULL") - } - } - - return pushQueue.Push(opts) -} - -// pushUpdates generates push action history feeds for push updating multiple refs -func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { +// pushQueueHandleUpdates generates push action history feeds for push updating multiple refs +func pushQueueHandleUpdates(optsList []*repo_module.PushUpdateOptions) error { if len(optsList) == 0 { return nil } @@ -94,7 +81,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { defer gitRepo.Close() if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { - return fmt.Errorf("Failed to update size for repository: %v", err) + return fmt.Errorf("failed to update size for repository: %v", err) } addTags := make([]string, 0, len(optsList)) @@ -104,10 +91,11 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { for _, opts := range optsList { log.Trace("pushUpdates: %-v %s %s %s", repo, opts.OldCommitID, opts.NewCommitID, opts.RefFullName) - if opts.IsNewRef() && opts.IsDelRef() { - return fmt.Errorf("old and new revisions are both %s", objectFormat.EmptyObjectID()) + setting.PanicInDevOrTesting("invalid push update (add+del): %+v", opts) + continue } + if opts.RefFullName.IsTag() { if pusher == nil || pusher.ID != opts.PusherID { if opts.PusherID == user_model.ActionsUserID { @@ -188,11 +176,14 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { return err } - // delete cache for divergence + // sync branch related database data if branch == repo.DefaultBranch { if err := DelRepoDivergenceFromCache(ctx, repo.ID); err != nil { log.Error("DelRepoDivergenceFromCache: %v", err) } + if err := AddRepoToLicenseUpdaterQueue(&LicenseUpdaterOptions{RepoID: repo.ID}); err != nil { + log.Error("AddRepoToLicenseUpdaterQueue: %v", err) + } } else { if err := DelDivergenceFromCache(repo.ID, branch); err != nil { log.Error("DelDivergenceFromCache: %v", err) diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index 7d8f2bfc5b..6d1fa21151 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -4,7 +4,6 @@
{{template "base/alert" .}} {{template "repo/sub_menu" .}} - {{if .DefaultBranchBranch}}

{{ctx.Locale.Tr "repo.default_branch"}} {{if and $.IsWriter $.Repository.CanContentChange (not .IsDeleted)}} @@ -14,7 +13,8 @@ {{end}}

-
+ {{if .DefaultBranchBranch}} +
@@ -69,6 +69,10 @@
+ {{else}} +
+ {{ctx.Locale.Tr "repo.branch.default_branch_not_exist" $.Repository.DefaultBranch}} +
{{end}}

diff --git a/tests/integration/branches_test.go b/tests/integration/branches_test.go index 22b1549252..0763aef68f 100644 --- a/tests/integration/branches_test.go +++ b/tests/integration/branches_test.go @@ -8,6 +8,9 @@ import ( "net/url" "testing" + "gitea.dev/models/db" + repo_model "gitea.dev/models/repo" + "gitea.dev/models/unittest" "gitea.dev/modules/translation" "gitea.dev/tests" @@ -18,55 +21,52 @@ import ( func TestViewBranches(t *testing.T) { defer tests.PrepareTestEnv(t)() + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) req := NewRequest(t, "GET", "/user2/repo1/branches") resp := MakeRequest(t, req, http.StatusOK) - htmlDoc := NewHTMLParser(t, resp.Body) - _, exists := htmlDoc.doc.Find(".delete-branch-button").Attr("data-url") - assert.False(t, exists, "The template has changed") -} + AssertHTMLElement(t, htmlDoc, "[data-testid=branches-default-branch-list]", 1) + AssertHTMLElement(t, htmlDoc, "[data-testid=branches-default-branch-not-exist]", 0) -func TestDeleteBranch(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - deleteBranch(t) + repo1.DefaultBranch = "non-existent-branch" + _, _ = db.GetEngine(t.Context()).ID(repo1.ID).Cols("default_branch").Update(repo1) + req = NewRequest(t, "GET", "/user2/repo1/branches") + resp = MakeRequest(t, req, http.StatusOK) + htmlDoc = NewHTMLParser(t, resp.Body) + AssertHTMLElement(t, htmlDoc, "[data-testid=branches-default-branch-list]", 0) + AssertHTMLElement(t, htmlDoc, "[data-testid=branches-default-branch-not-exist]", 1) } func TestUndoDeleteBranch(t *testing.T) { + branchAction := func(t *testing.T, button string) (*HTMLDoc, string) { + session := loginUser(t, "user2") + req := NewRequest(t, "GET", "/user2/repo1/branches") + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + link, exists := htmlDoc.doc.Find(button).Attr("data-url") + require.True(t, exists, "The template has changed") + linkURL, err := url.Parse(link) + require.NoError(t, err) + + req = NewRequest(t, "POST", link) + session.MakeRequest(t, req, http.StatusOK) + req = NewRequest(t, "GET", "/user2/repo1/branches") + resp = session.MakeRequest(t, req, http.StatusOK) + + return NewHTMLParser(t, resp.Body), linkURL.Query().Get("name") + } + onGiteaRun(t, func(t *testing.T, u *url.URL) { - deleteBranch(t) - htmlDoc, name := branchAction(t, ".restore-branch-button") + htmlDoc, name := branchAction(t, ".delete-branch-button") + assert.Contains(t, + htmlDoc.doc.Find(".ui.positive.message").Text(), + translation.NewLocale("en-US").TrString("repo.branch.deletion_success", name), + ) + htmlDoc, name = branchAction(t, ".restore-branch-button") assert.Contains(t, htmlDoc.doc.Find(".ui.positive.message").Text(), translation.NewLocale("en-US").TrString("repo.branch.restore_success", name), ) }) } - -func deleteBranch(t *testing.T) { - htmlDoc, name := branchAction(t, ".delete-branch-button") - assert.Contains(t, - htmlDoc.doc.Find(".ui.positive.message").Text(), - translation.NewLocale("en-US").TrString("repo.branch.deletion_success", name), - ) -} - -func branchAction(t *testing.T, button string) (*HTMLDoc, string) { - session := loginUser(t, "user2") - req := NewRequest(t, "GET", "/user2/repo1/branches") - resp := session.MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - link, exists := htmlDoc.doc.Find(button).Attr("data-url") - require.True(t, exists, "The template has changed") - - req = NewRequest(t, "POST", link) - session.MakeRequest(t, req, http.StatusOK) - - url, err := url.Parse(link) - assert.NoError(t, err) - req = NewRequest(t, "GET", "/user2/repo1/branches") - resp = session.MakeRequest(t, req, http.StatusOK) - - return NewHTMLParser(t, resp.Body), url.Query().Get("name") -}