appview,knotserver: produce combined patch in comparisons

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 <[e-mail]>

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, "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}, nil
 }
 

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
 }

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

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)

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)
 

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)

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 {