Monorepo for Tangled

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

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>

authored by

oppiliappan and committed by tangled.org 86081872 0fd947f3

+37 -31
+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,