domi/forgejo-domi
1
0
Fork 0
forked from forgejo/forgejo
Code Pull requests Activity

Ensure that all unmerged files are merged when conflict checking (#20528) (#20536)

Browse source 
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>
Co-authored-by: zeripath <art27@cantab.net>
This commit is contained in:
6543 2022-07-29 15:37:18 +02:00 • committed by GitHub
parent d6bc1558c6
commit 210b096da7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 27 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

25
services/pull/patch_unmerged.go
View file

@ -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
}