[MER-3309] Add an extra merge commit detection based on commit messages

When a PR gets closed by "Close #123", usually GitHub marks the PR to be
closed by certain commit. However, this mechanism is not reliable, and
sometimes GitHub doesn't mark it so and just close it. This breaks the
merge commit detection since av-cli doesn't have a way to know it from
GitHub API.

By using git-log, we can figure out whether there's actually a commit
that closes the PRs with this comment. This adds that extra detection
method.

In order to make this work, it moves the timing of git-fetch. This is
needed in order for a local repository to have commits in the upstream
to check the commit messages.

internal/actions/sync_branch.go

@@ -61,6 +61,10 @@ func SyncBranch(
 		}
 	} else {
 		if opts.Fetch {
+			fetchHead, err := fetchUpstreamCommits(repo, tx, branch)
+			if err != nil {
+				return nil, err
+			}
 			update, err := UpdatePullRequestState(ctx, client, tx, branch.Name)
 			if err != nil {
 				_, _ = fmt.Fprint(os.Stderr, colors.Failure("      - error: ", err.Error()), "\n")
@@ -77,6 +81,14 @@ func SyncBranch(
 						" (create one with ", colors.CliCmd("av pr create"),
 					" or ", colors.CliCmd("av stack submit"), ")\n",
 				)
+			} else if branch.PullRequest.State == githubv4.PullRequestStateClosed && branch.MergeCommit == "" {
+				branch.MergeCommit, err = findMergeCommitWithGitLog(repo, fetchHead, branch)
+				if err != nil {
+					return nil, errors.Wrap(err, "failed to find the merge commit from git-log")
+				}
+				if branch.MergeCommit != "" {
+					tx.SetBranch(branch)
+				}
 			}
 		}
 
@@ -243,15 +255,6 @@ func syncBranchRebase(
 			"  - rebasing ", colors.UserInput(branch.Name),
 			" on top of merge commit ", colors.UserInput(short), "\n",
 		)
-		if opts.Fetch {
-			if _, err := repo.Git("fetch", "origin", origParentBranch.MergeCommit); err != nil {
-				return nil, errors.WrapIff(
-					err,
-					"failed to fetch merge commit %q from origin",
-					short,
-				)
-			}
-		}
 
 		// Replay the commits from this branch directly onto the merge commit.
 		// The HEAD of trunk might have moved forward since this, but this is
@@ -420,6 +423,36 @@ func syncBranchRebase(
 	return nil, nil
 }
 
+func fetchUpstreamCommits(repo *git.Repo, tx meta.WriteTx, branch meta.Branch) (string, error) {
+	parent := branch
+	for parent.Parent.Name != "" {
+		parent, _ = tx.Branch(parent.Parent.Name)
+	}
+
+	if _, err := repo.Git("fetch", "origin", parent.Name); err != nil {
+		return "", errors.WrapIff(err, "failed to fetch %q from origin", parent.Name)
+	}
+	commitHash, err := repo.RevParse(&git.RevParse{Rev: "FETCH_HEAD"})
+	if err != nil {
+		return "", errors.WrapIff(err, "failed to read the commit hash of %q", parent.Name)
+	}
+	return commitHash, nil
+}
+
+// findMergeCommitWithGitLog looks for the merge commit for a specified PR.
+//
+// Usually, GitHub should set which commit closes a pull request. This is known to be not that
+// reliable. When we cannot find a merge commit for a closed PR, we try to find if any commit in the
+// upstream closes the pull request as a fallback.
+func findMergeCommitWithGitLog(repo *git.Repo, upstreamCommit string, branch meta.Branch) (string, error) {
+	cis, err := repo.Log(git.LogOpts{RevisionRange: branch.Name + ".." + upstreamCommit})
+	if err != nil {
+		return "", err
+	}
+	closedPRs := git.FindClosedPRs(cis)
+	return closedPRs[branch.PullRequest.Number], nil
+}
+
 func syncBranchContinue(
 	ctx context.Context,
 	repo *git.Repo,

internal/git/log.go

@@ -0,0 +1,93 @@
+package git
+
+import (
+	"bufio"
+	"bytes"
+	"io"
+	"regexp"
+	"strconv"
+	"strings"
+
+	"github.com/sirupsen/logrus"
+)
+
+var (
+	closeCommitPattern = regexp.MustCompile(`(?i)\b(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)\W+#(\d+)\b`)
+)
+
+type LogOpts struct {
+	// RevisionRange is the range of the commits specified by the format described in
+	// git-log(1).
+	RevisionRange string
+}
+
+// Log returns a list of commits specified by the range.
+func (r *Repo) Log(opts LogOpts) ([]*CommitInfo, error) {
+	res, err := r.Run(&RunOpts{
+		Args:      []string{"log", "--format=%H%x00%h%x00%s%x00%b%x00", opts.RevisionRange},
+		ExitError: true,
+	})
+	if err != nil {
+		return nil, err
+	}
+	logrus.WithFields(logrus.Fields{"range": opts.RevisionRange}).Debug("got git-log")
+
+	rd := bufio.NewReader(bytes.NewBuffer(res.Stdout))
+	var ret []*CommitInfo
+	for {
+		ci, err := readLogEntry(rd)
+		if err != nil {
+			if err == io.EOF {
+				break
+			}
+			return nil, err
+		}
+		ret = append(ret, ci)
+	}
+	return ret, nil
+}
+
+func readLogEntry(rd *bufio.Reader) (*CommitInfo, error) {
+	commitHash, err := rd.ReadString('\x00')
+	if err != nil {
+		return nil, err
+	}
+	abbrevHash, err := rd.ReadString('\x00')
+	if err != nil {
+		return nil, err
+	}
+	subject, err := rd.ReadString('\x00')
+	if err != nil {
+		return nil, err
+	}
+	body, err := rd.ReadString('\x00')
+	if err != nil {
+		return nil, err
+	}
+	return &CommitInfo{
+		Hash:      strings.TrimSpace(trimNUL(commitHash)),
+		ShortHash: strings.TrimSpace(trimNUL(abbrevHash)),
+		Subject:   trimNUL(subject),
+		Body:      trimNUL(body),
+	}, nil
+}
+
+func trimNUL(s string) string {
+	return strings.Trim(s, "\x00")
+}
+
+// FindClosedPRs finds the "closes #123" instructions from the commit messages. This returns a PR
+// number to commit hash mapping.
+//
+// See https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
+func FindClosedPRs(cis []*CommitInfo) map[int64]string {
+	ret := map[int64]string{}
+	for _, ci := range cis {
+		matches := closeCommitPattern.FindAllStringSubmatch(ci.Body, -1)
+		for _, match := range matches {
+			prNum, _ := strconv.ParseInt(match[1], 10, 64)
+			ret[prNum] = ci.Hash
+		}
+	}
+	return ret
+}

internal/git/log_test.go

@@ -0,0 +1,51 @@
+package git_test
+
+import (
+	"testing"
+
+	"github.com/aviator-co/av/internal/git"
+	"github.com/aviator-co/av/internal/git/gittest"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRepo_Log(t *testing.T) {
+	repo := gittest.NewTempRepo(t)
+	c1 := gittest.CommitFile(t, repo, "file", []byte("first commit\n"), gittest.WithMessage("commit 1\n\ncommit 1 body"))
+	c2 := gittest.CommitFile(t, repo, "file", []byte("first commit\nsecond commit\n"), gittest.WithMessage("commit 2\n\ncommit 2 body"))
+
+	cis, err := repo.Log(git.LogOpts{RevisionRange: c1 + "^1.." + c2})
+	assert.NoError(t, err)
+	assert.Equal(t, []*git.CommitInfo{
+		{
+			Hash:      c2,
+			ShortHash: c2[:7],
+			Subject:   "commit 2",
+			Body:      "commit 2 body\n",
+		},
+		{
+			Hash:      c1,
+			ShortHash: c1[:7],
+			Subject:   "commit 1",
+			Body:      "commit 1 body\n",
+		},
+	}, cis)
+}
+
+func TestFindClosedPRs(t *testing.T) {
+	cis := []*git.CommitInfo{
+		{
+			Hash: "fake_1",
+			Body: "some comments. close #123. fixed #433",
+		},
+		{
+			Hash: "fake_2",
+			Body: "some other comments.\nfix #234",
+		},
+	}
+
+	assert.Equal(t, map[int64]string{
+		123: "fake_1",
+		234: "fake_2",
+		433: "fake_1",
+	}, git.FindClosedPRs(cis))
+}