Skip to content

Commit 46bcf3e

Browse files
committed
Implement the git-merge-one-file algorithm
Signed-off-by: Andrew Thornton <[email protected]>
1 parent 2f65bf9 commit 46bcf3e

File tree

3 files changed

+335
-50
lines changed

3 files changed

+335
-50
lines changed

modules/git/repo_compare.go

+4
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ func (repo *Repository) GetDiff(base, head string, w io.Writer) error {
237237

238238
// GetDiffBinary generates and returns patch data between given revisions, including binary diffs.
239239
func (repo *Repository) GetDiffBinary(base, head string, w io.Writer) error {
240+
if CheckGitVersionAtLeast("1.7.7") == nil {
241+
return NewCommandContext(repo.Ctx, "diff", "-p", "--binary", "--histogram", base, head).
242+
RunInDirPipeline(repo.Path, w, nil)
243+
}
240244
return NewCommandContext(repo.Ctx, "diff", "-p", "--binary", "--patience", base, head).
241245
RunInDirPipeline(repo.Path, w, nil)
242246
}

services/pull/patch.go

+151-50
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ import (
1616
"code.gitea.io/gitea/models"
1717
"code.gitea.io/gitea/models/unit"
1818
"code.gitea.io/gitea/modules/git"
19+
"code.gitea.io/gitea/modules/graceful"
1920
"code.gitea.io/gitea/modules/log"
21+
"code.gitea.io/gitea/modules/process"
2022
"code.gitea.io/gitea/modules/util"
2123

2224
"github.com/gobwas/glob"
@@ -98,74 +100,170 @@ func TestPatch(pr *models.PullRequest) error {
98100
return nil
99101
}
100102

103+
func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, gitRepo *git.Repository, markConflict func(string)) error {
104+
switch {
105+
case file.stage1 != nil && (file.stage2 == nil || file.stage3 == nil):
106+
// 1. Deleted in one or both:
107+
//
108+
// Conflict <==> the stage1 !Equivalent to the undeleted one
109+
if (file.stage2 != nil && !file.stage1.SameAs(file.stage2)) || (file.stage3 != nil && !file.stage1.SameAs(file.stage3)) {
110+
// Conflict!
111+
markConflict(file.stage1.path)
112+
return nil
113+
}
114+
115+
// Not a genuine conflict and we can simply remove the file from the index
116+
return gitRepo.RemoveFilesFromIndex(file.stage1.path)
117+
case file.stage1 == nil && file.stage2 != nil && (file.stage3 == nil || file.stage2.SameAs(file.stage3)):
118+
// 2. Added in ours but not in theirs or identical in both
119+
//
120+
// Not a genuine conflict just add to the index
121+
if err := gitRepo.AddObjectToIndex(file.stage2.mode, git.MustIDFromString(file.stage2.sha), file.stage2.path); err != nil {
122+
return err
123+
}
124+
return nil
125+
case file.stage1 == nil && file.stage2 != nil && file.stage3 != nil && file.stage2.sha == file.stage3.sha && file.stage2.mode != file.stage3.mode:
126+
// 3. Added in both with the same sha but the modes are different
127+
//
128+
// Conflict! (Not sure that this can actually happen but we should handle)
129+
markConflict(file.stage2.path)
130+
return nil
131+
case file.stage1 == nil && file.stage2 == nil && file.stage3 != nil:
132+
// 4. Added in theirs but not ours:
133+
//
134+
// Not a genuine conflict just add to the index
135+
return gitRepo.AddObjectToIndex(file.stage3.mode, git.MustIDFromString(file.stage3.sha), file.stage3.path)
136+
case file.stage1 == nil:
137+
// 5. Created by new in both
138+
//
139+
// Conflict!
140+
markConflict(file.stage2.path)
141+
return nil
142+
case file.stage2 != nil && file.stage3 != nil:
143+
// 5. Modified in both - we should try to merge in the changes but first:
144+
//
145+
if file.stage2.mode == "120000" || file.stage3.mode == "120000" {
146+
// 5a. Conflicting symbolic link change
147+
markConflict(file.stage2.path)
148+
return nil
149+
}
150+
if file.stage2.mode == "160000" || file.stage3.mode == "160000" {
151+
// 5b. Conflicting submodule change
152+
markConflict(file.stage2.path)
153+
return nil
154+
}
155+
if file.stage2.mode != file.stage3.mode {
156+
// 5c. Conflicting mode change
157+
markConflict(file.stage2.path)
158+
return nil
159+
}
160+
161+
// Need to get the objects from the object db to attempt to merge
162+
root, err := git.NewCommandContext(ctx, "unpack-file", file.stage1.sha).RunInDir(tmpBasePath)
163+
if err != nil {
164+
return fmt.Errorf("unable to get root object: %s at path: %s for merging. Error: %w", file.stage1.sha, file.stage1.path, err)
165+
}
166+
root = strings.TrimSpace(root)
167+
defer util.Remove(root)
168+
base, err := git.NewCommandContext(ctx, "unpack-file", file.stage2.sha).RunInDir(tmpBasePath)
169+
if err != nil {
170+
return fmt.Errorf("unable to get base object: %s at path: %s for merging. Error: %w", file.stage2.sha, file.stage2.path, err)
171+
}
172+
base = strings.TrimSpace(base)
173+
defer util.Remove(base)
174+
head, err := git.NewCommandContext(ctx, "unpack-file", file.stage3.sha).RunInDir(tmpBasePath)
175+
if err != nil {
176+
return fmt.Errorf("unable to get head object:%s at path: %s for merging. Error: %w", file.stage3.sha, file.stage3.path, err)
177+
}
178+
head = strings.TrimSpace(head)
179+
defer util.Remove(head)
180+
181+
// now git merge-file annoyingly takes a different order to the merge-tree ...
182+
_, conflictErr := git.NewCommandContext(ctx, "merge-file", base, root, head).RunInDir(tmpBasePath)
183+
if conflictErr != nil {
184+
markConflict(file.stage2.path)
185+
return nil
186+
}
187+
188+
// base now contains the merged data
189+
hash, err := git.NewCommandContext(ctx, "hash-object", "-w", "--path", file.stage2.path, base).RunInDir(tmpBasePath)
190+
if err != nil {
191+
return err
192+
}
193+
hash = strings.TrimSpace(hash)
194+
return gitRepo.AddObjectToIndex(file.stage2.mode, git.MustIDFromString(hash), file.stage2.path)
195+
default:
196+
if file.stage1 != nil {
197+
markConflict(file.stage1.path)
198+
} else if file.stage2 != nil {
199+
markConflict(file.stage2.path)
200+
} else if file.stage3 != nil {
201+
markConflict(file.stage2.path)
202+
} else {
203+
// This shouldn't happen!
204+
}
205+
}
206+
return nil
207+
}
208+
101209
func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath string) (bool, error) {
102-
if _, err := git.NewCommand("read-tree", "-m", pr.MergeBase, "base", "tracking").RunInDir(tmpBasePath); err != nil {
210+
ctx, cancel, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("checkConflicts: pr[%d] %s/%s#%d", pr.ID, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Index))
211+
defer finished()
212+
213+
// First we use read-tree to do a simple three-way merge
214+
if _, err := git.NewCommandContext(ctx, "read-tree", "-m", pr.MergeBase, "base", "tracking").RunInDir(tmpBasePath); err != nil {
103215
log.Error("Unable to run read-tree -m! Error: %v", err)
104-
return false, fmt.Errorf("Unable to run read-tree -m! Error: %v", err)
216+
return false, fmt.Errorf("unable to run read-tree -m! Error: %v", err)
105217
}
106218

107-
diffReader, diffWriter, err := os.Pipe()
108-
if err != nil {
109-
log.Error("Unable to open stderr pipe: %v", err)
110-
return false, fmt.Errorf("Unable to open stderr pipe: %v", err)
111-
}
219+
// Then we use git ls-files -u to list the unmerged files and collate the triples in unmergedfiles
220+
unmerged := make(chan *unmergedFile)
221+
go unmergedFiles(ctx, tmpBasePath, unmerged)
222+
112223
defer func() {
113-
_ = diffReader.Close()
114-
_ = diffWriter.Close()
224+
cancel()
225+
for range unmerged {
226+
// empty the unmerged channel
227+
}
115228
}()
116-
stderr := &strings.Builder{}
117229

118-
conflict := false
119230
numberOfConflicts := 0
120-
err = git.NewCommand("diff", "--name-status", "--diff-filter=U").
121-
RunInDirTimeoutEnvFullPipelineFunc(
122-
nil, -1, tmpBasePath,
123-
diffWriter, stderr, nil,
124-
func(ctx context.Context, cancel context.CancelFunc) error {
125-
// Close the writer end of the pipe to begin processing
126-
_ = diffWriter.Close()
127-
defer func() {
128-
// Close the reader on return to terminate the git command if necessary
129-
_ = diffReader.Close()
130-
}()
131-
132-
// Now scan the output from the command
133-
scanner := bufio.NewScanner(diffReader)
134-
for scanner.Scan() {
135-
line := scanner.Text()
136-
split := strings.SplitN(line, "\t", 2)
137-
file := ""
138-
if len(split) == 2 {
139-
file = split[1]
140-
}
141-
142-
if file != "" {
143-
conflict = true
144-
if numberOfConflicts < 10 {
145-
pr.ConflictedFiles = append(pr.ConflictedFiles, file)
146-
}
147-
numberOfConflicts++
148-
}
149-
}
231+
conflict := false
232+
markConflict := func(filepath string) {
233+
log.Trace("Conflict: %s in PR[%d] %s/%s#%d", filepath, pr.ID, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Index)
234+
conflict = true
235+
if numberOfConflicts < 10 {
236+
pr.ConflictedFiles = append(pr.ConflictedFiles, filepath)
237+
}
238+
numberOfConflicts++
239+
}
150240

151-
return nil
152-
})
241+
for file := range unmerged {
242+
if file == nil {
243+
break
244+
}
245+
if file.err != nil {
246+
cancel()
247+
return false, file.err
248+
}
153249

154-
if err != nil {
155-
return false, fmt.Errorf("git diff --name-status --filter=U: %v", git.ConcatenateError(err, stderr.String()))
250+
// OK now we have the unmerged file triplet attempt to merge it
251+
if err := attemptMerge(ctx, file, tmpBasePath, gitRepo, markConflict); err != nil {
252+
return false, err
253+
}
156254
}
157255

158256
if !conflict {
159257
return false, nil
160258
}
161259

162-
// OK read-tree has failed so we need to try a different thing
260+
// OK read-tree has failed so we need to try a different thing - this might actually suceed where the above fails due to whitespace handling.
163261

164262
// 1. Create a plain patch from head to base
165263
tmpPatchFile, err := os.CreateTemp("", "patch")
166264
if err != nil {
167265
log.Error("Unable to create temporary patch file! Error: %v", err)
168-
return false, fmt.Errorf("Unable to create temporary patch file! Error: %v", err)
266+
return false, fmt.Errorf("unable to create temporary patch file! Error: %v", err)
169267
}
170268
defer func() {
171269
_ = util.Remove(tmpPatchFile.Name())
@@ -174,12 +272,12 @@ func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath
174272
if err := gitRepo.GetDiffBinary(pr.MergeBase, "tracking", tmpPatchFile); err != nil {
175273
tmpPatchFile.Close()
176274
log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err)
177-
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)
275+
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)
178276
}
179277
stat, err := tmpPatchFile.Stat()
180278
if err != nil {
181279
tmpPatchFile.Close()
182-
return false, fmt.Errorf("Unable to stat patch file: %v", err)
280+
return false, fmt.Errorf("unable to stat patch file: %v", err)
183281
}
184282
patchPath := tmpPatchFile.Name()
185283
tmpPatchFile.Close()
@@ -216,6 +314,9 @@ func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath
216314
if prConfig.IgnoreWhitespaceConflicts {
217315
args = append(args, "--ignore-whitespace")
218316
}
317+
if git.CheckGitVersionAtLeast("2.32.0") == nil {
318+
args = append(args, "--3way")
319+
}
219320
args = append(args, patchPath)
220321
pr.ConflictedFiles = make([]string, 0, 5)
221322

@@ -230,7 +331,7 @@ func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath
230331
stderrReader, stderrWriter, err := os.Pipe()
231332
if err != nil {
232333
log.Error("Unable to open stderr pipe: %v", err)
233-
return false, fmt.Errorf("Unable to open stderr pipe: %v", err)
334+
return false, fmt.Errorf("unable to open stderr pipe: %v", err)
234335
}
235336
defer func() {
236337
_ = stderrReader.Close()

0 commit comments

Comments
 (0)