appview,knotserver: produce combined patch in comparisons #609

+9

appview/db/db.go

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

+21 -17

appview/db/pulls.go

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

+9

appview/models/pull.go

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

+23 -40

appview/pulls/pulls.go

··· 146 146 stack, _ := r.Context().Value("stack").(models.Stack) 147 147 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 148 148 149 149 - totalIdents := 1 150 150 - for _, submission := range pull.Submissions { 151 151 - totalIdents += len(submission.Comments) 152 152 - } 153 153 - 154 154 - identsToResolve := make([]string, totalIdents) 155 155 - 156 156 - // populate idents 157 157 - identsToResolve[0] = pull.OwnerDid 158 158 - idx := 1 159 159 - for _, submission := range pull.Submissions { 160 160 - for _, comment := range submission.Comments { 161 161 - identsToResolve[idx] = comment.OwnerDid 162 162 - idx += 1 163 163 - } 164 164 - } 165 165 - 166 149 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 167 150 branchDeleteStatus := s.branchDeleteStatus(r, f, pull) 168 151 resubmitResult := pages.Unknown ··· 460 443 return 461 444 } 462 445 463 463 - patch := pull.Submissions[roundIdInt].Patch 446 446 + patch := pull.Submissions[roundIdInt].CombinedPatch() 464 447 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 465 448 466 449 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 511 494 return 512 495 } 513 496 514 514 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 497 497 + currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch()) 515 498 if err != nil { 516 499 log.Println("failed to interdiff; current patch malformed") 517 500 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 518 501 return 519 502 } 520 503 521 521 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 504 504 + previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch()) 522 505 if err != nil { 523 506 log.Println("failed to interdiff; previous patch malformed") 524 507 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 720 703 721 704 createdAt := time.Now().Format(time.RFC3339) 722 705 723 723 - pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId) 724 724 - if err != nil { 725 725 - log.Println("failed to get pull at", err) 726 726 - s.pages.Notice(w, "pull-comment", "Failed to create comment.") 727 727 - return 728 728 - } 729 729 - 730 706 client, err := s.oauth.AuthorizedClient(r) 731 707 if err != nil { 732 708 log.Println("failed to get authorized client", err) ··· 739 715 Rkey: tid.TID(), 740 716 Record: &lexutil.LexiconTypeDecoder{ 741 717 Val: &tangled.RepoPullComment{ 742 742 - Pull: string(pullAt), 718 718 + Pull: pull.PullAt().String(), 743 719 Body: body, 744 720 CreatedAt: createdAt, 745 721 }, ··· 987 963 } 988 964 989 965 sourceRev := comparison.Rev2 990 990 - patch := comparison.Patch 966 966 + patch := comparison.FormatPatchRaw 967 967 + combined := comparison.CombinedPatchRaw 991 968 992 969 if !patchutil.IsPatchValid(patch) { 993 970 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1002 979 Sha: comparison.Rev2, 1003 980 } 1004 981 1005 1005 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 982 982 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1006 983 } 1007 984 1008 985 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 1011 988 return 1012 989 } 1013 990 1014 1014 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 991 991 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked) 1015 992 } 1016 993 1017 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) { ··· 1094 1071 } 1095 1072 1096 1073 sourceRev := comparison.Rev2 1097 1097 - patch := comparison.Patch 1074 1074 + patch := comparison.FormatPatchRaw 1075 1075 + combined := comparison.CombinedPatchRaw 1098 1076 1099 1077 if !patchutil.IsPatchValid(patch) { 1100 1078 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1114 1092 Sha: sourceRev, 1115 1093 } 1116 1094 1117 1117 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1095 1095 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1118 1096 } 1119 1097 1120 1098 func (s *Pulls) createPullRequest( ··· 1124 1102 user *oauth.User, 1125 1103 title, body, targetBranch string, 1126 1104 patch string, 1105 1105 + combined string, 1127 1106 sourceRev string, 1128 1107 pullSource *models.PullSource, 1129 1108 recordPullSource *tangled.RepoPull_Source, ··· 1183 1162 rkey := tid.TID() 1184 1163 initialSubmission := models.PullSubmission{ 1185 1164 Patch: patch, 1165 1165 + Combined: combined, 1186 1166 SourceRev: sourceRev, 1187 1167 } 1188 1168 pull := &models.Pull{ ··· 1612 1592 1613 1593 patch := r.FormValue("patch") 1614 1594 1615 1615 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1595 1595 + s.resubmitPullHelper(w, r, f, user, pull, patch, "", "") 1616 1596 } 1617 1597 1618 1598 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1673 1653 } 1674 1654 1675 1655 sourceRev := comparison.Rev2 1676 1676 - patch := comparison.Patch 1656 1656 + patch := comparison.FormatPatchRaw 1657 1657 + combined := comparison.CombinedPatchRaw 1677 1658 1678 1678 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1659 1659 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1679 1660 } 1680 1661 1681 1662 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1768 1749 comparison := forkComparison 1769 1750 1770 1751 sourceRev := comparison.Rev2 1771 1771 - patch := comparison.Patch 1752 1752 + patch := comparison.FormatPatchRaw 1753 1753 + combined := comparison.CombinedPatchRaw 1772 1754 1773 1773 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1755 1755 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1774 1756 } 1775 1757 1776 1758 // validate a resubmission against a pull request ··· 1797 1779 user *oauth.User, 1798 1780 pull *models.Pull, 1799 1781 patch string, 1782 1782 + combined string, 1800 1783 sourceRev string, 1801 1784 ) { 1802 1785 if pull.IsStacked() { ··· 1826 1809 } 1827 1810 defer tx.Rollback() 1828 1811 1829 1829 - err = db.ResubmitPull(tx, pull, patch, sourceRev) 1812 1812 + err = db.ResubmitPull(tx, pull, patch, combined, sourceRev) 1830 1813 if err != nil { 1831 1814 log.Println("failed to create pull request", err) 1832 1815 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 2042 2025 submission := np.Submissions[np.LastRoundNumber()] 2043 2026 2044 2027 // resubmit the old pull 2045 2045 - err := db.ResubmitPull(tx, op, submission.Patch, submission.SourceRev) 2028 2028 + err := db.ResubmitPull(tx, op, submission.Patch, submission.Combined, submission.SourceRev) 2046 2029 2047 2030 if err != nil { 2048 2031 log.Println("failed to update pull", err, op.PullId)

+6 -1

appview/repo/repo.go

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

+20 -4

knotserver/xrpc/repo_compare.go

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

+7 -5

types/repo.go

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