Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking: Remove feat/filestore branch Filecoin dependency #254

Closed
5 of 6 tasks
rvagg opened this issue Oct 7, 2021 · 5 comments
Closed
5 of 6 tasks

Tracking: Remove feat/filestore branch Filecoin dependency #254

rvagg opened this issue Oct 7, 2021 · 5 comments

Comments

@rvagg
Copy link
Member

rvagg commented Oct 7, 2021

#24 is a long-standing PR that has a branch that is relied on for Filecoin. It has the Blockstore API and null padding, both of which are used by Filecoin. But we've never agreed to integrate these changes to master here so they have lingered for a ~year and we need to resolve it.

It's not believed that the Blockstore API is needed at this stage. But the null padding is. So this is the proposed plan:

rvagg added a commit to filecoin-project/go-fil-markets that referenced this issue Oct 7, 2021
rvagg added a commit to filecoin-project/lotus that referenced this issue Oct 7, 2021
@mvdan
Copy link
Contributor

mvdan commented Oct 7, 2021

Indeed I am doint item 1, in #171.

@rvagg
Copy link
Member Author

rvagg commented Oct 8, 2021

I've added filecoin-project/go-commp-utils#7 to the list. It's in the lotus dependency tree and pulls in v0.1.1 of go-car even though it doesn't use it.

With that change, and switching go-fil-markets and lotus to go-car@master (as per the PRs linked above), I'm growing increasingly confident that we need neither of the commits on the custom branch. So I don't believe #255 is necessary. Only the most basic CAR workflows go through go-car, all of the storage/sealing/retrieval paths go through go-car/v2 which has the zero=eof option in-built and enabled.

I've got greens for all of my PRs, just using master here for v0. So I think the next step is to cut a v0.3.2 tag on master so the SelectiveCar changes can get a tag, and then go ahead and get those 3 PRs merged.

What to do with #255 is likely a separate question, will take it up over there.

rvagg added a commit to filecoin-project/go-fil-markets that referenced this issue Oct 15, 2021
rvagg added a commit to filecoin-project/go-fil-markets that referenced this issue Oct 15, 2021
rvagg added a commit to filecoin-project/go-fil-markets that referenced this issue May 10, 2022
@rvagg
Copy link
Member Author

rvagg commented May 10, 2022

go-fil-markets update:

@rvagg
Copy link
Member Author

rvagg commented May 10, 2022

Lotus deps for go-car and go-commp-utils got updated; and it's got the latest go-fil-markets too so as per above comment it's not using the filestore branch. So I've ticked that off.

I'm pretty sure that "remove feat/filestore branch Filecoin dependency" has been completed. Getting the tagged version on go-fil-markets would be nice, but is not essential to resolve this. I'll try this week to get agreement to merge that but I'm going to close this out in the next couple of days unless someone sees a reason to disagree.

hannahhoward pushed a commit to filecoin-project/go-fil-markets that referenced this issue May 11, 2022
@rvagg
Copy link
Member Author

rvagg commented May 12, 2022

🥳 🎆

calling this done

@rvagg rvagg closed this as completed May 12, 2022
dirkmc pushed a commit to filecoin-project/go-fil-markets that referenced this issue May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎉 Done
Development

No branches or pull requests

2 participants