diff --git a/git/obj_iter.go b/git/obj_iter.go index c367f11..9431285 100644 --- a/git/obj_iter.go +++ b/git/obj_iter.go @@ -18,6 +18,13 @@ type ObjectIter struct { headerCh chan BatchHeader } +// maxRevListLineLength is the maximum length of a single line of `git +// rev-list --objects` output that we are willing to process. A line +// can be much longer than usual if an object has a very long path +// name. 64 MiB is far larger than any plausible path name while still +// bounding how much memory a single line can consume. +const maxRevListLineLength = 64 * 1024 * 1024 + // NewObjectIter returns an iterator that iterates over objects in // `repo`. The arguments are passed to `git rev-list --objects`. The // 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) ), // Read the output of `git rev-list --objects`, strip off any - // trailing information, and write the OIDs to `git cat-file`: - pipe.LinewiseFunction( + // trailing information, and write the OIDs to `git cat-file`. + // + // A single line of `git rev-list --objects` output can be very + // long if an object has a very long path name. We therefore + // can't use `pipe.LinewiseFunction()`, whose scanner rejects + // any line longer than 64 KiB with "bufio.Scanner: token too + // long" (see + // https://github.com/github/git-sizer/issues/157). Instead, use + // `pipe.ScannerFunction()` with a scanner whose buffer starts + // small but is allowed to grow up to `maxRevListLineLength`, so + // that normal usage stays cheap while pathologically long lines + // can still be processed. + pipe.ScannerFunction( "copy-oids", + func(r io.Reader) (pipe.Scanner, error) { + scanner := bufio.NewScanner(r) + scanner.Buffer( + make([]byte, 0, bufio.MaxScanTokenSize), + maxRevListLineLength, + ) + scanner.Split(pipe.ScanLFTerminatedLines) + return scanner, nil + }, func(_ context.Context, _ pipe.Env, line []byte, stdout *bufio.Writer) error { if len(line) < hashHexSize { return fmt.Errorf("line too short: '%s'", line) diff --git a/git/obj_iter_test.go b/git/obj_iter_test.go new file mode 100644 index 0000000..fe032bc --- /dev/null +++ b/git/obj_iter_test.go @@ -0,0 +1,106 @@ +package git_test + +import ( + "bytes" + "context" + "fmt" + "io" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/github/git-sizer/git" + "github.com/github/git-sizer/internal/testutils" +) + +// hashObjectLiterally writes an object of the given type to `repo` +// using `git hash-object --literally`, which (unlike the normal path) +// does not reject "malformed" objects such as trees with excessively +// long path names. It returns the new object's OID. +func hashObjectLiterally( + t *testing.T, repo *testutils.TestRepo, otype string, content []byte, +) git.OID { + t.Helper() + + cmd := repo.GitCommand( + t, "hash-object", "--literally", "-w", "-t", otype, "--stdin", + ) + cmd.Stdin = bytes.NewReader(content) + out, err := cmd.Output() + require.NoError(t, err) + + oid, err := git.NewOID(string(bytes.TrimSpace(out))) + require.NoError(t, err) + return oid +} + +// TestObjectIterLongPath checks that the object iterator can process a +// repository that contains an object whose path name is longer than +// the 64 KiB line length that `bufio.Scanner` accepts by default. Such +// a path makes `git rev-list --objects` emit a line longer than 64 KiB, +// which used to abort the walk with "bufio.Scanner: token too long". +// See https://github.com/github/git-sizer/issues/157. +func TestObjectIterLongPath(t *testing.T) { + t.Parallel() + + repo := testutils.NewTestRepo(t, true, "long-path") + defer repo.Remove(t) + + r := repo.Repository(t) + + blob := repo.CreateObject(t, "blob", func(w io.Writer) error { + _, err := io.WriteString(w, "hello\n") + return err + }) + + // A tree that references the blob under a file name that is far + // longer than 64 KiB, so that the corresponding `git rev-list + // --objects` line exceeds the default scanner limit: + longName := strings.Repeat("a", 100*1024) + var treeContent bytes.Buffer + fmt.Fprintf(&treeContent, "100644 %s\x00", longName) + treeContent.Write(blob.Bytes()) + tree := hashObjectLiterally(t, repo, "tree", treeContent.Bytes()) + + commit := repo.CreateObject(t, "commit", func(w io.Writer) error { + _, err := fmt.Fprintf( + w, + "tree %s\n"+ + "author Example 1112911993 -0700\n"+ + "committer Example 1112911993 -0700\n"+ + "\n"+ + "Commit with a very long path\n", + tree, + ) + return err + }) + + repo.UpdateRef(t, "refs/heads/main", commit) + + ctx := context.Background() + iter, err := r.NewObjectIter(ctx) + require.NoError(t, err) + + errChan := make(chan error, 1) + go func() { + defer iter.Close() + errChan <- iter.AddRoot(commit) + }() + + var oids []git.OID + for { + header, ok, err := iter.Next() + require.NoError(t, err) + if !ok { + break + } + oids = append(oids, header.OID) + } + require.NoError(t, <-errChan) + + assert.Contains(t, oids, commit) + assert.Contains(t, oids, tree) + assert.Contains(t, oids, blob) +}