Don't return binary file changes in raw PR diffs by default (#17158)

* return diffs without binary file content change

* ?binary=true option to restore old behaviour

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Co-authored-by: zeripath <art27@cantab.net>
This commit is contained in:
Norwin 2021-09-27 23:09:49 +02:00 committed by GitHub
parent e8574f2f7d
commit f48dce3176
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 32 additions and 11 deletions

View file

@ -215,20 +215,29 @@ func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int,
} }
// GetDiffOrPatch generates either diff or formatted patch data between given revisions // GetDiffOrPatch generates either diff or formatted patch data between given revisions
func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, formatted bool) error { func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, patch, binary bool) error {
if formatted { if patch {
return repo.GetPatch(base, head, w) return repo.GetPatch(base, head, w)
} }
if binary {
return repo.GetDiffBinary(base, head, w)
}
return repo.GetDiff(base, head, w) 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 { func (repo *Repository) GetDiff(base, head string, w io.Writer) error {
return NewCommand("diff", "-p", base, head).
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). return NewCommand("diff", "-p", "--binary", base, head).
RunInDirPipeline(repo.Path, w, nil) 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 { func (repo *Repository) GetPatch(base, head string, w io.Writer) error {
stderr := new(bytes.Buffer) stderr := new(bytes.Buffer)
err := NewCommand("format-patch", "--binary", "--stdout", base+"..."+head). err := NewCommand("format-patch", "--binary", "--stdout", base+"..."+head).
@ -246,8 +255,7 @@ func (repo *Repository) GetDiffFromMergeBase(base, head string, w io.Writer) err
err := NewCommand("diff", "-p", "--binary", base+"..."+head). err := NewCommand("diff", "-p", "--binary", base+"..."+head).
RunInDirPipeline(repo.Path, w, stderr) RunInDirPipeline(repo.Path, w, stderr)
if err != nil && bytes.Contains(stderr.Bytes(), []byte("no merge base")) { if err != nil && bytes.Contains(stderr.Bytes(), []byte("no merge base")) {
return NewCommand("diff", "-p", "--binary", base, head). return repo.GetDiffBinary(base, head, w)
RunInDirPipeline(repo.Path, w, nil)
} }
return err return err
} }

View file

@ -204,6 +204,10 @@ func DownloadPullDiffOrPatch(ctx *context.APIContext) {
// type: string // type: string
// enum: [diff, patch] // enum: [diff, patch]
// required: true // 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: // responses:
// "200": // "200":
// "$ref": "#/responses/string" // "$ref": "#/responses/string"
@ -225,7 +229,9 @@ func DownloadPullDiffOrPatch(ctx *context.APIContext) {
patch = true 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) ctx.InternalServerError(err)
return return
} }

View file

@ -1345,8 +1345,9 @@ func DownloadPullDiffOrPatch(ctx *context.Context, patch bool) {
} }
pr := issue.PullRequest 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) ctx.ServerError("DownloadDiffOrPatch", err)
return return
} }

View file

@ -22,7 +22,7 @@ import (
) )
// DownloadDiffOrPatch will write the patch for the pr to the writer // 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 { 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) log.Error("Unable to load base repository ID %d for pr #%d [%d]", pr.BaseRepoID, pr.Index, pr.ID)
return err return err
@ -33,7 +33,7 @@ func DownloadDiffOrPatch(pr *models.PullRequest, w io.Writer, patch bool) error
return fmt.Errorf("OpenRepository: %v", err) return fmt.Errorf("OpenRepository: %v", err)
} }
defer gitRepo.Close() 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) 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) 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()) _ = 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() 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) 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) 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)

View file

@ -7400,6 +7400,12 @@
"name": "diffType", "name": "diffType",
"in": "path", "in": "path",
"required": true "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": { "responses": {