From 023c465ab4526d7f53dc9cc4b749e7193cc14717 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 26 Jan 2020 10:45:41 +0000 Subject: [PATCH 1/2] Initial implementation of ApplyDiffPatch This code adds a simple endpoint to add diff patch application to Gitea --- modules/repofiles/patch.go | 159 +++++++++++++++++++++++++++++++ modules/structs/repo_file.go | 8 ++ options/locale/locale_en-US.ini | 3 + routers/api/v1/api.go | 1 + routers/api/v1/repo/patch.go | 87 +++++++++++++++++ routers/repo/patch.go | 110 +++++++++++++++++++++ routers/routes/routes.go | 2 + templates/repo/editor/patch.tmpl | 60 ++++++++++++ templates/swagger/v1_json.tmpl | 44 +++++++++ 9 files changed, 474 insertions(+) create mode 100644 modules/repofiles/patch.go create mode 100644 routers/api/v1/repo/patch.go create mode 100644 routers/repo/patch.go create mode 100644 templates/repo/editor/patch.tmpl diff --git a/modules/repofiles/patch.go b/modules/repofiles/patch.go new file mode 100644 index 0000000000000..90b3848df28bc --- /dev/null +++ b/modules/repofiles/patch.go @@ -0,0 +1,159 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repofiles + +import ( + "fmt" + "strings" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + repo_module "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/modules/structs" + api "code.gitea.io/gitea/modules/structs" +) + +// ApplyDiffPatchOptions holds the repository diff patch update options +type ApplyDiffPatchOptions struct { + LastCommitID string + OldBranch string + NewBranch string + Message string + Content string + SHA string + Author *IdentityOptions + Committer *IdentityOptions + Dates *CommitDateOptions +} + +// ApplyDiffPatch applies a patch to the given repository +func ApplyDiffPatch(repo *models.Repository, doer *models.User, opts *ApplyDiffPatchOptions) (*structs.FileResponse, error) { + // If no branch name is set, assume master + if opts.OldBranch == "" { + opts.OldBranch = repo.DefaultBranch + } + if opts.NewBranch == "" { + opts.NewBranch = opts.OldBranch + } + + // oldBranch must exist for this operation + if _, err := repo_module.GetBranch(repo, opts.OldBranch); err != nil { + return nil, err + } + + // A NewBranch can be specified for the patch to be applied to. + // Check to make sure the branch does not already exist, otherwise we can't proceed. + // If we aren't branching to a new branch, make sure user can commit to the given branch + if opts.NewBranch != opts.OldBranch { + existingBranch, err := repo_module.GetBranch(repo, opts.NewBranch) + if existingBranch != nil { + return nil, models.ErrBranchAlreadyExists{ + BranchName: opts.NewBranch, + } + } + if err != nil && !git.IsErrBranchNotExist(err) { + return nil, err + } + } else { + protectedBranch, err := repo.GetBranchProtection(opts.OldBranch) + if err != nil { + return nil, err + } + if protectedBranch != nil && !protectedBranch.CanUserPush(doer.ID) { + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + if protectedBranch != nil && protectedBranch.RequireSignedCommits { + _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) + if err != nil { + if !models.IsErrWontSign(err) { + return nil, err + } + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + } + } + + message := strings.TrimSpace(opts.Message) + + author, committer := GetAuthorAndCommitterUsers(opts.Author, opts.Committer, doer) + + t, err := NewTemporaryUploadRepository(repo) + if err != nil { + log.Error("%v", err) + } + defer t.Close() + if err := t.Clone(opts.OldBranch); err != nil { + return nil, err + } + if err := t.SetDefaultIndex(); err != nil { + return nil, err + } + + // Get the commit of the original branch + commit, err := t.GetBranchCommit(opts.OldBranch) + if err != nil { + return nil, err // Couldn't get a commit for the branch + } + + // Assigned LastCommitID in opts if it hasn't been set + if opts.LastCommitID == "" { + opts.LastCommitID = commit.ID.String() + } else { + lastCommitID, err := t.gitRepo.ConvertToSHA1(opts.LastCommitID) + if err != nil { + return nil, fmt.Errorf("ApplyPatch: Invalid last commit ID: %v", err) + } + opts.LastCommitID = lastCommitID.String() + } + + stdout := &strings.Builder{} + stderr := &strings.Builder{} + + err = git.NewCommand("apply", "--index", "--cached", "--ignore-whitespace", "--whitespace=fix").RunInDirFullPipeline(t.basePath, stdout, stderr, strings.NewReader(opts.Content)) + if err != nil { + return nil, fmt.Errorf("Error: Stdout: %s\nStderr: %s\nErr: %v", stdout.String(), stderr.String(), err) + } + + // Now write the tree + treeHash, err := t.WriteTree() + if err != nil { + return nil, err + } + + // Now commit the tree + var commitHash string + if opts.Dates != nil { + commitHash, err = t.CommitTreeWithDate(author, committer, treeHash, message, opts.Dates.Author, opts.Dates.Committer) + } else { + commitHash, err = t.CommitTree(author, committer, treeHash, message) + } + if err != nil { + return nil, err + } + + // Then push this tree to NewBranch + if err := t.Push(doer, commitHash, opts.NewBranch); err != nil { + return nil, err + } + + commit, err = t.GetCommit(commitHash) + if err != nil { + return nil, err + } + + fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil + verification := GetPayloadCommitVerification(commit) + fileResponse := &api.FileResponse{ + Commit: fileCommitResponse, + Verification: verification, + } + + return fileResponse, nil +} diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index c34923e389dfc..6da2faca884a8 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -48,6 +48,14 @@ type UpdateFileOptions struct { FromPath string `json:"from_path" binding:"MaxSize(500)"` } +// ApplyDiffPatchFileOptions options for applying a diff patch +// Note: `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 ApplyDiffPatchFileOptions struct { + DeleteFileOptions + // required: true + Content string `json:"content"` +} + // FileLinksResponse contains the links for a repo's file type FileLinksResponse struct { Self *string `json:"self"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 662ea49acaebe..c0e76a38413ac 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -795,6 +795,9 @@ editor.add_tmpl = Add '' editor.add = Add '%s' editor.update = Update '%s' editor.delete = Delete '%s' +editor.patch = Apply Patch +editor.fail_to_apply_patch = Unable to apply patch '%s' +editor.new_patch = New Patch editor.commit_message_desc = Add an optional extended description… editor.commit_directly_to_this_branch = Commit directly to the %s branch. editor.create_new_branch = Create a new branch for this commit and start a pull request. diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index f356d7a2bc185..30b5650e1c78d 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -838,6 +838,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("/blobs/:sha", context.RepoRef(), repo.GetBlob) m.Get("/tags/:sha", context.RepoRef(), repo.GetTag) }, reqRepoReader(models.UnitTypeCode)) + m.Post("/diffpatch", reqRepoWriter(models.UnitTypeCode), reqToken(), bind(api.ApplyDiffPatchFileOptions{}), repo.ApplyDiffPatch) m.Group("/contents", func() { m.Get("", repo.GetContentsList) m.Get("/*", repo.GetContents) diff --git a/routers/api/v1/repo/patch.go b/routers/api/v1/repo/patch.go new file mode 100644 index 0000000000000..bcc01f3f6484d --- /dev/null +++ b/routers/api/v1/repo/patch.go @@ -0,0 +1,87 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "net/http" + "time" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/repofiles" + api "code.gitea.io/gitea/modules/structs" +) + +// ApplyDiffPatch handles API call for applying a patch +func ApplyDiffPatch(ctx *context.APIContext, apiOpts api.ApplyDiffPatchFileOptions) { + // swagger:operation POST /repos/{owner}/{repo}/diffpatch repository repoApplyDiffPatch + // --- + // summary: Apply diff patch to repository + // 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: body + // in: body + // required: true + // schema: + // "$ref": "#/definitions/UpdateFileOptions" + // responses: + // "200": + // "$ref": "#/responses/FileResponse" + opts := &repofiles.ApplyDiffPatchOptions{ + Content: apiOpts.Content, + SHA: apiOpts.SHA, + Message: apiOpts.Message, + OldBranch: apiOpts.BranchName, + NewBranch: apiOpts.NewBranchName, + Committer: &repofiles.IdentityOptions{ + Name: apiOpts.Committer.Name, + Email: apiOpts.Committer.Email, + }, + Author: &repofiles.IdentityOptions{ + Name: apiOpts.Author.Name, + Email: apiOpts.Author.Email, + }, + Dates: &repofiles.CommitDateOptions{ + Author: apiOpts.Dates.Author, + Committer: apiOpts.Dates.Committer, + }, + } + if opts.Dates.Author.IsZero() { + opts.Dates.Author = time.Now() + } + if opts.Dates.Committer.IsZero() { + opts.Dates.Committer = time.Now() + } + + if opts.Message == "" { + opts.Message = "apply-patch" + } + + if !canWriteFiles(ctx.Repo) { + ctx.Error(http.StatusInternalServerError, "ApplyPatch", models.ErrUserDoesNotHaveAccessToRepo{ + UserID: ctx.User.ID, + RepoName: ctx.Repo.Repository.LowerName, + }) + } + + if fileResponse, err := repofiles.ApplyDiffPatch(ctx.Repo.Repository, ctx.User, opts); err != nil { + ctx.Error(http.StatusInternalServerError, "ApplyPatch", err) + } else { + ctx.JSON(http.StatusCreated, fileResponse) + } +} diff --git a/routers/repo/patch.go b/routers/repo/patch.go new file mode 100644 index 0000000000000..d927b82b67964 --- /dev/null +++ b/routers/repo/patch.go @@ -0,0 +1,110 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "strings" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/auth" + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/repofiles" + "code.gitea.io/gitea/modules/setting" +) + +const ( + tplPatchFile base.TplName = "repo/editor/patch" +) + +// NewDiffPatch render create patch page +func NewDiffPatch(ctx *context.Context) { + ctx.Data["RequireHighlightJS"] = true + + canCommit := renderCommitRights(ctx) + + ctx.Data["TreePath"] = "patch" + + ctx.Data["commit_summary"] = "" + ctx.Data["commit_message"] = "" + if canCommit { + ctx.Data["commit_choice"] = frmCommitChoiceDirect + } else { + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + } + ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx) + ctx.Data["last_commit"] = ctx.Repo.CommitID + ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() + + ctx.HTML(200, tplPatchFile) +} + +// NewDiffPatchPost response for sending patch page +func NewDiffPatchPost(ctx *context.Context, form auth.EditRepoFileForm) { + canCommit := renderCommitRights(ctx) + branchName := ctx.Repo.BranchName + if form.CommitChoice == frmCommitChoiceNewBranch { + branchName = form.NewBranchName + } + ctx.Data["RequireHighlightJS"] = true + ctx.Data["TreePath"] = "patch" + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/branch/" + ctx.Repo.BranchName + ctx.Data["FileContent"] = form.Content + ctx.Data["commit_summary"] = form.CommitSummary + ctx.Data["commit_message"] = form.CommitMessage + ctx.Data["commit_choice"] = form.CommitChoice + ctx.Data["new_branch_name"] = form.NewBranchName + ctx.Data["last_commit"] = ctx.Repo.CommitID + ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") + + if ctx.HasError() { + ctx.HTML(200, tplPatchFile) + return + } + + // Cannot commit to a an existing branch if user doesn't have rights + if branchName == ctx.Repo.BranchName && !canCommit { + ctx.Data["Err_NewBranchName"] = true + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + ctx.RenderWithErr(ctx.Tr("repo.editor.cannot_commit_to_protected_branch", branchName), tplEditFile, &form) + return + } + + // CommitSummary is optional in the web form, if empty, give it a default message based on add or update + // `message` will be both the summary and message combined + message := strings.TrimSpace(form.CommitSummary) + if len(message) == 0 { + message = ctx.Tr("repo.editor.patch") + } + + form.CommitMessage = strings.TrimSpace(form.CommitMessage) + if len(form.CommitMessage) > 0 { + message += "\n\n" + form.CommitMessage + } + + if _, err := repofiles.ApplyDiffPatch(ctx.Repo.Repository, ctx.User, &repofiles.ApplyDiffPatchOptions{ + LastCommitID: form.LastCommit, + OldBranch: ctx.Repo.BranchName, + NewBranch: branchName, + Message: message, + Content: strings.Replace(form.Content, "\r", "", -1), + }); err != nil { + if models.IsErrBranchAlreadyExists(err) { + // For when a user specifies a new branch that already exists + ctx.Data["Err_NewBranchName"] = true + if branchErr, ok := err.(models.ErrBranchAlreadyExists); ok { + ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchErr.BranchName), tplEditFile, &form) + } else { + ctx.Error(500, err.Error()) + } + } else if models.IsErrCommitIDDoesNotMatch(err) { + ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplPatchFile, &form) + } else { + ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_apply_patch", err), tplPatchFile, &form) + } + } + ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + ctx.Repo.BranchName + "..." + form.NewBranchName) +} diff --git a/routers/routes/routes.go b/routers/routes/routes.go index d739f0b6ca5d2..db03505b7518f 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -791,6 +791,8 @@ func RegisterRoutes(m *macaron.Macaron) { m.Combo("/_upload/*", repo.MustBeAbleToUpload). Get(repo.UploadFile). Post(bindIgnErr(auth.UploadRepoFileForm{}), repo.UploadFilePost) + m.Combo("/_diffpatch/*").Get(repo.NewDiffPatch). + Post(bindIgnErr(auth.EditRepoFileForm{}), repo.NewDiffPatchPost) }, context.RepoRefByType(context.RepoRefBranch), repo.MustBeEditable) m.Group("", func() { m.Post("/upload-file", repo.UploadFileToServer) diff --git a/templates/repo/editor/patch.tmpl b/templates/repo/editor/patch.tmpl new file mode 100644 index 0000000000000..5cd4f296ca643 --- /dev/null +++ b/templates/repo/editor/patch.tmpl @@ -0,0 +1,60 @@ +{{template "base/head" .}} +
+ {{template "repo/header" .}} +
+ {{template "base/alert" .}} +
+ {{.CsrfTokenHtml}} + +
+
+
+ {{.Repository.Name}} +
/
+ {{.BranchName}} + + {{.i18n.Tr "repo.editor.or"}} {{.i18n.Tr "repo.editor.cancel_lower"}} + + +
+
+
+
+
+ {{.i18n.Tr "repo.editor.new_patch"}} +
+
+ +
+ {{.i18n.Tr "loading"}} +
+
+
+ {{template "repo/editor/commit_form" .}} +
+
+ +
+
+ + {{.i18n.Tr "repo.editor.commit_empty_file_header"}} +
+
+

{{.i18n.Tr "repo.editor.commit_empty_file_text"}}

+
+
+
+ + {{.i18n.Tr "repo.editor.cancel"}} +
+
+ + {{.i18n.Tr "repo.editor.commit_changes"}} +
+
+
+
+{{template "base/footer" .}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 34a4b42f2ffd8..cec2370b0829d 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2922,6 +2922,50 @@ } } }, + "/repos/{owner}/{repo}/diffpatch": { + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Apply diff patch to repository", + "operationId": "repoApplyDiffPatch", + "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 + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/UpdateFileOptions" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/FileResponse" + } + } + } + }, "/repos/{owner}/{repo}/editorconfig/{filepath}": { "get": { "produces": [ From 18824a5ead412bbe334a2ab4316916891cd21132 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 11 Jun 2020 21:33:07 +0100 Subject: [PATCH 2/2] Change diff to use go-routine reader Signed-off-by: Andrew Thornton --- modules/git/diff.go | 25 ++++++++++++++----------- modules/git/diff_test.go | 12 ++++++------ modules/migrations/gitea.go | 16 +++++++++++----- routers/repo/commit.go | 1 + services/pull/review.go | 18 ++++++++++++++---- 5 files changed, 46 insertions(+), 26 deletions(-) diff --git a/modules/git/diff.go b/modules/git/diff.go index 6f876e4964035..882f96db6f18b 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -28,23 +28,23 @@ const ( ) // GetRawDiff dumps diff results of repository in given commit ID to io.Writer. -func GetRawDiff(repoPath, commitID string, diffType RawDiffType, writer io.Writer) error { - return GetRawDiffForFile(repoPath, "", commitID, diffType, "", writer) +func GetRawDiff(ctx context.Context, repoPath, commitID string, diffType RawDiffType, writer io.Writer) error { + return GetRawDiffForFile(ctx, repoPath, "", commitID, diffType, "", writer) } // GetRawDiffForFile dumps diff results of file in given commit ID to io.Writer. -func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error { +func GetRawDiffForFile(ctx context.Context, repoPath, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error { repo, err := OpenRepository(repoPath) if err != nil { return fmt.Errorf("OpenRepository: %v", err) } defer repo.Close() - return GetRepoRawDiffForFile(repo, startCommit, endCommit, diffType, file, writer) + return GetRepoRawDiffForFile(ctx, repo, startCommit, endCommit, diffType, file, writer) } // GetRepoRawDiffForFile dumps diff results of file in given commit ID to io.Writer according given repository -func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error { +func GetRepoRawDiffForFile(ctx context.Context, repo *Repository, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error { commit, err := repo.GetCommit(endCommit) if err != nil { return fmt.Errorf("GetCommit: %v", err) @@ -54,7 +54,7 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff fileArgs = append(fileArgs, "--", file) } // FIXME: graceful: These commands should have a timeout - ctx, cancel := context.WithCancel(DefaultContext) + ctx, cancel := context.WithCancel(ctx) defer cancel() var cmd *exec.Cmd @@ -132,10 +132,10 @@ func isHeader(lof string) bool { // CutDiffAroundLine cuts a diff of a file in way that only the given line + numberOfLine above it will be shown // it also recalculates hunks and adds the appropriate headers to the new diff. // Warning: Only one-file diffs are allowed. -func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLine int) string { +func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLine int) (string, error) { if line == 0 || numbersOfLine == 0 { // no line or num of lines => no diff - return "" + return "", nil } scanner := bufio.NewScanner(originalDiff) hunk := make([]string, 0) @@ -213,15 +213,18 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi } } } + if err := scanner.Err(); err != nil { + return "", err + } // No hunk found if currentLine == 0 { - return "" + return "", nil } // headerLines + hunkLine (1) = totalNonCodeLines if len(hunk)-headerLines-1 <= numbersOfLine { // No need to cut the hunk => return existing hunk - return strings.Join(hunk, "\n") + return strings.Join(hunk, "\n"), nil } var oldBegin, oldNumOfLines, newBegin, newNumOfLines int64 if old { @@ -256,5 +259,5 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi // construct the new hunk header newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@", oldBegin, oldNumOfLines, newBegin, newNumOfLines) - return strings.Join(newHunk, "\n") + return strings.Join(newHunk, "\n"), nil } diff --git a/modules/git/diff_test.go b/modules/git/diff_test.go index 4258abfe50983..49d2e05983cf2 100644 --- a/modules/git/diff_test.go +++ b/modules/git/diff_test.go @@ -24,7 +24,7 @@ const exampleDiff = `diff --git a/README.md b/README.md + cut off` func TestCutDiffAroundLine(t *testing.T) { - result := CutDiffAroundLine(strings.NewReader(exampleDiff), 4, false, 3) + result, _ := CutDiffAroundLine(strings.NewReader(exampleDiff), 4, false, 3) resultByLine := strings.Split(result, "\n") assert.Len(t, resultByLine, 7) // Check if headers got transferred @@ -37,17 +37,17 @@ func TestCutDiffAroundLine(t *testing.T) { assert.Equal(t, "+ Build Status", resultByLine[4]) // Must be same result as before since old line 3 == new line 5 - newResult := CutDiffAroundLine(strings.NewReader(exampleDiff), 3, true, 3) + newResult, _ := CutDiffAroundLine(strings.NewReader(exampleDiff), 3, true, 3) assert.Equal(t, result, newResult, "Must be same result as before since old line 3 == new line 5") - newResult = CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 300) + newResult, _ = CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 300) assert.Equal(t, exampleDiff, newResult) - emptyResult := CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 0) + emptyResult, _ := CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 0) assert.Empty(t, emptyResult) // Line is out of scope - emptyResult = CutDiffAroundLine(strings.NewReader(exampleDiff), 434, false, 0) + emptyResult, _ = CutDiffAroundLine(strings.NewReader(exampleDiff), 434, false, 0) assert.Empty(t, emptyResult) } @@ -69,7 +69,7 @@ func ExampleCutDiffAroundLine() { Docker Pulls + cut off + cut off` - result := CutDiffAroundLine(strings.NewReader(diff), 4, false, 3) + result, _ := CutDiffAroundLine(strings.NewReader(diff), 4, false, 3) println(result) } diff --git a/modules/migrations/gitea.go b/modules/migrations/gitea.go index 8da1bd4683a66..d130cffe374b3 100644 --- a/modules/migrations/gitea.go +++ b/modules/migrations/gitea.go @@ -6,7 +6,6 @@ package migrations import ( - "bytes" "context" "fmt" "io" @@ -795,13 +794,20 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { return fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) } + preader, pwriter := io.Pipe() + ctx, cancel := context.WithCancel(git.DefaultContext) + var patch string - patchBuf := new(bytes.Buffer) - if err := git.GetRepoRawDiffForFile(g.gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, comment.TreePath, patchBuf); err != nil { + go func() { + err := git.GetRepoRawDiffForFile(ctx, g.gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, comment.TreePath, pwriter) + _ = pwriter.CloseWithError(err) + }() + patch, err = git.CutDiffAroundLine(preader, int64((&models.Comment{Line: int64(line + comment.Position - 1)}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) + _ = preader.Close() + cancel() + if err != nil { // We should ignore the error since the commit maybe removed when force push to the pull request log.Warn("GetRepoRawDiffForFile failed when migrating [%s, %s, %s, %s]: %v", g.gitRepo.Path, pr.MergeBase, headCommitID, comment.TreePath, err) - } else { - patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: int64(line + comment.Position - 1)}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) } var c = models.Comment{ diff --git a/routers/repo/commit.go b/routers/repo/commit.go index 004d4915df8bd..38a0d62308ecd 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -329,6 +329,7 @@ func RawDiff(ctx *context.Context) { repoPath = models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) } if err := git.GetRawDiff( + ctx.Req.Context(), repoPath, ctx.Params(":sha"), git.RawDiffType(ctx.Params(":ext")), diff --git a/services/pull/review.go b/services/pull/review.go index 25e0ca858a6cc..b41d96ea1ec16 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -6,8 +6,9 @@ package pull import ( - "bytes" + "context" "fmt" + "io" "strings" "code.gitea.io/gitea/models" @@ -138,11 +139,20 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models if err != nil { return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) } - patchBuf := new(bytes.Buffer) - if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil { + + ctx, cancel := context.WithCancel(git.DefaultContext) + preader, pwriter := io.Pipe() + go func() { + err = git.GetRepoRawDiffForFile(ctx, gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, pwriter) + _ = pwriter.CloseWithError(err) + }() + + patch, err = git.CutDiffAroundLine(preader, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) + _ = preader.Close() + cancel() + if err != nil { return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath) } - patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) } return models.CreateComment(&models.CreateCommentOptions{ Type: models.CommentTypeCode,