Skip to content

Commit 40eb644

Browse files
committed
Handle very long lines in git rev-list --objects output
The `copy-oids` stage used `pipe.LinewiseFunction`, whose scanner rejects any line longer than 64 KiB with "bufio.Scanner: token too long". A single line of `git rev-list --objects` output can exceed that limit when an object has a very long path name, which aborts the whole object walk. Use `pipe.ScannerFunction` with a scanner that uses the same LF line-splitting but whose buffer starts at the usual 64 KiB and may grow up to 64 MiB, so normal usage stays cheap while pathologically long lines can still be processed. Closes #157
1 parent 88eaa80 commit 40eb644

2 files changed

Lines changed: 135 additions & 2 deletions

File tree

git/obj_iter.go

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ type ObjectIter struct {
1818
headerCh chan BatchHeader
1919
}
2020

21+
// maxRevListLineLength is the maximum length of a single line of `git
22+
// rev-list --objects` output that we are willing to process. A line
23+
// can be much longer than usual if an object has a very long path
24+
// name. 64 MiB is far larger than any plausible path name while still
25+
// bounding how much memory a single line can consume.
26+
const maxRevListLineLength = 64 * 1024 * 1024
27+
2128
// NewObjectIter returns an iterator that iterates over objects in
2229
// `repo`. The arguments are passed to `git rev-list --objects`. The
2330
// second return value is the stdin of the `rev-list` command. The
@@ -64,9 +71,29 @@ func (repo *Repository) NewObjectIter(ctx context.Context) (*ObjectIter, error)
6471
),
6572

6673
// Read the output of `git rev-list --objects`, strip off any
67-
// trailing information, and write the OIDs to `git cat-file`:
68-
pipe.LinewiseFunction(
74+
// trailing information, and write the OIDs to `git cat-file`.
75+
//
76+
// A single line of `git rev-list --objects` output can be very
77+
// long if an object has a very long path name. We therefore
78+
// can't use `pipe.LinewiseFunction()`, whose scanner rejects
79+
// any line longer than 64 KiB with "bufio.Scanner: token too
80+
// long" (see
81+
// https://github.com/github/git-sizer/issues/157). Instead, use
82+
// `pipe.ScannerFunction()` with a scanner whose buffer starts
83+
// small but is allowed to grow up to `maxRevListLineLength`, so
84+
// that normal usage stays cheap while pathologically long lines
85+
// can still be processed.
86+
pipe.ScannerFunction(
6987
"copy-oids",
88+
func(r io.Reader) (pipe.Scanner, error) {
89+
scanner := bufio.NewScanner(r)
90+
scanner.Buffer(
91+
make([]byte, 0, bufio.MaxScanTokenSize),
92+
maxRevListLineLength,
93+
)
94+
scanner.Split(pipe.ScanLFTerminatedLines)
95+
return scanner, nil
96+
},
7097
func(_ context.Context, _ pipe.Env, line []byte, stdout *bufio.Writer) error {
7198
if len(line) < hashHexSize {
7299
return fmt.Errorf("line too short: '%s'", line)

git/obj_iter_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
package git_test
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"fmt"
7+
"io"
8+
"strings"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
14+
"github.com/github/git-sizer/git"
15+
"github.com/github/git-sizer/internal/testutils"
16+
)
17+
18+
// hashObjectLiterally writes an object of the given type to `repo`
19+
// using `git hash-object --literally`, which (unlike the normal path)
20+
// does not reject "malformed" objects such as trees with excessively
21+
// long path names. It returns the new object's OID.
22+
func hashObjectLiterally(
23+
t *testing.T, repo *testutils.TestRepo, otype string, content []byte,
24+
) git.OID {
25+
t.Helper()
26+
27+
cmd := repo.GitCommand(
28+
t, "hash-object", "--literally", "-w", "-t", otype, "--stdin",
29+
)
30+
cmd.Stdin = bytes.NewReader(content)
31+
out, err := cmd.Output()
32+
require.NoError(t, err)
33+
34+
oid, err := git.NewOID(string(bytes.TrimSpace(out)))
35+
require.NoError(t, err)
36+
return oid
37+
}
38+
39+
// TestObjectIterLongPath checks that the object iterator can process a
40+
// repository that contains an object whose path name is longer than
41+
// the 64 KiB line length that `bufio.Scanner` accepts by default. Such
42+
// a path makes `git rev-list --objects` emit a line longer than 64 KiB,
43+
// which used to abort the walk with "bufio.Scanner: token too long".
44+
// See https://github.com/github/git-sizer/issues/157.
45+
func TestObjectIterLongPath(t *testing.T) {
46+
t.Parallel()
47+
48+
repo := testutils.NewTestRepo(t, true, "long-path")
49+
defer repo.Remove(t)
50+
51+
r := repo.Repository(t)
52+
53+
blob := repo.CreateObject(t, "blob", func(w io.Writer) error {
54+
_, err := io.WriteString(w, "hello\n")
55+
return err
56+
})
57+
58+
// A tree that references the blob under a file name that is far
59+
// longer than 64 KiB, so that the corresponding `git rev-list
60+
// --objects` line exceeds the default scanner limit:
61+
longName := strings.Repeat("a", 100*1024)
62+
var treeContent bytes.Buffer
63+
fmt.Fprintf(&treeContent, "100644 %s\x00", longName)
64+
treeContent.Write(blob.Bytes())
65+
tree := hashObjectLiterally(t, repo, "tree", treeContent.Bytes())
66+
67+
commit := repo.CreateObject(t, "commit", func(w io.Writer) error {
68+
_, err := fmt.Fprintf(
69+
w,
70+
"tree %s\n"+
71+
"author Example <example@example.com> 1112911993 -0700\n"+
72+
"committer Example <example@example.com> 1112911993 -0700\n"+
73+
"\n"+
74+
"Commit with a very long path\n",
75+
tree,
76+
)
77+
return err
78+
})
79+
80+
repo.UpdateRef(t, "refs/heads/main", commit)
81+
82+
ctx := context.Background()
83+
iter, err := r.NewObjectIter(ctx)
84+
require.NoError(t, err)
85+
86+
errChan := make(chan error, 1)
87+
go func() {
88+
defer iter.Close()
89+
errChan <- iter.AddRoot(commit)
90+
}()
91+
92+
var oids []git.OID
93+
for {
94+
header, ok, err := iter.Next()
95+
require.NoError(t, err)
96+
if !ok {
97+
break
98+
}
99+
oids = append(oids, header.OID)
100+
}
101+
require.NoError(t, <-errChan)
102+
103+
assert.Contains(t, oids, commit)
104+
assert.Contains(t, oids, tree)
105+
assert.Contains(t, oids, blob)
106+
}

0 commit comments

Comments
 (0)