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

···
 		return err
 	})
 
+	// 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_id, repo_at, round_number, patch, source_rev)
+		insert into pull_submissions (pull_id, repo_at, round_number, patch, combined, source_rev)
 		values (?, ?, ?, ?, ?)
-	`, pull.PullId, pull.RepoAt, 0, pull.Submissions[0].Patch, pull.Submissions[0].SourceRev)
+	`, pull.PullId, pull.RepoAt, 0, pull.Submissions[0].Patch, pull.Submissions[0].Combined, pull.Submissions[0].SourceRev)
 	return err
 }
 
···
 	inClause := strings.TrimSuffix(strings.Repeat("?, ", len(pulls)), ", ")
 	submissionsQuery := fmt.Sprintf(`
 		select
-			id, pull_id, round_number, patch, created, source_rev
+			id, pull_id, round_number, patch, combined, created, source_rev
 		from
 			pull_submissions
 		where
···
 
 	for submissionsRows.Next() {
 		var s models.PullSubmission
-		var sourceRev sql.NullString
+		var sourceRev, combined sql.NullString
 		var createdAt string
 		err := submissionsRows.Scan(
 			&s.ID,
 			&s.PullId,
 			&s.RoundNumber,
 			&s.Patch,
+			&combined,
 			&createdAt,
 			&sourceRev,
 		)
···
 
 		if sourceRev.Valid {
 			s.SourceRev = sourceRev.String
+		}
+
+		if combined.Valid {
+			s.Combined = combined.String
 		}
 
 		if p, ok := pulls[s.PullId]; ok {
···
 
 	submissionsQuery := `
 		select
-			id, pull_id, repo_at, round_number, patch, created, source_rev
+			id, pull_id, repo_at, round_number, patch, combined, created, source_rev
 		from
 			pull_submissions
 		where
···
 	for submissionsRows.Next() {
 		var submission models.PullSubmission
 		var submissionCreatedStr string
-		var submissionSourceRev sql.NullString
+		var submissionSourceRev, submissionCombined sql.NullString
 		err := submissionsRows.Scan(
 			&submission.ID,
 			&submission.PullId,
 			&submission.RepoAt,
 			&submission.RoundNumber,
 			&submission.Patch,
+			&submissionCombined,
 			&submissionCreatedStr,
 			&submissionSourceRev,
 		)
···
 
 		if submissionSourceRev.Valid {
 			submission.SourceRev = submissionSourceRev.String
+		}
+
+		if submissionCombined.Valid {
+			submission.Combined = submissionCombined.String
 		}
 
 		submissionsMap[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_id, repo_at, round_number, patch, source_rev)
+		insert into pull_submissions (pull_id, repo_at, round_number, patch, combined, source_rev)
 		values (?, ?, ?, ?, ?)
 	`, pull.PullId, pull.RepoAt, newRoundNumber, newPatch, sourceRev)
 

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 patches
+}
+
+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)
 	resubmitResult := pages.Unknown
 	if user != nil && user.Did == pull.OwnerDid {
···
 		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 {