appview,knotserver: produce combined patch in comparisons #609

+9

appview/db/db.go

··· 1097 }) 1098 conn.ExecContext(ctx, "pragma foreign_keys = on;") 1099 0 0 0 0 0 0 0 0 0 1100 return &DB{ 1101 db, 1102 logger,

··· 1097 }) 1098 conn.ExecContext(ctx, "pragma foreign_keys = on;") 1099 1100 + // knots may report the combined patch for a comparison, we can store that on the appview side 1101 + // (but not on the pds record), because calculating the combined patch requires a git index 1102 + runMigration(conn, logger, "add-combined-column-submissions", func(tx *sql.Tx) error { 1103 + _, err := tx.Exec(` 1104 + alter table pull_submissions add column combined text; 1105 + `) 1106 + return err 1107 + }) 1108 + 1109 return &DB{ 1110 db, 1111 logger,

+21 -17

appview/db/pulls.go

··· 90 pull.ID = int(id) 91 92 _, err = tx.Exec(` 93 - insert into pull_submissions (pull_at, round_number, patch, source_rev) 94 - values (?, ?, ?, ?) 95 - `, pull.PullAt(), 0, pull.Submissions[0].Patch, pull.Submissions[0].SourceRev) 96 return err 97 } 98 ··· 313 pull_at, 314 round_number, 315 patch, 0 316 created, 317 source_rev 318 from ··· 332 333 for rows.Next() { 334 var submission models.PullSubmission 335 - var createdAt string 336 - var sourceRev sql.NullString 337 err := rows.Scan( 338 &submission.ID, 339 &submission.PullAt, 340 &submission.RoundNumber, 341 &submission.Patch, 342 - &createdAt, 343 - &sourceRev, 0 344 ) 345 if err != nil { 346 return nil, err 347 } 348 349 - createdTime, err := time.Parse(time.RFC3339, createdAt) 350 - if err != nil { 351 - return nil, err 0 0 0 352 } 353 - submission.Created = createdTime 354 355 - if sourceRev.Valid { 356 - submission.SourceRev = sourceRev.String 357 } 358 359 submissionMap[submission.ID] = &submission ··· 590 return err 591 } 592 593 - func ResubmitPull(e Execer, pull *models.Pull, newPatch, sourceRev string) error { 594 newRoundNumber := len(pull.Submissions) 595 _, err := e.Exec(` 596 - insert into pull_submissions (pull_at, round_number, patch, source_rev) 597 - values (?, ?, ?, ?) 598 - `, pull.PullAt(), newRoundNumber, newPatch, sourceRev) 599 600 return err 601 }

··· 90 pull.ID = int(id) 91 92 _, err = tx.Exec(` 93 + insert into pull_submissions (pull_at, round_number, patch, combined, source_rev) 94 + values (?, ?, ?, ?, ?) 95 + `, pull.PullAt(), 0, pull.Submissions[0].Patch, pull.Submissions[0].Combined, pull.Submissions[0].SourceRev) 96 return err 97 } 98 ··· 313 pull_at, 314 round_number, 315 patch, 316 + combined, 317 created, 318 source_rev 319 from ··· 333 334 for rows.Next() { 335 var submission models.PullSubmission 336 + var submissionCreatedStr string 337 + var submissionSourceRev, submissionCombined sql.NullString 338 err := rows.Scan( 339 &submission.ID, 340 &submission.PullAt, 341 &submission.RoundNumber, 342 &submission.Patch, 343 + &submissionCombined, 344 + &submissionCreatedStr, 345 + &submissionSourceRev, 346 ) 347 if err != nil { 348 return nil, err 349 } 350 351 + if t, err := time.Parse(time.RFC3339, submissionCreatedStr); err == nil { 352 + submission.Created = t 353 + } 354 + 355 + if submissionSourceRev.Valid { 356 + submission.SourceRev = submissionSourceRev.String 357 } 0 358 359 + if submissionCombined.Valid { 360 + submission.Combined = submissionCombined.String 361 } 362 363 submissionMap[submission.ID] = &submission ··· 594 return err 595 } 596 597 + func ResubmitPull(e Execer, pull *models.Pull, newPatch, combined, sourceRev string) error { 598 newRoundNumber := len(pull.Submissions) 599 _, err := e.Exec(` 600 + insert into pull_submissions (pull_at, round_number, patch, combined, source_rev) 601 + values (?, ?, ?, ?, ?) 602 + `, pull.PullAt(), newRoundNumber, newPatch, combined, sourceRev) 603 604 return err 605 }

+9

appview/models/pull.go

··· 134 // content 135 RoundNumber int 136 Patch string 0 137 Comments []PullComment 138 SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 139 ··· 263 return participants 264 } 265 0 0 0 0 0 0 0 0 266 type Stack []*Pull 267 268 // position of this pull in the stack

··· 134 // content 135 RoundNumber int 136 Patch string 137 + Combined string 138 Comments []PullComment 139 SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 140 ··· 264 return participants 265 } 266 267 + func (s PullSubmission) CombinedPatch() string { 268 + if s.Combined == "" { 269 + return s.Patch 270 + } 271 + 272 + return s.Combined 273 + } 274 + 275 type Stack []*Pull 276 277 // position of this pull in the stack

+23 -40

appview/pulls/pulls.go

··· 146 stack, _ := r.Context().Value("stack").(models.Stack) 147 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 148 149 - totalIdents := 1 150 - for _, submission := range pull.Submissions { 151 - totalIdents += len(submission.Comments) 152 - } 153 - 154 - identsToResolve := make([]string, totalIdents) 155 - 156 - // populate idents 157 - identsToResolve[0] = pull.OwnerDid 158 - idx := 1 159 - for _, submission := range pull.Submissions { 160 - for _, comment := range submission.Comments { 161 - identsToResolve[idx] = comment.OwnerDid 162 - idx += 1 163 - } 164 - } 165 - 166 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 167 branchDeleteStatus := s.branchDeleteStatus(r, f, pull) 168 resubmitResult := pages.Unknown ··· 460 return 461 } 462 463 - patch := pull.Submissions[roundIdInt].Patch 464 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 465 466 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 511 return 512 } 513 514 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 515 if err != nil { 516 log.Println("failed to interdiff; current patch malformed") 517 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 518 return 519 } 520 521 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 522 if err != nil { 523 log.Println("failed to interdiff; previous patch malformed") 524 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 720 721 createdAt := time.Now().Format(time.RFC3339) 722 723 - pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId) 724 - if err != nil { 725 - log.Println("failed to get pull at", err) 726 - s.pages.Notice(w, "pull-comment", "Failed to create comment.") 727 - return 728 - } 729 - 730 client, err := s.oauth.AuthorizedClient(r) 731 if err != nil { 732 log.Println("failed to get authorized client", err) ··· 739 Rkey: tid.TID(), 740 Record: &lexutil.LexiconTypeDecoder{ 741 Val: &tangled.RepoPullComment{ 742 - Pull: string(pullAt), 743 Body: body, 744 CreatedAt: createdAt, 745 }, ··· 987 } 988 989 sourceRev := comparison.Rev2 990 - patch := comparison.Patch 0 991 992 if !patchutil.IsPatchValid(patch) { 993 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1002 Sha: comparison.Rev2, 1003 } 1004 1005 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1006 } 1007 1008 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 1011 return 1012 } 1013 1014 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 1015 } 1016 1017 func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, forkRepo string, title, body, targetBranch, sourceBranch string, isStacked bool) { ··· 1094 } 1095 1096 sourceRev := comparison.Rev2 1097 - patch := comparison.Patch 0 1098 1099 if !patchutil.IsPatchValid(patch) { 1100 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1114 Sha: sourceRev, 1115 } 1116 1117 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1118 } 1119 1120 func (s *Pulls) createPullRequest( ··· 1124 user *oauth.User, 1125 title, body, targetBranch string, 1126 patch string, 0 1127 sourceRev string, 1128 pullSource *models.PullSource, 1129 recordPullSource *tangled.RepoPull_Source, ··· 1183 rkey := tid.TID() 1184 initialSubmission := models.PullSubmission{ 1185 Patch: patch, 0 1186 SourceRev: sourceRev, 1187 } 1188 pull := &models.Pull{ ··· 1612 1613 patch := r.FormValue("patch") 1614 1615 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1616 } 1617 1618 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1673 } 1674 1675 sourceRev := comparison.Rev2 1676 - patch := comparison.Patch 0 1677 1678 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1679 } 1680 1681 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1768 comparison := forkComparison 1769 1770 sourceRev := comparison.Rev2 1771 - patch := comparison.Patch 0 1772 1773 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1774 } 1775 1776 // validate a resubmission against a pull request ··· 1797 user *oauth.User, 1798 pull *models.Pull, 1799 patch string, 0 1800 sourceRev string, 1801 ) { 1802 if pull.IsStacked() { ··· 1826 } 1827 defer tx.Rollback() 1828 1829 - err = db.ResubmitPull(tx, pull, patch, sourceRev) 1830 if err != nil { 1831 log.Println("failed to create pull request", err) 1832 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 2042 submission := np.Submissions[np.LastRoundNumber()] 2043 2044 // resubmit the old pull 2045 - err := db.ResubmitPull(tx, op, submission.Patch, submission.SourceRev) 2046 2047 if err != nil { 2048 log.Println("failed to update pull", err, op.PullId)

··· 146 stack, _ := r.Context().Value("stack").(models.Stack) 147 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 148 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 149 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 150 branchDeleteStatus := s.branchDeleteStatus(r, f, pull) 151 resubmitResult := pages.Unknown ··· 443 return 444 } 445 446 + patch := pull.Submissions[roundIdInt].CombinedPatch() 447 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 448 449 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 494 return 495 } 496 497 + currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch()) 498 if err != nil { 499 log.Println("failed to interdiff; current patch malformed") 500 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 501 return 502 } 503 504 + previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch()) 505 if err != nil { 506 log.Println("failed to interdiff; previous patch malformed") 507 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 703 704 createdAt := time.Now().Format(time.RFC3339) 705 0 0 0 0 0 0 0 706 client, err := s.oauth.AuthorizedClient(r) 707 if err != nil { 708 log.Println("failed to get authorized client", err) ··· 715 Rkey: tid.TID(), 716 Record: &lexutil.LexiconTypeDecoder{ 717 Val: &tangled.RepoPullComment{ 718 + Pull: pull.PullAt().String(), 719 Body: body, 720 CreatedAt: createdAt, 721 }, ··· 963 } 964 965 sourceRev := comparison.Rev2 966 + patch := comparison.FormatPatchRaw 967 + combined := comparison.CombinedPatchRaw 968 969 if !patchutil.IsPatchValid(patch) { 970 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 979 Sha: comparison.Rev2, 980 } 981 982 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 983 } 984 985 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 988 return 989 } 990 991 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked) 992 } 993 994 func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, forkRepo string, title, body, targetBranch, sourceBranch string, isStacked bool) { ··· 1071 } 1072 1073 sourceRev := comparison.Rev2 1074 + patch := comparison.FormatPatchRaw 1075 + combined := comparison.CombinedPatchRaw 1076 1077 if !patchutil.IsPatchValid(patch) { 1078 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1092 Sha: sourceRev, 1093 } 1094 1095 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1096 } 1097 1098 func (s *Pulls) createPullRequest( ··· 1102 user *oauth.User, 1103 title, body, targetBranch string, 1104 patch string, 1105 + combined string, 1106 sourceRev string, 1107 pullSource *models.PullSource, 1108 recordPullSource *tangled.RepoPull_Source, ··· 1162 rkey := tid.TID() 1163 initialSubmission := models.PullSubmission{ 1164 Patch: patch, 1165 + Combined: combined, 1166 SourceRev: sourceRev, 1167 } 1168 pull := &models.Pull{ ··· 1592 1593 patch := r.FormValue("patch") 1594 1595 + s.resubmitPullHelper(w, r, f, user, pull, patch, "", "") 1596 } 1597 1598 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1653 } 1654 1655 sourceRev := comparison.Rev2 1656 + patch := comparison.FormatPatchRaw 1657 + combined := comparison.CombinedPatchRaw 1658 1659 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1660 } 1661 1662 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1749 comparison := forkComparison 1750 1751 sourceRev := comparison.Rev2 1752 + patch := comparison.FormatPatchRaw 1753 + combined := comparison.CombinedPatchRaw 1754 1755 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1756 } 1757 1758 // validate a resubmission against a pull request ··· 1779 user *oauth.User, 1780 pull *models.Pull, 1781 patch string, 1782 + combined string, 1783 sourceRev string, 1784 ) { 1785 if pull.IsStacked() { ··· 1809 } 1810 defer tx.Rollback() 1811 1812 + err = db.ResubmitPull(tx, pull, patch, combined, sourceRev) 1813 if err != nil { 1814 log.Println("failed to create pull request", err) 1815 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 2025 submission := np.Submissions[np.LastRoundNumber()] 2026 2027 // resubmit the old pull 2028 + err := db.ResubmitPull(tx, op, submission.Patch, submission.Combined, submission.SourceRev) 2029 2030 if err != nil { 2031 log.Println("failed to update pull", err, op.PullId)

+6 -1

appview/repo/repo.go

··· 2565 return 2566 } 2567 2568 - diff := patchutil.AsNiceDiff(formatPatch.Patch, base) 0 0 0 0 0 2569 2570 repoinfo := f.RepoInfo(user) 2571

··· 2565 return 2566 } 2567 2568 + var diff types.NiceDiff 2569 + if formatPatch.CombinedPatchRaw != "" { 2570 + diff = patchutil.AsNiceDiff(formatPatch.CombinedPatchRaw, base) 2571 + } else { 2572 + diff = patchutil.AsNiceDiff(formatPatch.FormatPatchRaw, base) 2573 + } 2574 2575 repoinfo := f.RepoInfo(user) 2576

+20 -4

knotserver/xrpc/repo_compare.go

··· 4 "fmt" 5 "net/http" 6 0 7 "tangled.org/core/knotserver/git" 8 "tangled.org/core/types" 9 xrpcerr "tangled.org/core/xrpc/errors" ··· 71 return 72 } 73 0 0 0 0 0 0 0 0 0 0 0 0 0 74 response := types.RepoFormatPatchResponse{ 75 - Rev1: commit1.Hash.String(), 76 - Rev2: commit2.Hash.String(), 77 - FormatPatch: formatPatch, 78 - Patch: rawPatch, 0 0 79 } 80 81 writeJson(w, response)

··· 4 "fmt" 5 "net/http" 6 7 + "github.com/bluekeyes/go-gitdiff/gitdiff" 8 "tangled.org/core/knotserver/git" 9 "tangled.org/core/types" 10 xrpcerr "tangled.org/core/xrpc/errors" ··· 72 return 73 } 74 75 + var combinedPatch []*gitdiff.File 76 + var combinedPatchRaw string 77 + // we need the combined patch 78 + if len(formatPatch) >= 2 { 79 + diffTree, err := gr.DiffTree(commit1, commit2) 80 + if err != nil { 81 + x.Logger.Error("error comparing revisions", "msg", err.Error()) 82 + } else { 83 + combinedPatch = diffTree.Diff 84 + combinedPatchRaw = diffTree.Patch 85 + } 86 + } 87 + 88 response := types.RepoFormatPatchResponse{ 89 + Rev1: commit1.Hash.String(), 90 + Rev2: commit2.Hash.String(), 91 + FormatPatch: formatPatch, 92 + FormatPatchRaw: rawPatch, 93 + CombinedPatch: combinedPatch, 94 + CombinedPatchRaw: combinedPatchRaw, 95 } 96 97 writeJson(w, response)

+7 -5

types/repo.go

··· 1 package types 2 3 import ( 0 4 "github.com/go-git/go-git/v5/plumbing/object" 5 ) 6 ··· 33 } 34 35 type RepoFormatPatchResponse struct { 36 - Rev1 string `json:"rev1,omitempty"` 37 - Rev2 string `json:"rev2,omitempty"` 38 - FormatPatch []FormatPatch `json:"format_patch,omitempty"` 39 - MergeBase string `json:"merge_base,omitempty"` // deprecated 40 - Patch string `json:"patch,omitempty"` 0 41 } 42 43 type RepoTreeResponse struct {

··· 1 package types 2 3 import ( 4 + "github.com/bluekeyes/go-gitdiff/gitdiff" 5 "github.com/go-git/go-git/v5/plumbing/object" 6 ) 7 ··· 34 } 35 36 type RepoFormatPatchResponse struct { 37 + Rev1 string `json:"rev1,omitempty"` 38 + Rev2 string `json:"rev2,omitempty"` 39 + FormatPatch []FormatPatch `json:"format_patch,omitempty"` 40 + FormatPatchRaw string `json:"patch,omitempty"` 41 + CombinedPatch []*gitdiff.File `json:"combined_patch,omitempty"` 42 + CombinedPatchRaw string `json:"combined_patch_raw,omitempty"` 43 } 44 45 type RepoTreeResponse struct {