appview,knotserver: produce combined patch in comparisons #609

merged
opened by oppi.li targeting master from push-tvqsxkkxqolv

this is calculated by the knotserver in sh.tangled.repo.compare and cached by the appview in pull submissions, this cannot be calculated on the appview side with just the format-patch because this calculation requires a git-index.

Signed-off-by: oppiliappan me@oppi.li

Changed files
+95 -67
appview
db
models
pulls
repo
knotserver
types
+9
appview/db/db.go
···
})
conn.ExecContext(ctx, "pragma foreign_keys = on;")
+
// knots may report the combined patch for a comparison, we can store that on the appview side
+
// (but not on the pds record), because calculating the combined patch requires a git index
+
runMigration(conn, logger, "add-combined-column-submissions", func(tx *sql.Tx) error {
+
_, err := tx.Exec(`
+
alter table pull_submissions add column combined text;
+
`)
+
return err
+
})
+
return &DB{
db,
logger,
+21 -17
appview/db/pulls.go
···
pull.ID = int(id)
_, err = tx.Exec(`
-
insert into pull_submissions (pull_at, round_number, patch, source_rev)
-
values (?, ?, ?, ?)
-
`, pull.PullAt(), 0, pull.Submissions[0].Patch, pull.Submissions[0].SourceRev)
+
insert into pull_submissions (pull_at, round_number, patch, combined, source_rev)
+
values (?, ?, ?, ?, ?)
+
`, pull.PullAt(), 0, pull.Submissions[0].Patch, pull.Submissions[0].Combined, pull.Submissions[0].SourceRev)
return err
}
···
pull_at,
round_number,
patch,
+
combined,
created,
source_rev
from
···
for rows.Next() {
var submission models.PullSubmission
-
var createdAt string
-
var sourceRev sql.NullString
+
var submissionCreatedStr string
+
var submissionSourceRev, submissionCombined sql.NullString
err := rows.Scan(
&submission.ID,
&submission.PullAt,
&submission.RoundNumber,
&submission.Patch,
-
&createdAt,
-
&sourceRev,
+
&submissionCombined,
+
&submissionCreatedStr,
+
&submissionSourceRev,
)
if err != nil {
return nil, err
}
-
createdTime, err := time.Parse(time.RFC3339, createdAt)
-
if err != nil {
-
return nil, err
+
if t, err := time.Parse(time.RFC3339, submissionCreatedStr); err == nil {
+
submission.Created = t
+
}
+
+
if submissionSourceRev.Valid {
+
submission.SourceRev = submissionSourceRev.String
}
-
submission.Created = createdTime
-
if sourceRev.Valid {
-
submission.SourceRev = sourceRev.String
+
if submissionCombined.Valid {
+
submission.Combined = submissionCombined.String
}
submissionMap[submission.ID] = &submission
···
return err
}
-
func ResubmitPull(e Execer, pull *models.Pull, newPatch, sourceRev string) error {
+
func ResubmitPull(e Execer, pull *models.Pull, newPatch, combined, sourceRev string) error {
newRoundNumber := len(pull.Submissions)
_, err := e.Exec(`
-
insert into pull_submissions (pull_at, round_number, patch, source_rev)
-
values (?, ?, ?, ?)
-
`, pull.PullAt(), newRoundNumber, newPatch, sourceRev)
+
insert into pull_submissions (pull_at, round_number, patch, combined, source_rev)
+
values (?, ?, ?, ?, ?)
+
`, pull.PullAt(), newRoundNumber, newPatch, combined, sourceRev)
return err
}
+9
appview/models/pull.go
···
// content
RoundNumber int
Patch string
+
Combined string
Comments []PullComment
SourceRev string // include the rev that was used to create this submission: only for branch/fork PRs
···
return participants
}
+
func (s PullSubmission) CombinedPatch() string {
+
if s.Combined == "" {
+
return s.Patch
+
}
+
+
return s.Combined
+
}
+
type Stack []*Pull
// position of this pull in the stack
+23 -40
appview/pulls/pulls.go
···
stack, _ := r.Context().Value("stack").(models.Stack)
abandonedPulls, _ := r.Context().Value("abandonedPulls").([]*models.Pull)
-
totalIdents := 1
-
for _, submission := range pull.Submissions {
-
totalIdents += len(submission.Comments)
-
}
-
-
identsToResolve := make([]string, totalIdents)
-
-
// populate idents
-
identsToResolve[0] = pull.OwnerDid
-
idx := 1
-
for _, submission := range pull.Submissions {
-
for _, comment := range submission.Comments {
-
identsToResolve[idx] = comment.OwnerDid
-
idx += 1
-
}
-
}
-
mergeCheckResponse := s.mergeCheck(r, f, pull, stack)
branchDeleteStatus := s.branchDeleteStatus(r, f, pull)
resubmitResult := pages.Unknown
···
return
}
-
patch := pull.Submissions[roundIdInt].Patch
+
patch := pull.Submissions[roundIdInt].CombinedPatch()
diff := patchutil.AsNiceDiff(patch, pull.TargetBranch)
s.pages.RepoPullPatchPage(w, pages.RepoPullPatchParams{
···
return
}
-
currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].Patch)
+
currentPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt].CombinedPatch())
if err != nil {
log.Println("failed to interdiff; current patch malformed")
s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; current patch is invalid.")
return
}
-
previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].Patch)
+
previousPatch, err := patchutil.AsDiff(pull.Submissions[roundIdInt-1].CombinedPatch())
if err != nil {
log.Println("failed to interdiff; previous patch malformed")
s.pages.Notice(w, fmt.Sprintf("interdiff-error-%d", roundIdInt), "Failed to calculate interdiff; previous patch is invalid.")
···
createdAt := time.Now().Format(time.RFC3339)
-
pullAt, err := db.GetPullAt(s.db, f.RepoAt(), pull.PullId)
-
if err != nil {
-
log.Println("failed to get pull at", err)
-
s.pages.Notice(w, "pull-comment", "Failed to create comment.")
-
return
-
}
-
client, err := s.oauth.AuthorizedClient(r)
if err != nil {
log.Println("failed to get authorized client", err)
···
Rkey: tid.TID(),
Record: &lexutil.LexiconTypeDecoder{
Val: &tangled.RepoPullComment{
-
Pull: string(pullAt),
+
Pull: pull.PullAt().String(),
Body: body,
CreatedAt: createdAt,
},
···
}
sourceRev := comparison.Rev2
-
patch := comparison.Patch
+
patch := comparison.FormatPatchRaw
+
combined := comparison.CombinedPatchRaw
if !patchutil.IsPatchValid(patch) {
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
···
Sha: comparison.Rev2,
}
-
s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked)
+
s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked)
}
func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) {
···
return
}
-
s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", nil, nil, isStacked)
+
s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, "", "", nil, nil, isStacked)
}
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) {
···
sourceRev := comparison.Rev2
-
patch := comparison.Patch
+
patch := comparison.FormatPatchRaw
+
combined := comparison.CombinedPatchRaw
if !patchutil.IsPatchValid(patch) {
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
···
Sha: sourceRev,
-
s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, sourceRev, pullSource, recordPullSource, isStacked)
+
s.createPullRequest(w, r, f, user, title, body, targetBranch, patch, combined, sourceRev, pullSource, recordPullSource, isStacked)
func (s *Pulls) createPullRequest(
···
user *oauth.User,
title, body, targetBranch string,
patch string,
+
combined string,
sourceRev string,
pullSource *models.PullSource,
recordPullSource *tangled.RepoPull_Source,
···
rkey := tid.TID()
initialSubmission := models.PullSubmission{
Patch: patch,
+
Combined: combined,
SourceRev: sourceRev,
pull := &models.Pull{
···
patch := r.FormValue("patch")
-
s.resubmitPullHelper(w, r, f, user, pull, patch, "")
+
s.resubmitPullHelper(w, r, f, user, pull, patch, "", "")
func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) {
···
sourceRev := comparison.Rev2
-
patch := comparison.Patch
+
patch := comparison.FormatPatchRaw
+
combined := comparison.CombinedPatchRaw
-
s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev)
+
s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev)
func (s *Pulls) resubmitFork(w http.ResponseWriter, r *http.Request) {
···
comparison := forkComparison
sourceRev := comparison.Rev2
-
patch := comparison.Patch
+
patch := comparison.FormatPatchRaw
+
combined := comparison.CombinedPatchRaw
-
s.resubmitPullHelper(w, r, f, user, pull, patch, sourceRev)
+
s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev)
// validate a resubmission against a pull request
···
user *oauth.User,
pull *models.Pull,
patch string,
+
combined string,
sourceRev string,
) {
if pull.IsStacked() {
···
defer tx.Rollback()
-
err = db.ResubmitPull(tx, pull, patch, sourceRev)
+
err = db.ResubmitPull(tx, pull, patch, combined, sourceRev)
if err != nil {
log.Println("failed to create pull request", err)
s.pages.Notice(w, "resubmit-error", "Failed to create pull request. Try again later.")
···
submission := np.Submissions[np.LastRoundNumber()]
// resubmit the old pull
-
err := db.ResubmitPull(tx, op, submission.Patch, submission.SourceRev)
+
err := db.ResubmitPull(tx, op, submission.Patch, submission.Combined, submission.SourceRev)
if err != nil {
log.Println("failed to update pull", err, op.PullId)
+6 -1
appview/repo/repo.go
···
return
-
diff := patchutil.AsNiceDiff(formatPatch.Patch, base)
+
var diff types.NiceDiff
+
if formatPatch.CombinedPatchRaw != "" {
+
diff = patchutil.AsNiceDiff(formatPatch.CombinedPatchRaw, base)
+
} else {
+
diff = patchutil.AsNiceDiff(formatPatch.FormatPatchRaw, base)
+
}
repoinfo := f.RepoInfo(user)
+20 -4
knotserver/xrpc/repo_compare.go
···
"fmt"
"net/http"
+
"github.com/bluekeyes/go-gitdiff/gitdiff"
"tangled.org/core/knotserver/git"
"tangled.org/core/types"
xrpcerr "tangled.org/core/xrpc/errors"
···
return
}
+
var combinedPatch []*gitdiff.File
+
var combinedPatchRaw string
+
// we need the combined patch
+
if len(formatPatch) >= 2 {
+
diffTree, err := gr.DiffTree(commit1, commit2)
+
if err != nil {
+
x.Logger.Error("error comparing revisions", "msg", err.Error())
+
} else {
+
combinedPatch = diffTree.Diff
+
combinedPatchRaw = diffTree.Patch
+
}
+
}
+
response := types.RepoFormatPatchResponse{
-
Rev1: commit1.Hash.String(),
-
Rev2: commit2.Hash.String(),
-
FormatPatch: formatPatch,
-
Patch: rawPatch,
+
Rev1: commit1.Hash.String(),
+
Rev2: commit2.Hash.String(),
+
FormatPatch: formatPatch,
+
FormatPatchRaw: rawPatch,
+
CombinedPatch: combinedPatch,
+
CombinedPatchRaw: combinedPatchRaw,
}
writeJson(w, response)
+7 -5
types/repo.go
···
package types
import (
+
"github.com/bluekeyes/go-gitdiff/gitdiff"
"github.com/go-git/go-git/v5/plumbing/object"
)
···
}
type RepoFormatPatchResponse struct {
-
Rev1 string `json:"rev1,omitempty"`
-
Rev2 string `json:"rev2,omitempty"`
-
FormatPatch []FormatPatch `json:"format_patch,omitempty"`
-
MergeBase string `json:"merge_base,omitempty"` // deprecated
-
Patch string `json:"patch,omitempty"`
+
Rev1 string `json:"rev1,omitempty"`
+
Rev2 string `json:"rev2,omitempty"`
+
FormatPatch []FormatPatch `json:"format_patch,omitempty"`
+
FormatPatchRaw string `json:"patch,omitempty"`
+
CombinedPatch []*gitdiff.File `json:"combined_patch,omitempty"`
+
CombinedPatchRaw string `json:"combined_patch_raw,omitempty"`
}
type RepoTreeResponse struct {