refactor: WithErrorOnEmptyRoots(bool)

this allows us to keep backward compatibility
and explicitly enable both behaviors, which means
code that expects error can be migrated into implicit flag
over time

car.go

@@ -124,31 +124,36 @@ var bufioReaderPool = sync.Pool{
 }
 
 type CarReader struct {
-	br     *bufio.Reader // set nil on EOF
-	Header *CarHeader
+	br                *bufio.Reader // set nil on EOF
+	Header            *CarHeader
+	errorOnEmptyRoots bool
 }
 
-type carReaderConfig struct {
-	allowEmptyRoots bool
-}
-
-type CarReaderOpt func(carReaderConfig)
-
-// CarReaderAllowEmptyRoots is an option that can be passed to NewCarReader to
-// change the behavior when reading a car file that does not have any root
-// cids set. With this option a reader will be returned, instead of an error.
-func CarReaderAllowEmptyRoot() CarReaderOpt {
-	return func(cr carReaderConfig) {
-		cr.allowEmptyRoots = true
+type CarReaderOption func(*CarReader) error
+
+// WithErrorOnEmptyRoots is an option that can be passed to NewCarReader to
+// specify the behavior when reading a car file that does not have any root
+// cids set in the CAR header.
+// Setting this option to true will cause CarReader to error on CAR that has
+// no root CIDs listed in the header.
+func WithErrorOnEmptyRoots(flag bool) CarReaderOption {
+	return func(cr *CarReader) error {
+		cr.errorOnEmptyRoots = flag
+		return nil
 	}
 }
 
-func NewCarReader(r io.Reader, opts ...CarReaderOpt) (*CarReader, error) {
-	conf := carReaderConfig{}
-	for _, o := range opts {
-		o(conf)
-	}
+func NewCarReader(r io.Reader) (*CarReader, error) {
+	// BACKWARD COMPATIBILITY NOTE:
+	// WithErrorOnEmptyRoots(true) here is the legacy behavior
+	// which we need to keep for reasons described in
+	// https://github.com/ipfs/specs/pull/402#issuecomment-1599428849
+	// (mainly, we need to migrate Filecoin code to use explicit
+	// WithErrorOnEmptyRoots(true) before we can change the default here).
+	return NewCarReaderWithOptions(r, WithErrorOnEmptyRoots(true))
+}
 
+func NewCarReaderWithOptions(r io.Reader, opts ...CarReaderOption) (*CarReader, error) {
 	br := bufioReaderPool.Get().(*bufio.Reader)
 	br.Reset(r)
 	ch, err := ReadHeader(br)
@@ -161,14 +166,23 @@ func NewCarReader(r io.Reader, opts ...CarReaderOpt) (*CarReader, error) {
 		return nil, fmt.Errorf("invalid car version: %d", ch.Version)
 	}
 
-	if !conf.allowEmptyRoots && len(ch.Roots) == 0 {
+	carReader := &CarReader{
+		br:     br,
+		Header: ch,
+	}
+
+	for _, o := range opts {
+		err := o(carReader)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if carReader.errorOnEmptyRoots && len(ch.Roots) == 0 {
 		return nil, fmt.Errorf("empty car, no roots")
 	}
 
-	return &CarReader{
-		br:     br,
-		Header: ch,
-	}, nil
+	return carReader, nil
 }
 
 func (cr *CarReader) Next() (blocks.Block, error) {

car_test.go

@@ -139,6 +139,52 @@ func TestEOFHandling(t *testing.T) {
 		}
 	})
 }
+func TestEmptyRootsInHeader(t *testing.T) {
+	testCases := []struct {
+		name              string
+		errorOnEmptyRoots bool
+		hex               string
+		errStr            string // either the whole error string
+	}{
+		{
+			"errorOnEmptyRoots:true",
+			true,
+			"0aa16776657273696f6e01",
+			"empty car, no roots",
+		}, {
+			"errorOnEmptyRoots:false",
+			false,
+			"0aa16776657273696f6e01",
+			"",
+		},
+	}
+
+	makeCar := func(t *testing.T, byts string, flag bool) error {
+		fixture, err := hex.DecodeString(byts)
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, err = car.NewCarReaderWithOptions(bytes.NewReader(fixture), car.WithErrorOnEmptyRoots(flag))
+		return err
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := makeCar(t, tc.hex, tc.errorOnEmptyRoots)
+			if err == nil && tc.errorOnEmptyRoots {
+				t.Fatal("expected error from empty roots, didn't get one")
+			}
+			if err != nil && !tc.errorOnEmptyRoots {
+				t.Fatal("expected no error from empty roots, but got one")
+			}
+			if tc.errStr != "" {
+				if err.Error() != tc.errStr {
+					t.Fatalf("bad error: %v", err)
+				}
+			}
+		})
+	}
+}
 
 func TestBadHeaders(t *testing.T) {
 	testCases := []struct {
@@ -163,6 +209,11 @@ func TestBadHeaders(t *testing.T) {
 			"{version:\"1\",roots:[baeaaaa3bmjrq]}",
 			"1da265726f6f747381d82a4800010000036162636776657273696f6e6131",
 			"", "invalid header: ",
+		}, {
+			"{version:1}",
+			"0aa16776657273696f6e01",
+			"empty car, no roots",
+			"",
 		}, {
 			"{version:1,roots:{cid:baeaaaa3bmjrq}}",
 			"20a265726f6f7473a163636964d82a4800010000036162636776657273696f6e01",