From 12a3fb337488903af53b1118e0611ee9d3d0e905 Mon Sep 17 00:00:00 2001 From: nelind Date: Sun, 1 Jun 2025 03:12:28 +0200 Subject: [PATCH] lexicons: pulls: add stack information to pull records Change-Id: pmktqsszrmpvrymrxrlwxllqtvomkvzv Signed-off-by: nelind --- api/tangled/cbor_gen.go | 203 ++++++++++++++++++++++++++++++++++++++- api/tangled/repopull.go | 23 +++-- appview/db/db.go | 8 ++ appview/db/pulls.go | 47 +++++++-- appview/pulls/pulls.go | 23 ++++- cmd/gen.go | 1 + lexicons/pulls/pull.json | 21 ++++ 7 files changed, 306 insertions(+), 20 deletions(-) diff --git a/api/tangled/cbor_gen.go b/api/tangled/cbor_gen.go index f898501..9246554 100644 --- a/api/tangled/cbor_gen.go +++ b/api/tangled/cbor_gen.go @@ -6939,6 +6939,164 @@ func (t *RepoIssueState) UnmarshalCBOR(r io.Reader) (err error) { return nil } +func (t *RepoPull_StackInfo) MarshalCBOR(w io.Writer) error { + if t == nil { + _, err := w.Write(cbg.CborNull) + return err + } + + cw := cbg.NewCborWriter(w) + fieldCount := 2 + + if t.Parent == nil { + fieldCount-- + } + + if _, err := cw.Write(cbg.CborEncodeMajorType(cbg.MajMap, uint64(fieldCount))); err != nil { + return err + } + + // t.Parent (string) (string) + if t.Parent != nil { + + if len("parent") > 1000000 { + return xerrors.Errorf("Value in field \"parent\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("parent"))); err != nil { + return err + } + if _, err := cw.WriteString(string("parent")); err != nil { + return err + } + + if t.Parent == nil { + if _, err := cw.Write(cbg.CborNull); err != nil { + return err + } + } else { + if len(*t.Parent) > 1000000 { + return xerrors.Errorf("Value in field t.Parent was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(*t.Parent))); err != nil { + return err + } + if _, err := cw.WriteString(string(*t.Parent)); err != nil { + return err + } + } + } + + // t.ChangeId (string) (string) + if len("changeId") > 1000000 { + return xerrors.Errorf("Value in field \"changeId\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("changeId"))); err != nil { + return err + } + if _, err := cw.WriteString(string("changeId")); err != nil { + return err + } + + if len(t.ChangeId) > 1000000 { + return xerrors.Errorf("Value in field t.ChangeId was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(t.ChangeId))); err != nil { + return err + } + if _, err := cw.WriteString(string(t.ChangeId)); err != nil { + return err + } + return nil +} + +func (t *RepoPull_StackInfo) UnmarshalCBOR(r io.Reader) (err error) { + *t = RepoPull_StackInfo{} + + cr := cbg.NewCborReader(r) + + maj, extra, err := cr.ReadHeader() + if err != nil { + return err + } + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + + if maj != cbg.MajMap { + return fmt.Errorf("cbor input should be of type map") + } + + if extra > cbg.MaxLength { + return fmt.Errorf("RepoPull_StackInfo: map struct too large (%d)", extra) + } + + n := extra + + nameBuf := make([]byte, 8) + for i := uint64(0); i < n; i++ { + nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 1000000) + if err != nil { + return err + } + + if !ok { + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { + return err + } + continue + } + + switch string(nameBuf[:nameLen]) { + // t.Parent (string) (string) + case "parent": + + { + b, err := cr.ReadByte() + if err != nil { + return err + } + if b != cbg.CborNull[0] { + if err := cr.UnreadByte(); err != nil { + return err + } + + sval, err := cbg.ReadStringWithMax(cr, 1000000) + if err != nil { + return err + } + + t.Parent = (*string)(&sval) + } + } + // t.ChangeId (string) (string) + case "changeId": + + { + sval, err := cbg.ReadStringWithMax(cr, 1000000) + if err != nil { + return err + } + + t.ChangeId = string(sval) + } + + default: + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { + return err + } + } + } + + return nil +} func (t *RepoPull_Target) MarshalCBOR(w io.Writer) error { if t == nil { _, err := w.Write(cbg.CborNull) @@ -7080,7 +7238,7 @@ func (t *RepoPull) MarshalCBOR(w io.Writer) error { } cw := cbg.NewCborWriter(w) - fieldCount := 7 + fieldCount := 8 if t.Body == nil { fieldCount-- @@ -7090,6 +7248,10 @@ func (t *RepoPull) MarshalCBOR(w io.Writer) error { fieldCount-- } + if t.StackInfo == nil { + fieldCount-- + } + if _, err := cw.Write(cbg.CborEncodeMajorType(cbg.MajMap, uint64(fieldCount))); err != nil { return err } @@ -7248,6 +7410,25 @@ func (t *RepoPull) MarshalCBOR(w io.Writer) error { if _, err := cw.WriteString(string(t.CreatedAt)); err != nil { return err } + + // t.StackInfo (tangled.RepoPull_StackInfo) (struct) + if t.StackInfo != nil { + + if len("stackInfo") > 1000000 { + return xerrors.Errorf("Value in field \"stackInfo\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("stackInfo"))); err != nil { + return err + } + if _, err := cw.WriteString(string("stackInfo")); err != nil { + return err + } + + if err := t.StackInfo.MarshalCBOR(cw); err != nil { + return err + } + } return nil } @@ -7397,6 +7578,26 @@ func (t *RepoPull) UnmarshalCBOR(r io.Reader) (err error) { t.CreatedAt = string(sval) } + // t.StackInfo (tangled.RepoPull_StackInfo) (struct) + case "stackInfo": + + { + + b, err := cr.ReadByte() + if err != nil { + return err + } + if b != cbg.CborNull[0] { + if err := cr.UnreadByte(); err != nil { + return err + } + t.StackInfo = new(RepoPull_StackInfo) + if err := t.StackInfo.UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling t.StackInfo pointer: %w", err) + } + } + + } default: // Field doesn't exist on this type, so ignore it diff --git a/api/tangled/repopull.go b/api/tangled/repopull.go index c56931c..e9a939b 100644 --- a/api/tangled/repopull.go +++ b/api/tangled/repopull.go @@ -17,13 +17,14 @@ func init() { } // // RECORDTYPE: RepoPull type RepoPull struct { - LexiconTypeID string `json:"$type,const=sh.tangled.repo.pull" cborgen:"$type,const=sh.tangled.repo.pull"` - Body *string `json:"body,omitempty" cborgen:"body,omitempty"` - CreatedAt string `json:"createdAt" cborgen:"createdAt"` - Patch string `json:"patch" cborgen:"patch"` - Source *RepoPull_Source `json:"source,omitempty" cborgen:"source,omitempty"` - Target *RepoPull_Target `json:"target" cborgen:"target"` - Title string `json:"title" cborgen:"title"` + LexiconTypeID string `json:"$type,const=sh.tangled.repo.pull" cborgen:"$type,const=sh.tangled.repo.pull"` + Body *string `json:"body,omitempty" cborgen:"body,omitempty"` + CreatedAt string `json:"createdAt" cborgen:"createdAt"` + Patch string `json:"patch" cborgen:"patch"` + Source *RepoPull_Source `json:"source,omitempty" cborgen:"source,omitempty"` + StackInfo *RepoPull_StackInfo `json:"stackInfo,omitempty" cborgen:"stackInfo,omitempty"` + Target *RepoPull_Target `json:"target" cborgen:"target"` + Title string `json:"title" cborgen:"title"` } // RepoPull_Source is a "source" in the sh.tangled.repo.pull schema. @@ -33,6 +34,14 @@ type RepoPull_Source struct { Sha string `json:"sha" cborgen:"sha"` } +// RepoPull_StackInfo is a "stackInfo" in the sh.tangled.repo.pull schema. +type RepoPull_StackInfo struct { + // changeId: Change ID of this commit/change. Principly also available in the patch itself as a line in the commit footer. + ChangeId string `json:"changeId" cborgen:"changeId"` + // parent: AT-URI of the PR for the parent commit/change in the change stack. + Parent *string `json:"parent,omitempty" cborgen:"parent,omitempty"` +} + // RepoPull_Target is a "target" in the sh.tangled.repo.pull schema. type RepoPull_Target struct { Branch string `json:"branch" cborgen:"branch"` diff --git a/appview/db/db.go b/appview/db/db.go index e18646d..2f036db 100644 --- a/appview/db/db.go +++ b/appview/db/db.go @@ -655,6 +655,14 @@ func Make(dbPath string) (*DB, error) { return err }) + + runMigration(db, "add-parent-at-for-stacks-to-pulls", func(tx *sql.Tx) error { + _, err := tx.Exec(` + alter table pulls add column parent_at text; + `) + return err + }) + return &DB{db}, nil } diff --git a/appview/db/pulls.go b/appview/db/pulls.go index ffeefb5..5169a03 100644 --- a/appview/db/pulls.go +++ b/appview/db/pulls.go @@ -72,6 +72,7 @@ type Pull struct { // stacking StackId string // nullable string ChangeId string // nullable string + ParentAt *syntax.ATURI ParentChangeId string // nullable string // meta @@ -91,15 +92,19 @@ func (p Pull) AsRecord() tangled.RepoPull { } record := tangled.RepoPull{ - Title: p.Title, - Body: &p.Body, - CreatedAt: p.Created.Format(time.RFC3339), + Title: p.Title, + Body: &p.Body, + CreatedAt: p.Created.Format(time.RFC3339), Target: &tangled.RepoPull_Target{ Repo: p.RepoAt.String(), Branch: p.TargetBranch, }, - Patch: p.LatestPatch(), - Source: source, + Patch: p.LatestPatch(), + Source: source, + StackInfo: &tangled.RepoPull_StackInfo{ + ChangeId: p.ChangeId, + Parent: (*string)(p.ParentAt), + }, } return record } @@ -255,13 +260,16 @@ func NewPull(tx *sql.Tx, pull *Pull) error { } } - var stackId, changeId, parentChangeId *string + var stackId, changeId, parentAt, parentChangeId *string if pull.StackId != "" { stackId = &pull.StackId } if pull.ChangeId != "" { changeId = &pull.ChangeId } + if pull.ParentAt != nil { + parentAt = (*string)(pull.ParentAt) + } if pull.ParentChangeId != "" { parentChangeId = &pull.ParentChangeId } @@ -269,9 +277,9 @@ func NewPull(tx *sql.Tx, pull *Pull) error { _, err = tx.Exec( ` insert into pulls ( - repo_at, owner_did, pull_id, title, target_branch, body, rkey, state, source_branch, source_repo_at, stack_id, change_id, parent_change_id + repo_at, owner_did, pull_id, title, target_branch, body, rkey, state, source_branch, source_repo_at, stack_id, change_id, parent_at, parent_change_id ) - values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, pull.RepoAt, pull.OwnerDid, pull.PullId, @@ -284,6 +292,7 @@ func NewPull(tx *sql.Tx, pull *Pull) error { sourceRepoAt, stackId, changeId, + parentAt, parentChangeId, ) if err != nil { @@ -341,6 +350,7 @@ func GetPulls(e Execer, filters ...filter) ([]*Pull, error) { source_repo_at, stack_id, change_id, + parent_at, parent_change_id from pulls @@ -356,7 +366,7 @@ func GetPulls(e Execer, filters ...filter) ([]*Pull, error) { for rows.Next() { var pull Pull var createdAt string - var sourceBranch, sourceRepoAt, stackId, changeId, parentChangeId sql.NullString + var sourceBranch, sourceRepoAt, stackId, changeId, parentAt, parentChangeId sql.NullString err := rows.Scan( &pull.OwnerDid, &pull.RepoAt, @@ -371,6 +381,7 @@ func GetPulls(e Execer, filters ...filter) ([]*Pull, error) { &sourceRepoAt, &stackId, &changeId, + &parentAt, &parentChangeId, ) if err != nil { @@ -402,6 +413,13 @@ func GetPulls(e Execer, filters ...filter) ([]*Pull, error) { if changeId.Valid { pull.ChangeId = changeId.String } + if parentAt.Valid { + parentAtParsed, err := syntax.ParseATURI(parentAt.String) + if err != nil { + return nil, err + } + pull.ParentAt = &parentAtParsed + } if parentChangeId.Valid { pull.ParentChangeId = parentChangeId.String } @@ -530,6 +548,7 @@ func GetPull(e Execer, repoAt syntax.ATURI, pullId int) (*Pull, error) { source_repo_at, stack_id, change_id, + parent_at, parent_change_id from pulls @@ -540,7 +559,7 @@ func GetPull(e Execer, repoAt syntax.ATURI, pullId int) (*Pull, error) { var pull Pull var createdAt string - var sourceBranch, sourceRepoAt, stackId, changeId, parentChangeId sql.NullString + var sourceBranch, sourceRepoAt, stackId, changeId, parentAt, parentChangeId sql.NullString err := row.Scan( &pull.OwnerDid, &pull.PullId, @@ -555,6 +574,7 @@ func GetPull(e Execer, repoAt syntax.ATURI, pullId int) (*Pull, error) { &sourceRepoAt, &stackId, &changeId, + &parentAt, &parentChangeId, ) if err != nil { @@ -587,6 +607,13 @@ func GetPull(e Execer, repoAt syntax.ATURI, pullId int) (*Pull, error) { if changeId.Valid { pull.ChangeId = changeId.String } + if parentAt.Valid { + parsedParentAt, err := syntax.ParseATURI(parentAt.String) + if err != nil { + return nil, err + } + pull.ParentAt = &parsedParentAt + } if parentChangeId.Valid { pull.ParentChangeId = parentChangeId.String } diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go index 1fbc403..ba78cab 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -1693,17 +1693,28 @@ func (s *Pulls) resubmitStackedPullHelper( newStack, err := newStack(f, user, targetBranch, patch, pull.PullSource, stackId) if err != nil { log.Println("failed to create resubmitted stack", err) - s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") + s.pages.Notice(w, "pull-resubmit-error", "Failed to merge pull request. Try again later.") return } // find the diff between the stacks, first, map them by changeId origById := make(map[string]*db.Pull) newById := make(map[string]*db.Pull) + chIdToAtUri := make(map[string]*syntax.ATURI) for _, p := range origStack { origById[p.ChangeId] = p + + // build map from change id to existing at uris (ignore error as it shouldnt be possible here) + pAtUri, _ := syntax.ParseATURI(fmt.Sprintf("at://%s/%s/%s", user.Did, tangled.RepoPullNSID, p.Rkey)) + chIdToAtUri[p.ChangeId] = &pAtUri } for _, p := range newStack { + // if change id has already been given a PR use its at uri instead of the newly created (and thus incorrect) + // one made by newStack + if ppAt, ok := chIdToAtUri[p.ParentChangeId]; ok { + p.ParentAt = ppAt + } + newById[p.ChangeId] = p } @@ -1751,7 +1762,8 @@ func (s *Pulls) resubmitStackedPullHelper( // we still need to update the hash in submission.Patch and submission.SourceRev if patchutil.Equal(newFiles, origFiles) && origHeader.Title == newHeader.Title && - origHeader.Body == newHeader.Body { + origHeader.Body == newHeader.Body && + op.ParentChangeId == np.ParentChangeId { unchanged[op.ChangeId] = struct{}{} } else { updated[op.ChangeId] = struct{}{} @@ -1825,6 +1837,7 @@ func (s *Pulls) resubmitStackedPullHelper( record := op.AsRecord() record.Patch = submission.Patch + record.StackInfo.Parent = (*string)(np.ParentAt) writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ RepoApplyWrites_Update: &comatproto.RepoApplyWrites_Update{ @@ -2173,6 +2186,7 @@ func newStack(f *reporesolver.ResolvedRepo, user *oauth.User, targetBranch, patc // the stack is identified by a UUID var stack db.Stack parentChangeId := "" + var parentAt *syntax.ATURI = nil for _, fp := range formatPatches { // all patches must have a jj change-id changeId, err := fp.ChangeId() @@ -2203,12 +2217,17 @@ func newStack(f *reporesolver.ResolvedRepo, user *oauth.User, targetBranch, patc StackId: stackId, ChangeId: changeId, + ParentAt: parentAt, ParentChangeId: parentChangeId, } stack = append(stack, &pull) parentChangeId = changeId + // this is a bit of an ugly way to create the ATURI but its the best we can do with the data flow here + // (igore error as it shouldnt be possible here) + parsedParentAt, _ := syntax.ParseATURI(fmt.Sprintf("at://%s/%s/%s", user.Did, tangled.RepoPullNSID, pull.Rkey)); + parentAt = &parsedParentAt } return stack, nil diff --git a/cmd/gen.go b/cmd/gen.go index 5996ae7..69585f5 100644 --- a/cmd/gen.go +++ b/cmd/gen.go @@ -46,6 +46,7 @@ func main() { tangled.RepoIssueState{}, tangled.RepoPull{}, tangled.RepoPullComment{}, + tangled.RepoPull_StackInfo{}, tangled.RepoPull_Source{}, tangled.RepoPull_Target{}, tangled.RepoPullStatus{}, diff --git a/lexicons/pulls/pull.json b/lexicons/pulls/pull.json index 143131d..e34e060 100644 --- a/lexicons/pulls/pull.json +++ b/lexicons/pulls/pull.json @@ -29,6 +29,10 @@ "patch": { "type": "string" }, + "stackInfo": { + "type": "ref", + "ref": "#stackInfo" + }, "source": { "type": "ref", "ref": "#source" @@ -76,6 +80,23 @@ "format": "at-uri" } } + }, + "stackInfo": { + "type": "object", + "required": [ + "changeId" + ], + "properties": { + "changeId": { + "type": "string", + "description": "Change ID of this commit/change." + }, + "parent": { + "type": "string", + "description": "AT-URI of the PR for the parent commit/change in the change stack.", + "format": "at-uri" + } + } } } } -- 2.50.0