Remove recursive call to 'independents'

Signed-off-by: David Pordomingo <[email protected]>

Author: dpordomingo

merge_base.go

@@ -90,52 +90,53 @@ func ancestorsIndex(excluded, starting *object.Commit) (map[plumbing.Hash]struct
 // It mimics the behavior of `git merge-base --independent commit...`.
 func Independents(commits []*object.Commit) ([]*object.Commit, error) {
 	// use sortedByCommitDateDesc strategy
-	cleaned := sortByCommitDateDesc(commits...)
-	cleaned = removeDuplicated(cleaned)
-	return independents(cleaned, map[plumbing.Hash]bool{}, 0)
-}
+	candidates := sortByCommitDateDesc(commits...)
+	candidates = removeDuplicated(candidates)
 
-func independents(
-	candidates []*object.Commit,
-	excluded map[plumbing.Hash]bool,
-	start int,
-) ([]*object.Commit, error) {
-	if len(candidates) == 1 {
+	seen := map[plumbing.Hash]struct{}{}
+	var isLimit object.CommitFilter = func(commit *object.Commit) bool {
+		_, ok := seen[commit.Hash]
+		return ok
+	}
+
+	if len(candidates) < 2 {
 		return candidates, nil
 	}
 
-	res := candidates
-	for i := start; i < len(candidates); i++ {
-		from := candidates[i]
-		others := remove(res, from)
-		fromHistoryIter := object.NewCommitIterBSF(from, excluded, nil)
+	pos := 0
+	for {
+		from := candidates[pos]
+		others := remove(candidates, from)
+		fromHistoryIter := object.NewFilterCommitIter(from, nil, &isLimit)
 		err := fromHistoryIter.ForEach(func(fromAncestor *object.Commit) error {
 			for _, other := range others {
 				if fromAncestor.Hash == other.Hash {
-					res = remove(res, other)
+					candidates = remove(candidates, other)
 					others = remove(others, other)
 				}
 			}
 
-			if len(res) == 1 {
+			if len(candidates) == 1 {
 				return storer.ErrStop
 			}
 
-			excluded[fromAncestor.Hash] = true
+			seen[fromAncestor.Hash] = struct{}{}
 			return nil
 		})
 
 		if err != nil {
 			return nil, err
 		}
 
-		if len(res) < len(candidates) {
-			return independents(res, excluded, indexOf(res, from)+1)
+		nextPos := indexOf(candidates, from) + 1
+		if nextPos >= len(candidates) {
+			break
 		}
 
+		pos = nextPos
 	}
 
-	return res, nil
+	return candidates, nil
 }
 
 // sortByCommitDateDesc returns the passed commits, sorted by `committer.When desc`

merge_base_test.go

@@ -39,7 +39,7 @@ func alphabeticSortCommits(commits []*object.Commit) {
 
 The following tests consider this history having two root commits: V and W
 
-V---o---M----AB----A---CD1--R---C--------S-------------------Q < master
+V---o---M----AB----A---CD1--P---C--------S-------------------Q < master
                \         \ /            /                   /
                 \         X            GQ1---G < feature   /
                  \       / \          /     /             /
@@ -57,14 +57,16 @@ passed  merge-base
 
 Independents
 ----------------------------
-candidates        result
- A                 A           Only one commit returns it
- A, A, A           A           Repeated commits are ignored
- A, A, M, M, N     A, N        M is reachable from A, no it is not independent
- CD1, CD2, M, N    CD1, CD2    M and N are reachable from CD2, so they're not
- C, G, dev, M, N   C, G, dev   M and N are reachable from G, so they're not
- A, A^, A, N, N^   A, N        A^ and N^ are rechable from A and N
-
+candidates           result
+ A                    A           Only one commit returns it
+ A, A, A              A           Repeated commits are ignored
+ A, A, M, M, N        A, N        M is reachable from A, so it is not independent
+ S, G, P              S, G        P is reachable from S, so it is not independent
+ CD1, CD2, M, N       CD1, CD2    M and N are reachable from CD2, so they're not
+ C, G, dev, M, N      C, G, dev   M and N are reachable from G, so they're not
+ C, D, M, N           C, D        M and N are reachable from C, so they're not
+ A, A^, A, N, N^      A, N        A^ and N^ are rechable from A and N
+ A^^^, A^, A^^, A, N  A, N        A^^^, A^^ and A^ are rechable from A, so they're not
 
 IsAncestor
 ----------------------------
@@ -246,14 +248,22 @@ func (s *orphansSuite) TestIndependentAcrossCrossMerges(c *C) {
 	s.AssertIndependents(c, revs, expectedRevs)
 }
 
-// TestIndependentChangingOrder validates that Independents works well
+// TestIndependentChangingOrderRepetition validates that Independents works well
 // when the order and repetition is tricky (A, A^, A, N, N^ -> A, N)
-func (s *orphansSuite) TestIndependentChangingOrder(c *C) {
+func (s *orphansSuite) TestIndependentChangingOrderRepetition(c *C) {
 	revs := []string{"A", "A^", "A", "N", "N^"}
 	expectedRevs := []string{"A", "N"}
 	s.AssertIndependents(c, revs, expectedRevs)
 }
 
+// TestIndependentChangingOrder validates that Independents works well
+// when the order is tricky (A^^^, A^, A^^, A, N -> A, N)
+func (s *orphansSuite) TestIndependentChangingOrder(c *C) {
+	revs := []string{"A^^^", "A^", "A^^", "A", "N"}
+	expectedRevs := []string{"A", "N"}
+	s.AssertIndependents(c, revs, expectedRevs)
+}
+
 // TestAncestor validates that IsAncestor returns true if walking from first
 // commit, through its parents, it can be reached the second ( A^^, A -> true )
 func (s *orphansSuite) TestAncestor(c *C) {