Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 29 additions & 32 deletions internal/gh/ghui/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ghui

import (
"context"
"maps"
"strings"

"charm.land/bubbles/v2/spinner"
Expand All @@ -15,7 +16,6 @@ import (
"github.com/aviator-co/av/internal/utils/stackutils"
"github.com/go-git/go-git/v6/config"
"github.com/go-git/go-git/v6/plumbing"
"github.com/go-git/go-git/v6/plumbing/object"
)

func NewGitHubFetchModel(
Expand Down Expand Up @@ -203,12 +203,15 @@ func (vm *GitHubFetchModel) runGitHubAPIFetch() tea.Msg {
}

func (vm *GitHubFetchModel) updateMergeCommitsFromCommitMessage() tea.Msg {
trunkRefs := map[plumbing.ReferenceName]bool{}
ctx := context.Background()
trunkBranches := map[plumbing.ReferenceName][]string{}
for _, br := range vm.targetBranches {
avbr, _ := vm.db.ReadTx().Branch(br.Short())
if avbr.Parent.Trunk {
trunkRefs[plumbing.NewBranchReferenceName(avbr.Parent.Name)] = true
trunk, ok := meta.Trunk(vm.db.ReadTx(), br.Short())
if !ok {
continue
}
trunkRef := plumbing.NewBranchReferenceName(trunk)
trunkBranches[trunkRef] = append(trunkBranches[trunkRef], br.Short())
}

repo := vm.repo.GoGitRepo()
Expand All @@ -218,15 +221,8 @@ func (vm *GitHubFetchModel) updateMergeCommitsFromCommitMessage() tea.Msg {
}
remoteConfig := remote.Config()

// For each trunk commits, look first 10000 commits to find the recently merge
// commit. If we want to do this properly, we should look into the commits between
// tip of the trunks and the merge bases of the branches. However, without
// pre-calculated generation numbers (which is available in commit-graph), this
// anyway would require a full history traversal.
//
// This is anyway a best-effort approach, so we just look into the first 10000.
mergedPRs := map[int64]plumbing.Hash{}
for trunkRef := range trunkRefs {
mergedPRs := map[int64]string{}
for trunkRef, branches := range trunkBranches {
rtb := mapToRemoteTrackingBranch(remoteConfig, trunkRef)
if rtb == nil {
// No remote tracking branch. Skip.
Expand All @@ -236,25 +232,26 @@ func (vm *GitHubFetchModel) updateMergeCommitsFromCommitMessage() tea.Msg {
if err != nil {
return errors.Errorf("failed to get reference %q: %v", rtb, err)
}
c, err := repo.CommitObject(ref.Hash())
tip := ref.Hash().String()

// A commit that closes a PR of one of the target branches cannot be
// older than the point where their stacks forked from the trunk, so
// only that range of the trunk history needs to be scanned. The
// commit count cap is a best-effort safety valve for when the fork
// point is very old (or cannot be determined).
revisionRange := []string{"--max-count=10000", tip}
mergeBaseArgs := append([]string{"merge-base", "--octopus", tip}, branches...)
if base, err := vm.repo.Git(ctx, mergeBaseArgs...); err == nil {
if base == tip {
continue
}
revisionRange = []string{"--max-count=10000", base + ".." + tip}
}
Comment on lines +242 to +249

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using git merge-base --octopus can return multiple merge bases separated by newlines if there is no single common ancestor. Since vm.repo.Git only trims leading/trailing whitespace, base will contain internal newlines (e.g., "sha1\nsha2"). Constructing base + ".." + tip will result in "sha1\nsha2..tip", which is an invalid revision range and will cause git log to fail with a fatal error.

Instead of using git merge-base to find a single fork point, we can leverage Git's native revision exclusion syntax. Passing ^branch for each branch to git log (e.g., git log tip ^branch1 ^branch2) will automatically exclude all commits reachable from those branches. This is extremely robust, handles multiple branches and complex histories perfectly, and completely eliminates the extra git merge-base process call.

		revisionRange := []string{"--max-count=10000", tip}
		for _, branch := range branches {
			revisionRange = append(revisionRange, "^"+branch)
		}

cis, err := vm.repo.Log(ctx, git.LogOpts{RevisionRange: revisionRange})
if err != nil {
return errors.Errorf("failed to get commit %q: %v", ref.Hash(), err)
return errors.Errorf("failed to read the commit history of %q: %v", rtb, err)
}
visited := 0
_ = object.NewCommitPreorderIter(c, nil, nil).ForEach(func(c *object.Commit) error {
m := git.FindClosesPullRequestComments([]*git.CommitInfo{{
Hash: c.Hash.String(),
Body: c.Message,
}})
for pr := range m {
mergedPRs[pr] = c.Hash
}
visited += 1
if visited >= 10000 {
return errors.New("stop")
}
return nil
})
maps.Copy(mergedPRs, git.FindClosesPullRequestComments(cis))
}
for _, br := range vm.targetBranches {
tx := vm.db.WriteTx()
Expand All @@ -265,7 +262,7 @@ func (vm *GitHubFetchModel) updateMergeCommitsFromCommitMessage() tea.Msg {
}
if avbr.PullRequest != nil && avbr.PullRequest.Number != 0 {
if hash, ok := mergedPRs[avbr.PullRequest.Number]; ok {
avbr.MergeCommit = hash.String()
avbr.MergeCommit = hash
tx.SetBranch(avbr)
}
}
Expand Down
67 changes: 60 additions & 7 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,25 +309,78 @@ func (r *Repo) DoesRefExist(ctx context.Context, ref string) (bool, error) {
return false, nil
}

func (r *Repo) LsRemote(ctx context.Context, remote string) (map[string]string, error) {
const tempFetchRefPrefix = "refs/av/fetch"

func tempFetchRef(ref string) string {
return tempFetchRefPrefix + "/" + strings.TrimPrefix(ref, "refs/")
}

// FetchRemoteRefs fetches the given fully-qualified refs from the remote in a
// single round trip and returns a map of ref name to commit SHA. Refs that do
// not exist on the remote (or otherwise fail to fetch) are omitted from the
// result.
//
// This is intentionally not implemented with ls-remote: ls-remote patterns
// cannot filter the server-side ref advertisement (only --heads/--tags can),
// so on remotes with a very large number of refs (e.g. GitHub repositories
// with hundreds of thousands of refs/pull/* refs) it downloads the entire
// advertisement, whereas git fetch sends protocol v2 ref-prefix filters so
// that the server advertises only the requested refs.
func (r *Repo) FetchRemoteRefs(
ctx context.Context,
remote string,
refs []string,
) (map[string]string, error) {
if len(refs) == 0 {
return nil, nil
}
args := []string{"fetch", "--no-tags", remote}
for _, ref := range refs {
args = append(args, "+"+ref+":"+tempFetchRef(ref))
}
if _, err := r.Git(ctx, args...); err != nil {
// A single missing ref fails the entire batch fetch. Fall back to
// fetching individually, omitting the refs that fail.
ret := make(map[string]string)
for _, ref := range refs {
sha, err := r.FetchRemoteRef(ctx, remote, ref)
if err != nil {
r.log.WithError(err).Debugf("failed to fetch remote ref %q", ref)
continue
}
ret[ref] = sha
}
return ret, nil
}
out, err := r.Run(ctx, &RunOpts{
Args: []string{"ls-remote", remote},
Args: []string{"for-each-ref", "--format=%(objectname)%00%(refname)", tempFetchRefPrefix},
ExitError: true,
})
if err != nil {
return nil, errors.Errorf("failed to get remote branches: %v", err)
return nil, errors.Errorf("failed to read the fetched refs: %v", err)
}
ret := make(map[string]string)
for _, line := range out.Lines() {
ss := strings.Split(line, "\t")
if len(ss) != 2 {
return nil, errors.Errorf("failed to parse the ls-remote output: %q", line)
objectName, refName, ok := strings.Cut(line, "\x00")
if !ok {
return nil, errors.Errorf("failed to parse the for-each-ref output: %q", line)
}
ret[ss[1]] = ss[0]
ret["refs/"+strings.TrimPrefix(refName, tempFetchRefPrefix+"/")] = objectName
_, _ = r.Git(ctx, "update-ref", "-d", refName)
}
return ret, nil
}

// FetchRemoteRef fetches a single fully-qualified ref from the remote and
// returns its commit SHA. See FetchRemoteRefs for why this uses git fetch
// rather than ls-remote.
func (r *Repo) FetchRemoteRef(ctx context.Context, remote string, ref string) (string, error) {
if _, err := r.Git(ctx, "fetch", "--no-tags", remote, ref); err != nil {
return "", err
}
return r.Git(ctx, "rev-parse", "FETCH_HEAD")
}

type CheckoutBranch struct {
// The name of the branch to checkout.
Name string
Expand Down
76 changes: 51 additions & 25 deletions internal/git/gitui/prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const (

reasonNoPullRequest = "PR not found."
reasonHasChild = "PR is already merged, but still have a child."
reasonPRHeadNotFound = "PR is already merged, but we cannot find which commit is merged."
reasonPRHeadFetchFailed = "PR is already merged, but failed to fetch the merged commit from the remote."
reasonPRHeadIsDifferent = "PR is already merged, but the local branch points to a different commit than the merged commit."
)

Expand Down Expand Up @@ -153,18 +153,16 @@ func (vm *PruneBranchModel) View() tea.View {
}

sb := strings.Builder{}
if len(vm.deleteCandidates) == 0 {
if vm.chooseNoPrune {
sb.WriteString(colors.SuccessStyle.Render("✓ Not deleting merged branches\n"))
} else if len(vm.deleteCandidates) == 0 {
sb.WriteString(colors.SuccessStyle.Render("✓ No merged branches to delete\n"))
} else if vm.askingForConfirmation {
sb.WriteString("Confirming the deletion of merged branches\n")
} else if vm.runningDeletion {
sb.WriteString(colors.ProgressStyle.Render(vm.spinner.View() + "Deleting merged branches...\n"))
} else if vm.done {
if vm.chooseNoPrune {
sb.WriteString(colors.SuccessStyle.Render("✓ Not deleting merged branches\n"))
} else {
sb.WriteString(colors.SuccessStyle.Render("✓ Deleted the merged branches\n"))
}
sb.WriteString(colors.SuccessStyle.Render("✓ Deleted the merged branches\n"))
}

if len(vm.noDeleteBranches) > 0 {
Expand Down Expand Up @@ -341,12 +339,15 @@ func (vm *PruneBranchModel) CheckoutInitialState() error {
}

func (vm *PruneBranchModel) calculateMergedBranches() tea.Msg {
remoteBranches, err := vm.repo.LsRemote(context.Background(), vm.repo.GetRemoteName())
if err != nil {
return err
if vm.chooseNoPrune {
return &PruneBranchProgress{candidateCalculationDone: true}
}
type mergedBranch struct {
branch plumbing.ReferenceName
prNumber int64
}
var noDeleteBranches []noDeleteBranch
var deleteCandidates []deleteCandidate
var mergedBranches []mergedBranch
for _, br := range vm.targetBranches {
avbr, _ := vm.db.ReadTx().Branch(br.Short())
if avbr.MergeCommit == "" {
Expand All @@ -366,26 +367,51 @@ func (vm *PruneBranchModel) calculateMergedBranches() tea.Msg {
)
continue
}
remoteHash, ok := remoteBranches[fmt.Sprintf("refs/pull/%d/head", avbr.PullRequest.Number)]
if !ok {
noDeleteBranches = append(
noDeleteBranches,
noDeleteBranch{branch: br, reason: reasonPRHeadNotFound},
)
continue
mergedBranches = append(
mergedBranches,
mergedBranch{branch: br, prNumber: avbr.PullRequest.Number},
)
}

var deleteCandidates []deleteCandidate
if len(mergedBranches) > 0 {
refs := make([]string, 0, len(mergedBranches))
for _, mb := range mergedBranches {
refs = append(refs, fmt.Sprintf("refs/pull/%d/head", mb.prNumber))
}
ref, err := vm.repo.GoGitRepo().Reference(br, true)
remoteRefs, err := vm.repo.FetchRemoteRefs(
context.Background(),
vm.repo.GetRemoteName(),
refs,
)
if err != nil {
return err
}
if ref.Hash().String() != remoteHash {
noDeleteBranches = append(
noDeleteBranches,
noDeleteBranch{branch: br, reason: reasonPRHeadIsDifferent},
for _, mb := range mergedBranches {
remoteHash, ok := remoteRefs[fmt.Sprintf("refs/pull/%d/head", mb.prNumber)]
if !ok {
noDeleteBranches = append(
noDeleteBranches,
noDeleteBranch{branch: mb.branch, reason: reasonPRHeadFetchFailed},
)
continue
}
ref, err := vm.repo.GoGitRepo().Reference(mb.branch, true)
if err != nil {
return err
}
if ref.Hash().String() != remoteHash {
noDeleteBranches = append(
noDeleteBranches,
noDeleteBranch{branch: mb.branch, reason: reasonPRHeadIsDifferent},
)
continue
}
deleteCandidates = append(
deleteCandidates,
deleteCandidate{branch: mb.branch, commit: ref.Hash()},
)
continue
}
deleteCandidates = append(deleteCandidates, deleteCandidate{branch: br, commit: ref.Hash()})
}
vm.noDeleteBranches = noDeleteBranches
vm.deleteCandidates = deleteCandidates
Expand Down
2 changes: 1 addition & 1 deletion internal/git/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func trimNUL(s string) string {
func FindClosesPullRequestComments(cis []*CommitInfo) map[int64]string {
ret := map[int64]string{}
for _, ci := range cis {
matches := closeCommitPattern.FindAllStringSubmatch(ci.Body, -1)
matches := closeCommitPattern.FindAllStringSubmatch(ci.Subject+"\n"+ci.Body, -1)
for _, match := range matches {
prNum, _ := strconv.ParseInt(match[1], 10, 64)
ret[prNum] = ci.Hash
Expand Down