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, pullAt syntax.ATURI, newRoundNumber int, newPatch string, newSourceRev string) error { 597 597 + func ResubmitPull(e Execer, pullAt syntax.ATURI, newRoundNumber int, newPatch string, combinedPatch string, newSourceRev string) error { 594 598 _, err := e.Exec(` 595 595 - insert into pull_submissions (pull_at, round_number, patch, source_rev) 596 596 - values (?, ?, ?, ?) 597 597 - `, pullAt, newRoundNumber, newPatch, newSourceRev) 599 599 + insert into pull_submissions (pull_at, round_number, patch, combined, source_rev) 600 600 + values (?, ?, ?, ?, ?) 601 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 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 154 + func (p *Pull) LastRoundNumber() int { 155 155 + return len(p.Submissions) - 1 156 156 + } 157 157 + 158 158 + func (p *Pull) LatestSubmission() *PullSubmission { 159 159 + return p.Submissions[p.LastRoundNumber()] 160 160 + } 161 161 + 153 162 func (p *Pull) LatestPatch() string { 154 154 - latestSubmission := p.Submissions[p.LastRoundNumber()] 155 155 - return latestSubmission.Patch 163 163 + return p.LatestSubmission().Patch 156 164 } 157 165 158 166 func (p *Pull) LatestSha() string { 159 159 - latestSubmission := p.Submissions[p.LastRoundNumber()] 160 160 - return latestSubmission.SourceRev 167 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 172 } 166 173 167 167 - func (p *Pull) LastRoundNumber() int { 168 168 - return len(p.Submissions) - 1 169 169 - } 170 170 - 171 174 func (p *Pull) IsPatchBased() bool { 172 175 return p.PullSource == nil 173 176 } ··· 254 257 return participants 255 258 } 256 259 260 260 + func (s PullSubmission) CombinedPatch() string { 261 261 + if s.Combined == "" { 262 262 + return s.Patch 263 263 + } 264 264 + 265 265 + return s.Combined 266 266 + } 267 267 + 257 268 type Stack []*Pull 258 269 259 270 // position of this pull in the stack

+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 148 - totalIdents := 1 149 149 - for _, submission := range pull.Submissions { 150 150 - totalIdents += len(submission.Comments) 151 151 - } 152 152 - 153 153 - identsToResolve := make([]string, totalIdents) 154 154 - 155 155 - // populate idents 156 156 - identsToResolve[0] = pull.OwnerDid 157 157 - idx := 1 158 158 - for _, submission := range pull.Submissions { 159 159 - for _, comment := range submission.Comments { 160 160 - identsToResolve[idx] = comment.OwnerDid 161 161 - idx += 1 162 162 - } 163 163 - } 164 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 462 - patch := pull.Submissions[roundIdInt].Patch 445 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 513 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 496 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 520 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 503 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 722 - pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId) 723 723 - if err != nil { 724 724 - log.Println("failed to get pull at", err) 725 725 - s.pages.Notice(w, "pull-comment", "Failed to create comment.") 726 726 - return 727 727 - } 728 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 741 - Pull: string(pullAt), 717 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 989 - patch := comparison.Patch 965 965 + patch := comparison.FormatPatchRaw 966 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 1004 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 981 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 1013 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 990 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 1096 - patch := comparison.Patch 1073 1073 + patch := comparison.FormatPatchRaw 1074 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 1116 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1094 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 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 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 1614 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1594 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 1675 - patch := comparison.Patch 1655 1655 + patch := comparison.FormatPatchRaw 1656 1656 + combined := comparison.CombinedPatchRaw 1676 1657 1677 1677 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1658 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 1770 - patch := comparison.Patch 1751 1751 + patch := comparison.FormatPatchRaw 1752 1752 + combined := comparison.CombinedPatchRaw 1771 1753 1772 1772 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1754 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 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 1832 - err = db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, newSourceRev) 1815 1815 + combinedPatch := combined 1816 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 2011 + combinedPatch := np.LatestSubmission().Combined 2027 2012 newSourceRev := np.LatestSha() 2028 2028 - err := db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, newSourceRev) 2029 2029 - 2013 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 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 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 {