Skip to content

Commit c030b62

Browse files
authored
refac(git): use iterators for rev-list operations (#682)
1 parent 0ace6c2 commit c030b62

File tree

5 files changed

+97
-99
lines changed

5 files changed

+97
-99
lines changed

branch_split.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"go.abhg.dev/gs/internal/forge"
1212
"go.abhg.dev/gs/internal/git"
1313
"go.abhg.dev/gs/internal/silog"
14+
"go.abhg.dev/gs/internal/sliceutil"
1415
"go.abhg.dev/gs/internal/spice"
1516
"go.abhg.dev/gs/internal/spice/state"
1617
"go.abhg.dev/gs/internal/text"
@@ -78,10 +79,10 @@ func (cmd *branchSplitCmd) Run(
7879

7980
// Commits are in oldest-to-newest order,
8081
// with last commit being branch head.
81-
branchCommits, err := repo.ListCommitsDetails(ctx,
82+
branchCommits, err := sliceutil.CollectErr(repo.ListCommitsDetails(ctx,
8283
git.CommitRangeFrom(branch.Head).
8384
ExcludeFrom(branch.BaseHash).
84-
Reverse())
85+
Reverse()))
8586
if err != nil {
8687
return fmt.Errorf("list commits: %w", err)
8788
}

branch_track.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,6 @@ func (cmd *branchTrackCmd) Run(
6060
log.Debugf("Looking for base branch in range: %v..%v",
6161
trunkHash.Short(), branchHash.Short())
6262

63-
// Find all revisions between the current branch and the trunk branch
64-
// and check if we know any branches at those revisions.
65-
// If not, we'll use the trunk branch as the base.
66-
revs, err := repo.ListCommits(ctx,
67-
git.CommitRangeFrom(branchHash).ExcludeFrom(trunkHash))
68-
if err != nil {
69-
return fmt.Errorf("list commits: %w", err)
70-
}
71-
7263
trackedBranches, err := store.ListBranches(ctx)
7364
if err != nil {
7465
return fmt.Errorf("list tracked branches: %w", err)
@@ -94,8 +85,17 @@ func (cmd *branchTrackCmd) Run(
9485
return hash, nil
9586
}
9687

88+
// Find all revisions between the current branch and the trunk branch
89+
// and check if we know any branches at those revisions.
90+
// If not, we'll use the trunk branch as the base.
91+
revIter := repo.ListCommits(ctx,
92+
git.CommitRangeFrom(branchHash).ExcludeFrom(trunkHash))
9793
revLoop:
98-
for _, rev := range revs {
94+
for rev, err := range revIter {
95+
if err != nil {
96+
return fmt.Errorf("list commits: %w", err)
97+
}
98+
9999
for branchIdx, branchName := range trackedBranches {
100100
if branchName == cmd.Branch {
101101
continue

internal/git/integration_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"go.abhg.dev/gs/internal/git"
1010
"go.abhg.dev/gs/internal/git/gittest"
1111
"go.abhg.dev/gs/internal/silog/silogtest"
12+
"go.abhg.dev/gs/internal/sliceutil"
1213
"go.abhg.dev/gs/internal/text"
1314
)
1415

@@ -59,7 +60,7 @@ func TestIntegrationCommitListing(t *testing.T) {
5960

6061
t.Run("ListCommitsDetails", func(t *testing.T) {
6162
ctx := t.Context()
62-
commits, err := repo.ListCommitsDetails(ctx, git.CommitRangeFrom("HEAD").Limit(2))
63+
commits, err := sliceutil.CollectErr(repo.ListCommitsDetails(ctx, git.CommitRangeFrom("HEAD").Limit(2)))
6364
require.NoError(t, err)
6465

6566
// Normalize to UTC

internal/git/rev_list.go

Lines changed: 79 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,29 @@
11
package git
22

33
import (
4-
"bufio"
4+
"bytes"
55
"context"
66
"fmt"
7+
"iter"
78
"strconv"
89
"strings"
910
"time"
1011
)
1112

1213
// ListCommits returns a list of commits matched by the given range.
13-
func (r *Repository) ListCommits(ctx context.Context, commits CommitRange) ([]Hash, error) {
14-
lines, err := r.listCommitsFormat(ctx, commits, "")
15-
if err != nil {
16-
return nil, err
17-
}
14+
func (r *Repository) ListCommits(ctx context.Context, commits CommitRange) iter.Seq2[Hash, error] {
15+
return func(yield func(Hash, error) bool) {
16+
for line, err := range r.listCommitsFormat(ctx, commits, "") {
17+
if err != nil {
18+
yield(Hash(""), err)
19+
return
20+
}
1821

19-
hashes := make([]Hash, len(lines))
20-
for i, line := range lines {
21-
hashes[i] = Hash(line)
22+
if !yield(Hash(line), nil) {
23+
return
24+
}
25+
}
2226
}
23-
24-
return hashes, nil
2527
}
2628

2729
// CommitDetail contains information about a commit.
@@ -44,103 +46,96 @@ func (cd *CommitDetail) String() string {
4446
}
4547

4648
// ListCommitsDetails returns details about commits matched by the given range.
47-
func (r *Repository) ListCommitsDetails(ctx context.Context, commits CommitRange) ([]CommitDetail, error) {
48-
lines, err := r.listCommitsFormat(ctx, commits, "%H %h %at %s")
49-
if err != nil {
50-
return nil, err
51-
}
49+
func (r *Repository) ListCommitsDetails(ctx context.Context, commits CommitRange) iter.Seq2[CommitDetail, error] {
50+
return func(yield func(CommitDetail, error) bool) {
51+
for line, err := range r.listCommitsFormat(ctx, commits, "%H %h %at %s") {
52+
if err != nil {
53+
yield(CommitDetail{}, err)
54+
return
55+
}
5256

53-
details := make([]CommitDetail, len(lines))
54-
for i, line := range lines {
55-
hash, line, ok := strings.Cut(line, " ")
56-
if !ok {
57-
r.log.Warn("Bad rev-list output", "line", line, "error", "missing a hash")
58-
continue
59-
}
57+
hash, line, ok := strings.Cut(line, " ")
58+
if !ok {
59+
r.log.Warn("Bad rev-list output", "line", line, "error", "missing a hash")
60+
continue
61+
}
6062

61-
shortHash, line, ok := strings.Cut(line, " ")
62-
if !ok {
63-
r.log.Warn("Bad rev-list output", "line", line, "error", "missing a short hash")
64-
continue
65-
}
63+
shortHash, line, ok := strings.Cut(line, " ")
64+
if !ok {
65+
r.log.Warn("Bad rev-list output", "line", line, "error", "missing a short hash")
66+
continue
67+
}
6668

67-
epochstr, subject, ok := strings.Cut(line, " ")
68-
if !ok {
69-
r.log.Warn("Bad rev-list output", "line", line, "error", "missing an time")
70-
continue
71-
}
72-
epoch, err := strconv.ParseInt(epochstr, 10, 64)
73-
if err != nil {
74-
r.log.Warn("Bad rev-list output", "line", line, "error", err)
75-
continue
76-
}
69+
epochstr, subject, ok := strings.Cut(line, " ")
70+
if !ok {
71+
r.log.Warn("Bad rev-list output", "line", line, "error", "missing an time")
72+
continue
73+
}
74+
epoch, err := strconv.ParseInt(epochstr, 10, 64)
75+
if err != nil {
76+
r.log.Warn("Bad rev-list output", "line", line, "error", err)
77+
continue
78+
}
7779

78-
details[i] = CommitDetail{
79-
Hash: Hash(hash),
80-
ShortHash: Hash(shortHash),
81-
Subject: subject,
82-
AuthorDate: time.Unix(epoch, 0),
80+
if !yield(CommitDetail{
81+
Hash: Hash(hash),
82+
ShortHash: Hash(shortHash),
83+
Subject: subject,
84+
AuthorDate: time.Unix(epoch, 0),
85+
}, nil) {
86+
return
87+
}
8388
}
8489
}
85-
86-
return details, nil
8790
}
8891

8992
// ListCommitsFormat lists commits matched by the given range,
9093
// formatted according to the given format string.
9194
//
9295
// See git-log(1) for details on the format string.
93-
func (r *Repository) listCommitsFormat(ctx context.Context, commits CommitRange, format string) ([]string, error) {
96+
func (r *Repository) listCommitsFormat(ctx context.Context, commits CommitRange, format string) iter.Seq2[string, error] {
9497
args := make([]string, 0, len(commits)+3)
9598
args = append(args, "rev-list")
9699
if format != "" {
97100
args = append(args, "--format="+format)
98101
}
99102
args = append(args, []string(commits)...)
100103

101-
cmd := r.gitCmd(ctx, args...)
102-
out, err := cmd.StdoutPipe()
103-
if err != nil {
104-
return nil, err
105-
}
106-
107-
if err := cmd.Start(r.exec); err != nil {
108-
return nil, fmt.Errorf("start rev-list: %w", err)
109-
}
104+
return func(yield func(string, error) bool) {
105+
cmd := r.gitCmd(ctx, args...)
110106

111-
// TODO: Return a string iterator
112-
var lines []string
113-
scanner := bufio.NewScanner(out)
114-
for scanner.Scan() {
115-
line := scanner.Text()
116-
117-
// With --format, rev-list output is in the form:
118-
//
119-
// commit <hash>
120-
// <formatted message>
121-
//
122-
// We'll need to ignore the first line.
123-
//
124-
// This is a bit of a hack, but the --no-commit-header flag
125-
// that suppresses this line is only available in git 2.33+.
126-
if format != "" && strings.HasPrefix(line, "commit ") {
127-
if !scanner.Scan() {
128-
break
107+
var sawCommitHeader bool
108+
for bs, err := range cmd.ScanLines(r.exec) {
109+
if err != nil {
110+
yield("", fmt.Errorf("git rev-list: %w", err))
111+
return
129112
}
130-
}
131-
132-
lines = append(lines, scanner.Text())
133-
}
134113

135-
if err := scanner.Err(); err != nil {
136-
return nil, fmt.Errorf("scan: %w", err)
137-
}
114+
// With --format, rev-list output is in the form:
115+
//
116+
// commit <hash>
117+
// <formatted message>
118+
//
119+
// We'll need to ignore the first line.
120+
//
121+
// This is a bit of a hack, but the --no-commit-header flag
122+
// that suppresses this line is only available in git 2.33+.
123+
if format == "" || sawCommitHeader {
124+
sawCommitHeader = false
125+
if !yield(string(bs), nil) {
126+
return
127+
}
128+
continue
129+
}
138130

139-
if err := cmd.Wait(r.exec); err != nil {
140-
return nil, fmt.Errorf("rev-list: %w", err)
131+
if format != "" && !sawCommitHeader {
132+
if bytes.HasPrefix(bs, []byte("commit ")) {
133+
sawCommitHeader = true
134+
continue
135+
}
136+
}
137+
}
141138
}
142-
143-
return lines, nil
144139
}
145140

146141
// CountCommits reports the number of commits matched by the given range.

log.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"go.abhg.dev/gs/internal/git"
1616
"go.abhg.dev/gs/internal/must"
1717
"go.abhg.dev/gs/internal/silog"
18+
"go.abhg.dev/gs/internal/sliceutil"
1819
"go.abhg.dev/gs/internal/spice"
1920
"go.abhg.dev/gs/internal/spice/state"
2021
"go.abhg.dev/gs/internal/ui"
@@ -180,10 +181,10 @@ func (cmd *branchLogCmd) run(
180181
}
181182

182183
if opts.Commits {
183-
commits, err := repo.ListCommitsDetails(ctx,
184+
commits, err := sliceutil.CollectErr(repo.ListCommitsDetails(ctx,
184185
git.CommitRangeFrom(branch.Head).
185186
ExcludeFrom(branch.BaseHash).
186-
FirstParent())
187+
FirstParent()))
187188
if err != nil {
188189
log.Warn("Could not list commits for branch. Skipping.", "branch", branch.Name, "error", err)
189190
continue

0 commit comments

Comments
 (0)