From e741976ee3b6eeaf2ec8643bfdbc5b086e50c516 Mon Sep 17 00:00:00 2001 From: Norwin Date: Sun, 26 Sep 2021 18:37:04 +0200 Subject: [PATCH 1/2] return diffs without binary file content change --- modules/git/repo_compare.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 5d1208aab18fe..a5b04e1098262 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -210,13 +210,13 @@ func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, formatted return repo.GetDiff(base, head, w) } -// GetDiff generates and returns patch data between given revisions. +// GetDiff generates and returns patch data between given revisions, optimized for human readability func (repo *Repository) GetDiff(base, head string, w io.Writer) error { - return NewCommand("diff", "-p", "--binary", base, head). + return NewCommand("diff", "-p", base, head). RunInDirPipeline(repo.Path, w, nil) } -// GetPatch generates and returns format-patch data between given revisions. +// GetPatch generates and returns format-patch data between given revisions, able to be used with `git apply` func (repo *Repository) GetPatch(base, head string, w io.Writer) error { stderr := new(bytes.Buffer) err := NewCommand("format-patch", "--binary", "--stdout", base+"..."+head). From f8e0b9f6a8f9af1ffaa0ea73cd7f1b1a91861ecf Mon Sep 17 00:00:00 2001 From: Norwin Date: Mon, 27 Sep 2021 12:50:42 +0200 Subject: [PATCH 2/2] ?binary=true option to restore old behaviour --- modules/git/repo_compare.go | 16 ++++++++++++---- routers/api/v1/repo/pull.go | 8 +++++++- routers/web/repo/pull.go | 3 ++- services/pull/patch.go | 6 +++--- templates/swagger/v1_json.tmpl | 6 ++++++ 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index a5b04e1098262..0f4c5a399fe96 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -203,10 +203,13 @@ func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int, } // GetDiffOrPatch generates either diff or formatted patch data between given revisions -func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, formatted bool) error { - if formatted { +func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, patch, binary bool) error { + if patch { return repo.GetPatch(base, head, w) } + if binary { + return repo.GetDiffBinary(base, head, w) + } return repo.GetDiff(base, head, w) } @@ -216,6 +219,12 @@ func (repo *Repository) GetDiff(base, head string, w io.Writer) error { RunInDirPipeline(repo.Path, w, nil) } +// GetDiffBinary generates and returns patch data between given revisions, including binary diffs. +func (repo *Repository) GetDiffBinary(base, head string, w io.Writer) error { + return NewCommand("diff", "-p", "--binary", base, head). + RunInDirPipeline(repo.Path, w, nil) +} + // GetPatch generates and returns format-patch data between given revisions, able to be used with `git apply` func (repo *Repository) GetPatch(base, head string, w io.Writer) error { stderr := new(bytes.Buffer) @@ -234,8 +243,7 @@ func (repo *Repository) GetDiffFromMergeBase(base, head string, w io.Writer) err err := NewCommand("diff", "-p", "--binary", base+"..."+head). RunInDirPipeline(repo.Path, w, stderr) if err != nil && bytes.Contains(stderr.Bytes(), []byte("no merge base")) { - return NewCommand("diff", "-p", "--binary", base, head). - RunInDirPipeline(repo.Path, w, nil) + return repo.GetDiffBinary(base, head, w) } return err } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index b9767b413d7b6..b41cf9a853164 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -204,6 +204,10 @@ func DownloadPullDiffOrPatch(ctx *context.APIContext) { // type: string // enum: [diff, patch] // required: true + // - name: binary + // in: query + // description: whether to include binary file changes. if true, the diff is applicable with `git apply` + // type: boolean // responses: // "200": // "$ref": "#/responses/string" @@ -225,7 +229,9 @@ func DownloadPullDiffOrPatch(ctx *context.APIContext) { patch = true } - if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch); err != nil { + binary := ctx.FormBool("binary") + + if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch, binary); err != nil { ctx.InternalServerError(err) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index c370e7f04d63d..1aac003e7bfd6 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1345,8 +1345,9 @@ func DownloadPullDiffOrPatch(ctx *context.Context, patch bool) { } pr := issue.PullRequest + binary := ctx.FormBool("binary") - if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch); err != nil { + if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch, binary); err != nil { ctx.ServerError("DownloadDiffOrPatch", err) return } diff --git a/services/pull/patch.go b/services/pull/patch.go index 184da9f659214..27d799191917e 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -22,7 +22,7 @@ import ( ) // DownloadDiffOrPatch will write the patch for the pr to the writer -func DownloadDiffOrPatch(pr *models.PullRequest, w io.Writer, patch bool) error { +func DownloadDiffOrPatch(pr *models.PullRequest, w io.Writer, patch, binary bool) error { if err := pr.LoadBaseRepo(); err != nil { log.Error("Unable to load base repository ID %d for pr #%d [%d]", pr.BaseRepoID, pr.Index, pr.ID) return err @@ -33,7 +33,7 @@ func DownloadDiffOrPatch(pr *models.PullRequest, w io.Writer, patch bool) error return fmt.Errorf("OpenRepository: %v", err) } defer gitRepo.Close() - if err := gitRepo.GetDiffOrPatch(pr.MergeBase, pr.GetGitRefName(), w, patch); err != nil { + if err := gitRepo.GetDiffOrPatch(pr.MergeBase, pr.GetGitRefName(), w, patch, binary); err != nil { log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) return fmt.Errorf("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) } @@ -108,7 +108,7 @@ func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath _ = util.Remove(tmpPatchFile.Name()) }() - if err := gitRepo.GetDiff(pr.MergeBase, "tracking", tmpPatchFile); err != nil { + if err := gitRepo.GetDiffBinary(pr.MergeBase, "tracking", tmpPatchFile); err != nil { tmpPatchFile.Close() log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) return false, fmt.Errorf("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 63eba4583288e..ef4f9247cf171 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7400,6 +7400,12 @@ "name": "diffType", "in": "path", "required": true + }, + { + "type": "boolean", + "description": "whether to include binary file changes. if true, the diff is applicable with `git apply`", + "name": "binary", + "in": "query" } ], "responses": {