diff --git a/internal/actions/sync_branch.go b/internal/actions/sync_branch.go index a268814b..5fc3f70b 100644 --- a/internal/actions/sync_branch.go +++ b/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, diff --git a/internal/git/log.go b/internal/git/log.go new file mode 100644 index 00000000..a8a28776 --- /dev/null +++ b/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 +} diff --git a/internal/git/log_test.go b/internal/git/log_test.go new file mode 100644 index 00000000..6dd3205d --- /dev/null +++ b/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)) +}