fix: opt-in way to allow empty list of roots in CAR headers

[willscott]

Allow reading a car even when roots is empty

[hannahhoward]

This needs @rvagg 's eyes. The issue has never been the technical implementation. The problem, according to @rvagg, is that aspects of Filecoin dealmaking depend on this not being empty and are indirectly using go-car to validate the root CID is not empty. I'm not 100% clear this is still a serious issue but it requires some audit of ecosystem code.

[willscott]

@lidel did you see my one comment on your commit?

you keep the state of the header check on the reader, even though it's only used in the constructor. I asked:

do we need to keep this state around on the car reader? we only use the option in the constructor itself, so it seems preferable to have a transient config struct for the option to > modulate, rather than holding onto the boolean for no reason.

[lidel]

@willscott I did not see it. In the future, please drop comments like this on PRs for visibility :pray:

I made a concious decision to remove carReaderConfig because this is not a one-off feature. We create a surface for future options, and roots is not a representative case.

Other flags will modify Reader behavior beyond the header.

For example, we could add WithErrorOnIdentityBlock or WithIgnoreInvalidBlocks and these need to be kept around for the entire life of the reader to serve their purpose, so no point in having a separate struct just for a single boolean around roots behavior.

[willscott]

cool. I'm happy with this as is.

@masih or @rvagg can you approve as a code owner?

[rvagg]

I guess this is OK, although we did say that we weren't going to touch the v0 package anymore.

I do have a v3 branch here that's nearly ready to propose for a semver-major bump that combines v0 and v2 code into a single package so we no longer have this awkward ipld/go-car and ipld/go-car/v2 import business and still need to rely on this older v0 stuff. One of the aims of that was to do away with the zero-roots checks entirely. I could push forward with that if this is actually that important. But as I said on Slack, it seems to me that getting to the bottom of why we are seeing zero-root CARs in the Rhea flow seems like a pretty important thing to be working on; we don't have a good explanation for it and that's a bit of a problem.

[willscott]

@rvagg - but we have continued to update v1. - e.g with the updated use of go-merkledag / etc.

I think we do this now so we can move forward, rather than blocking on not having a good interface for reading this type of car