forked from tangled.org/core
this repo has no description

appview: pulls: refactor ResubmitPull

Changed files
+604 -262
appview
db
pages
templates
repo
pulls
state
patchutil
+69
appview/db/db.go
··· 386 386 return err 387 387 }) 388 388 389 + // disable foreign-keys for the next migration 390 + // NOTE: this cannot be done in a transaction, so it is run outside [0] 391 + // 392 + // [0]: https://sqlite.org/pragma.html#pragma_foreign_keys 393 + db.Exec("pragma foreign_keys = off;") 394 + runMigration(db, "recreate-pulls-column-for-stacking-support", func(tx *sql.Tx) error { 395 + _, err := tx.Exec(` 396 + -- disable fk to not delete submissions table 397 + pragma foreign_keys = off; 398 + 399 + create table pulls_new ( 400 + -- identifiers 401 + id integer primary key autoincrement, 402 + pull_id integer not null, 403 + 404 + -- at identifiers 405 + repo_at text not null, 406 + owner_did text not null, 407 + rkey text not null, 408 + 409 + -- content 410 + title text not null, 411 + body text not null, 412 + target_branch text not null, 413 + state integer not null default 0 check (state in (0, 1, 2, 3)), -- closed, open, merged, deleted 414 + 415 + -- source info 416 + source_branch text, 417 + source_repo_at text, 418 + 419 + -- stacking 420 + stack_id text, 421 + change_id text, 422 + parent_change_id text, 423 + 424 + -- meta 425 + created text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), 426 + 427 + -- constraints 428 + unique(repo_at, pull_id), 429 + foreign key (repo_at) references repos(at_uri) on delete cascade 430 + ); 431 + 432 + insert into pulls_new ( 433 + id, pull_id, 434 + repo_at, owner_did, rkey, 435 + title, body, target_branch, state, 436 + source_branch, source_repo_at, 437 + created 438 + ) 439 + select 440 + id, pull_id, 441 + repo_at, owner_did, rkey, 442 + title, body, target_branch, state, 443 + source_branch, source_repo_at, 444 + created 445 + FROM pulls; 446 + 447 + drop table pulls; 448 + alter table pulls_new rename to pulls; 449 + 450 + -- reenable fk 451 + pragma foreign_keys = on; 452 + `) 453 + return err 454 + }) 455 + db.Exec("pragma foreign_keys = on;") 456 + 457 + >>>>>>> Conflict 1 of 1 ends 389 458 return &DB{db}, nil 390 459 } 391 460
+90 -9
appview/db/pulls.go
··· 22 22 PullClosed PullState = iota 23 23 PullOpen 24 24 PullMerged 25 + PullDeleted 25 26 ) 26 27 27 28 func (p PullState) String() string { ··· 32 33 return "merged" 33 34 case PullClosed: 34 35 return "closed" 36 + case PullDeleted: 37 + return "deleted" 35 38 default: 36 39 return "closed" 37 40 } ··· 45 48 } 46 49 func (p PullState) IsClosed() bool { 47 50 return p == PullClosed 51 + } 52 + func (p PullState) IsDelete() bool { 53 + return p == PullDeleted 48 54 } 49 55 50 56 type Pull struct { ··· 77 83 Repo *Repo 78 84 } 79 85 86 + func (p Pull) AsRecord() tangled.RepoPull { 87 + var source *tangled.RepoPull_Source 88 + if p.PullSource != nil { 89 + s := p.PullSource.AsRecord() 90 + source = &s 91 + } 92 + 93 + record := tangled.RepoPull{ 94 + Title: p.Title, 95 + Body: &p.Body, 96 + CreatedAt: p.Created.Format(time.RFC3339), 97 + PullId: int64(p.PullId), 98 + TargetRepo: p.RepoAt.String(), 99 + TargetBranch: p.TargetBranch, 100 + Patch: p.LatestPatch(), 101 + Source: source, 102 + } 103 + return record 104 + } 105 + 80 106 type PullSource struct { 81 107 Branch string 82 108 RepoAt *syntax.ATURI ··· 85 111 Repo *Repo 86 112 } 87 113 114 + func (p PullSource) AsRecord() tangled.RepoPull_Source { 115 + var repoAt *string 116 + if p.RepoAt != nil { 117 + s := p.RepoAt.String() 118 + repoAt = &s 119 + } 120 + record := tangled.RepoPull_Source{ 121 + Branch: p.Branch, 122 + Repo: repoAt, 123 + } 124 + return record 125 + } 126 + 88 127 type PullSubmission struct { 89 128 // ids 90 129 ID int ··· 97 136 RoundNumber int 98 137 Patch string 99 138 Comments []PullComment 100 - SourceRev string // include the rev that was used to create this submission: only for branch PRs 139 + SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs 101 140 102 141 // meta 103 142 Created time.Time ··· 434 473 inClause := strings.TrimSuffix(strings.Repeat("?, ", len(pulls)), ", ") 435 474 submissionsQuery := fmt.Sprintf(` 436 475 select 437 - id, pull_id, round_number, patch 476 + id, pull_id, round_number, patch, source_rev 438 477 from 439 478 pull_submissions 440 479 where ··· 459 498 460 499 for submissionsRows.Next() { 461 500 var s PullSubmission 501 + var sourceRev sql.NullString 462 502 err := submissionsRows.Scan( 463 503 &s.ID, 464 504 &s.PullId, 465 505 &s.RoundNumber, 466 506 &s.Patch, 507 + &sourceRev, 467 508 ) 468 509 if err != nil { 469 510 return nil, err 511 + } 512 + 513 + if sourceRev.Valid { 514 + s.SourceRev = sourceRev.String 470 515 } 471 516 472 517 if p, ok := pulls[s.PullId]; ok { ··· 839 884 } 840 885 841 886 func SetPullState(e Execer, repoAt syntax.ATURI, pullId int, pullState PullState) error { 842 - _, err := e.Exec(`update pulls set state = ? where repo_at = ? and pull_id = ?`, pullState, repoAt, pullId) 887 + _, err := e.Exec( 888 + `update pulls set state = ? where repo_at = ? and pull_id = ? and state <> ?`, 889 + pullState, 890 + repoAt, 891 + pullId, 892 + PullDeleted, // only update state of non-deleted pulls 893 + ) 843 894 return err 844 895 } 845 896 ··· 858 909 return err 859 910 } 860 911 912 + func DeletePull(e Execer, repoAt syntax.ATURI, pullId int) error { 913 + err := SetPullState(e, repoAt, pullId, PullDeleted) 914 + return err 915 + } 916 + 861 917 func ResubmitPull(e Execer, pull *Pull, newPatch, sourceRev string) error { 862 918 newRoundNumber := len(pull.Submissions) 863 919 _, err := e.Exec(` ··· 868 924 return err 869 925 } 870 926 927 + func SetPullParentChangeId(e Execer, parentChangeId string, filters ...filter) error { 928 + var conditions []string 929 + var args []any 930 + 931 + args = append(args, parentChangeId) 932 + 933 + for _, filter := range filters { 934 + conditions = append(conditions, filter.Condition()) 935 + args = append(args, filter.arg) 936 + } 937 + 938 + whereClause := "" 939 + if conditions != nil { 940 + whereClause = " where " + strings.Join(conditions, " and ") 941 + } 942 + 943 + query := fmt.Sprintf("update pulls set parent_change_id = ? %s", whereClause) 944 + _, err := e.Exec(query, args...) 945 + 946 + return err 947 + } 948 + 871 949 type PullCount struct { 872 - Open int 873 - Merged int 874 - Closed int 950 + Open int 951 + Merged int 952 + Closed int 953 + Deleted int 875 954 } 876 955 877 956 func GetPullCount(e Execer, repoAt syntax.ATURI) (PullCount, error) { ··· 879 958 select 880 959 count(case when state = ? then 1 end) as open_count, 881 960 count(case when state = ? then 1 end) as merged_count, 882 - count(case when state = ? then 1 end) as closed_count 961 + count(case when state = ? then 1 end) as closed_count, 962 + count(case when state = ? then 1 end) as deleted_count 883 963 from pulls 884 964 where repo_at = ?`, 885 965 PullOpen, 886 966 PullMerged, 887 967 PullClosed, 968 + PullDeleted, 888 969 repoAt, 889 970 ) 890 971 891 972 var count PullCount 892 - if err := row.Scan(&count.Open, &count.Merged, &count.Closed); err != nil { 893 - return PullCount{0, 0, 0}, err 973 + if err := row.Scan(&count.Open, &count.Merged, &count.Closed, &count.Deleted); err != nil { 974 + return PullCount{0, 0, 0, 0}, err 894 975 } 895 976 896 977 return count, nil
+16 -9
appview/pages/templates/repo/pulls/pull.html
··· 207 207 {{ i "triangle-alert" "w-4 h-4" }} 208 208 <span class="font-medium">merge conflicts detected</span> 209 209 </div> 210 - <ul class="space-y-1"> 211 - {{ range .MergeCheck.Conflicts }} 212 - {{ if .Filename }} 213 - <li class="flex items-center"> 214 - {{ i "file-warning" "w-4 h-4 mr-1.5 text-red-500 dark:text-red-300" }} 215 - <span class="font-mono">{{ slice .Filename 0 (sub (len .Filename) 2) }}</span> 216 - </li> 210 + {{ if gt (len .MergeCheck.Conflicts) 0 }} 211 + <ul class="space-y-1"> 212 + {{ range .MergeCheck.Conflicts }} 213 + {{ if .Filename }} 214 + <li class="flex items-center"> 215 + {{ i "file-warning" "w-4 h-4 mr-1.5 text-red-500 dark:text-red-300" }} 216 + <span class="font-mono">{{ slice .Filename 0 (sub (len .Filename) 2) }}</span> 217 + </li> 218 + {{ else if .Reason }} 219 + <li class="flex items-center"> 220 + {{ i "file-warning" "w-4 h-4 mr-1.5 text-red-500 dark:text-red-300" }} 221 + <span>{{.Reason}}</span> 222 + </li> 223 + {{ end }} 217 224 {{ end }} 218 - {{ end }} 219 - </ul> 225 + </ul> 226 + {{ end }} 220 227 </div> 221 228 </div> 222 229 {{ else if .MergeCheck }}
+369 -244
appview/state/pull.go
··· 10 10 "net/http" 11 11 "sort" 12 12 "strconv" 13 + "strings" 13 14 "time" 14 15 15 16 "tangled.sh/tangled.sh/core/api/tangled" ··· 21 22 "tangled.sh/tangled.sh/core/patchutil" 22 23 "tangled.sh/tangled.sh/core/types" 23 24 25 + "github.com/bluekeyes/go-gitdiff/gitdiff" 24 26 comatproto "github.com/bluesky-social/indigo/api/atproto" 25 27 "github.com/bluesky-social/indigo/atproto/syntax" 26 28 lexutil "github.com/bluesky-social/indigo/lex/util" ··· 47 49 } 48 50 49 51 // can be nil if this pull is not stacked 50 - stack := r.Context().Value("stack").(db.Stack) 52 + stack, _ := r.Context().Value("stack").(db.Stack) 51 53 52 54 roundNumberStr := chi.URLParam(r, "round") 53 55 roundNumber, err := strconv.Atoi(roundNumberStr) ··· 63 65 mergeCheckResponse := s.mergeCheck(f, pull, stack) 64 66 resubmitResult := pages.Unknown 65 67 if user.Did == pull.OwnerDid { 66 - resubmitResult = s.resubmitCheck(f, pull) 68 + resubmitResult = s.resubmitCheck(f, pull, stack) 67 69 } 68 70 69 71 s.pages.PullActionsFragment(w, pages.PullActionsParams{ ··· 94 96 } 95 97 96 98 // can be nil if this pull is not stacked 97 - stack := r.Context().Value("stack").(db.Stack) 99 + stack, _ := r.Context().Value("stack").(db.Stack) 98 100 99 101 totalIdents := 1 100 102 for _, submission := range pull.Submissions { ··· 126 128 mergeCheckResponse := s.mergeCheck(f, pull, stack) 127 129 resubmitResult := pages.Unknown 128 130 if user != nil && user.Did == pull.OwnerDid { 129 - resubmitResult = s.resubmitCheck(f, pull) 131 + resubmitResult = s.resubmitCheck(f, pull, stack) 130 132 } 131 133 132 134 s.pages.RepoSinglePull(w, pages.RepoSinglePullParams{ ··· 169 171 var mergeable db.Stack 170 172 for _, p := range subStack { 171 173 // stop at the first merged PR 172 - if p.State == db.PullMerged { 174 + if p.State == db.PullMerged || p.State == db.PullClosed { 173 175 break 174 176 } 175 177 176 - // skip over closed PRs 177 - // 178 - // we will close PRs that are "removed" from a stack 179 - if p.State != db.PullClosed { 178 + // skip over deleted PRs 179 + if p.State != db.PullDeleted { 180 180 mergeable = append(mergeable, p) 181 181 } 182 182 } ··· 223 223 return mergeCheckResponse 224 224 } 225 225 226 - func (s *State) resubmitCheck(f *FullyResolvedRepo, pull *db.Pull) pages.ResubmitResult { 226 + func (s *State) resubmitCheck(f *FullyResolvedRepo, pull *db.Pull, stack db.Stack) pages.ResubmitResult { 227 227 if pull.State == db.PullMerged || pull.PullSource == nil { 228 228 return pages.Unknown 229 229 } ··· 273 273 return pages.Unknown 274 274 } 275 275 276 - latestSubmission := pull.Submissions[pull.LastRoundNumber()] 277 - if latestSubmission.SourceRev != result.Branch.Hash { 278 - fmt.Println(latestSubmission.SourceRev, result.Branch.Hash) 276 + latestSourceRev := pull.Submissions[pull.LastRoundNumber()].SourceRev 277 + 278 + if pull.IsStacked() && stack != nil { 279 + top := stack[0] 280 + latestSourceRev = top.Submissions[top.LastRoundNumber()].SourceRev 281 + } 282 + 283 + if latestSourceRev != result.Branch.Hash { 284 + log.Println(latestSourceRev, result.Branch.Hash) 279 285 return pages.ShouldResubmit 280 286 } 281 287 ··· 650 656 RepoInfo: f.RepoInfo(s, user), 651 657 Branches: result.Branches, 652 658 }) 659 + 653 660 case http.MethodPost: 654 661 title := r.FormValue("title") 655 662 body := r.FormValue("body") ··· 884 891 r, 885 892 f, 886 893 user, 887 - title, body, targetBranch, 894 + targetBranch, 888 895 patch, 889 896 sourceRev, 890 897 pullSource, 891 - recordPullSource, 892 898 ) 893 899 return 894 900 } ··· 988 994 r *http.Request, 989 995 f *FullyResolvedRepo, 990 996 user *oauth.User, 991 - title, body, targetBranch string, 997 + targetBranch string, 992 998 patch string, 993 999 sourceRev string, 994 1000 pullSource *db.PullSource, 995 - recordPullSource *tangled.RepoPull_Source, 996 1001 ) { 997 1002 // run some necessary checks for stacked-prs first 998 1003 ··· 1005 1010 1006 1011 formatPatches, err := patchutil.ExtractPatches(patch) 1007 1012 if err != nil { 1013 + log.Println("failed to extract patches", err) 1008 1014 s.pages.Notice(w, "pull", fmt.Sprintf("Failed to extract patches: %v", err)) 1009 1015 return 1010 1016 } 1011 1017 1012 1018 // must have atleast 1 patch to begin with 1013 1019 if len(formatPatches) == 0 { 1020 + log.Println("empty patches") 1014 1021 s.pages.Notice(w, "pull", "No patches found in the generated format-patch.") 1015 1022 return 1016 1023 } 1017 1024 1018 - tx, err := s.db.BeginTx(r.Context(), nil) 1025 + // build a stack out of this patch 1026 + stackId := uuid.New() 1027 + stack, err := newStack(f, user, targetBranch, patch, pullSource, stackId.String()) 1028 + if err != nil { 1029 + log.Println("failed to create stack", err) 1030 + s.pages.Notice(w, "pull", fmt.Sprintf("Failed to create stack: %v", err)) 1031 + return 1032 + } 1033 + 1034 + client, err := s.oauth.AuthorizedClient(r) 1019 1035 if err != nil { 1020 - log.Println("failed to start tx") 1036 + log.Println("failed to get authorized client", err) 1021 1037 s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") 1022 1038 return 1023 1039 } 1024 - defer tx.Rollback() 1025 1040 1026 - // create a series of pull requests, and write records from them at once 1041 + // apply all record creations at once 1027 1042 var writes []*comatproto.RepoApplyWrites_Input_Writes_Elem 1028 - 1029 - // the stack is identified by a UUID 1030 - stackId := uuid.New() 1031 - parentChangeId := "" 1032 - for _, fp := range formatPatches { 1033 - // all patches must have a jj change-id 1034 - changeId, err := fp.ChangeId() 1035 - if err != nil { 1036 - s.pages.Notice(w, "pull", "Stacking is only supported if all patches contain a change-id commit header.") 1037 - return 1038 - } 1039 - 1040 - title = fp.Title 1041 - body = fp.Body 1042 - rkey := appview.TID() 1043 - 1044 - // TODO: can we just use a format-patch string here? 1045 - initialSubmission := db.PullSubmission{ 1046 - Patch: fp.Raw, 1047 - SourceRev: sourceRev, 1048 - } 1049 - err = db.NewPull(tx, &db.Pull{ 1050 - Title: title, 1051 - Body: body, 1052 - TargetBranch: targetBranch, 1053 - OwnerDid: user.Did, 1054 - RepoAt: f.RepoAt, 1055 - Rkey: rkey, 1056 - Submissions: []*db.PullSubmission{ 1057 - &initialSubmission, 1058 - }, 1059 - PullSource: pullSource, 1060 - 1061 - StackId: stackId.String(), 1062 - ChangeId: changeId, 1063 - ParentChangeId: parentChangeId, 1064 - }) 1065 - if err != nil { 1066 - log.Println("failed to create pull request", err) 1067 - s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") 1068 - return 1069 - } 1070 - 1071 - record := tangled.RepoPull{ 1072 - Title: title, 1073 - TargetRepo: string(f.RepoAt), 1074 - TargetBranch: targetBranch, 1075 - Patch: fp.Raw, 1076 - Source: recordPullSource, 1077 - } 1078 - writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ 1043 + for _, p := range stack { 1044 + record := p.AsRecord() 1045 + write := comatproto.RepoApplyWrites_Input_Writes_Elem{ 1079 1046 RepoApplyWrites_Create: &comatproto.RepoApplyWrites_Create{ 1080 1047 Collection: tangled.RepoPullNSID, 1081 - Rkey: &rkey, 1048 + Rkey: &p.Rkey, 1082 1049 Value: &lexutil.LexiconTypeDecoder{ 1083 1050 Val: &record, 1084 1051 }, 1085 1052 }, 1086 - }) 1087 - 1088 - parentChangeId = changeId 1053 + } 1054 + writes = append(writes, &write) 1089 1055 } 1090 - 1091 - client, err := s.oauth.AuthorizedClient(r) 1092 - if err != nil { 1093 - log.Println("failed to get authorized client", err) 1094 - s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") 1095 - return 1096 - } 1097 - 1098 - // apply all record creations at once 1099 1056 _, err = client.RepoApplyWrites(r.Context(), &comatproto.RepoApplyWrites_Input{ 1100 1057 Repo: user.Did, 1101 1058 Writes: writes, ··· 1107 1064 } 1108 1065 1109 1066 // create all pulls at once 1067 + tx, err := s.db.BeginTx(r.Context(), nil) 1068 + if err != nil { 1069 + log.Println("failed to start tx") 1070 + s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") 1071 + return 1072 + } 1073 + defer tx.Rollback() 1074 + 1075 + for _, p := range stack { 1076 + err = db.NewPull(tx, p) 1077 + if err != nil { 1078 + log.Println("failed to create pull request", err) 1079 + s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") 1080 + return 1081 + } 1082 + } 1083 + 1110 1084 if err = tx.Commit(); err != nil { 1111 1085 log.Println("failed to create pull request", err) 1112 1086 s.pages.Notice(w, "pull", "Failed to create pull request. Try again later.") ··· 1371 1345 1372 1346 patch := r.FormValue("patch") 1373 1347 1374 - if err = validateResubmittedPatch(pull, patch); err != nil { 1375 - s.pages.Notice(w, "resubmit-error", err.Error()) 1376 - return 1377 - } 1378 - 1379 - tx, err := s.db.BeginTx(r.Context(), nil) 1380 - if err != nil { 1381 - log.Println("failed to start tx") 1382 - s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") 1383 - return 1384 - } 1385 - defer tx.Rollback() 1386 - 1387 - err = db.ResubmitPull(tx, pull, patch, "") 1388 - if err != nil { 1389 - log.Println("failed to resubmit pull request", err) 1390 - s.pages.Notice(w, "resubmit-error", "Failed to resubmit pull request. Try again later.") 1391 - return 1392 - } 1393 - client, err := s.oauth.AuthorizedClient(r) 1394 - if err != nil { 1395 - log.Println("failed to get authorized client", err) 1396 - s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") 1397 - return 1398 - } 1399 - 1400 - ex, err := client.RepoGetRecord(r.Context(), "", tangled.RepoPullNSID, user.Did, pull.Rkey) 1401 - if err != nil { 1402 - // failed to get record 1403 - s.pages.Notice(w, "resubmit-error", "Failed to update pull, no record found on PDS.") 1404 - return 1405 - } 1406 - 1407 - _, err = client.RepoPutRecord(r.Context(), &comatproto.RepoPutRecord_Input{ 1408 - Collection: tangled.RepoPullNSID, 1409 - Repo: user.Did, 1410 - Rkey: pull.Rkey, 1411 - SwapRecord: ex.Cid, 1412 - Record: &lexutil.LexiconTypeDecoder{ 1413 - Val: &tangled.RepoPull{ 1414 - Title: pull.Title, 1415 - PullId: int64(pull.PullId), 1416 - TargetRepo: string(f.RepoAt), 1417 - TargetBranch: pull.TargetBranch, 1418 - Patch: patch, // new patch 1419 - }, 1420 - }, 1421 - }) 1422 - if err != nil { 1423 - log.Println("failed to update record", err) 1424 - s.pages.Notice(w, "resubmit-error", "Failed to update pull request on the PDS. Try again later.") 1425 - return 1426 - } 1427 - 1428 - if err = tx.Commit(); err != nil { 1429 - log.Println("failed to commit transaction", err) 1430 - s.pages.Notice(w, "resubmit-error", "Failed to resubmit pull.") 1431 - return 1432 - } 1433 - 1434 - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) 1435 - return 1348 + s.resubmitPullHelper(w, r, f, user, pull, patch, "") 1436 1349 } 1437 1350 1438 1351 func (s *State) resubmitBranch(w http.ResponseWriter, r *http.Request) { ··· 1480 1393 sourceRev := comparison.Rev2 1481 1394 patch := comparison.Patch 1482 1395 1483 - if err = validateResubmittedPatch(pull, patch); err != nil { 1484 - s.pages.Notice(w, "resubmit-error", err.Error()) 1485 - return 1486 - } 1487 - 1488 - if sourceRev == pull.Submissions[pull.LastRoundNumber()].SourceRev { 1489 - s.pages.Notice(w, "resubmit-error", "This branch has not changed since the last submission.") 1490 - return 1491 - } 1492 - 1493 - tx, err := s.db.BeginTx(r.Context(), nil) 1494 - if err != nil { 1495 - log.Println("failed to start tx") 1496 - s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") 1497 - return 1498 - } 1499 - defer tx.Rollback() 1500 - 1501 - err = db.ResubmitPull(tx, pull, patch, sourceRev) 1502 - if err != nil { 1503 - log.Println("failed to create pull request", err) 1504 - s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") 1505 - return 1506 - } 1507 - client, err := s.oauth.AuthorizedClient(r) 1508 - if err != nil { 1509 - log.Println("failed to authorize client") 1510 - s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") 1511 - return 1512 - } 1513 - 1514 - ex, err := client.RepoGetRecord(r.Context(), "", tangled.RepoPullNSID, user.Did, pull.Rkey) 1515 - if err != nil { 1516 - // failed to get record 1517 - s.pages.Notice(w, "resubmit-error", "Failed to update pull, no record found on PDS.") 1518 - return 1519 - } 1520 - 1521 - recordPullSource := &tangled.RepoPull_Source{ 1522 - Branch: pull.PullSource.Branch, 1523 - } 1524 - _, err = client.RepoPutRecord(r.Context(), &comatproto.RepoPutRecord_Input{ 1525 - Collection: tangled.RepoPullNSID, 1526 - Repo: user.Did, 1527 - Rkey: pull.Rkey, 1528 - SwapRecord: ex.Cid, 1529 - Record: &lexutil.LexiconTypeDecoder{ 1530 - Val: &tangled.RepoPull{ 1531 - Title: pull.Title, 1532 - PullId: int64(pull.PullId), 1533 - TargetRepo: string(f.RepoAt), 1534 - TargetBranch: pull.TargetBranch, 1535 - Patch: patch, // new patch 1536 - Source: recordPullSource, 1537 - }, 1538 - }, 1539 - }) 1540 - if err != nil { 1541 - log.Println("failed to update record", err) 1542 - s.pages.Notice(w, "resubmit-error", "Failed to update pull request on the PDS. Try again later.") 1543 - return 1544 - } 1545 - 1546 - if err = tx.Commit(); err != nil { 1547 - log.Println("failed to commit transaction", err) 1548 - s.pages.Notice(w, "resubmit-error", "Failed to resubmit pull.") 1549 - return 1550 - } 1551 - 1552 - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) 1553 - return 1396 + s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1554 1397 } 1555 1398 1556 1399 func (s *State) resubmitFork(w http.ResponseWriter, r *http.Request) { ··· 1623 1466 sourceRev := comparison.Rev2 1624 1467 patch := comparison.Patch 1625 1468 1626 - if err = validateResubmittedPatch(pull, patch); err != nil { 1627 - s.pages.Notice(w, "resubmit-error", err.Error()) 1469 + s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev) 1470 + } 1471 + 1472 + // validate a resubmission against a pull request 1473 + func validateResubmittedPatch(pull *db.Pull, patch string) error { 1474 + if patch == "" { 1475 + return fmt.Errorf("Patch is empty.") 1476 + } 1477 + 1478 + if patch == pull.LatestPatch() { 1479 + return fmt.Errorf("Patch is identical to previous submission.") 1480 + } 1481 + 1482 + if !patchutil.IsPatchValid(patch) { 1483 + return fmt.Errorf("Invalid patch format. Please provide a valid diff.") 1484 + } 1485 + 1486 + return nil 1487 + } 1488 + 1489 + func (s *State) resubmitPullHelper( 1490 + w http.ResponseWriter, 1491 + r *http.Request, 1492 + f *FullyResolvedRepo, 1493 + user *oauth.User, 1494 + pull *db.Pull, 1495 + patch string, 1496 + sourceRev string, 1497 + ) { 1498 + if pull.IsStacked() { 1499 + log.Println("resubmitting stacked PR") 1500 + s.resubmitStackedPullHelper(w, r, f, user, pull, patch, pull.StackId) 1628 1501 return 1629 1502 } 1630 1503 1631 - if sourceRev == pull.Submissions[pull.LastRoundNumber()].SourceRev { 1632 - s.pages.Notice(w, "resubmit-error", "This branch has not changed since the last submission.") 1504 + if err := validateResubmittedPatch(pull, patch); err != nil { 1505 + s.pages.Notice(w, "resubmit-error", err.Error()) 1633 1506 return 1634 1507 } 1635 1508 1509 + // validate sourceRev if branch/fork based 1510 + if pull.IsBranchBased() || pull.IsForkBased() { 1511 + if sourceRev == pull.Submissions[pull.LastRoundNumber()].SourceRev { 1512 + s.pages.Notice(w, "resubmit-error", "This branch has not changed since the last submission.") 1513 + return 1514 + } 1515 + } 1516 + 1636 1517 tx, err := s.db.BeginTx(r.Context(), nil) 1637 1518 if err != nil { 1638 1519 log.Println("failed to start tx") ··· 1649 1530 } 1650 1531 client, err := s.oauth.AuthorizedClient(r) 1651 1532 if err != nil { 1652 - log.Println("failed to get client") 1533 + log.Println("failed to authorize client") 1653 1534 s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") 1654 1535 return 1655 1536 } ··· 1661 1542 return 1662 1543 } 1663 1544 1664 - repoAt := pull.PullSource.RepoAt.String() 1665 - recordPullSource := &tangled.RepoPull_Source{ 1666 - Branch: pull.PullSource.Branch, 1667 - Repo: &repoAt, 1545 + var recordPullSource *tangled.RepoPull_Source 1546 + if pull.IsBranchBased() { 1547 + recordPullSource = &tangled.RepoPull_Source{ 1548 + Branch: pull.PullSource.Branch, 1549 + } 1550 + } 1551 + if pull.IsForkBased() { 1552 + repoAt := pull.PullSource.RepoAt.String() 1553 + recordPullSource = &tangled.RepoPull_Source{ 1554 + Branch: pull.PullSource.Branch, 1555 + Repo: &repoAt, 1556 + } 1668 1557 } 1558 + 1669 1559 _, err = client.RepoPutRecord(r.Context(), &comatproto.RepoPutRecord_Input{ 1670 1560 Collection: tangled.RepoPullNSID, 1671 1561 Repo: user.Did, ··· 1698 1588 return 1699 1589 } 1700 1590 1701 - // validate a resubmission against a pull request 1702 - func validateResubmittedPatch(pull *db.Pull, patch string) error { 1703 - if patch == "" { 1704 - return fmt.Errorf("Patch is empty.") 1591 + func (s *State) resubmitStackedPullHelper( 1592 + w http.ResponseWriter, 1593 + r *http.Request, 1594 + f *FullyResolvedRepo, 1595 + user *oauth.User, 1596 + pull *db.Pull, 1597 + patch string, 1598 + stackId string, 1599 + ) { 1600 + targetBranch := pull.TargetBranch 1601 + 1602 + origStack, _ := r.Context().Value("stack").(db.Stack) 1603 + newStack, err := newStack(f, user, targetBranch, patch, pull.PullSource, stackId) 1604 + if err != nil { 1605 + log.Println("failed to create resubmitted stack", err) 1606 + s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") 1607 + return 1608 + } 1609 + 1610 + // find the diff between the stacks, first, map them by changeId 1611 + origById := make(map[string]*db.Pull) 1612 + newById := make(map[string]*db.Pull) 1613 + for _, p := range origStack { 1614 + origById[p.ChangeId] = p 1615 + } 1616 + for _, p := range newStack { 1617 + newById[p.ChangeId] = p 1618 + } 1619 + 1620 + // commits that got deleted: corresponding pull is closed 1621 + // commits that got added: new pull is created 1622 + // commits that got updated: corresponding pull is resubmitted & new round begins 1623 + // 1624 + // for commits that were unchanged: no changes, parent-change-id is updated as necessary 1625 + additions := make(map[string]*db.Pull) 1626 + deletions := make(map[string]*db.Pull) 1627 + unchanged := make(map[string]struct{}) 1628 + updated := make(map[string]struct{}) 1629 + 1630 + // pulls in orignal stack but not in new one 1631 + for _, op := range origStack { 1632 + if _, ok := newById[op.ChangeId]; !ok { 1633 + deletions[op.ChangeId] = op 1634 + } 1705 1635 } 1706 1636 1707 - if patch == pull.LatestPatch() { 1708 - return fmt.Errorf("Patch is identical to previous submission.") 1637 + // pulls in new stack but not in original one 1638 + for _, np := range newStack { 1639 + if _, ok := origById[np.ChangeId]; !ok { 1640 + additions[np.ChangeId] = np 1641 + } 1709 1642 } 1710 1643 1711 - if !patchutil.IsPatchValid(patch) { 1712 - return fmt.Errorf("Invalid patch format. Please provide a valid diff.") 1644 + // NOTE: this loop can be written in any of above blocks, 1645 + // but is written separately in the interest of simpler code 1646 + for _, np := range newStack { 1647 + if op, ok := origById[np.ChangeId]; ok { 1648 + // pull exists in both stacks 1649 + // TODO: can we avoid reparse? 1650 + origFiles, _, _ := gitdiff.Parse(strings.NewReader(op.LatestPatch())) 1651 + newFiles, _, _ := gitdiff.Parse(strings.NewReader(np.LatestPatch())) 1652 + 1653 + patchutil.SortPatch(newFiles) 1654 + patchutil.SortPatch(origFiles) 1655 + 1656 + if patchutil.Equal(newFiles, origFiles) { 1657 + unchanged[op.ChangeId] = struct{}{} 1658 + } else { 1659 + updated[op.ChangeId] = struct{}{} 1660 + } 1661 + } 1713 1662 } 1714 1663 1715 - return nil 1664 + tx, err := s.db.Begin() 1665 + if err != nil { 1666 + log.Println("failed to start transaction", err) 1667 + s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") 1668 + return 1669 + } 1670 + defer tx.Rollback() 1671 + 1672 + // pds updates to make 1673 + var writes []*comatproto.RepoApplyWrites_Input_Writes_Elem 1674 + 1675 + // deleted pulls are marked as deleted in the DB 1676 + for _, p := range deletions { 1677 + err := db.DeletePull(tx, p.RepoAt, p.PullId) 1678 + if err != nil { 1679 + log.Println("failed to delete pull", err, p.PullId) 1680 + s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") 1681 + return 1682 + } 1683 + writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ 1684 + RepoApplyWrites_Delete: &comatproto.RepoApplyWrites_Delete{ 1685 + Collection: tangled.RepoPullNSID, 1686 + Rkey: p.Rkey, 1687 + }, 1688 + }) 1689 + } 1690 + 1691 + // new pulls are created 1692 + for _, p := range additions { 1693 + err := db.NewPull(tx, p) 1694 + if err != nil { 1695 + log.Println("failed to create pull", err, p.PullId) 1696 + s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") 1697 + return 1698 + } 1699 + 1700 + record := p.AsRecord() 1701 + writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ 1702 + RepoApplyWrites_Create: &comatproto.RepoApplyWrites_Create{ 1703 + Collection: tangled.RepoPullNSID, 1704 + Rkey: &p.Rkey, 1705 + Value: &lexutil.LexiconTypeDecoder{ 1706 + Val: &record, 1707 + }, 1708 + }, 1709 + }) 1710 + } 1711 + 1712 + // updated pulls are, well, updated; to start a new round 1713 + for id := range updated { 1714 + op, _ := origById[id] 1715 + np, _ := newById[id] 1716 + 1717 + submission := np.Submissions[np.LastRoundNumber()] 1718 + 1719 + // resubmit the old pull 1720 + err := db.ResubmitPull(tx, op, submission.Patch, submission.SourceRev) 1721 + 1722 + if err != nil { 1723 + log.Println("failed to update pull", err, op.PullId) 1724 + s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") 1725 + return 1726 + } 1727 + 1728 + record := op.AsRecord() 1729 + record.Patch = submission.Patch 1730 + 1731 + writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ 1732 + RepoApplyWrites_Update: &comatproto.RepoApplyWrites_Update{ 1733 + Collection: tangled.RepoPullNSID, 1734 + Rkey: op.Rkey, 1735 + Value: &lexutil.LexiconTypeDecoder{ 1736 + Val: &record, 1737 + }, 1738 + }, 1739 + }) 1740 + } 1741 + 1742 + // update parent-change-id relations for the entire stack 1743 + for _, p := range newStack { 1744 + err := db.SetPullParentChangeId( 1745 + tx, 1746 + p.ParentChangeId, 1747 + // these should be enough filters to be unique per-stack 1748 + db.Filter("repo_at", p.RepoAt.String()), 1749 + db.Filter("owner_did", p.OwnerDid), 1750 + db.Filter("change_id", p.ChangeId), 1751 + ) 1752 + 1753 + if err != nil { 1754 + log.Println("failed to update pull", err, p.PullId) 1755 + s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") 1756 + return 1757 + } 1758 + } 1759 + 1760 + err = tx.Commit() 1761 + if err != nil { 1762 + log.Println("failed to resubmit pull", err) 1763 + s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") 1764 + return 1765 + } 1766 + 1767 + client, err := s.oauth.AuthorizedClient(r) 1768 + if err != nil { 1769 + log.Println("failed to authorize client") 1770 + s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.") 1771 + return 1772 + } 1773 + 1774 + _, err = client.RepoApplyWrites(r.Context(), &comatproto.RepoApplyWrites_Input{ 1775 + Repo: user.Did, 1776 + Writes: writes, 1777 + }) 1778 + if err != nil { 1779 + log.Println("failed to create stacked pull request", err) 1780 + s.pages.Notice(w, "pull", "Failed to create stacked pull request. Try again later.") 1781 + return 1782 + } 1783 + 1784 + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) 1785 + return 1716 1786 } 1717 1787 1718 1788 func (s *State) MergePull(w http.ResponseWriter, r *http.Request) { ··· 1745 1815 1746 1816 // collect the portion of the stack that is mergeable 1747 1817 for _, p := range subStack { 1748 - // stop at the first merged PR 1749 - if p.State == db.PullMerged { 1818 + // stop at the first merged/closed PR 1819 + if p.State == db.PullMerged || p.State == db.PullClosed { 1750 1820 break 1751 1821 } 1752 1822 1753 - // skip over closed PRs 1754 - // 1755 - // TODO: we need a "deleted" state for such PRs, but without losing discussions 1756 - // we will close PRs that are "removed" from a stack 1757 - if p.State == db.PullClosed { 1823 + // skip over deleted PRs 1824 + if p.State == db.PullDeleted { 1758 1825 continue 1759 1826 } 1760 1827 ··· 1806 1873 1807 1874 tx, err := s.db.Begin() 1808 1875 if err != nil { 1809 - log.Printf("failed to start transcation", err) 1876 + log.Println("failed to start transcation", err) 1810 1877 s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") 1811 1878 return 1812 1879 } 1880 + defer tx.Rollback() 1813 1881 1814 1882 for _, p := range pullsToMerge { 1815 1883 err := db.MergePull(tx, f.RepoAt, p.PullId) ··· 1865 1933 s.pages.Notice(w, "pull-close", "Failed to close pull.") 1866 1934 return 1867 1935 } 1936 + defer tx.Rollback() 1868 1937 1869 1938 var pullsToClose []*db.Pull 1870 1939 pullsToClose = append(pullsToClose, pull) ··· 1932 2001 s.pages.Notice(w, "pull-reopen", "Failed to reopen pull.") 1933 2002 return 1934 2003 } 2004 + defer tx.Rollback() 1935 2005 1936 2006 var pullsToReopen []*db.Pull 1937 2007 pullsToReopen = append(pullsToReopen, pull) ··· 1963 2033 s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) 1964 2034 return 1965 2035 } 2036 + 2037 + func newStack(f *FullyResolvedRepo, user *oauth.User, targetBranch, patch string, pullSource *db.PullSource, stackId string) (db.Stack, error) { 2038 + formatPatches, err := patchutil.ExtractPatches(patch) 2039 + if err != nil { 2040 + return nil, fmt.Errorf("Failed to extract patches: %v", err) 2041 + } 2042 + 2043 + // must have atleast 1 patch to begin with 2044 + if len(formatPatches) == 0 { 2045 + return nil, fmt.Errorf("No patches found in the generated format-patch.") 2046 + } 2047 + 2048 + // the stack is identified by a UUID 2049 + var stack db.Stack 2050 + parentChangeId := "" 2051 + for _, fp := range formatPatches { 2052 + // all patches must have a jj change-id 2053 + changeId, err := fp.ChangeId() 2054 + if err != nil { 2055 + return nil, fmt.Errorf("Stacking is only supported if all patches contain a change-id commit header.") 2056 + } 2057 + 2058 + title := fp.Title 2059 + body := fp.Body 2060 + rkey := appview.TID() 2061 + 2062 + initialSubmission := db.PullSubmission{ 2063 + Patch: fp.Raw, 2064 + SourceRev: fp.SHA, 2065 + } 2066 + pull := db.Pull{ 2067 + Title: title, 2068 + Body: body, 2069 + TargetBranch: targetBranch, 2070 + OwnerDid: user.Did, 2071 + RepoAt: f.RepoAt, 2072 + Rkey: rkey, 2073 + Submissions: []*db.PullSubmission{ 2074 + &initialSubmission, 2075 + }, 2076 + PullSource: pullSource, 2077 + Created: time.Now(), 2078 + 2079 + StackId: stackId, 2080 + ChangeId: changeId, 2081 + ParentChangeId: parentChangeId, 2082 + } 2083 + 2084 + stack = append(stack, &pull) 2085 + 2086 + parentChangeId = changeId 2087 + } 2088 + 2089 + return stack, nil 2090 + }
+60
patchutil/patchutil.go
··· 5 5 "os" 6 6 "os/exec" 7 7 "regexp" 8 + "slices" 8 9 "strings" 9 10 10 11 "github.com/bluekeyes/go-gitdiff/gitdiff" ··· 203 204 204 205 return string(output), nil 205 206 } 207 + 208 + // are two patches identical 209 + func Equal(a, b []*gitdiff.File) bool { 210 + return slices.EqualFunc(a, b, func(x, y *gitdiff.File) bool { 211 + // same pointer 212 + if x == y { 213 + return true 214 + } 215 + if x == nil || y == nil { 216 + return x == y 217 + } 218 + 219 + // compare file metadata 220 + if x.OldName != y.OldName || x.NewName != y.NewName { 221 + return false 222 + } 223 + if x.OldMode != y.OldMode || x.NewMode != y.NewMode { 224 + return false 225 + } 226 + if x.IsNew != y.IsNew || x.IsDelete != y.IsDelete || x.IsCopy != y.IsCopy || x.IsRename != y.IsRename { 227 + return false 228 + } 229 + 230 + if len(x.TextFragments) != len(y.TextFragments) { 231 + return false 232 + } 233 + 234 + for i, xFrag := range x.TextFragments { 235 + yFrag := y.TextFragments[i] 236 + 237 + // Compare fragment headers 238 + if xFrag.OldPosition != yFrag.OldPosition || xFrag.OldLines != yFrag.OldLines || 239 + xFrag.NewPosition != yFrag.NewPosition || xFrag.NewLines != yFrag.NewLines { 240 + return false 241 + } 242 + 243 + // Compare fragment changes 244 + if len(xFrag.Lines) != len(yFrag.Lines) { 245 + return false 246 + } 247 + 248 + for j, xLine := range xFrag.Lines { 249 + yLine := yFrag.Lines[j] 250 + if xLine.Op != yLine.Op || xLine.Line != yLine.Line { 251 + return false 252 + } 253 + } 254 + } 255 + 256 + return true 257 + }) 258 + } 259 + 260 + // sort patch files in alphabetical order 261 + func SortPatch(patch []*gitdiff.File) { 262 + slices.SortFunc(patch, func(a, b *gitdiff.File) int { 263 + return strings.Compare(bestName(a), bestName(b)) 264 + }) 265 + }