appview,knotserver: produce combined patch in comparisons #609

+9

appview/db/db.go

··· 954 return err 955 }) 956 0 0 0 0 0 0 0 0 0 957 return &DB{db}, nil 958 } 959

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

+18 -8

appview/db/pulls.go

··· 87 pull.ID = int(id) 88 89 _, err = tx.Exec(` 90 - insert into pull_submissions (pull_id, repo_at, round_number, patch, source_rev) 91 values (?, ?, ?, ?, ?) 92 - `, pull.PullId, pull.RepoAt, 0, pull.Submissions[0].Patch, pull.Submissions[0].SourceRev) 93 return err 94 } 95 ··· 218 inClause := strings.TrimSuffix(strings.Repeat("?, ", len(pulls)), ", ") 219 submissionsQuery := fmt.Sprintf(` 220 select 221 - id, pull_id, round_number, patch, created, source_rev 222 from 223 pull_submissions 224 where ··· 243 244 for submissionsRows.Next() { 245 var s models.PullSubmission 246 - var sourceRev sql.NullString 247 var createdAt string 248 err := submissionsRows.Scan( 249 &s.ID, 250 &s.PullId, 251 &s.RoundNumber, 252 &s.Patch, 0 253 &createdAt, 254 &sourceRev, 255 ) ··· 267 s.SourceRev = sourceRev.String 268 } 269 0 0 0 0 270 if p, ok := pulls[s.PullId]; ok { 271 p.Submissions = make([]*models.PullSubmission, s.RoundNumber+1) 272 p.Submissions[s.RoundNumber] = &s ··· 412 413 submissionsQuery := ` 414 select 415 - id, pull_id, repo_at, round_number, patch, created, source_rev 416 from 417 pull_submissions 418 where ··· 429 for submissionsRows.Next() { 430 var submission models.PullSubmission 431 var submissionCreatedStr string 432 - var submissionSourceRev sql.NullString 433 err := submissionsRows.Scan( 434 &submission.ID, 435 &submission.PullId, 436 &submission.RepoAt, 437 &submission.RoundNumber, 438 &submission.Patch, 0 439 &submissionCreatedStr, 440 &submissionSourceRev, 441 ) ··· 453 submission.SourceRev = submissionSourceRev.String 454 } 455 0 0 0 0 456 submissionsMap[submission.ID] = &submission 457 } 458 if err = submissionsRows.Close(); err != nil { ··· 674 return err 675 } 676 677 - func ResubmitPull(e Execer, pull *models.Pull, newPatch, sourceRev string) error { 678 newRoundNumber := len(pull.Submissions) 679 _, err := e.Exec(` 680 - insert into pull_submissions (pull_id, repo_at, round_number, patch, source_rev) 681 values (?, ?, ?, ?, ?) 682 `, pull.PullId, pull.RepoAt, newRoundNumber, newPatch, sourceRev) 683

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

+9

appview/models/pull.go

··· 134 // content 135 RoundNumber int 136 Patch string 0 137 Comments []PullComment 138 SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 139 ··· 221 return patches 222 } 223 0 0 0 0 0 0 0 0 224 type Stack []*Pull 225 226 // position of this pull in the stack

··· 134 // content 135 RoundNumber int 136 Patch string 137 + Combined string 138 Comments []PullComment 139 SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 140 ··· 222 return patches 223 } 224 225 + func (s PullSubmission) CombinedPatch() string { 226 + if s.Combined == "" { 227 + return s.Patch 228 + } 229 + 230 + return s.Combined 231 + } 232 + 233 type Stack []*Pull 234 235 // position of this pull in the stack

+23 -40

appview/pulls/pulls.go

··· 135 stack, _ := r.Context().Value("stack").(models.Stack) 136 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 137 138 - totalIdents := 1 139 - for _, submission := range pull.Submissions { 140 - totalIdents += len(submission.Comments) 141 - } 142 - 143 - identsToResolve := make([]string, totalIdents) 144 - 145 - // populate idents 146 - identsToResolve[0] = pull.OwnerDid 147 - idx := 1 148 - for _, submission := range pull.Submissions { 149 - for _, comment := range submission.Comments { 150 - identsToResolve[idx] = comment.OwnerDid 151 - idx += 1 152 - } 153 - } 154 - 155 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 156 resubmitResult := pages.Unknown 157 if user != nil && user.Did == pull.OwnerDid { ··· 374 return 375 } 376 377 - patch := pull.Submissions[roundIdInt].Patch 378 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 379 380 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 425 return 426 } 427 428 - currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch) 429 if err != nil { 430 log.Println("failed to interdiff; current patch malformed") 431 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 432 return 433 } 434 435 - previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch) 436 if err != nil { 437 log.Println("failed to interdiff; previous patch malformed") 438 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 617 618 createdAt := time.Now().Format(time.RFC3339) 619 620 - pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId) 621 - if err != nil { 622 - log.Println("failed to get pull at", err) 623 - s.pages.Notice(w, "pull-comment", "Failed to create comment.") 624 - return 625 - } 626 - 627 client, err := s.oauth.AuthorizedClient(r) 628 if err != nil { 629 log.Println("failed to get authorized client", err) ··· 636 Rkey: tid.TID(), 637 Record: &lexutil.LexiconTypeDecoder{ 638 Val: &tangled.RepoPullComment{ 639 - Pull: string(pullAt), 640 Body: body, 641 CreatedAt: createdAt, 642 }, ··· 884 } 885 886 sourceRev := comparison.Rev2 887 - patch := comparison.Patch 0 888 889 if !patchutil.IsPatchValid(patch) { 890 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 899 Sha: comparison.Rev2, 900 } 901 902 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 903 } 904 905 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 908 return 909 } 910 911 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked) 912 } 913 914 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 } 992 993 sourceRev := comparison.Rev2 994 - patch := comparison.Patch 0 995 996 if !patchutil.IsPatchValid(patch) { 997 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 1011 Sha: sourceRev, 1012 } 1013 1014 - s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked) 1015 } 1016 1017 func (s *Pulls) createPullRequest( ··· 1021 user *oauth.User, 1022 title, body, targetBranch string, 1023 patch string, 0 1024 sourceRev string, 1025 pullSource *models.PullSource, 1026 recordPullSource *tangled.RepoPull_Source, ··· 1076 rkey := tid.TID() 1077 initialSubmission := models.PullSubmission{ 1078 Patch: patch, 0 1079 SourceRev: sourceRev, 1080 } 1081 pull := &models.Pull{ ··· 1504 1505 patch := r.FormValue("patch") 1506 1507 - s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1508 } 1509 1510 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1565 } 1566 1567 sourceRev := comparison.Rev2 1568 - patch := comparison.Patch 0 1569 1570 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1571 } 1572 1573 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1660 comparison := forkComparison 1661 1662 sourceRev := comparison.Rev2 1663 - patch := comparison.Patch 0 1664 1665 - s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1666 } 1667 1668 // validate a resubmission against a pull request ··· 1689 user *oauth.User, 1690 pull *models.Pull, 1691 patch string, 0 1692 sourceRev string, 1693 ) { 1694 if pull.IsStacked() { ··· 1718 } 1719 defer tx.Rollback() 1720 1721 - err = db.ResubmitPull(tx, pull, patch, sourceRev) 1722 if err != nil { 1723 log.Println("failed to create pull request", err) 1724 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 1933 submission := np.Submissions[np.LastRoundNumber()] 1934 1935 // resubmit the old pull 1936 - err := db.ResubmitPull(tx, op, submission.Patch, submission.SourceRev) 1937 1938 if err != nil { 1939 log.Println("failed to update pull", err, op.PullId)

··· 135 stack, _ := r.Context().Value("stack").(models.Stack) 136 abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull) 137 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 138 mergeCheckResponse := s.mergeCheck(r, f, pull, stack) 139 resubmitResult := pages.Unknown 140 if user != nil && user.Did == pull.OwnerDid { ··· 357 return 358 } 359 360 + patch := pull.Submissions[roundIdInt].CombinedPatch() 361 diff := patchutil.AsNiceDiff(patch, pull.TargetBranch) 362 363 s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{ ··· 408 return 409 } 410 411 + currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch()) 412 if err != nil { 413 log.Println("failed to interdiff; current patch malformed") 414 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.") 415 return 416 } 417 418 + previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch()) 419 if err != nil { 420 log.Println("failed to interdiff; previous patch malformed") 421 s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.") ··· 600 601 createdAt := time.Now().Format(time.RFC3339) 602 0 0 0 0 0 0 0 603 client, err := s.oauth.AuthorizedClient(r) 604 if err != nil { 605 log.Println("failed to get authorized client", err) ··· 612 Rkey: tid.TID(), 613 Record: &lexutil.LexiconTypeDecoder{ 614 Val: &tangled.RepoPullComment{ 615 + Pull: pull.PullAt().String(), 616 Body: body, 617 CreatedAt: createdAt, 618 }, ··· 860 } 861 862 sourceRev := comparison.Rev2 863 + patch := comparison.FormatPatchRaw 864 + combined := comparison.CombinedPatchRaw 865 866 if !patchutil.IsPatchValid(patch) { 867 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 876 Sha: comparison.Rev2, 877 } 878 879 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 880 } 881 882 func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) { ··· 885 return 886 } 887 888 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked) 889 } 890 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) { ··· 968 } 969 970 sourceRev := comparison.Rev2 971 + patch := comparison.FormatPatchRaw 972 + combined := comparison.CombinedPatchRaw 973 974 if !patchutil.IsPatchValid(patch) { 975 s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.") ··· 989 Sha: sourceRev, 990 } 991 992 + s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked) 993 } 994 995 func (s *Pulls) createPullRequest( ··· 999 user *oauth.User, 1000 title, body, targetBranch string, 1001 patch string, 1002 + combined string, 1003 sourceRev string, 1004 pullSource *models.PullSource, 1005 recordPullSource *tangled.RepoPull_Source, ··· 1055 rkey := tid.TID() 1056 initialSubmission := models.PullSubmission{ 1057 Patch: patch, 1058 + Combined: combined, 1059 SourceRev: sourceRev, 1060 } 1061 pull := &models.Pull{ ··· 1484 1485 patch := r.FormValue("patch") 1486 1487 + s.resubmitPullHelper(w, r, f, user, pull, patch, "", "") 1488 } 1489 1490 func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1545 } 1546 1547 sourceRev := comparison.Rev2 1548 + patch := comparison.FormatPatchRaw 1549 + combined := comparison.CombinedPatchRaw 1550 1551 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1552 } 1553 1554 func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1641 comparison := forkComparison 1642 1643 sourceRev := comparison.Rev2 1644 + patch := comparison.FormatPatchRaw 1645 + combined := comparison.CombinedPatchRaw 1646 1647 + s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev) 1648 } 1649 1650 // validate a resubmission against a pull request ··· 1671 user *oauth.User, 1672 pull *models.Pull, 1673 patch string, 1674 + combined string, 1675 sourceRev string, 1676 ) { 1677 if pull.IsStacked() { ··· 1701 } 1702 defer tx.Rollback() 1703 1704 + err = db.ResubmitPull(tx, pull, patch, combined, sourceRev) 1705 if err != nil { 1706 log.Println("failed to create pull request", err) 1707 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") ··· 1916 submission := np.Submissions[np.LastRoundNumber()] 1917 1918 // resubmit the old pull 1919 + err := db.ResubmitPull(tx, op, submission.Patch, submission.Combined, submission.SourceRev) 1920 1921 if err != nil { 1922 log.Println("failed to update pull", err, op.PullId)

+6 -1

appview/repo/repo.go

··· 2503 return 2504 } 2505 2506 - diff := patchutil.AsNiceDiff(formatPatch.Patch, base) 0 0 0 0 0 2507 2508 repoinfo := f.RepoInfo(user) 2509

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

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