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, pullAt syntax.ATURI, newRoundNumber int, newPatch string, newSourceRev string) error { 594 _, err := e.Exec(` 595 - insert into pull_submissions (pull_at, round_number, patch, source_rev) 596 - values (?, ?, ?, ?) 597 - `, pullAt, newRoundNumber, newPatch, newSourceRev) 598 599 return err 600 }

··· 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, pullAt syntax.ATURI, newRoundNumber int, newPatch string, combinedPatch string, newSourceRev string) error { 598 _, err := e.Exec(` 599 + insert into pull_submissions (pull_at, round_number, patch, combined, source_rev) 600 + values (?, ?, ?, ?, ?) 601 + `, pullAt, newRoundNumber, newPatch, combinedPatch, newSourceRev) 602 603 return err 604 }

+19 -8

appview/models/pull.go

··· 125 // content 126 RoundNumber int 127 Patch string 0 128 Comments []PullComment 129 SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 130 ··· 150 Created time.Time 151 } 152 0 0 0 0 0 0 0 0 153 func (p *Pull) LatestPatch() string { 154 - latestSubmission := p.Submissions[p.LastRoundNumber()] 155 - return latestSubmission.Patch 156 } 157 158 func (p *Pull) LatestSha() string { 159 - latestSubmission := p.Submissions[p.LastRoundNumber()] 160 - return latestSubmission.SourceRev 161 } 162 163 func (p *Pull) PullAt() syntax.ATURI { 164 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 - } 170 - 171 func (p *Pull) IsPatchBased() bool { 172 return p.PullSource == nil 173 } ··· 254 return participants 255 } 256 0 0 0 0 0 0 0 0 257 type Stack []*Pull 258 259 // position of this pull in the stack

··· 125 // content 126 RoundNumber int 127 Patch string 128 + Combined string 129 Comments []PullComment 130 SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 131 ··· 151 Created time.Time 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 + 162 func (p *Pull) LatestPatch() string { 163 + return p.LatestSubmission().Patch 0 164 } 165 166 func (p *Pull) LatestSha() string { 167 + return p.LatestSubmission().SourceRev 0 168 } 169 170 func (p *Pull) PullAt() syntax.ATURI { 171 return syntax.ATURI(fmt.Sprintf("at://%s/%s/%s", p.OwnerDid, tangled.RepoPullNSID, p.Rkey)) 172 } 173 0 0 0 0 174 func (p *Pull) IsPatchBased() bool { 175 return p.PullSource == nil 176 } ··· 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 266 + } 267 + 268 type Stack []*Pull 269 270 // position of this pull in the stack

+26 -41

appview/pulls/pulls.go

··· 145 stack, _ := r.Context().Value("stack").(models.Stack) 146 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 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 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 166 branchDeleteStatus := s.branchDeleteStatus(r, f, pull) 167 resubmitResult := pages.Unknown ··· 459 return 460 } 461 462 - patch := pull.Submissions[roundIdInt].Patch 463 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 464 465 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 510 return 511 } 512 513 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 514 if err != nil { 515 log.Println("failed to interdiff; current patch malformed") 516 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 517 return 518 } 519 520 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 521 if err != nil { 522 log.Println("failed to interdiff; previous patch malformed") 523 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 719 720 createdAt := time.Now().Format(time.RFC3339) 721 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 client, err := s.oauth.AuthorizedClient(r) 730 if err != nil { 731 log.Println("failed to get authorized client", err) ··· 738 Rkey: tid.TID(), 739 Record: &lexutil.LexiconTypeDecoder{ 740 Val: &tangled.RepoPullComment{ 741 - Pull: string(pullAt), 742 Body: body, 743 CreatedAt: createdAt, 744 }, ··· 986 } 987 988 sourceRev := comparison.Rev2 989 - patch := comparison.Patch 0 990 991 if !patchutil.IsPatchValid(patch) { 992 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1001 Sha: comparison.Rev2, 1002 } 1003 1004 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1005 } 1006 1007 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 1010 return 1011 } 1012 1013 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 1014 } 1015 1016 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 } 1094 1095 sourceRev := comparison.Rev2 1096 - patch := comparison.Patch 0 1097 1098 if !patchutil.IsPatchValid(patch) { 1099 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1113 Sha: sourceRev, 1114 } 1115 1116 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1117 } 1118 1119 func (s *Pulls) createPullRequest( ··· 1123 user *oauth.User, 1124 title, body, targetBranch string, 1125 patch string, 0 1126 sourceRev string, 1127 pullSource *models.PullSource, 1128 recordPullSource *tangled.RepoPull_Source, ··· 1182 rkey := tid.TID() 1183 initialSubmission := models.PullSubmission{ 1184 Patch: patch, 0 1185 SourceRev: sourceRev, 1186 } 1187 pull := &models.Pull{ ··· 1611 1612 patch := r.FormValue("patch") 1613 1614 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1615 } 1616 1617 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1672 } 1673 1674 sourceRev := comparison.Rev2 1675 - patch := comparison.Patch 0 1676 1677 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1678 } 1679 1680 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1767 comparison := forkComparison 1768 1769 sourceRev := comparison.Rev2 1770 - patch := comparison.Patch 0 1771 1772 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1773 } 1774 1775 // validate a resubmission against a pull request ··· 1796 user *oauth.User, 1797 pull *models.Pull, 1798 patch string, 0 1799 sourceRev string, 1800 ) { 1801 if pull.IsStacked() { ··· 1829 newRoundNumber := len(pull.Submissions) 1830 newPatch := patch 1831 newSourceRev := sourceRev 1832 - err = db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, newSourceRev) 0 1833 if err != nil { 1834 log.Println("failed to create pull request", err) 1835 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 2024 pullAt := op.PullAt() 2025 newRoundNumber := len(op.Submissions) 2026 newPatch := np.LatestPatch() 0 2027 newSourceRev := np.LatestSha() 2028 - err := db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, newSourceRev) 2029 - 2030 if err != nil { 2031 log.Println("failed to update pull", err, op.PullId) 2032 s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") ··· 2374 initialSubmission := models.PullSubmission{ 2375 Patch: fp.Raw, 2376 SourceRev: fp.SHA, 0 2377 } 2378 pull := models.Pull{ 2379 Title: title,

··· 145 stack, _ := r.Context().Value("stack").(models.Stack) 146 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 147 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 148 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 149 branchDeleteStatus := s.branchDeleteStatus(r, f, pull) 150 resubmitResult := pages.Unknown ··· 442 return 443 } 444 445 + patch := pull.Submissions[roundIdInt].CombinedPatch() 446 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 447 448 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 493 return 494 } 495 496 + currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch()) 497 if err != nil { 498 log.Println("failed to interdiff; current patch malformed") 499 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 500 return 501 } 502 503 + previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch()) 504 if err != nil { 505 log.Println("failed to interdiff; previous patch malformed") 506 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 702 703 createdAt := time.Now().Format(time.RFC3339) 704 0 0 0 0 0 0 0 705 client, err := s.oauth.AuthorizedClient(r) 706 if err != nil { 707 log.Println("failed to get authorized client", err) ··· 714 Rkey: tid.TID(), 715 Record: &lexutil.LexiconTypeDecoder{ 716 Val: &tangled.RepoPullComment{ 717 + Pull: pull.PullAt().String(), 718 Body: body, 719 CreatedAt: createdAt, 720 }, ··· 962 } 963 964 sourceRev := comparison.Rev2 965 + patch := comparison.FormatPatchRaw 966 + combined := comparison.CombinedPatchRaw 967 968 if !patchutil.IsPatchValid(patch) { 969 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 978 Sha: comparison.Rev2, 979 } 980 981 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 982 } 983 984 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 987 return 988 } 989 990 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked) 991 } 992 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) { ··· 1070 } 1071 1072 sourceRev := comparison.Rev2 1073 + patch := comparison.FormatPatchRaw 1074 + combined := comparison.CombinedPatchRaw 1075 1076 if !patchutil.IsPatchValid(patch) { 1077 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1091 Sha: sourceRev, 1092 } 1093 1094 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1095 } 1096 1097 func (s *Pulls) createPullRequest( ··· 1101 user *oauth.User, 1102 title, body, targetBranch string, 1103 patch string, 1104 + combined string, 1105 sourceRev string, 1106 pullSource *models.PullSource, 1107 recordPullSource *tangled.RepoPull_Source, ··· 1161 rkey := tid.TID() 1162 initialSubmission := models.PullSubmission{ 1163 Patch: patch, 1164 + Combined: combined, 1165 SourceRev: sourceRev, 1166 } 1167 pull := &models.Pull{ ··· 1591 1592 patch := r.FormValue("patch") 1593 1594 + s.resubmitPullHelper(w, r, f, user, pull, patch, "", "") 1595 } 1596 1597 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1652 } 1653 1654 sourceRev := comparison.Rev2 1655 + patch := comparison.FormatPatchRaw 1656 + combined := comparison.CombinedPatchRaw 1657 1658 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1659 } 1660 1661 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1748 comparison := forkComparison 1749 1750 sourceRev := comparison.Rev2 1751 + patch := comparison.FormatPatchRaw 1752 + combined := comparison.CombinedPatchRaw 1753 1754 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1755 } 1756 1757 // validate a resubmission against a pull request ··· 1778 user *oauth.User, 1779 pull *models.Pull, 1780 patch string, 1781 + combined string, 1782 sourceRev string, 1783 ) { 1784 if pull.IsStacked() { ··· 1812 newRoundNumber := len(pull.Submissions) 1813 newPatch := patch 1814 newSourceRev := sourceRev 1815 + combinedPatch := combined 1816 + err = db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, combinedPatch, newSourceRev) 1817 if err != nil { 1818 log.Println("failed to create pull request", err) 1819 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 2008 pullAt := op.PullAt() 2009 newRoundNumber := len(op.Submissions) 2010 newPatch := np.LatestPatch() 2011 + combinedPatch := np.LatestSubmission().Combined 2012 newSourceRev := np.LatestSha() 2013 + err := db.ResubmitPull(tx, pullAt, newRoundNumber, newPatch, combinedPatch, newSourceRev) 0 2014 if err != nil { 2015 log.Println("failed to update pull", err, op.PullId) 2016 s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") ··· 2358 initialSubmission := models.PullSubmission{ 2359 Patch: fp.Raw, 2360 SourceRev: fp.SHA, 2361 + Combined: fp.Raw, 2362 } 2363 pull := models.Pull{ 2364 Title: title,

+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 {