appview,knotserver: produce combined patch in comparisons #609

+9

appview/db/db.go

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

+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

··· 142 142 stack, _ := r.Context().Value("stack").(models.Stack) 143 143 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 144 144 145 145 - totalIdents := 1 146 146 - for _, submission := range pull.Submissions { 147 147 - totalIdents += len(submission.Comments) 148 148 - } 149 149 - 150 150 - identsToResolve := make([]string, totalIdents) 151 151 - 152 152 - // populate idents 153 153 - identsToResolve[0] = pull.OwnerDid 154 154 - idx := 1 155 155 - for _, submission := range pull.Submissions { 156 156 - for _, comment := range submission.Comments { 157 157 - identsToResolve[idx] = comment.OwnerDid 158 158 - idx += 1 159 159 - } 160 160 - } 161 161 - 162 145 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 163 146 branchDeleteStatus := s.branchDeleteStatus(r, f, pull) 164 147 resubmitResult := pages.Unknown ··· 456 439 return 457 440 } 458 441 459 459 - patch := pull.Submissions[roundIdInt].Patch 442 442 + patch := pull.Submissions[roundIdInt].CombinedPatch() 460 443 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 461 444 462 445 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 507 490 return 508 491 } 509 492 510 510 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 493 493 + currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch()) 511 494 if err != nil { 512 495 log.Println("failed to interdiff; current patch malformed") 513 496 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 514 497 return 515 498 } 516 499 517 517 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 500 500 + previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch()) 518 501 if err != nil { 519 502 log.Println("failed to interdiff; previous patch malformed") 520 503 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 716 699 717 700 createdAt := time.Now().Format(time.RFC3339) 718 701 719 719 - pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId) 720 720 - if err != nil { 721 721 - log.Println("failed to get pull at", err) 722 722 - s.pages.Notice(w, "pull-comment", "Failed to create comment.") 723 723 - return 724 724 - } 725 725 - 726 702 client, err := s.oauth.AuthorizedClient(r) 727 703 if err != nil { 728 704 log.Println("failed to get authorized client", err) ··· 735 711 Rkey: tid.TID(), 736 712 Record: &lexutil.LexiconTypeDecoder{ 737 713 Val: &tangled.RepoPullComment{ 738 738 - Pull: string(pullAt), 714 714 + Pull: pull.PullAt().String(), 739 715 Body: body, 740 716 CreatedAt: createdAt, 741 717 }, ··· 983 959 } 984 960 985 961 sourceRev := comparison.Rev2 986 986 - patch := comparison.Patch 962 962 + patch := comparison.FormatPatchRaw 963 963 + combined := comparison.CombinedPatchRaw 987 964 988 965 if !patchutil.IsPatchValid(patch) { 989 966 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 998 975 Sha: comparison.Rev2, 999 976 } 1000 977 1001 1001 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 978 978 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1002 979 } 1003 980 1004 981 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 1007 984 return 1008 985 } 1009 986 1010 1010 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 987 987 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked) 1011 988 } 1012 989 1013 990 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) { ··· 1090 1067 } 1091 1068 1092 1069 sourceRev := comparison.Rev2 1093 1093 - patch := comparison.Patch 1070 1070 + patch := comparison.FormatPatchRaw 1071 1071 + combined := comparison.CombinedPatchRaw 1094 1072 1095 1073 if !patchutil.IsPatchValid(patch) { 1096 1074 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1110 1088 Sha: sourceRev, 1111 1089 } 1112 1090 1113 1113 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1091 1091 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1114 1092 } 1115 1093 1116 1094 func (s *Pulls) createPullRequest( ··· 1120 1098 user *oauth.User, 1121 1099 title, body, targetBranch string, 1122 1100 patch string, 1101 1101 + combined string, 1123 1102 sourceRev string, 1124 1103 pullSource *models.PullSource, 1125 1104 recordPullSource *tangled.RepoPull_Source, ··· 1179 1158 rkey := tid.TID() 1180 1159 initialSubmission := models.PullSubmission{ 1181 1160 Patch: patch, 1161 1161 + Combined: combined, 1182 1162 SourceRev: sourceRev, 1183 1163 } 1184 1164 pull := &models.Pull{ ··· 1608 1588 1609 1589 patch := r.FormValue("patch") 1610 1590 1611 1611 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1591 1591 + s.resubmitPullHelper(w, r, f, user, pull, patch, "", "") 1612 1592 } 1613 1593 1614 1594 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1669 1649 } 1670 1650 1671 1651 sourceRev := comparison.Rev2 1672 1672 - patch := comparison.Patch 1652 1652 + patch := comparison.FormatPatchRaw 1653 1653 + combined := comparison.CombinedPatchRaw 1673 1654 1674 1674 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1655 1655 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1675 1656 } 1676 1657 1677 1658 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1764 1745 comparison := forkComparison 1765 1746 1766 1747 sourceRev := comparison.Rev2 1767 1767 - patch := comparison.Patch 1748 1748 + patch := comparison.FormatPatchRaw 1749 1749 + combined := comparison.CombinedPatchRaw 1768 1750 1769 1769 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1751 1751 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1770 1752 } 1771 1753 1772 1754 // validate a resubmission against a pull request ··· 1793 1775 user *oauth.User, 1794 1776 pull *models.Pull, 1795 1777 patch string, 1778 1778 + combined string, 1796 1779 sourceRev string, 1797 1780 ) { 1798 1781 if pull.IsStacked() { ··· 1822 1805 } 1823 1806 defer tx.Rollback() 1824 1807 1825 1825 - err = db.ResubmitPull(tx, pull, patch, sourceRev) 1808 1808 + err = db.ResubmitPull(tx, pull, patch, combined, sourceRev) 1826 1809 if err != nil { 1827 1810 log.Println("failed to create pull request", err) 1828 1811 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 2038 2021 submission := np.Submissions[np.LastRoundNumber()] 2039 2022 2040 2023 // resubmit the old pull 2041 2041 - err := db.ResubmitPull(tx, op, submission.Patch, submission.SourceRev) 2024 2024 + err := db.ResubmitPull(tx, op, submission.Patch, submission.Combined, submission.SourceRev) 2042 2025 2043 2026 if err != nil { 2044 2027 log.Println("failed to update pull", err, op.PullId)

+6 -1

appview/repo/repo.go

··· 2527 2527 return 2528 2528 } 2529 2529 2530 2530 - diff := patchutil.AsNiceDiff(formatPatch.Patch, base) 2530 2530 + var diff types.NiceDiff 2531 2531 + if formatPatch.CombinedPatchRaw != "" { 2532 2532 + diff = patchutil.AsNiceDiff(formatPatch.CombinedPatchRaw, base) 2533 2533 + } else { 2534 2534 + diff = patchutil.AsNiceDiff(formatPatch.FormatPatchRaw, base) 2535 2535 + } 2531 2536 2532 2537 repoinfo := f.RepoInfo(user) 2533 2538

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