shithub: furgit

Download patch

ref: 7d6c80d1540f8bc06bc6a31086b3723bed7c95db
parent: 236a666b716cac51872edf93f14ffe6b553f259b
author: Runxi Yu <me@runxiyu.org>
date: Sat Mar 7 11:43:13 EST 2026

mergebase: No fake iterator API first and idempotency

--- a/mergebase/base.go
+++ b/mergebase/base.go
@@ -16,28 +16,15 @@
 	right objectid.ObjectID,
 ) (objectid.ObjectID, bool, error) {
 	query := Query(store, graph, left, right)
-	seq := query.Seq()
 
-	var (
-		first objectid.ObjectID
-		ok    bool
-	)
-
-	seq(func(id objectid.ObjectID) bool {
-		first = id
-		ok = true
-
-		return false
-	})
-
-	err := query.Err()
+	bases, err := query.All()
 	if err != nil {
 		return objectid.ObjectID{}, false, err
 	}
 
-	if !ok {
+	if len(bases) == 0 {
 		return objectid.ObjectID{}, false, nil
 	}
 
-	return first, true, nil
+	return bases[0], true, nil
 }
--- a/mergebase/compute.go
+++ b/mergebase/compute.go
@@ -8,14 +8,25 @@
 	"codeberg.org/lindenii/furgit/objectid"
 )
 
-func (query *Bases) compute() ([]objectid.ObjectID, error) {
+// All returns all merge bases in Git's merge-base --all order.
+func (query *Bases) All() ([]objectid.ObjectID, error) {
+	if query.computed {
+		return slices.Clone(query.bases), query.err
+	}
+
+	query.computed = true
+
 	leftCommit, err := peel.ToCommit(query.store, query.left)
 	if err != nil {
+		query.err = err
+
 		return nil, err
 	}
 
 	rightCommit, err := peel.ToCommit(query.store, query.right)
 	if err != nil {
+		query.err = err
+
 		return nil, err
 	}
 
@@ -23,16 +34,22 @@
 
 	leftIdx, err := ctx.ResolveOID(leftCommit)
 	if err != nil {
+		query.err = err
+
 		return nil, err
 	}
 
 	rightIdx, err := ctx.ResolveOID(rightCommit)
 	if err != nil {
+		query.err = err
+
 		return nil, err
 	}
 
 	candidates, err := commitquery.MergeBases(ctx, leftIdx, rightIdx)
 	if err != nil {
+		query.err = err
+
 		return nil, err
 	}
 
@@ -47,10 +64,10 @@
 		}
 	})
 
-	out := make([]objectid.ObjectID, 0, len(candidates))
+	query.bases = make([]objectid.ObjectID, 0, len(candidates))
 	for _, idx := range candidates {
-		out = append(out, ctx.ID(idx))
+		query.bases = append(query.bases, ctx.ID(idx))
 	}
 
-	return out, nil
+	return slices.Clone(query.bases), nil
 }
--- a/mergebase/integration_test.go
+++ b/mergebase/integration_test.go
@@ -35,12 +35,11 @@
 		store := testRepo.OpenObjectStore(t)
 
 		query := mergebase.Query(store, nil, left, tag)
-		got := oidSetFromSeq(query.Seq())
-
-		err := query.Err()
+		all, err := query.All()
 		if err != nil {
-			t.Fatalf("query.Err(): %v", err)
+			t.Fatalf("query.All(): %v", err)
 		}
+		got := oidSetFromSlice(all)
 
 		want := gitMergeBaseAllSet(t, testRepo, left, tag)
 		if !maps.Equal(got, want) {
@@ -77,12 +76,11 @@
 		store := testRepo.OpenObjectStore(t)
 
 		query := mergebase.Query(store, nil, left, right)
-		got := oidSetFromSeq(query.Seq())
-
-		err := query.Err()
+		all, err := query.All()
 		if err != nil {
-			t.Fatalf("query.Err(): %v", err)
+			t.Fatalf("query.All(): %v", err)
 		}
+		got := oidSetFromSlice(all)
 
 		want := gitMergeBaseAllSet(t, testRepo, left, right)
 		if !maps.Equal(got, want) {
@@ -137,12 +135,11 @@
 		graph := testRepo.OpenCommitGraph(t)
 
 		query := mergebase.Query(store, graph, left, right)
-		got := oidSetFromSeq(query.Seq())
-
-		err := query.Err()
+		all, err := query.All()
 		if err != nil {
-			t.Fatalf("query.Err(): %v", err)
+			t.Fatalf("query.All(): %v", err)
 		}
+		got := oidSetFromSlice(all)
 
 		want := gitMergeBaseAllSet(t, testRepo, left, right)
 		if !maps.Equal(got, want) {
@@ -232,15 +229,13 @@
 	})
 }
 
-// oidSetFromSeq collects one object ID sequence into a set.
-func oidSetFromSeq(seq func(func(objectid.ObjectID) bool)) map[objectid.ObjectID]struct{} {
+// oidSetFromSlice collects one object ID slice into a set.
+func oidSetFromSlice(ids []objectid.ObjectID) map[objectid.ObjectID]struct{} {
 	out := make(map[objectid.ObjectID]struct{})
 
-	seq(func(id objectid.ObjectID) bool {
+	for _, id := range ids {
 		out[id] = struct{}{}
-
-		return true
-	})
+	}
 
 	return out
 }
--- a/mergebase/mergebase.go
+++ b/mergebase/mergebase.go
@@ -7,7 +7,7 @@
 	"codeberg.org/lindenii/furgit/objectstore"
 )
 
-// Bases is one iterator merge-base query.
+// Bases is one merge-base query over two commit roots.
 type Bases struct {
 	store objectstore.Store
 	graph *commitgraphread.Reader
@@ -14,6 +14,7 @@
 	left  objectid.ObjectID
 	right objectid.ObjectID
 
-	seqUsed bool
-	err     error
+	computed bool
+	bases    []objectid.ObjectID
+	err      error
 }
--- a/mergebase/query.go
+++ b/mergebase/query.go
@@ -6,7 +6,7 @@
 	"codeberg.org/lindenii/furgit/objectstore"
 )
 
-// Query builds one single-use merge-base query over two commit roots.
+// Query builds one merge-base query over two commit roots.
 //
 // Both inputs are peeled through annotated tags before commit traversal.
 func Query(
--- a/mergebase/seq.go
+++ /dev/null
@@ -1,47 +1,0 @@
-package mergebase
-
-import (
-	"errors"
-	"iter"
-
-	"codeberg.org/lindenii/furgit/objectid"
-)
-
-// Seq returns the merge-base sequence. It is single-use.
-func (query *Bases) Seq() iter.Seq[objectid.ObjectID] {
-	if query.seqUsed {
-		return func(yield func(objectid.ObjectID) bool) {
-			_ = yield
-
-			if query.err == nil {
-				query.err = errors.New("mergebase: sequence already consumed")
-			}
-		}
-	}
-
-	query.seqUsed = true
-
-	return func(yield func(objectid.ObjectID) bool) {
-		if query.err != nil {
-			return
-		}
-
-		bases, err := query.compute()
-		if err != nil {
-			query.err = err
-
-			return
-		}
-
-		for _, id := range bases {
-			if !yield(id) {
-				return
-			}
-		}
-	}
-}
-
-// Err returns the terminal error, if any, once Seq has been consumed.
-func (query *Bases) Err() error {
-	return query.err
-}
--- a/mergebase/unit_test.go
+++ b/mergebase/unit_test.go
@@ -38,19 +38,6 @@
 	return fmt.Appendf(nil, "object %s\ntype %s\ntag t\n\nmsg\n", target.String(), targetName)
 }
 
-// collectSeq collects one object ID sequence into a slice.
-func collectSeq(seq func(func(objectid.ObjectID) bool)) []objectid.ObjectID {
-	var out []objectid.ObjectID
-
-	seq(func(id objectid.ObjectID) bool {
-		out = append(out, id)
-
-		return true
-	})
-
-	return out
-}
-
 // toSet converts one slice of object IDs into a set.
 func toSet(ids []objectid.ObjectID) map[objectid.ObjectID]struct{} {
 	set := make(map[objectid.ObjectID]struct{}, len(ids))
@@ -97,11 +84,9 @@
 		right := store.AddObject(objecttype.TypeCommit, commitBody(tree, left))
 
 		query := mergebase.Query(store, nil, left, right)
-		got := collectSeq(query.Seq())
-
-		err := query.Err()
+		got, err := query.All()
 		if err != nil {
-			t.Fatalf("query.Err(): %v", err)
+			t.Fatalf("query.All(): %v", err)
 		}
 
 		if !slices.Equal(got, []objectid.ObjectID{left}) {
@@ -145,11 +130,9 @@
 		tag := store.AddObject(objecttype.TypeTag, tagBody(right, objecttype.TypeCommit))
 
 		query := mergebase.Query(store, nil, left, tag)
-		got := collectSeq(query.Seq())
-
-		err := query.Err()
+		got, err := query.All()
 		if err != nil {
-			t.Fatalf("query.Err(): %v", err)
+			t.Fatalf("query.All(): %v", err)
 		}
 
 		if !slices.Equal(got, []objectid.ObjectID{base}) {
@@ -196,12 +179,11 @@
 		right := store.AddObject(objecttype.TypeCommit, commitBody(rightTree, base2, base1))
 
 		query := mergebase.Query(store, nil, left, right)
-		got := toSet(collectSeq(query.Seq()))
-
-		err := query.Err()
+		all, err := query.All()
 		if err != nil {
-			t.Fatalf("query.Err(): %v", err)
+			t.Fatalf("query.All(): %v", err)
 		}
+		got := toSet(all)
 
 		want := map[objectid.ObjectID]struct{}{base1: {}, base2: {}}
 		if !maps.Equal(got, want) {
@@ -244,11 +226,9 @@
 		right := store.AddObject(objecttype.TypeCommit, commitBody(rightTree))
 
 		query := mergebase.Query(store, nil, left, right)
-		got := collectSeq(query.Seq())
-
-		err := query.Err()
+		got, err := query.All()
 		if err != nil {
-			t.Fatalf("query.Err(): %v", err)
+			t.Fatalf("query.All(): %v", err)
 		}
 
 		if len(got) != 0 {
@@ -281,9 +261,7 @@
 		tagToTree := store.AddObject(objecttype.TypeTag, tagBody(tree, objecttype.TypeTree))
 
 		query := mergebase.Query(store, nil, commit, tagToTree)
-		_ = collectSeq(query.Seq())
-
-		err := query.Err()
+		_, err := query.All()
 		if err == nil {
 			t.Fatal("expected error")
 		}
@@ -299,7 +277,7 @@
 	})
 }
 
-func TestQuerySeqSingleUse(t *testing.T) {
+func TestQueryAllIsRepeatable(t *testing.T) {
 	t.Parallel()
 
 	testgit.ForEachAlgorithm(t, func(t *testing.T, algo objectid.Algorithm) { //nolint:thelper
@@ -316,20 +294,33 @@
 
 		query := mergebase.Query(store, nil, left, right)
 
-		_ = collectSeq(query.Seq())
-		again := collectSeq(query.Seq())
+		first, err := query.All()
+		if err != nil {
+			t.Fatalf("query.All() first call: %v", err)
+		}
 
-		if len(again) != 0 {
-			t.Fatalf("second Seq() unexpectedly yielded %v", again)
+		again, err := query.All()
+		if err != nil {
+			t.Fatalf("query.All() second call: %v", err)
 		}
 
-		err := query.Err()
-		if err == nil {
-			t.Fatal("expected error after second Seq()")
+		if !slices.Equal(again, first) {
+			t.Fatalf("second All()=%v, want %v", again, first)
 		}
 
-		if err.Error() != "mergebase: sequence already consumed" {
-			t.Fatalf("unexpected error: %v", err)
+		if len(first) == 0 {
+			t.Fatal("first All() unexpectedly returned no results")
+		}
+
+		first[0] = objectid.ObjectID{}
+
+		third, err := query.All()
+		if err != nil {
+			t.Fatalf("query.All() third call: %v", err)
+		}
+
+		if third[0] == (objectid.ObjectID{}) {
+			t.Fatal("query.All() exposed internal slice state")
 		}
 	})
 }
--