From dfd75944992fc6508ec891b4c29715c23e59e4ed Mon Sep 17 00:00:00 2001 From: RiceChuan Date: Thu, 12 Dec 2024 12:26:11 +0800 Subject: [PATCH 01/12] chore: use errors.New to replace fmt.Errorf with no parameters (#32800) use errors.New to replace fmt.Errorf with no parameters Signed-off-by: RiceChuan --- models/db/context_committer_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/db/context_committer_test.go b/models/db/context_committer_test.go index 38e91f22edb41..849c5dea411d1 100644 --- a/models/db/context_committer_test.go +++ b/models/db/context_committer_test.go @@ -4,7 +4,7 @@ package db // it's not db_test, because this file is for testing the private type halfCommitter import ( - "fmt" + "errors" "testing" "github.com/stretchr/testify/assert" @@ -80,7 +80,7 @@ func Test_halfCommitter(t *testing.T) { testWithCommitter(mockCommitter, func(committer Committer) error { defer committer.Close() if true { - return fmt.Errorf("error") + return errors.New("error") } return committer.Commit() }) @@ -94,7 +94,7 @@ func Test_halfCommitter(t *testing.T) { testWithCommitter(mockCommitter, func(committer Committer) error { committer.Close() committer.Commit() - return fmt.Errorf("error") + return errors.New("error") }) mockCommitter.Assert(t) From 1e751d81b321c07f14ad25e034bf87a1e060e5ef Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 12 Dec 2024 12:37:25 +0800 Subject: [PATCH 02/12] Fix JS error when dropping a file to a editor without dropzone (#32804) `dropzoneEl` may not exist --- web_src/js/features/comp/EditorUpload.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/js/features/comp/EditorUpload.ts b/web_src/js/features/comp/EditorUpload.ts index b1f49cbe92667..89982747ea5de 100644 --- a/web_src/js/features/comp/EditorUpload.ts +++ b/web_src/js/features/comp/EditorUpload.ts @@ -178,6 +178,7 @@ export function initTextareaEvents(textarea, dropzoneEl) { }); textarea.addEventListener('drop', (e) => { if (!e.dataTransfer.files.length) return; + if (!dropzoneEl) return; handleUploadFiles(new TextareaEditor(textarea), dropzoneEl, e.dataTransfer.files, e); }); dropzoneEl?.dropzone.on(DropzoneCustomEventRemovedFile, ({fileUuid}) => { From 01b1896bf5eacfd7f4f64d9ebb0ad165e3e60a5c Mon Sep 17 00:00:00 2001 From: Kemal Zebari <60799661+kemzeb@users.noreply.github.com> Date: Wed, 11 Dec 2024 21:02:35 -0800 Subject: [PATCH 03/12] Implement update branch API (#32433) Resolves #22526. Builds upon #23061. --------- Co-authored-by: sillyguodong <33891828+sillyguodong@users.noreply.github.com> Co-authored-by: wxiaoguang --- modules/structs/repo.go | 10 ++++ routers/api/v1/api.go | 1 + routers/api/v1/repo/branch.go | 71 +++++++++++++++++++++++++++ routers/api/v1/swagger/options.go | 2 + templates/swagger/v1_json.tmpl | 73 ++++++++++++++++++++++++++++ tests/integration/api_branch_test.go | 32 ++++++++++++ 6 files changed, 189 insertions(+) diff --git a/modules/structs/repo.go b/modules/structs/repo.go index 832ffa8bcc958..fb784bd8b37f8 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -278,6 +278,16 @@ type CreateBranchRepoOption struct { OldRefName string `json:"old_ref_name" binding:"GitRefName;MaxSize(100)"` } +// UpdateBranchRepoOption options when updating a branch in a repository +// swagger:model +type UpdateBranchRepoOption struct { + // New branch name + // + // required: true + // unique: true + Name string `json:"name" binding:"Required;GitRefName;MaxSize(100)"` +} + // TransferRepoOption options when transfer a repository's ownership // swagger:model type TransferRepoOption struct { diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index f28ee980e1043..96365e7c14dfc 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1195,6 +1195,7 @@ func Routes() *web.Router { m.Get("/*", repo.GetBranch) m.Delete("/*", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, repo.DeleteBranch) m.Post("", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, bind(api.CreateBranchRepoOption{}), repo.CreateBranch) + m.Patch("/*", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, bind(api.UpdateBranchRepoOption{}), repo.UpdateBranch) }, context.ReferencesGitRepo(), reqRepoReader(unit.TypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 53f3b4648a592..946203e97ec0a 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -386,6 +386,77 @@ func ListBranches(ctx *context.APIContext) { ctx.JSON(http.StatusOK, apiBranches) } +// UpdateBranch updates a repository's branch. +func UpdateBranch(ctx *context.APIContext) { + // swagger:operation PATCH /repos/{owner}/{repo}/branches/{branch} repository repoUpdateBranch + // --- + // summary: Update a branch + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: branch + // in: path + // description: name of the branch + // type: string + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/UpdateBranchRepoOption" + // responses: + // "204": + // "$ref": "#/responses/empty" + // "403": + // "$ref": "#/responses/forbidden" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" + + opt := web.GetForm(ctx).(*api.UpdateBranchRepoOption) + + oldName := ctx.PathParam("*") + repo := ctx.Repo.Repository + + if repo.IsEmpty { + ctx.Error(http.StatusNotFound, "", "Git Repository is empty.") + return + } + + if repo.IsMirror { + ctx.Error(http.StatusForbidden, "", "Git Repository is a mirror.") + return + } + + msg, err := repo_service.RenameBranch(ctx, repo, ctx.Doer, ctx.Repo.GitRepo, oldName, opt.Name) + if err != nil { + ctx.Error(http.StatusInternalServerError, "RenameBranch", err) + return + } + if msg == "target_exist" { + ctx.Error(http.StatusUnprocessableEntity, "", "Cannot rename a branch using the same name or rename to a branch that already exists.") + return + } + if msg == "from_not_exist" { + ctx.Error(http.StatusNotFound, "", "Branch doesn't exist.") + return + } + + ctx.Status(http.StatusNoContent) +} + // GetBranchProtection gets a branch protection func GetBranchProtection(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/branch_protections/{name} repository repoGetBranchProtection diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index 39c98c666e545..125605d98f5aa 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -90,6 +90,8 @@ type swaggerParameterBodies struct { // in:body EditRepoOption api.EditRepoOption // in:body + UpdateBranchRepoOption api.UpdateBranchRepoOption + // in:body TransferRepoOption api.TransferRepoOption // in:body CreateForkOption api.CreateForkOption diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index c06c0ad1541c2..82a301da2fe99 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -5045,6 +5045,63 @@ "$ref": "#/responses/repoArchivedError" } } + }, + "patch": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Update a branch", + "operationId": "repoUpdateBranch", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the branch", + "name": "branch", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/UpdateBranchRepoOption" + } + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } } }, "/repos/{owner}/{repo}/collaborators": { @@ -24968,6 +25025,22 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "UpdateBranchRepoOption": { + "description": "UpdateBranchRepoOption options when updating a branch in a repository", + "type": "object", + "required": [ + "name" + ], + "properties": { + "name": { + "description": "New branch name", + "type": "string", + "uniqueItems": true, + "x-go-name": "Name" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "UpdateFileOptions": { "description": "UpdateFileOptions options for updating files\nNote: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)", "type": "object", diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index 8e49516aa7220..24a041de17e1b 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -5,6 +5,7 @@ package integration import ( "net/http" + "net/http/httptest" "net/url" "testing" @@ -186,6 +187,37 @@ func testAPICreateBranch(t testing.TB, session *TestSession, user, repo, oldBran return resp.Result().StatusCode == status } +func TestAPIUpdateBranch(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + t.Run("UpdateBranchWithEmptyRepo", func(t *testing.T) { + testAPIUpdateBranch(t, "user10", "repo6", "master", "test", http.StatusNotFound) + }) + t.Run("UpdateBranchWithSameBranchNames", func(t *testing.T) { + resp := testAPIUpdateBranch(t, "user2", "repo1", "master", "master", http.StatusUnprocessableEntity) + assert.Contains(t, resp.Body.String(), "Cannot rename a branch using the same name or rename to a branch that already exists.") + }) + t.Run("UpdateBranchThatAlreadyExists", func(t *testing.T) { + resp := testAPIUpdateBranch(t, "user2", "repo1", "master", "branch2", http.StatusUnprocessableEntity) + assert.Contains(t, resp.Body.String(), "Cannot rename a branch using the same name or rename to a branch that already exists.") + }) + t.Run("UpdateBranchWithNonExistentBranch", func(t *testing.T) { + resp := testAPIUpdateBranch(t, "user2", "repo1", "i-dont-exist", "new-branch-name", http.StatusNotFound) + assert.Contains(t, resp.Body.String(), "Branch doesn't exist.") + }) + t.Run("RenameBranchNormalScenario", func(t *testing.T) { + testAPIUpdateBranch(t, "user2", "repo1", "branch2", "new-branch-name", http.StatusNoContent) + }) + }) +} + +func testAPIUpdateBranch(t *testing.T, ownerName, repoName, from, to string, expectedHTTPStatus int) *httptest.ResponseRecorder { + token := getUserToken(t, ownerName, auth_model.AccessTokenScopeWriteRepository) + req := NewRequestWithJSON(t, "PATCH", "api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.UpdateBranchRepoOption{ + Name: to, + }).AddTokenAuth(token) + return MakeRequest(t, req, expectedHTTPStatus) +} + func TestAPIBranchProtection(t *testing.T) { defer tests.PrepareTestEnv(t)() From 22bf2ca6ba1b89bcb88217541f31900dd606391e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 12 Dec 2024 16:10:09 +0800 Subject: [PATCH 04/12] Make API "compare" accept commit IDs (#32801) --- modules/git/commit.go | 14 ++ modules/git/ref.go | 4 +- modules/git/repo_ref.go | 28 ++++ modules/globallock/globallock_test.go | 2 +- routers/api/v1/repo/compare.go | 15 +-- routers/api/v1/repo/pull.go | 147 ++++++++++----------- routers/web/repo/issue.go | 2 - services/context/repo.go | 30 +---- tests/integration/api_repo_compare_test.go | 28 ++-- tests/integration/integration_test.go | 2 +- 10 files changed, 151 insertions(+), 121 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 010b56948ef60..0ed268e3469cc 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -474,3 +474,17 @@ func (c *Commit) GetRepositoryDefaultPublicGPGKey(forceUpdate bool) (*GPGSetting } return c.repo.GetDefaultPublicGPGKey(forceUpdate) } + +func IsStringLikelyCommitID(objFmt ObjectFormat, s string, minLength ...int) bool { + minLen := util.OptionalArg(minLength, objFmt.FullLength()) + if len(s) < minLen || len(s) > objFmt.FullLength() { + return false + } + for _, c := range s { + isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') + if !isHex { + return false + } + } + return true +} diff --git a/modules/git/ref.go b/modules/git/ref.go index 2db630e2ea9ad..aab4c5d77d75c 100644 --- a/modules/git/ref.go +++ b/modules/git/ref.go @@ -142,7 +142,6 @@ func (ref RefName) RemoteName() string { // ShortName returns the short name of the reference name func (ref RefName) ShortName() string { - refName := string(ref) if ref.IsBranch() { return ref.BranchName() } @@ -158,8 +157,7 @@ func (ref RefName) ShortName() string { if ref.IsFor() { return ref.ForBranchName() } - - return refName + return string(ref) // usually it is a commit ID } // RefGroup returns the group type of the reference diff --git a/modules/git/repo_ref.go b/modules/git/repo_ref.go index 8eaa17cb04182..850ec65502940 100644 --- a/modules/git/repo_ref.go +++ b/modules/git/repo_ref.go @@ -61,3 +61,31 @@ func parseTags(refs []string) []string { } return results } + +// UnstableGuessRefByShortName does the best guess to see whether a "short name" provided by user is a branch, tag or commit. +// It could guess wrongly if the input is already ambiguous. For example: +// * "refs/heads/the-name" vs "refs/heads/refs/heads/the-name" +// * "refs/tags/1234567890" vs commit "1234567890" +// In most cases, it SHOULD AVOID using this function, unless there is an irresistible reason (eg: make API friendly to end users) +// If the function is used, the caller SHOULD CHECK the ref type carefully. +func (repo *Repository) UnstableGuessRefByShortName(shortName string) RefName { + if repo.IsBranchExist(shortName) { + return RefNameFromBranch(shortName) + } + if repo.IsTagExist(shortName) { + return RefNameFromTag(shortName) + } + if strings.HasPrefix(shortName, "refs/") { + if repo.IsReferenceExist(shortName) { + return RefName(shortName) + } + } + commit, err := repo.GetCommit(shortName) + if err == nil { + commitIDString := commit.ID.String() + if strings.HasPrefix(commitIDString, shortName) { + return RefName(commitIDString) + } + } + return "" +} diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index 88a555c86f3bd..f14c7d656b64b 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -64,7 +64,7 @@ func TestLockAndDo(t *testing.T) { } func testLockAndDo(t *testing.T) { - const concurrency = 1000 + const concurrency = 50 ctx := context.Background() count := 0 diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index 38e5330b3acb8..1678bc033c639 100644 --- a/routers/api/v1/repo/compare.go +++ b/routers/api/v1/repo/compare.go @@ -64,22 +64,19 @@ func CompareDiff(ctx *context.APIContext) { } } - _, headGitRepo, ci, _, _ := parseCompareInfo(ctx, api.CreatePullRequestOption{ - Base: infos[0], - Head: infos[1], - }) + compareResult, closer := parseCompareInfo(ctx, api.CreatePullRequestOption{Base: infos[0], Head: infos[1]}) if ctx.Written() { return } - defer headGitRepo.Close() + defer closer() verification := ctx.FormString("verification") == "" || ctx.FormBool("verification") files := ctx.FormString("files") == "" || ctx.FormBool("files") - apiCommits := make([]*api.Commit, 0, len(ci.Commits)) + apiCommits := make([]*api.Commit, 0, len(compareResult.compareInfo.Commits)) userCache := make(map[string]*user_model.User) - for i := 0; i < len(ci.Commits); i++ { - apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, ci.Commits[i], userCache, + for i := 0; i < len(compareResult.compareInfo.Commits); i++ { + apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, compareResult.compareInfo.Commits[i], userCache, convert.ToCommitOptions{ Stat: true, Verification: verification, @@ -93,7 +90,7 @@ func CompareDiff(ctx *context.APIContext) { } ctx.JSON(http.StatusOK, &api.Compare{ - TotalCommits: len(ci.Commits), + TotalCommits: len(compareResult.compareInfo.Commits), Commits: apiCommits, }) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 86b204f51e250..6f4f3efaa10fa 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -389,8 +389,7 @@ func CreatePullRequest(ctx *context.APIContext) { form := *web.GetForm(ctx).(*api.CreatePullRequestOption) if form.Head == form.Base { - ctx.Error(http.StatusUnprocessableEntity, "BaseHeadSame", - "Invalid PullRequest: There are no changes between the head and the base") + ctx.Error(http.StatusUnprocessableEntity, "BaseHeadSame", "Invalid PullRequest: There are no changes between the head and the base") return } @@ -401,14 +400,22 @@ func CreatePullRequest(ctx *context.APIContext) { ) // Get repo/branch information - headRepo, headGitRepo, compareInfo, baseBranch, headBranch := parseCompareInfo(ctx, form) + compareResult, closer := parseCompareInfo(ctx, form) if ctx.Written() { return } - defer headGitRepo.Close() + defer closer() + + if !compareResult.baseRef.IsBranch() || !compareResult.headRef.IsBranch() { + ctx.Error(http.StatusUnprocessableEntity, "BaseHeadInvalidRefType", "Invalid PullRequest: base and head must be branches") + return + } // Check if another PR exists with the same targets - existingPr, err := issues_model.GetUnmergedPullRequest(ctx, headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch, issues_model.PullRequestFlowGithub) + existingPr, err := issues_model.GetUnmergedPullRequest(ctx, compareResult.headRepo.ID, ctx.Repo.Repository.ID, + compareResult.headRef.ShortName(), compareResult.baseRef.ShortName(), + issues_model.PullRequestFlowGithub, + ) if err != nil { if !issues_model.IsErrPullRequestNotExist(err) { ctx.Error(http.StatusInternalServerError, "GetUnmergedPullRequest", err) @@ -484,13 +491,13 @@ func CreatePullRequest(ctx *context.APIContext) { DeadlineUnix: deadlineUnix, } pr := &issues_model.PullRequest{ - HeadRepoID: headRepo.ID, + HeadRepoID: compareResult.headRepo.ID, BaseRepoID: repo.ID, - HeadBranch: headBranch, - BaseBranch: baseBranch, - HeadRepo: headRepo, + HeadBranch: compareResult.headRef.ShortName(), + BaseBranch: compareResult.baseRef.ShortName(), + HeadRepo: compareResult.headRepo, BaseRepo: repo, - MergeBase: compareInfo.MergeBase, + MergeBase: compareResult.compareInfo.MergeBase, Type: issues_model.PullRequestGitea, } @@ -1080,32 +1087,32 @@ func MergePullRequest(ctx *context.APIContext) { ctx.Status(http.StatusOK) } -func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) (*repo_model.Repository, *git.Repository, *git.CompareInfo, string, string) { - baseRepo := ctx.Repo.Repository +type parseCompareInfoResult struct { + headRepo *repo_model.Repository + headGitRepo *git.Repository + compareInfo *git.CompareInfo + baseRef git.RefName + headRef git.RefName +} +// parseCompareInfo returns non-nil if it succeeds, it always writes to the context and returns nil if it fails +func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) (result *parseCompareInfoResult, closer func()) { + var err error // Get compared branches information // format: ...[:] // base<-head: master...head:feature // same repo: master...feature + baseRepo := ctx.Repo.Repository + baseRefToGuess := form.Base - // TODO: Validate form first? - - baseBranch := form.Base - - var ( - headUser *user_model.User - headBranch string - isSameRepo bool - err error - ) - - // If there is no head repository, it means pull request between same repository. - headInfos := strings.Split(form.Head, ":") - if len(headInfos) == 1 { - isSameRepo = true - headUser = ctx.Repo.Owner - headBranch = headInfos[0] + headUser := ctx.Repo.Owner + headRefToGuess := form.Head + if headInfos := strings.Split(form.Head, ":"); len(headInfos) == 1 { + // If there is no head repository, it means pull request between same repository. + // Do nothing here because the head variables have been assigned above. } else if len(headInfos) == 2 { + // There is a head repository (the head repository could also be the same base repo) + headRefToGuess = headInfos[1] headUser, err = user_model.GetUserByName(ctx, headInfos[0]) if err != nil { if user_model.IsErrUserNotExist(err) { @@ -1113,38 +1120,29 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) } else { ctx.Error(http.StatusInternalServerError, "GetUserByName", err) } - return nil, nil, nil, "", "" + return nil, nil } - headBranch = headInfos[1] - // The head repository can also point to the same repo - isSameRepo = ctx.Repo.Owner.ID == headUser.ID } else { ctx.NotFound() - return nil, nil, nil, "", "" + return nil, nil } - ctx.Repo.PullRequest.SameRepo = isSameRepo - log.Trace("Repo path: %q, base branch: %q, head branch: %q", ctx.Repo.GitRepo.Path, baseBranch, headBranch) - // Check if base branch is valid. - if !ctx.Repo.GitRepo.IsBranchExist(baseBranch) && !ctx.Repo.GitRepo.IsTagExist(baseBranch) { - ctx.NotFound("BaseNotExist") - return nil, nil, nil, "", "" - } + isSameRepo := ctx.Repo.Owner.ID == headUser.ID // Check if current user has fork of repository or in the same repository. headRepo := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) if headRepo == nil && !isSameRepo { - err := baseRepo.GetBaseRepo(ctx) + err = baseRepo.GetBaseRepo(ctx) if err != nil { ctx.Error(http.StatusInternalServerError, "GetBaseRepo", err) - return nil, nil, nil, "", "" + return nil, nil } // Check if baseRepo's base repository is the same as headUser's repository. if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID { log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID) ctx.NotFound("GetBaseRepo") - return nil, nil, nil, "", "" + return nil, nil } // Assign headRepo so it can be used below. headRepo = baseRepo.BaseRepo @@ -1154,67 +1152,68 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) if isSameRepo { headRepo = ctx.Repo.Repository headGitRepo = ctx.Repo.GitRepo + closer = func() {} // no need to close the head repo because it shares the base repo } else { headGitRepo, err = gitrepo.OpenRepository(ctx, headRepo) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) - return nil, nil, nil, "", "" + return nil, nil } + closer = func() { _ = headGitRepo.Close() } } + defer func() { + if result == nil && !isSameRepo { + _ = headGitRepo.Close() + } + }() // user should have permission to read baseRepo's codes and pulls, NOT headRepo's permBase, err := access_model.GetUserRepoPermission(ctx, baseRepo, ctx.Doer) if err != nil { - headGitRepo.Close() ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err) - return nil, nil, nil, "", "" + return nil, nil } + if !permBase.CanReadIssuesOrPulls(true) || !permBase.CanRead(unit.TypeCode) { - if log.IsTrace() { - log.Trace("Permission Denied: User %-v cannot create/read pull requests or cannot read code in Repo %-v\nUser in baseRepo has Permissions: %-+v", - ctx.Doer, - baseRepo, - permBase) - } - headGitRepo.Close() + log.Trace("Permission Denied: User %-v cannot create/read pull requests or cannot read code in Repo %-v\nUser in baseRepo has Permissions: %-+v", ctx.Doer, baseRepo, permBase) ctx.NotFound("Can't read pulls or can't read UnitTypeCode") - return nil, nil, nil, "", "" + return nil, nil } - // user should have permission to read headrepo's codes + // user should have permission to read headRepo's codes + // TODO: could the logic be simplified if the headRepo is the same as the baseRepo? Need to think more about it. permHead, err := access_model.GetUserRepoPermission(ctx, headRepo, ctx.Doer) if err != nil { - headGitRepo.Close() ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err) - return nil, nil, nil, "", "" + return nil, nil } if !permHead.CanRead(unit.TypeCode) { - if log.IsTrace() { - log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v", - ctx.Doer, - headRepo, - permHead) - } - headGitRepo.Close() + log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v", ctx.Doer, headRepo, permHead) ctx.NotFound("Can't read headRepo UnitTypeCode") - return nil, nil, nil, "", "" + return nil, nil } - // Check if head branch is valid. - if !headGitRepo.IsBranchExist(headBranch) && !headGitRepo.IsTagExist(headBranch) { - headGitRepo.Close() + baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefToGuess) + headRef := headGitRepo.UnstableGuessRefByShortName(headRefToGuess) + + log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.GitRepo.Path, baseRefToGuess, baseRef, headRefToGuess, headRef) + + baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName()) + headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName()) + // Check if base&head ref are valid. + if !baseRefValid || !headRefValid { ctx.NotFound() - return nil, nil, nil, "", "" + return nil, nil } - compareInfo, err := headGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch, false, false) + compareInfo, err := headGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseRef.ShortName(), headRef.ShortName(), false, false) if err != nil { - headGitRepo.Close() ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err) - return nil, nil, nil, "", "" + return nil, nil } - return headRepo, headGitRepo, compareInfo, baseBranch, headBranch + result = &parseCompareInfoResult{headRepo: headRepo, headGitRepo: headGitRepo, compareInfo: compareInfo, baseRef: baseRef, headRef: headRef} + return result, closer } // UpdatePullRequest merge PR's baseBranch into headBranch diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 833f59981b46b..5397411b59cc2 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -9,7 +9,6 @@ import ( "fmt" "html/template" "net/http" - "net/url" "strconv" "strings" @@ -114,7 +113,6 @@ func MustAllowPulls(ctx *context.Context) { // User can send pull request if owns a forked repository. if ctx.IsSigned && repo_model.HasForkedRepo(ctx, ctx.Doer.ID, ctx.Repo.Repository.ID) { ctx.Repo.PullRequest.Allowed = true - ctx.Repo.PullRequest.HeadInfoSubURL = url.PathEscape(ctx.Doer.Name) + ":" + util.PathEscapeSegments(ctx.Repo.BranchName) } } diff --git a/services/context/repo.go b/services/context/repo.go index cf328ca97b77c..9b5443911030a 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -39,10 +39,9 @@ import ( // PullRequest contains information to make a pull request type PullRequest struct { - BaseRepo *repo_model.Repository - Allowed bool - SameRepo bool - HeadInfoSubURL string // [:] url segment + BaseRepo *repo_model.Repository + Allowed bool // it only used by the web tmpl: "PullRequestCtx.Allowed" + SameRepo bool // it only used by the web tmpl: "PullRequestCtx.SameRepo" } // Repository contains information to operate a repository @@ -401,6 +400,7 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) { // RepoAssignment returns a middleware to handle repository assignment func RepoAssignment(ctx *Context) context.CancelFunc { if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; repoAssignmentOnce { + // FIXME: it should panic in dev/test modes to have a clear behavior log.Trace("RepoAssignment was exec already, skipping second call ...") return nil } @@ -697,7 +697,6 @@ func RepoAssignment(ctx *Context) context.CancelFunc { ctx.Data["BaseRepo"] = repo.BaseRepo ctx.Repo.PullRequest.BaseRepo = repo.BaseRepo ctx.Repo.PullRequest.Allowed = canPush - ctx.Repo.PullRequest.HeadInfoSubURL = url.PathEscape(ctx.Repo.Owner.Name) + ":" + util.PathEscapeSegments(ctx.Repo.BranchName) } else if repo.AllowsPulls(ctx) { // Or, this is repository accepts pull requests between branches. canCompare = true @@ -705,7 +704,6 @@ func RepoAssignment(ctx *Context) context.CancelFunc { ctx.Repo.PullRequest.BaseRepo = repo ctx.Repo.PullRequest.Allowed = canPush ctx.Repo.PullRequest.SameRepo = true - ctx.Repo.PullRequest.HeadInfoSubURL = util.PathEscapeSegments(ctx.Repo.BranchName) } ctx.Data["CanCompareOrPull"] = canCompare ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequest @@ -771,20 +769,6 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool return "" } -func isStringLikelyCommitID(objFmt git.ObjectFormat, s string, minLength ...int) bool { - minLen := util.OptionalArg(minLength, objFmt.FullLength()) - if len(s) < minLen || len(s) > objFmt.FullLength() { - return false - } - for _, c := range s { - isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') - if !isHex { - return false - } - } - return true -} - func getRefNameLegacy(ctx *Base, repo *Repository, optionalExtraRef ...string) (string, RepoRefType) { extraRef := util.OptionalArg(optionalExtraRef) reqPath := ctx.PathParam("*") @@ -799,7 +783,7 @@ func getRefNameLegacy(ctx *Base, repo *Repository, optionalExtraRef ...string) ( // For legacy support only full commit sha parts := strings.Split(reqPath, "/") - if isStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), parts[0]) { + if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), parts[0]) { // FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists repo.TreePath = strings.Join(parts[1:], "/") return parts[0], RepoRefCommit @@ -849,7 +833,7 @@ func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string { return getRefNameFromPath(repo, path, repo.GitRepo.IsTagExist) case RepoRefCommit: parts := strings.Split(path, "/") - if isStringLikelyCommitID(repo.GetObjectFormat(), parts[0], 7) { + if git.IsStringLikelyCommitID(repo.GetObjectFormat(), parts[0], 7) { // FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists repo.TreePath = strings.Join(parts[1:], "/") return parts[0] @@ -985,7 +969,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func return cancel } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if isStringLikelyCommitID(ctx.Repo.GetObjectFormat(), refName, 7) { + } else if git.IsStringLikelyCommitID(ctx.Repo.GetObjectFormat(), refName, 7) { ctx.Repo.IsViewCommit = true ctx.Repo.CommitID = refName diff --git a/tests/integration/api_repo_compare_test.go b/tests/integration/api_repo_compare_test.go index f3188eb49f621..9565e4d2090bd 100644 --- a/tests/integration/api_repo_compare_test.go +++ b/tests/integration/api_repo_compare_test.go @@ -24,15 +24,27 @@ func TestAPICompareBranches(t *testing.T) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - repoName := "repo20" + t.Run("CompareBranches", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) - req := NewRequestf(t, "GET", "/api/v1/repos/user2/%s/compare/add-csv...remove-files-b", repoName). - AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusOK) + var apiResp *api.Compare + DecodeJSON(t, resp, &apiResp) - var apiResp *api.Compare - DecodeJSON(t, resp, &apiResp) + assert.Equal(t, 2, apiResp.TotalCommits) + assert.Len(t, apiResp.Commits, 2) + }) - assert.Equal(t, 2, apiResp.TotalCommits) - assert.Len(t, apiResp.Commits, 2) + t.Run("CompareCommits", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/808038d2f71b0ab02099...c8e31bc7688741a5287f").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + + var apiResp *api.Compare + DecodeJSON(t, resp, &apiResp) + + assert.Equal(t, 1, apiResp.TotalCommits) + assert.Len(t, apiResp.Commits, 1) + }) } diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 4338e19617468..8b6605eac8f3e 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -440,7 +440,7 @@ func DecodeJSON(t testing.TB, resp *httptest.ResponseRecorder, v any) { t.Helper() decoder := json.NewDecoder(resp.Body) - assert.NoError(t, decoder.Decode(v)) + require.NoError(t, decoder.Decode(v)) } func VerifyJSONSchema(t testing.TB, resp *httptest.ResponseRecorder, schemaFile string) { From 566f5356dbdddfa29eb131512c46a18fa3cdc3bd Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 12 Dec 2024 12:02:59 -0500 Subject: [PATCH 05/12] use dedicated runners for release artifacts (#32811) GH runners are having trouble, so switch the remaining release jobs to use dedicated runners. --- .github/workflows/release-nightly.yml | 6 +++--- .github/workflows/release-tag-rc.yml | 6 +++--- .github/workflows/release-tag-version.yml | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/release-nightly.yml b/.github/workflows/release-nightly.yml index 6e1b6e0758417..2264c9e822d1c 100644 --- a/.github/workflows/release-nightly.yml +++ b/.github/workflows/release-nightly.yml @@ -10,7 +10,7 @@ concurrency: jobs: nightly-binary: - runs-on: nscloud + runs-on: namespace-profile-gitea-release-binary steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions @@ -58,7 +58,7 @@ jobs: run: | aws s3 sync dist/release s3://${{ secrets.AWS_S3_BUCKET }}/gitea/${{ steps.clean_name.outputs.branch }} --no-progress nightly-docker-rootful: - runs-on: ubuntu-latest + runs-on: namespace-profile-gitea-release-docker steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions @@ -95,7 +95,7 @@ jobs: push: true tags: gitea/gitea:${{ steps.clean_name.outputs.branch }} nightly-docker-rootless: - runs-on: ubuntu-latest + runs-on: namespace-profile-gitea-release-docker steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions diff --git a/.github/workflows/release-tag-rc.yml b/.github/workflows/release-tag-rc.yml index 41037df29cbc0..a406602dc0a74 100644 --- a/.github/workflows/release-tag-rc.yml +++ b/.github/workflows/release-tag-rc.yml @@ -11,7 +11,7 @@ concurrency: jobs: binary: - runs-on: nscloud + runs-on: namespace-profile-gitea-release-binary steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions @@ -68,7 +68,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN }} docker-rootful: - runs-on: ubuntu-latest + runs-on: namespace-profile-gitea-release-docker steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions @@ -99,7 +99,7 @@ jobs: tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} docker-rootless: - runs-on: ubuntu-latest + runs-on: namespace-profile-gitea-release-docker steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions diff --git a/.github/workflows/release-tag-version.yml b/.github/workflows/release-tag-version.yml index a23e6982000ae..f67b76f40873b 100644 --- a/.github/workflows/release-tag-version.yml +++ b/.github/workflows/release-tag-version.yml @@ -13,7 +13,7 @@ concurrency: jobs: binary: - runs-on: nscloud + runs-on: namespace-profile-gitea-release-binary steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions @@ -70,7 +70,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN }} docker-rootful: - runs-on: ubuntu-latest + runs-on: namespace-profile-gitea-release-docker steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions @@ -105,7 +105,7 @@ jobs: tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} docker-rootless: - runs-on: ubuntu-latest + runs-on: namespace-profile-gitea-release-docker steps: - uses: actions/checkout@v4 # fetch all commits instead of only the last as some branches are long lived and could have many between versions From 00e2b339b6ba2df29c0c050221e58af5e7707ef8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 13 Dec 2024 02:37:44 +0800 Subject: [PATCH 06/12] Fix "unicode escape" JS error (#32806)
![image](https://github.com/user-attachments/assets/98aef2fb-791e-4b4a-b2ac-e880f8a52040) ![image](https://github.com/user-attachments/assets/532673ae-c4cf-4d84-a5c6-93e6eacd341c) ![image](https://github.com/user-attachments/assets/2a241a3d-b7f6-44ca-89d9-9d68386fbf3e) ![image](https://github.com/user-attachments/assets/1251e43d-41f2-42d1-a23b-3182e3812c3d)
--------- Co-authored-by: silverwind --- templates/repo/diff/box.tmpl | 4 ++-- templates/repo/wiki/view.tmpl | 6 +++--- web_src/js/features/repo-unicode-escape.ts | 21 +++++++++++---------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 0f1458bfbfd22..53ea4fd2e3918 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -167,8 +167,8 @@
{{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}} - - + + {{end}} {{if and (not $file.IsSubmodule) (not $.PageIsWiki)}} {{if $file.IsDeleted}} diff --git a/templates/repo/wiki/view.tmpl b/templates/repo/wiki/view.tmpl index 2bb0a4f006888..843a977e3e57c 100644 --- a/templates/repo/wiki/view.tmpl +++ b/templates/repo/wiki/view.tmpl @@ -44,13 +44,13 @@
diff --git a/web_src/js/features/repo-unicode-escape.ts b/web_src/js/features/repo-unicode-escape.ts index 0c7d2e8592aa0..49e34e22cdf28 100644 --- a/web_src/js/features/repo-unicode-escape.ts +++ b/web_src/js/features/repo-unicode-escape.ts @@ -1,27 +1,28 @@ import {addDelegatedEventListener, hideElem, queryElemSiblings, showElem, toggleElem} from '../utils/dom.ts'; export function initUnicodeEscapeButton() { + // buttons might appear on these pages: file view (code, rendered markdown), diff (commit, pr conversation, pr diff), blame, wiki addDelegatedEventListener(document, 'click', '.escape-button, .unescape-button, .toggle-escape-button', (btn, e) => { e.preventDefault(); - const fileContentElemId = btn.getAttribute('data-file-content-elem-id'); - const fileContent = fileContentElemId ? - document.querySelector(`#${fileContentElemId}`) : + const unicodeContentSelector = btn.getAttribute('data-unicode-content-selector'); + const container = unicodeContentSelector ? + document.querySelector(unicodeContentSelector) : btn.closest('.file-content, .non-diff-file-content'); - const fileView = fileContent?.querySelectorAll('.file-code, .file-view'); + const fileView = container.querySelector('.file-code, .file-view') ?? container; if (btn.matches('.escape-button')) { - for (const el of fileView) el.classList.add('unicode-escaped'); + fileView.classList.add('unicode-escaped'); hideElem(btn); showElem(queryElemSiblings(btn, '.unescape-button')); } else if (btn.matches('.unescape-button')) { - for (const el of fileView) el.classList.remove('unicode-escaped'); + fileView.classList.remove('unicode-escaped'); hideElem(btn); showElem(queryElemSiblings(btn, '.escape-button')); } else if (btn.matches('.toggle-escape-button')) { - const isEscaped = fileView[0]?.classList.contains('unicode-escaped'); - for (const el of fileView) el.classList.toggle('unicode-escaped', !isEscaped); - toggleElem(fileContent.querySelectorAll('.unescape-button'), !isEscaped); - toggleElem(fileContent.querySelectorAll('.escape-button'), isEscaped); + const isEscaped = fileView.classList.contains('unicode-escaped'); + fileView.classList.toggle('unicode-escaped', !isEscaped); + toggleElem(container.querySelectorAll('.unescape-button'), !isEscaped); + toggleElem(container.querySelectorAll('.escape-button'), isEscaped); } }); } From c9487a755b742fd2257f57cec1ead3f4c71174d7 Mon Sep 17 00:00:00 2001 From: Chai-Shi Date: Fri, 13 Dec 2024 03:02:54 +0800 Subject: [PATCH 07/12] Add "n commits" link to contributors in contributors graph page (#32799) Fixes Issue #29365 and inherit PR #29429 - I should extend the #29429 fork but the fork is not synced, so I created another PR. - Use `silenced` class for the link, as in #29847 --------- Co-authored-by: Ben Chang Co-authored-by: wxiaoguang --- templates/repo/contributors.tmpl | 1 + web_src/js/components/RepoContributors.vue | 27 ++++++++++++++++++---- web_src/js/features/contributors.ts | 1 + 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/templates/repo/contributors.tmpl b/templates/repo/contributors.tmpl index 6b8a63fe9964d..882d8b205cf2e 100644 --- a/templates/repo/contributors.tmpl +++ b/templates/repo/contributors.tmpl @@ -1,6 +1,7 @@ {{if .Permission.CanRead ctx.Consts.RepoUnitTypeCode}}
import {SvgIcon} from '../svg.ts'; +import dayjs from 'dayjs'; import { Chart, Title, @@ -26,6 +27,7 @@ import {sleep} from '../utils.ts'; import 'chartjs-adapter-dayjs-4/dist/chartjs-adapter-dayjs-4.esm'; import {fomanticQuery} from '../modules/fomantic/base.ts'; import type {Entries} from 'type-fest'; +import {pathEscapeSegments} from '../utils/url.ts'; const customEventListener: Plugin = { id: 'customEventListener', @@ -65,6 +67,10 @@ export default { type: String, required: true, }, + repoDefaultBranchName: { + type: String, + required: true, + }, }, data: () => ({ isLoading: false, @@ -100,6 +106,15 @@ export default { .slice(0, 100); }, + getContributorSearchQuery(contributorEmail: string) { + const min = dayjs(this.xAxisMin).format('YYYY-MM-DD'); + const max = dayjs(this.xAxisMax).format('YYYY-MM-DD'); + const params = new URLSearchParams({ + 'q': `after:${min}, before:${max}, author:${contributorEmail}`, + }); + return `${this.repoLink}/commits/branch/${pathEscapeSegments(this.repoDefaultBranchName)}/search?${params.toString()}`; + }, + async fetchGraphData() { this.isLoading = true; try { @@ -167,7 +182,7 @@ export default { // for details. user.max_contribution_type += 1; - filteredData[key] = {...user, weeks: filteredWeeks}; + filteredData[key] = {...user, weeks: filteredWeeks, email: key}; } return filteredData; @@ -215,7 +230,7 @@ export default { }; }, - updateOtherCharts({chart}: {chart: Chart}, reset?: boolean = false) { + updateOtherCharts({chart}: {chart: Chart}, reset: boolean = false) { const minVal = chart.options.scales.x.min; const maxVal = chart.options.scales.x.max; if (reset) { @@ -381,7 +396,7 @@ export default {
#{{ index + 1 }} - +

{{ contributor.name }}

@@ -389,7 +404,11 @@ export default { {{ contributor.name }}

- {{ contributor.total_commits.toLocaleString() }} {{ locale.contributionType.commits }} + + + {{ contributor.total_commits.toLocaleString() }} {{ locale.contributionType.commits }} + + {{ contributor.total_additions.toLocaleString() }}++ {{ contributor.total_deletions.toLocaleString() }}-- diff --git a/web_src/js/features/contributors.ts b/web_src/js/features/contributors.ts index 475c66e90060a..95fc81f5b368b 100644 --- a/web_src/js/features/contributors.ts +++ b/web_src/js/features/contributors.ts @@ -8,6 +8,7 @@ export async function initRepoContributors() { try { const View = createApp(RepoContributors, { repoLink: el.getAttribute('data-repo-link'), + repoDefaultBranchName: el.getAttribute('data-repo-default-branch-name'), locale: { filterLabel: el.getAttribute('data-locale-filter-label'), contributionType: { From 6370d2fb93a5ee897b82969ca30a9feb33667714 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 12 Dec 2024 11:28:23 -0800 Subject: [PATCH 08/12] Detect whether action view branch was deleted (#32764) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #32761 ![图片](https://github.com/user-attachments/assets/a5a7eef8-0fea-4242-b199-1b0b73d9bbdb) --- models/actions/run.go | 1 + models/git/branch.go | 18 +++++++++++-- routers/web/repo/actions/actions.go | 32 ++++++++++++++++++++++++ routers/web/repo/actions/view.go | 16 ++++++++++-- services/repository/branch.go | 2 +- templates/repo/actions/runs_list.tmpl | 6 ++--- web_src/js/components/RepoActionView.vue | 3 ++- 7 files changed, 69 insertions(+), 9 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 732fb48bb9a61..f40bc1eb3db1a 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -37,6 +37,7 @@ type ActionRun struct { TriggerUser *user_model.User `xorm:"-"` ScheduleID int64 Ref string `xorm:"index"` // the commit/tag/… that caused the run + IsRefDeleted bool `xorm:"-"` CommitSHA string IsForkPullRequest bool // If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow. NeedApproval bool // may need approval if it's a fork pull request diff --git a/models/git/branch.go b/models/git/branch.go index ba1ada5517dc0..e683ce47e657e 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -12,6 +12,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" @@ -169,9 +170,22 @@ func GetBranch(ctx context.Context, repoID int64, branchName string) (*Branch, e return &branch, nil } -func GetBranches(ctx context.Context, repoID int64, branchNames []string) ([]*Branch, error) { +func GetBranches(ctx context.Context, repoID int64, branchNames []string, includeDeleted bool) ([]*Branch, error) { branches := make([]*Branch, 0, len(branchNames)) - return branches, db.GetEngine(ctx).Where("repo_id=?", repoID).In("name", branchNames).Find(&branches) + + sess := db.GetEngine(ctx).Where("repo_id=?", repoID).In("name", branchNames) + if !includeDeleted { + sess.And("is_deleted=?", false) + } + return branches, sess.Find(&branches) +} + +func BranchesToNamesSet(branches []*Branch) container.Set[string] { + names := make(container.Set[string], len(branches)) + for _, branch := range branches { + names.Add(branch.Name) + } + return names } func AddBranches(ctx context.Context, branches []*Branch) error { diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index ad16b9fb4e4d5..7ed37ea26b2d1 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -245,6 +245,10 @@ func List(ctx *context.Context) { return } + if err := loadIsRefDeleted(ctx, runs); err != nil { + log.Error("LoadIsRefDeleted", err) + } + ctx.Data["Runs"] = runs actors, err := actions_model.GetActors(ctx, ctx.Repo.Repository.ID) @@ -267,6 +271,34 @@ func List(ctx *context.Context) { ctx.HTML(http.StatusOK, tplListActions) } +// loadIsRefDeleted loads the IsRefDeleted field for each run in the list. +// TODO: move this function to models/actions/run_list.go but now it will result in a circular import. +func loadIsRefDeleted(ctx *context.Context, runs actions_model.RunList) error { + branches := make(container.Set[string], len(runs)) + for _, run := range runs { + refName := git.RefName(run.Ref) + if refName.IsBranch() { + branches.Add(refName.ShortName()) + } + } + if len(branches) == 0 { + return nil + } + + branchInfos, err := git_model.GetBranches(ctx, ctx.Repo.Repository.ID, branches.Values(), false) + if err != nil { + return err + } + branchSet := git_model.BranchesToNamesSet(branchInfos) + for _, run := range runs { + refName := git.RefName(run.Ref) + if refName.IsBranch() && !branchSet.Contains(run.Ref) { + run.IsRefDeleted = true + } + } + return nil +} + type WorkflowDispatchInput struct { Name string `yaml:"name"` Description string `yaml:"description"` diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 73c6e54fbf50a..b711038da065a 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -19,6 +19,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" @@ -136,8 +137,9 @@ type ViewUser struct { } type ViewBranch struct { - Name string `json:"name"` - Link string `json:"link"` + Name string `json:"name"` + Link string `json:"link"` + IsDeleted bool `json:"isDeleted"` } type ViewJobStep struct { @@ -236,6 +238,16 @@ func ViewPost(ctx *context_module.Context) { Name: run.PrettyRef(), Link: run.RefLink(), } + refName := git.RefName(run.Ref) + if refName.IsBranch() { + b, err := git_model.GetBranch(ctx, ctx.Repo.Repository.ID, refName.ShortName()) + if err != nil && !git_model.IsErrBranchNotExist(err) { + log.Error("GetBranch: %v", err) + } else if git_model.IsErrBranchNotExist(err) || (b != nil && b.IsDeleted) { + branch.IsDeleted = true + } + } + resp.State.Run.Commit = ViewCommit{ ShortSha: base.ShortSha(run.CommitSHA), Link: fmt.Sprintf("%s/commit/%s", run.Repo.Link(), run.CommitSHA), diff --git a/services/repository/branch.go b/services/repository/branch.go index 508817c83e6c9..3a95aab264981 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -305,7 +305,7 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, branchNames, } return db.WithTx(ctx, func(ctx context.Context) error { - branches, err := git_model.GetBranches(ctx, repoID, branchNames) + branches, err := git_model.GetBranches(ctx, repoID, branchNames, true) if err != nil { return fmt.Errorf("git_model.GetBranches: %v", err) } diff --git a/templates/repo/actions/runs_list.tmpl b/templates/repo/actions/runs_list.tmpl index 5537e9e617215..fa1adb3e3ba2e 100644 --- a/templates/repo/actions/runs_list.tmpl +++ b/templates/repo/actions/runs_list.tmpl @@ -27,10 +27,10 @@

- {{if .RefLink}} - {{.PrettyRef}} + {{if .IsRefDeleted}} + {{.PrettyRef}} {{else}} - {{.PrettyRef}} + {{.PrettyRef}} {{end}}
{{svg "octicon-calendar" 16}}{{DateUtils.TimeSince .Updated}}
diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 7f647b668a641..eece2efaf868f 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -429,7 +429,8 @@ export function initRepositoryActionView() { {{ run.commit.pusher.displayName }} - {{ run.commit.branch.name }} + {{ run.commit.branch.name }} + {{ run.commit.branch.name }}
From ab6d819a89c11d2a2ca226c0728dc8c6d58d61cd Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 12 Dec 2024 15:45:27 -0500 Subject: [PATCH 09/12] Update actionlint.yaml --- .github/actionlint.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml index 023fb05a2969a..2adea23aa40aa 100644 --- a/.github/actionlint.yaml +++ b/.github/actionlint.yaml @@ -3,3 +3,5 @@ self-hosted-runner: - actuated-4cpu-8gb - actuated-4cpu-16gb - nscloud + - namespace-profile-gitea-release-docker + - namespace-profile-gitea-release-binary From a03fdd9566d62abd208af9ae30e58802a658e358 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Thu, 12 Dec 2024 15:10:47 -0600 Subject: [PATCH 10/12] Avoid MacOS keychain dialog in integration tests (#32813) Mac's git installation ships with a system wide config that configures the credential helper `osxkeychain`, which will prompt the user with a dialog. ``` $ git config list --system credential.helper=osxkeychain ``` By setting the environment variable [`GIT_CONFIG_NOSYSTEM=true`](https://git-scm.com/docs/git-config#ENVIRONMENT), Git will not load the system wide config, preventing the dialog from populating. Closes #26717 --- tests/integration/integration_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 8b6605eac8f3e..6b1b6b8b21ace 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -95,6 +95,11 @@ func TestMain(m *testing.M) { os.Unsetenv("GIT_COMMITTER_EMAIL") os.Unsetenv("GIT_COMMITTER_DATE") + // Avoid loading the default system config. On MacOS, this config + // sets the osxkeychain credential helper, which will cause tests + // to freeze with a dialog. + os.Setenv("GIT_CONFIG_NOSYSTEM", "true") + err := unittest.InitFixtures( unittest.FixturesOptions{ Dir: filepath.Join(filepath.Dir(setting.AppPath), "models/fixtures/"), From 0b8a8941a01ed4bf914843c88740ad6203550b85 Mon Sep 17 00:00:00 2001 From: hiifong Date: Fri, 13 Dec 2024 05:36:39 +0800 Subject: [PATCH 11/12] Fix lfs migration (#32812) Fix: #32803 --- modules/lfs/http_client.go | 1 + modules/lfs/shared.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/lfs/http_client.go b/modules/lfs/http_client.go index 3060e25754cc5..50f0e7a8d88a6 100644 --- a/modules/lfs/http_client.go +++ b/modules/lfs/http_client.go @@ -236,6 +236,7 @@ func createRequest(ctx context.Context, method, url string, headers map[string]s req.Header.Set(key, value) } req.Header.Set("Accept", AcceptHeader) + req.Header.Set("User-Agent", UserAgentHeader) return req, nil } diff --git a/modules/lfs/shared.go b/modules/lfs/shared.go index a4326b57b2f55..40ad789c1d9e8 100644 --- a/modules/lfs/shared.go +++ b/modules/lfs/shared.go @@ -15,7 +15,8 @@ const ( // MediaType contains the media type for LFS server requests MediaType = "application/vnd.git-lfs+json" // Some LFS servers offer content with other types, so fallback to '*/*' if application/vnd.git-lfs+json cannot be served - AcceptHeader = "application/vnd.git-lfs+json;q=0.9, */*;q=0.8" + AcceptHeader = "application/vnd.git-lfs+json;q=0.9, */*;q=0.8" + UserAgentHeader = "git-lfs" ) // BatchRequest contains multiple requests processed in one batch operation. From 30008fcfcfa84bf607baa493ffcebe7102363ba4 Mon Sep 17 00:00:00 2001 From: hiifong Date: Fri, 13 Dec 2024 08:45:06 +0800 Subject: [PATCH 12/12] Fix bug of branch/tag selector in the issue sidebar (#32744) Fix: #32731 --------- Co-authored-by: wxiaoguang --- models/issues/issue.go | 7 +++++-- .../repo/issue/branch_selector_field.tmpl | 18 ++++++++++++++++-- templates/repo/issue/new_form.tmpl | 3 ++- templates/repo/issue/view_content/sidebar.tmpl | 2 +- templates/shared/issuelist.tmpl | 2 +- web_src/js/features/repo-issue-sidebar.ts | 1 + 6 files changed, 26 insertions(+), 7 deletions(-) diff --git a/models/issues/issue.go b/models/issues/issue.go index 64fc20cc05e99..fe347c271560e 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -125,8 +125,11 @@ type Issue struct { IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not. PullRequest *PullRequest `xorm:"-"` NumComments int - Ref string - PinOrder int `xorm:"DEFAULT 0"` + + // TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl" + Ref string + + PinOrder int `xorm:"DEFAULT 0"` DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` diff --git a/templates/repo/issue/branch_selector_field.tmpl b/templates/repo/issue/branch_selector_field.tmpl index 286ac0cd051df..9183b7b46abc0 100644 --- a/templates/repo/issue/branch_selector_field.tmpl +++ b/templates/repo/issue/branch_selector_field.tmpl @@ -1,3 +1,17 @@ +{{/* TODO: RemoveIssueRef: the Issue.Ref will be removed in 1.24 or 1.25 if no end user really needs it or there could be better alternative then. +PR: https://github.com/go-gitea/gitea/pull/32744 + +The Issue.Ref was added by Add possibility to record branch or tag information in an issue (#780) +After 8 years, this "branch selector" does nothing more than saving the branch/tag name into database and displays it. + +There are still users using it: +* @didim99: it is a really useful feature to specify a branch in which issue found. + +Still needs to figure out: +* Could the "recording branch/tag name" be replaced by other approaches? + * Write the branch name in the issue title/body then it will still be displayed, eg: `[bug] (fix/ui-broken-bug) there is a bug ....` +* Is "GitHub-like development sidebar (`#31899`)" good enough (or better) for your usage? +*/}} {{if and (not .Issue.IsPull) (not .PageIsComparePull)}} {{else}} -
{{ctx.Locale.Tr "no_results_found"}}
+
{{ctx.Locale.Tr "no_results_found"}}
{{end}}
diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index ceaaebc4d5440..dd4c7617ce100 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -47,7 +47,8 @@
- {{template "repo/issue/branch_selector_field" $}} + {{template "repo/issue/branch_selector_field" $}}{{/* TODO: RemoveIssueRef: template "repo/issue/branch_selector_field" $*/}} + {{if .PageIsComparePull}} {{template "repo/issue/sidebar/reviewer_list" $.IssuePageMetaData}}
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 02f5d3e2df90f..987a882be7be5 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -1,5 +1,5 @@
- {{template "repo/issue/branch_selector_field" $}} + {{template "repo/issue/branch_selector_field" $}}{{/* TODO: RemoveIssueRef: template "repo/issue/branch_selector_field" $*/}} {{if .Issue.IsPull}} {{template "repo/issue/sidebar/reviewer_list" $.IssuePageMetaData}} diff --git a/templates/shared/issuelist.tmpl b/templates/shared/issuelist.tmpl index fe5184e7d224f..a2b802f2a2d0f 100644 --- a/templates/shared/issuelist.tmpl +++ b/templates/shared/issuelist.tmpl @@ -99,7 +99,7 @@ {{.Project.Title}} {{end}} - {{if .Ref}} + {{if .Ref}}{{/* TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl" */}} {{svg "octicon-git-branch" 14}} {{index $.IssueRefEndNames .ID}} diff --git a/web_src/js/features/repo-issue-sidebar.ts b/web_src/js/features/repo-issue-sidebar.ts index 45cd38d533d33..ef2b7d143cdd6 100644 --- a/web_src/js/features/repo-issue-sidebar.ts +++ b/web_src/js/features/repo-issue-sidebar.ts @@ -4,6 +4,7 @@ import {queryElems, toggleElem} from '../utils/dom.ts'; import {initIssueSidebarComboList} from './repo-issue-sidebar-combolist.ts'; function initBranchSelector() { + // TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl" const elSelectBranch = document.querySelector('.ui.dropdown.select-branch'); if (!elSelectBranch) return;