Skip to content

Commit

Permalink
Improve TestPatch to use git read-tree -m
Browse files Browse the repository at this point in the history
The current TestPatch conflict code uses a plain git apply which does not properly
account for 3-way merging. However, we can improve things using `git read-tree -m` to
do a three-way merge. We can also use `--patience` to generate a nicer diff for
applying patches too.

Fix go-gitea#13679

Signed-off-by: Andrew Thornton <[email protected]>
  • Loading branch information
zeripath committed Dec 16, 2021
1 parent 6e7d28c commit b23c35b
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 2 deletions.
2 changes: 1 addition & 1 deletion modules/git/repo_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (repo *Repository) GetDiff(base, head string, w io.Writer) error {

// GetDiffBinary generates and returns patch data between given revisions, including binary diffs.
func (repo *Repository) GetDiffBinary(base, head string, w io.Writer) error {
return NewCommandContext(repo.Ctx, "diff", "-p", "--binary", base, head).
return NewCommandContext(repo.Ctx, "diff", "-p", "--binary", "--patience", base, head).
RunInDirPipeline(repo.Path, w, nil)
}

Expand Down
64 changes: 63 additions & 1 deletion services/pull/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,68 @@ func TestPatch(pr *models.PullRequest) error {
}

func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath string) (bool, error) {
if _, err := git.NewCommand("read-tree", "-m", pr.MergeBase, "base", "tracking").RunInDir(tmpBasePath); err != nil {
log.Error("Unable to run read-tree -m! Error: %v", err)
return false, fmt.Errorf("Unable to run read-tree -m! Error: %v", err)
}

diffReader, diffWriter, err := os.Pipe()
if err != nil {
log.Error("Unable to open stderr pipe: %v", err)
return false, fmt.Errorf("Unable to open stderr pipe: %v", err)
}
defer func() {
_ = diffReader.Close()
_ = diffWriter.Close()
}()
stderr := &strings.Builder{}

conflict := false
numberOfConflicts := 0
err = git.NewCommand("diff", "--name-status", "--diff-filter=U").
RunInDirTimeoutEnvFullPipelineFunc(
nil, -1, tmpBasePath,
diffWriter, stderr, nil,
func(ctx context.Context, cancel context.CancelFunc) error {
// Close the writer end of the pipe to begin processing
_ = diffWriter.Close()
defer func() {
// Close the reader on return to terminate the git command if necessary
_ = diffReader.Close()
}()

// Now scan the output from the command
scanner := bufio.NewScanner(diffReader)
for scanner.Scan() {
line := scanner.Text()
split := strings.SplitN(line, "\t", 2)
file := ""
if len(split) == 2 {
file = split[1]
}

if file != "" {
conflict = true
if numberOfConflicts < 10 {
pr.ConflictedFiles = append(pr.ConflictedFiles, file)
}
numberOfConflicts++
}
}

return nil
})

if err != nil {
return false, fmt.Errorf("git diff --name-status --filter=U: %v", git.ConcatenateError(err, stderr.String()))
}

if !conflict {
return false, nil
}

// OK read-tree has failed so we need to try a different thing

// 1. Create a plain patch from head to base
tmpPatchFile, err := os.CreateTemp("", "patch")
if err != nil {
Expand Down Expand Up @@ -176,7 +238,7 @@ func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath
}()

// 7. Run the check command
conflict := false
conflict = false
err = git.NewCommand(args...).
RunInDirTimeoutEnvFullPipelineFunc(
nil, -1, tmpBasePath,
Expand Down

0 comments on commit b23c35b

Please sign in to comment.