From 060f0d62d840600dfe81159a2b0385d74b943810 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 | 7 ++ appview/db/pulls.go | 47 +++++++-- appview/pulls/pulls.go | 24 ++++- 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 174eaff..33c3068 100644 --- a/api/tangled/cbor_gen.go +++ b/api/tangled/cbor_gen.go @@ -6392,6 +6392,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) @@ -6533,7 +6691,7 @@ func (t *RepoPull) MarshalCBOR(w io.Writer) error { } cw := cbg.NewCborWriter(w) - fieldCount := 7 + fieldCount := 8 if t.Body == nil { fieldCount-- @@ -6543,6 +6701,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 } @@ -6701,6 +6863,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 } @@ -6850,6 +7031,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 45cafa3..e019698 100644 --- a/appview/db/db.go +++ b/appview/db/db.go @@ -678,6 +678,13 @@ func Make(dbPath string) (*DB, error) { return err }) + runMigration(conn, "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 b77eb29..9de7a30 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 { @@ -345,6 +354,7 @@ func GetPullsWithLimit(e Execer, limit int, filters ...filter) ([]*Pull, error) source_repo_at, stack_id, change_id, + parent_at, parent_change_id from pulls @@ -363,7 +373,7 @@ func GetPullsWithLimit(e Execer, limit int, 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, @@ -378,6 +388,7 @@ func GetPullsWithLimit(e Execer, limit int, filters ...filter) ([]*Pull, error) &sourceRepoAt, &stackId, &changeId, + &parentAt, &parentChangeId, ) if err != nil { @@ -409,6 +420,13 @@ func GetPullsWithLimit(e Execer, limit int, 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 } @@ -549,6 +567,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 @@ -559,7 +578,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, @@ -574,6 +593,7 @@ func GetPull(e Execer, repoAt syntax.ATURI, pullId int) (*Pull, error) { &sourceRepoAt, &stackId, &changeId, + &parentAt, &parentChangeId, ) if err != nil { @@ -606,6 +626,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 33c4b22..5fe5837 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -29,6 +29,7 @@ import ( "github.com/bluekeyes/go-gitdiff/gitdiff" comatproto "github.com/bluesky-social/indigo/api/atproto" + "github.com/bluesky-social/indigo/atproto/syntax" lexutil "github.com/bluesky-social/indigo/lex/util" "github.com/go-chi/chi/v5" "github.com/google/uuid" @@ -1633,17 +1634,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 } @@ -1691,7 +1703,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{}{} @@ -1775,6 +1788,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{ @@ -2123,6 +2137,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() @@ -2153,12 +2168,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 a463f57..21c81a2 100644 --- a/cmd/gen.go +++ b/cmd/gen.go @@ -44,6 +44,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.1