mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-09-11 04:30:06 +02:00
Ensure that all unmerged files are merged when conflict checking (#20528)
There is a subtle bug in the code relating to collating the results of `git ls-files -u -z` in `unmergedFiles()`. The code here makes the mistake of assuming that every unmerged file will always have a stage 1 conflict, and this results in conflicts that occur in stage 3 only being dropped. This PR simply adjusts this code to ensure that any empty unmergedFile will always be passed down the channel. The PR also adds a lot of Trace commands to attempt to help find future bugs in this code. Fix #19527 Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
parent
99f2f826ce
commit
7a428fae4b
2 changed files with 27 additions and 2 deletions
|
@ -124,6 +124,7 @@ func (e *errMergeConflict) Error() string {
|
|||
}
|
||||
|
||||
func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, gitRepo *git.Repository) error {
|
||||
log.Trace("Attempt to merge:\n%v", file)
|
||||
switch {
|
||||
case file.stage1 != nil && (file.stage2 == nil || file.stage3 == nil):
|
||||
// 1. Deleted in one or both:
|
||||
|
@ -295,7 +296,8 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
|
|||
var treeHash string
|
||||
treeHash, _, err = git.NewCommand(ctx, "write-tree").RunStdString(&git.RunOpts{Dir: tmpBasePath})
|
||||
if err != nil {
|
||||
return false, err
|
||||
lsfiles, _, _ := git.NewCommand(ctx, "ls-files", "-u").RunStdString(&git.RunOpts{Dir: tmpBasePath})
|
||||
return false, fmt.Errorf("unable to write unconflicted tree: %w\n`git ls-files -u`:\n%s", err, lsfiles)
|
||||
}
|
||||
treeHash = strings.TrimSpace(treeHash)
|
||||
baseTree, err := gitRepo.GetTree("base")
|
||||
|
|
|
@ -42,6 +42,17 @@ func (line *lsFileLine) SameAs(other *lsFileLine) bool {
|
|||
line.path == other.path
|
||||
}
|
||||
|
||||
// String provides a string representation for logging
|
||||
func (line *lsFileLine) String() string {
|
||||
if line == nil {
|
||||
return "<nil>"
|
||||
}
|
||||
if line.err != nil {
|
||||
return fmt.Sprintf("%d %s %s %s %v", line.stage, line.mode, line.path, line.sha, line.err)
|
||||
}
|
||||
return fmt.Sprintf("%d %s %s %s", line.stage, line.mode, line.path, line.sha)
|
||||
}
|
||||
|
||||
// readUnmergedLsFileLines calls git ls-files -u -z and parses the lines into mode-sha-stage-path quadruplets
|
||||
// it will push these to the provided channel closing it at the end
|
||||
func readUnmergedLsFileLines(ctx context.Context, tmpBasePath string, outputChan chan *lsFileLine) {
|
||||
|
@ -118,6 +129,17 @@ type unmergedFile struct {
|
|||
err error
|
||||
}
|
||||
|
||||
// String provides a string representation of the an unmerged file for logging
|
||||
func (u *unmergedFile) String() string {
|
||||
if u == nil {
|
||||
return "<nil>"
|
||||
}
|
||||
if u.err != nil {
|
||||
return fmt.Sprintf("error: %v\n%v\n%v\n%v", u.err, u.stage1, u.stage2, u.stage3)
|
||||
}
|
||||
return fmt.Sprintf("%v\n%v\n%v", u.stage1, u.stage2, u.stage3)
|
||||
}
|
||||
|
||||
// unmergedFiles will collate the output from readUnstagedLsFileLines in to file triplets and send them
|
||||
// to the provided channel, closing at the end.
|
||||
func unmergedFiles(ctx context.Context, tmpBasePath string, unmerged chan *unmergedFile) {
|
||||
|
@ -138,6 +160,7 @@ func unmergedFiles(ctx context.Context, tmpBasePath string, unmerged chan *unmer
|
|||
|
||||
next := &unmergedFile{}
|
||||
for line := range lsFileLineChan {
|
||||
log.Trace("Got line: %v Current State:\n%v", line, next)
|
||||
if line.err != nil {
|
||||
log.Error("Unable to run ls-files -u -z! Error: %v", line.err)
|
||||
unmerged <- &unmergedFile{err: fmt.Errorf("unable to run ls-files -u -z! Error: %v", line.err)}
|
||||
|
@ -149,7 +172,7 @@ func unmergedFiles(ctx context.Context, tmpBasePath string, unmerged chan *unmer
|
|||
case 0:
|
||||
// Should not happen as this represents successfully merged file - we will tolerate and ignore though
|
||||
case 1:
|
||||
if next.stage1 != nil {
|
||||
if next.stage1 != nil || next.stage2 != nil || next.stage3 != nil {
|
||||
// We need to handle the unstaged file stage1,stage2,stage3
|
||||
unmerged <- next
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue