go-car
go-car copied to clipboard
ipld
Open a ReadWrite blockstore without knowing the root Cid
Kinda connected to https://github.com/ipld/go-car/issues/196
It's quite odd to me that OpenReadWrite() require passing the expected root, while that information is in the file header. This prevent from simply opening the file and accessing the roots.
Is there a reason for this?
I think this is used when opening a new file, because it needs to write the header and so needs to select what root goes into the header
Yeah, this has bothered me slightly about the blockstore mechanics—when you reopen a CAR, the root must match, under the concept of a validating a "resume". But there's plenty of cases where you just don't care or it would be more convenient to just skip that form of validation. I ended up copying the same behaviour in storage.OpenReadableWritable, hence the logic is all in internal/store/resume.go.
The other property that is matched during a resume is the data padding (UseDataPadding which is mistakenly named WithDataPadding in various places in the docs). But, this could also be inferred from a load (see the wantPadding calculation).
So, I think it'd be reasonable to add an option to skip validation of these things; SkipResumeValidation or something like that. But it probably should require you then provide a nil root CIDs argument (or one that actually matches) and no non-zero DataPadding just to make sure there's no confusion; i.e. if we were to accept an alternative set of roots along with a SkipResumeValidation then the user may have the expectation that it's rewritten?
Alternatively we could actually rewrite roots if they provide a different set of roots and they take up the same amount of space. But then you have to account for the case where they don't want to rewrite, they just want to skip validation and you also need to make the user aware that they will get an error if the new header length doesn't match, (or doesn't fit in to the padded space available—you could take advantage of non-zero padding in the case of a different sized header and calculate a new DataOffset if you have to consume some of it).
it'd be reasonable to add an option to skip validation of these things;
SkipResumeValidationor something like that
Not sure that makes sense, but what about flipping that entirely, and have options to enable validation (WithDataPadding , WithRoots ...), and have no validation by default (aka, use what is in the file)? That would also simplify the constructor, which would not have roots in the parameters anymore (and double as an explicit breaking change).
There is also a need for a way to changes the roots (append and/or replace).
and double as an explicit breaking change
So this is the problem I'm trying to avoid. I don't really want to bump to a /v3 if we can help it. I'm happy to queue up breaking changes in some branch we shunt to the side and defer that until we have enough of them to warrant it, but major version bumps are annoying enough in Go that I wouldn't do it as flippantly as I do in JS, particularly in this repo where we already have the confusion of a /v2 directory. Perhaps it's time to collect a list of things that have been annoying and are worth breaking in a v3 (we could simplify the Finalize* stuff in that for instance).
@MichaelMure fwiw I'm happy for you to proceed with this, but I'd really prefer to not introduce a breaking change - either introducing a new constructor or having an opt-out option (that takes a bool so we could potentially use it to flip the behaviour in a future breaking release) seem to be the ideal path here. I don't think this is something that's going to be on a backlog for someone else to do unless it ends up getting in our way. I'm mostly using the storage/ interface for what I'm doing so I tend to bump into things there more than in blockstore/.
@BigLep I don't think I need that in the short term, so likely no. It's also likely that this will come back to haunt me at some point.
It'll haunt one of us, so whoever it haunts first can come back and address it!
Marking as a P2 for now, it can sit in our backlog.