appview,knotserver: produce combined patch in comparisons #609

+9

appview/db/db.go

··· 954 954 return err 955 955 }) 956 956 957 957 + // knots may report the combined patch for a comparison, we can store that on the appview side 958 958 + // (but not on the pds record), because calculating the combined patch requires a git index 959 959 + runMigration(conn, "add-combined-column-submissions", func(tx *sql.Tx) error { 960 960 + _, err := tx.Exec(` 961 961 + alter table pull_submissions add column combined text; 962 962 + `) 963 963 + return err 964 964 + }) 965 965 + 957 966 return &DB{db}, nil 958 967 } 959 968

+18 -8

appview/db/pulls.go

··· 87 87 pull.ID = int(id) 88 88 89 89 _, err = tx.Exec(` 90 90 - insert into pull_submissions (pull_id, repo_at, round_number, patch, source_rev) 90 90 + insert into pull_submissions (pull_id, repo_at, round_number, patch, combined, source_rev) 91 91 values (?, ?, ?, ?, ?) 92 92 - `, pull.PullId, pull.RepoAt, 0, pull.Submissions[0].Patch, pull.Submissions[0].SourceRev) 92 92 + `, pull.PullId, pull.RepoAt, 0, pull.Submissions[0].Patch, pull.Submissions[0].Combined, pull.Submissions[0].SourceRev) 93 93 return err 94 94 } 95 95 ··· 218 218 inClause := strings.TrimSuffix(strings.Repeat("?, ", len(pulls)), ", ") 219 219 submissionsQuery := fmt.Sprintf(` 220 220 select 221 221 - id, pull_id, round_number, patch, created, source_rev 221 221 + id, pull_id, round_number, patch, combined, created, source_rev 222 222 from 223 223 pull_submissions 224 224 where ··· 243 243 244 244 for submissionsRows.Next() { 245 245 var s models.PullSubmission 246 246 - var sourceRev sql.NullString 246 246 + var sourceRev, combined sql.NullString 247 247 var createdAt string 248 248 err := submissionsRows.Scan( 249 249 &s.ID, 250 250 &s.PullId, 251 251 &s.RoundNumber, 252 252 &s.Patch, 253 253 + &combined, 253 254 &createdAt, 254 255 &sourceRev, 255 256 ) ··· 267 268 s.SourceRev = sourceRev.String 268 269 } 269 270 271 271 + if combined.Valid { 272 272 + s.Combined = combined.String 273 273 + } 274 274 + 270 275 if p, ok := pulls[s.PullId]; ok { 271 276 p.Submissions = make([]*models.PullSubmission, s.RoundNumber+1) 272 277 p.Submissions[s.RoundNumber] = &s ··· 412 417 413 418 submissionsQuery := ` 414 419 select 415 415 - id, pull_id, repo_at, round_number, patch, created, source_rev 420 420 + id, pull_id, repo_at, round_number, patch, combined, created, source_rev 416 421 from 417 422 pull_submissions 418 423 where ··· 429 434 for submissionsRows.Next() { 430 435 var submission models.PullSubmission 431 436 var submissionCreatedStr string 432 432 - var submissionSourceRev sql.NullString 437 437 + var submissionSourceRev, submissionCombined sql.NullString 433 438 err := submissionsRows.Scan( 434 439 &submission.ID, 435 440 &submission.PullId, 436 441 &submission.RepoAt, 437 442 &submission.RoundNumber, 438 443 &submission.Patch, 444 444 + &submissionCombined, 439 445 &submissionCreatedStr, 440 446 &submissionSourceRev, 441 447 ) ··· 453 459 submission.SourceRev = submissionSourceRev.String 454 460 } 455 461 462 462 + if submissionCombined.Valid { 463 463 + submission.Combined = submissionCombined.String 464 464 + } 465 465 + 456 466 submissionsMap[submission.ID] = &submission 457 467 } 458 468 if err = submissionsRows.Close(); err != nil { ··· 674 684 return err 675 685 } 676 686 677 677 - func ResubmitPull(e Execer, pull *models.Pull, newPatch, sourceRev string) error { 687 687 + func ResubmitPull(e Execer, pull *models.Pull, newPatch, combined, sourceRev string) error { 678 688 newRoundNumber := len(pull.Submissions) 679 689 _, err := e.Exec(` 680 680 - insert into pull_submissions (pull_id, repo_at, round_number, patch, source_rev) 690 690 + insert into pull_submissions (pull_id, repo_at, round_number, patch, combined, source_rev) 681 691 values (?, ?, ?, ?, ?) 682 692 `, pull.PullId, pull.RepoAt, newRoundNumber, newPatch, sourceRev) 683 693

+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 ··· 221 222 return patches 222 223 } 223 224 225 225 + func (s PullSubmission) CombinedPatch() string { 226 226 + if s.Combined == "" { 227 227 + return s.Patch 228 228 + } 229 229 + 230 230 + return s.Combined 231 231 + } 232 232 + 224 233 type Stack []*Pull 225 234 226 235 // position of this pull in the stack

+23 -40

appview/pulls/pulls.go

··· 135 135 stack, _ := r.Context().Value("stack").(models.Stack) 136 136 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 137 137 138 138 - totalIdents := 1 139 139 - for _, submission := range pull.Submissions { 140 140 - totalIdents += len(submission.Comments) 141 141 - } 142 142 - 143 143 - identsToResolve := make([]string, totalIdents) 144 144 - 145 145 - // populate idents 146 146 - identsToResolve[0] = pull.OwnerDid 147 147 - idx := 1 148 148 - for _, submission := range pull.Submissions { 149 149 - for _, comment := range submission.Comments { 150 150 - identsToResolve[idx] = comment.OwnerDid 151 151 - idx += 1 152 152 - } 153 153 - } 154 154 - 155 138 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 156 139 resubmitResult := pages.Unknown 157 140 if user != nil && user.Did == pull.OwnerDid { ··· 374 357 return 375 358 } 376 359 377 377 - patch := pull.Submissions[roundIdInt].Patch 360 360 + patch := pull.Submissions[roundIdInt].CombinedPatch() 378 361 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 379 362 380 363 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 425 408 return 426 409 } 427 410 428 428 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 411 411 + currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch()) 429 412 if err != nil { 430 413 log.Println("failed to interdiff; current patch malformed") 431 414 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 432 415 return 433 416 } 434 417 435 435 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 418 418 + previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch()) 436 419 if err != nil { 437 420 log.Println("failed to interdiff; previous patch malformed") 438 421 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 617 600 618 601 createdAt := time.Now().Format(time.RFC3339) 619 602 620 620 - pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId) 621 621 - if err != nil { 622 622 - log.Println("failed to get pull at", err) 623 623 - s.pages.Notice(w, "pull-comment", "Failed to create comment.") 624 624 - return 625 625 - } 626 626 - 627 603 client, err := s.oauth.AuthorizedClient(r) 628 604 if err != nil { 629 605 log.Println("failed to get authorized client", err) ··· 636 612 Rkey: tid.TID(), 637 613 Record: &lexutil.LexiconTypeDecoder{ 638 614 Val: &tangled.RepoPullComment{ 639 639 - Pull: string(pullAt), 615 615 + Pull: pull.PullAt().String(), 640 616 Body: body, 641 617 CreatedAt: createdAt, 642 618 }, ··· 884 860 } 885 861 886 862 sourceRev := comparison.Rev2 887 887 - patch := comparison.Patch 863 863 + patch := comparison.FormatPatchRaw 864 864 + combined := comparison.CombinedPatchRaw 888 865 889 866 if !patchutil.IsPatchValid(patch) { 890 867 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 899 876 Sha: comparison.Rev2, 900 877 } 901 878 902 902 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 879 879 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 903 880 } 904 881 905 882 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 908 885 return 909 886 } 910 887 911 911 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 888 888 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked) 912 889 } 913 890 914 891 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) { ··· 991 968 } 992 969 993 970 sourceRev := comparison.Rev2 994 994 - patch := comparison.Patch 971 971 + patch := comparison.FormatPatchRaw 972 972 + combined := comparison.CombinedPatchRaw 995 973 996 974 if !patchutil.IsPatchValid(patch) { 997 975 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1011 989 Sha: sourceRev, 1012 990 } 1013 991 1014 1014 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 992 992 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 1015 993 } 1016 994 1017 995 func (s *Pulls) createPullRequest( ··· 1021 999 user *oauth.User, 1022 1000 title, body, targetBranch string, 1023 1001 patch string, 1002 1002 + combined string, 1024 1003 sourceRev string, 1025 1004 pullSource *models.PullSource, 1026 1005 recordPullSource *tangled.RepoPull_Source, ··· 1076 1055 rkey := tid.TID() 1077 1056 initialSubmission := models.PullSubmission{ 1078 1057 Patch: patch, 1058 1058 + Combined: combined, 1079 1059 SourceRev: sourceRev, 1080 1060 } 1081 1061 pull := &models.Pull{ ··· 1504 1484 1505 1485 patch := r.FormValue("patch") 1506 1486 1507 1507 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1487 1487 + s.resubmitPullHelper(w, r, f, user, pull, patch, "", "") 1508 1488 } 1509 1489 1510 1490 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1565 1545 } 1566 1546 1567 1547 sourceRev := comparison.Rev2 1568 1568 - patch := comparison.Patch 1548 1548 + patch := comparison.FormatPatchRaw 1549 1549 + combined := comparison.CombinedPatchRaw 1569 1550 1570 1570 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1551 1551 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1571 1552 } 1572 1553 1573 1554 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1660 1641 comparison := forkComparison 1661 1642 1662 1643 sourceRev := comparison.Rev2 1663 1663 - patch := comparison.Patch 1644 1644 + patch := comparison.FormatPatchRaw 1645 1645 + combined := comparison.CombinedPatchRaw 1664 1646 1665 1665 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1647 1647 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1666 1648 } 1667 1649 1668 1650 // validate a resubmission against a pull request ··· 1689 1671 user *oauth.User, 1690 1672 pull *models.Pull, 1691 1673 patch string, 1674 1674 + combined string, 1692 1675 sourceRev string, 1693 1676 ) { 1694 1677 if pull.IsStacked() { ··· 1718 1701 } 1719 1702 defer tx.Rollback() 1720 1703 1721 1721 - err = db.ResubmitPull(tx, pull, patch, sourceRev) 1704 1704 + err = db.ResubmitPull(tx, pull, patch, combined, sourceRev) 1722 1705 if err != nil { 1723 1706 log.Println("failed to create pull request", err) 1724 1707 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 1933 1916 submission := np.Submissions[np.LastRoundNumber()] 1934 1917 1935 1918 // resubmit the old pull 1936 1936 - err := db.ResubmitPull(tx, op, submission.Patch, submission.SourceRev) 1919 1919 + err := db.ResubmitPull(tx, op, submission.Patch, submission.Combined, submission.SourceRev) 1937 1920 1938 1921 if err != nil { 1939 1922 log.Println("failed to update pull", err, op.PullId)

+6 -1

appview/repo/repo.go

··· 2503 2503 return 2504 2504 } 2505 2505 2506 2506 - diff := patchutil.AsNiceDiff(formatPatch.Patch, base) 2506 2506 + var diff types.NiceDiff 2507 2507 + if formatPatch.CombinedPatchRaw != "" { 2508 2508 + diff = patchutil.AsNiceDiff(formatPatch.CombinedPatchRaw, base) 2509 2509 + } else { 2510 2510 + diff = patchutil.AsNiceDiff(formatPatch.FormatPatchRaw, base) 2511 2511 + } 2507 2512 2508 2513 repoinfo := f.RepoInfo(user) 2509 2514

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