forked from tangled.org/core
Monorepo for Tangled — https://tangled.org

appview,knotserver: produce combined patch in comparisons

this is calculated by the knotserver in sh.tangled.repo.compare and
cached by the appview in pull submissions, this cannot be calculated on
the appview side with just the format-patch because this calculation
requires a git-index.

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

authored by oppi.li and committed by Tangled 898d826c ce1f2b90

Changed files
+108 -76
appview
db
models
pulls
repo
knotserver
types
+9
appview/db/db.go
··· 1097 1097 }) 1098 1098 conn.ExecContext(ctx, "pragma foreign_keys = on;") 1099 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 + 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 - 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) 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 96 return err 97 97 } 98 98 ··· 313 313 pull_at, 314 314 round_number, 315 315 patch, 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 - var createdAt string 336 - var sourceRev sql.NullString 336 + var submissionCreatedStr string 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 - &createdAt, 343 - &sourceRev, 343 + &submissionCombined, 344 + &submissionCreatedStr, 345 + &submissionSourceRev, 344 346 ) 345 347 if err != nil { 346 348 return nil, err 347 349 } 348 350 349 - createdTime, err := time.Parse(time.RFC3339, createdAt) 350 - if err != nil { 351 - return nil, err 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 352 357 } 353 - submission.Created = createdTime 354 358 355 - if sourceRev.Valid { 356 - submission.SourceRev = sourceRev.String 359 + if submissionCombined.Valid { 360 + submission.Combined = submissionCombined.String 357 361 } 358 362 359 363 submissionMap[submission.ID] = &submission ··· 590 594 return err 591 595 } 592 596 593 - func ResubmitPull(e Execer, pullAt syntax.ATURI, newRoundNumber int, newPatch string, newSourceRev string) error { 597 + func ResubmitPull(e Execer, pullAt syntax.ATURI, newRoundNumber int, newPatch string, combinedPatch string, newSourceRev string) error { 594 598 _, err := e.Exec(` 595 - insert into pull_submissions (pull_at, round_number, patch, source_rev) 596 - values (?, ?, ?, ?) 597 - `, pullAt, newRoundNumber, newPatch, newSourceRev) 599 + insert into pull_submissions (pull_at, round_number, patch, combined, source_rev) 600 + values (?, ?, ?, ?, ?) 601 + `, pullAt, newRoundNumber, newPatch, combinedPatch, newSourceRev) 598 602 599 603 return err 600 604 }
+19 -8
appview/models/pull.go
··· 125 125 // content 126 126 RoundNumber int 127 127 Patch string 128 + Combined string 128 129 Comments []PullComment 129 130 SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 130 131 ··· 150 151 Created time.Time 151 152 } 152 153 154 + func (p *Pull) LastRoundNumber() int { 155 + return len(p.Submissions) - 1 156 + } 157 + 158 + func (p *Pull) LatestSubmission() *PullSubmission { 159 + return p.Submissions[p.LastRoundNumber()] 160 + } 161 + 153 162 func (p *Pull) LatestPatch() string { 154 - latestSubmission := p.Submissions[p.LastRoundNumber()] 155 - return latestSubmission.Patch 163 + return p.LatestSubmission().Patch 156 164 } 157 165 158 166 func (p *Pull) LatestSha() string { 159 - latestSubmission := p.Submissions[p.LastRoundNumber()] 160 - return latestSubmission.SourceRev 167 + return p.LatestSubmission().SourceRev 161 168 } 162 169 163 170 func (p *Pull) PullAt() syntax.ATURI { 164 171 return syntax.ATURI(fmt.Sprintf("at://%s/%s/%s", p.OwnerDid, tangled.RepoPullNSID, p.Rkey)) 165 - } 166 - 167 - func (p *Pull) LastRoundNumber() int { 168 - return len(p.Submissions) - 1 169 172 } 170 173 171 174 func (p *Pull) IsPatchBased() bool { ··· 252 255 } 253 256 254 257 return participants 258 + } 259 + 260 + func (s PullSubmission) CombinedPatch() string { 261 + if s.Combined == "" { 262 + return s.Patch 263 + } 264 + 265 + return s.Combined 255 266 } 256 267 257 268 type Stack []*Pull
+26 -41
appview/pulls/pulls.go
··· 145 145 stack, _ := r.Context().Value("stack").(models.Stack) 146 146 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 147 147 148 - totalIdents := 1 149 - for _, submission := range pull.Submissions { 150 - totalIdents += len(submission.Comments) 151 - } 152 - 153 - identsToResolve := make([]string, totalIdents) 154 - 155 - // populate idents 156 - identsToResolve[0] = pull.OwnerDid 157 - idx := 1 158 - for _, submission := range pull.Submissions { 159 - for _, comment := range submission.Comments { 160 - identsToResolve[idx] = comment.OwnerDid 161 - idx += 1 162 - } 163 - } 164 - 165 148 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 166 149 branchDeleteStatus := s.branchDeleteStatus(r, f, pull) 167 150 resubmitResult := pages.Unknown ··· 459 442 return 460 443 } 461 444 462 - patch := pull.Submissions[roundIdInt].Patch 445 + patch := pull.Submissions[roundIdInt].CombinedPatch() 463 446 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 464 447 465 448 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 510 493 return 511 494 } 512 495 513 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 496 + currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch()) 514 497 if err != nil { 515 498 log.Println("failed to interdiff; current patch malformed") 516 499 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 517 500 return 518 501 } 519 502 520 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 503 + previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch()) 521 504 if err != nil { 522 505 log.Println("failed to interdiff; previous patch malformed") 523 506 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 719 702 720 703 createdAt := time.Now().Format(time.RFC3339) 721 704 722 - pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId) 723 - if err != nil { 724 - log.Println("failed to get pull at", err) 725 - s.pages.Notice(w, "pull-comment", "Failed to create comment.") 726 - return 727 - } 728 - 729 705 client, err := s.oauth.AuthorizedClient(r) 730 706 if err != nil { 731 707 log.Println("failed to get authorized client", err) ··· 738 714 Rkey: tid.TID(), 739 715 Record: &lexutil.LexiconTypeDecoder{ 740 716 Val: &tangled.RepoPullComment{ 741 - Pull: string(pullAt), 717 + Pull: pull.PullAt().String(), 742 718 Body: body, 743 719 CreatedAt: createdAt, 744 720 }, ··· 986 962 } 987 963 988 964 sourceRev := comparison.Rev2 989 - patch := comparison.Patch 965 + patch := comparison.FormatPatchRaw 966 + combined := comparison.CombinedPatchRaw 990 967 991 968 if !patchutil.IsPatchValid(patch) { 992 969 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1001 978 Sha: comparison.Rev2, 1002 979 } 1003 980 1004 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 981 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1005 982 } 1006 983 1007 984 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 1010 987 return 1011 988 } 1012 989 1013 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 990 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked) 1014 991 } 1015 992 1016 993 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) { ··· 1093 1070 } 1094 1071 1095 1072 sourceRev := comparison.Rev2 1096 - patch := comparison.Patch 1073 + patch := comparison.FormatPatchRaw 1074 + combined := comparison.CombinedPatchRaw 1097 1075 1098 1076 if !patchutil.IsPatchValid(patch) { 1099 1077 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1113 1091 Sha: sourceRev, 1114 1092 } 1115 1093 1116 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1094 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1117 1095 } 1118 1096 1119 1097 func (s *Pulls) createPullRequest( ··· 1123 1101 user *oauth.User, 1124 1102 title, body, targetBranch string, 1125 1103 patch string, 1104 + combined string, 1126 1105 sourceRev string, 1127 1106 pullSource *models.PullSource, 1128 1107 recordPullSource *tangled.RepoPull_Source, ··· 1182 1161 rkey := tid.TID() 1183 1162 initialSubmission := models.PullSubmission{ 1184 1163 Patch: patch, 1164 + Combined: combined, 1185 1165 SourceRev: sourceRev, 1186 1166 } 1187 1167 pull := &models.Pull{ ··· 1611 1591 1612 1592 patch := r.FormValue("patch") 1613 1593 1614 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1594 + s.resubmitPullHelper(w, r, f, user, pull, patch, "", "") 1615 1595 } 1616 1596 1617 1597 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1672 1652 } 1673 1653 1674 1654 sourceRev := comparison.Rev2 1675 - patch := comparison.Patch 1655 + patch := comparison.FormatPatchRaw 1656 + combined := comparison.CombinedPatchRaw 1676 1657 1677 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1658 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1678 1659 } 1679 1660 1680 1661 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1767 1748 comparison := forkComparison 1768 1749 1769 1750 sourceRev := comparison.Rev2 1770 - patch := comparison.Patch 1751 + patch := comparison.FormatPatchRaw 1752 + combined := comparison.CombinedPatchRaw 1771 1753 1772 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1754 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1773 1755 } 1774 1756 1775 1757 // validate a resubmission against a pull request ··· 1796 1778 user *oauth.User, 1797 1779 pull *models.Pull, 1798 1780 patch string, 1781 + combined string, 1799 1782 sourceRev string, 1800 1783 ) { 1801 1784 if pull.IsStacked() { ··· 1829 1812 newRoundNumber := len(pull.Submissions) 1830 1813 newPatch := patch 1831 1814 newSourceRev := sourceRev 1832 - err = db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, newSourceRev) 1815 + combinedPatch := combined 1816 + err = db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, combinedPatch, newSourceRev) 1833 1817 if err != nil { 1834 1818 log.Println("failed to create pull request", err) 1835 1819 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 2024 2008 pullAt := op.PullAt() 2025 2009 newRoundNumber := len(op.Submissions) 2026 2010 newPatch := np.LatestPatch() 2011 + combinedPatch := np.LatestSubmission().Combined 2027 2012 newSourceRev := np.LatestSha() 2028 - err := db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, newSourceRev) 2029 - 2013 + err := db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, combinedPatch, newSourceRev) 2030 2014 if err != nil { 2031 2015 log.Println("failed to update pull", err, op.PullId) 2032 2016 s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") ··· 2374 2358 initialSubmission := models.PullSubmission{ 2375 2359 Patch: fp.Raw, 2376 2360 SourceRev: fp.SHA, 2361 + Combined: fp.Raw, 2377 2362 } 2378 2363 pull := models.Pull{ 2379 2364 Title: title,
+6 -1
appview/repo/repo.go
··· 2565 2565 return 2566 2566 } 2567 2567 2568 - diff := patchutil.AsNiceDiff(formatPatch.Patch, base) 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 + } 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 + "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 + 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 + 74 88 response := types.RepoFormatPatchResponse{ 75 - Rev1: commit1.Hash.String(), 76 - Rev2: commit2.Hash.String(), 77 - FormatPatch: formatPatch, 78 - Patch: rawPatch, 89 + Rev1: commit1.Hash.String(), 90 + Rev2: commit2.Hash.String(), 91 + FormatPatch: formatPatch, 92 + FormatPatchRaw: rawPatch, 93 + CombinedPatch: combinedPatch, 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 + "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 - 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"` 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"` 41 43 } 42 44 43 45 type RepoTreeResponse struct {