knotserver/git: rework merge check to also use `git am` #1033

we were using git apply in merge check and git am for the actual merge, but in reality, there are slight behavior changes among the two. this change switches out git apply in mergeCheck to use git am when dealing with mailbox style patches.

Signed-off-by: oppiliappan me@oppi.li

+37 -31

2 changed files

Diff round #1

expand all

unified split

knotserver

git

merge.go

xrpc

merge_check.go

+30 -30

knotserver/git/merge.go

··· 107 107 return fmt.Sprintf("merge failed: %s", e.Message) 108 108 } 109 109 110 - func (g *GitRepo) createTempFileWithPatch(patchData string) (string, error) { 110 + func createTemp(data string) (string, error) { 111 111 tmpFile, err := os.CreateTemp("", "git-patch-*.patch") 112 112 if err != nil { 113 113 return "", fmt.Errorf("failed to create temporary patch file: %w", err) 114 114 } 115 115 116 - if _, err := tmpFile.Write([]byte(patchData)); err != nil { 116 + if _, err := tmpFile.Write([]byte(data)); err != nil { 117 117 tmpFile.Close() 118 118 os.Remove(tmpFile.Name()) 119 119 return "", fmt.Errorf("failed to write patch data to temporary file: %w", err) ··· 127 127 return tmpFile.Name(), nil 128 128 } 129 129 130 - func (g *GitRepo) cloneRepository(targetBranch string) (string, error) { 130 + func (g *GitRepo) cloneTemp(targetBranch string) (string, error) { 131 131 tmpDir, err := os.MkdirTemp("", "git-clone-") 132 132 if err != nil { 133 133 return "", fmt.Errorf("failed to create temporary directory: %w", err) ··· 147 147 return tmpDir, nil 148 148 } 149 149 150 - func (g *GitRepo) checkPatch(tmpDir, patchFile string) error { 151 - var stderr bytes.Buffer 152 - 153 - cmd := exec.Command("git", "-C", tmpDir, "apply", "--check", "-v", patchFile) 154 - cmd.Stderr = &stderr 155 - 156 - if err := cmd.Run(); err != nil { 157 - conflicts := parseGitApplyErrors(stderr.String()) 158 - return &ErrMerge{ 159 - Message: "patch cannot be applied cleanly", 160 - Conflicts: conflicts, 161 - HasConflict: len(conflicts) > 0, 162 - OtherError: err, 163 - } 164 - } 165 - return nil 166 - } 167 - 168 150 func (g *GitRepo) applyPatch(patchData, patchFile string, opts MergeOptions) error { 169 151 var stderr bytes.Buffer 170 152 var cmd *exec.Cmd ··· 173 155 exec.Command("git", "-C", g.path, "config", "user.name", opts.CommitterName).Run() 174 156 exec.Command("git", "-C", g.path, "config", "user.email", opts.CommitterEmail).Run() 175 157 exec.Command("git", "-C", g.path, "config", "advice.mergeConflict", "false").Run() 158 + exec.Command("git", "-C", g.path, "config", "advice.amWorkDir", "false").Run() 176 159 177 160 // if patch is a format-patch, apply using 'git am' 178 161 if opts.FormatPatch { ··· 213 196 cmd.Stderr = &stderr 214 197 215 198 if err := cmd.Run(); err != nil { 216 - return fmt.Errorf("patch application failed: %s", stderr.String()) 199 + conflicts := parseGitApplyErrors(stderr.String()) 200 + return &ErrMerge{ 201 + Message: "patch cannot be applied cleanly", 202 + Conflicts: conflicts, 203 + HasConflict: len(conflicts) > 0, 204 + OtherError: err, 205 + } 217 206 } 218 207 219 208 return nil ··· 241 230 } 242 231 243 232 func (g *GitRepo) applySingleMailbox(singlePatch types.FormatPatch) (plumbing.Hash, error) { 244 - tmpPatch, err := g.createTempFileWithPatch(singlePatch.Raw) 233 + tmpPatch, err := createTemp(singlePatch.Raw) 245 234 if err != nil { 246 235 return plumbing.ZeroHash, fmt.Errorf("failed to create temporary patch file for singluar mailbox patch: %w", err) 247 236 } ··· 257 246 log.Println("head before apply", head.Hash().String()) 258 247 259 248 if err := cmd.Run(); err != nil { 260 - return plumbing.ZeroHash, fmt.Errorf("patch application failed: %s", stderr.String()) 249 + conflicts := parseGitApplyErrors(stderr.String()) 250 + return plumbing.ZeroHash, &ErrMerge{ 251 + Message: "patch cannot be applied cleanly", 252 + Conflicts: conflicts, 253 + HasConflict: len(conflicts) > 0, 254 + OtherError: err, 255 + } 261 256 } 262 257 263 258 if err := g.Refresh(); err != nil { ··· 324 319 return newHash, nil 325 320 } 326 321 327 - func (g *GitRepo) MergeCheck(patchData string, targetBranch string) error { 322 + func (g *GitRepo) MergeCheckWithOptions(patchData string, targetBranch string, mo MergeOptions) error { 328 323 if val, ok := mergeCheckCache.Get(g, patchData, targetBranch); ok { 329 324 return val 330 325 } 331 326 332 - patchFile, err := g.createTempFileWithPatch(patchData) 327 + patchFile, err := createTemp(patchData) 333 328 if err != nil { 334 329 return &ErrMerge{ 335 330 Message: err.Error(), ··· 338 333 } 339 334 defer os.Remove(patchFile) 340 335 341 - tmpDir, err := g.cloneRepository(targetBranch) 336 + tmpDir, err := g.cloneTemp(targetBranch) 342 337 if err != nil { 343 338 return &ErrMerge{ 344 339 Message: err.Error(), ··· 347 342 } 348 343 defer os.RemoveAll(tmpDir) 349 344 350 - result := g.checkPatch(tmpDir, patchFile) 345 + tmpRepo, err := PlainOpen(tmpDir) 346 + if err != nil { 347 + return err 348 + } 349 + 350 + result := tmpRepo.applyPatch(patchData, patchFile, mo) 351 351 mergeCheckCache.Set(g, patchData, targetBranch, result) 352 352 return result 353 353 } 354 354 355 355 func (g *GitRepo) MergeWithOptions(patchData string, targetBranch string, opts MergeOptions) error { 356 - patchFile, err := g.createTempFileWithPatch(patchData) 356 + patchFile, err := createTemp(patchData) 357 357 if err != nil { 358 358 return &ErrMerge{ 359 359 Message: err.Error(), ··· 362 362 } 363 363 defer os.Remove(patchFile) 364 364 365 - tmpDir, err := g.cloneRepository(targetBranch) 365 + tmpDir, err := g.cloneTemp(targetBranch) 366 366 if err != nil { 367 367 return &ErrMerge{ 368 368 Message: err.Error(),

+7 -1

knotserver/xrpc/merge_check.go

··· 9 9 securejoin "github.com/cyphar/filepath-securejoin" 10 10 "tangled.org/core/api/tangled" 11 11 "tangled.org/core/knotserver/git" 12 + "tangled.org/core/patchutil" 12 13 xrpcerr "tangled.org/core/xrpc/errors" 13 14 ) 14 15 ··· 51 52 return 52 53 } 53 54 54 - err = gr.MergeCheck(data.Patch, data.Branch) 55 + mo := git.MergeOptions{} 56 + mo.CommitterName = x.Config.Git.UserName 57 + mo.CommitterEmail = x.Config.Git.UserEmail 58 + mo.FormatPatch = patchutil.IsFormatPatch(data.Patch) 59 + 60 + err = gr.MergeCheckWithOptions(data.Patch, data.Branch, mo) 55 61 56 62 response := tangled.RepoMergeCheck_Output{ 57 63 Is_conflicted: false,