search

ipfs / boxo

A set of reference libraries for building IPFS applications and implementations in Go.
https://github.com/ipfs/boxo#readme
Other
216 stars 98 forks source link

Handle relationship between boxo and go-car and go-merkledag #218

Closed BigLep closed 1 year ago

BigLep commented 1 year ago

Current

Done criteria

boxo/car is removed. boxo will use ipld/go-car.

vvv OLD vvv

Done Criteria

Decision is made on the relation to be had between ipfs/boxo and ipld/go-car and ipfs/go-merkledag. This could mean having a dependency on ipfs/boxo (e.g., https://github.com/ipfs/go-merkledag/pull/100 ) or boxo taking a fork of the relevant functionality it needs and maintaining it.

Why Important

The https://github.com/ipfs/boxo/issues/196 effort (and particularly https://github.com/ipfs/boxo/pull/206) can't be completed without decisions here, and that work is blocking other refactoring work like https://github.com/ipfs/boxo/pull/176.

Resulting Actions

Below are the actions that are being taken as a result of the 2023-03-21 meeting:

✅ Item 1: Get go-car decoupled:

✅ Item 2: copy in go-merkledag to boxo This will get handled as part of https://github.com/ipfs/boxo/issues/202 A row will be added for go-merkledag. No deprecated types will be added.
A CODOWNERS will be added. https://github.com/ipfs/go-merkledag/pull/101

Additional items that have come up since>

  1. ✅ Surface this with maintainers (folks in PL EngRes #bedrock): https://filecoinproject.slack.com/archives/C03FK0JJ10E/p1680108077842429
  2. Clarify that boxo/ipld/car is not intended as a replacement for ipld/go-car: https://github.com/ipfs/boxo/pull/244
BigLep commented 1 year ago

2023-03-21 Sync Meeting

Attendees

  1. @BigLep
  2. @Jorropo
  3. @guseggert
  4. @rvagg
  5. @dvd

Goals

My primary goal in this conversation is how to best enable Bedrock to detach from the "boxo" effort, while also unblocking https://github.com/ipfs/boxo/issues/196

Agenda

Item 1: what to do with go-car

Item 2: what to do with go-merkledag

Item 3: any other disagreements that Rod wants to share? (Also encourage written thoughts to be shared publicly. They can be posted in this issue.)

BigLep commented 1 year ago

2023-03-21 meeting notes

Note @BigLep is not great at listening well and taking notes. Below is some of what was covered...

Rod: go-car and go-merkledag are different ends of the spectrum

go-merkledag is not something that IPFS implementations should depend on.
It shouldn't be permanent.
These aren't important things that should need for an IPFS implementation There is worry that it will become ore ossified by being in "boxo" Wants to see it die.
Don't want to care about it.
General worry is that by pulling all this stuff in, that are crowding out the ability for people to make their own choices about what libraries to use (that can't as easily mix and match components).

go-car Yes want to opt out of having a Boxo dependency.

Meta feedback Need to get people onboard with making changes If haven't gotten EngRes onboard, how about the rest of the ecosystem?

Currently has low trust towards Kubo maintainers as...

  1. See things getting merged without proper time open or owner review(example: go-car with a dependency on go-libipfs)
  2. Making decisions quickly without community involvement (e.g., changing name of go-libipfs)

Maybe can do a migration/breakage campaign but it can't be continual and it needs to be well thought through. Don't want to experience the repeated set of breakages like had with go-libp2p.

Steve: We're not in a position right now to lead a fully buttoned-up migration campaign. Focus is on how we decouple to not so Kubo maintainer work is not impacting Bedrock.

dvd: summary of the concerns:

  1. communication about changes
  2. get buy-in
  3. avoid breaking changes

Steve: Full acknowledgment that https://github.com/ipld/go-car/pull/356 was not handled correctly. It shouldn't have been merged.

[There were other things, but the above was the gist I got written down.]

defer: Discussion on bitswap in boxo.

Action Items

Actions around go-car and go-merkledag were added to the top-level description.

BigLep commented 1 year ago

Clarifying ownershp of go-car and requiring CODEOWNER PR approval is happening in: https://github.com/ipld/go-car/pull/403 https://github.com/ipld/github-mgmt/pull/51

BigLep commented 1 year ago

Clarifying the status of go-merkledag and adding a COEOWNERS is happening in https://github.com/ipfs/go-merkledag/pull/101

BigLep commented 1 year ago

I plan to send this message to #bedrock in FIL Slack on 2023-03-28. Review / 👍 welcome.

Hello Bedrock,

I know various on the Bedrock team have been snagged by efforts from the @kubo-maintainers to consolidate various repos into "boxo" (formerly called go-libipfs). Here's a quick update on where we're at...

Per conversations the week of 2023-03-20 in https://github.com/ipfs/boxo/issues/218, go-car and go-merkledag no longer depend on Boxo. That dependency established earlier in 2023 has been reverted.

The list of repos that will be getting copied into boxo (and their status) are listed here: https://github.com/ipfs/boxo/issues/202 . If anything seems off, please share in https://github.com/ipfs/boxo/issues/218 by EOD Friday 2023-03-31, as otherwise @kubo-maintainers will be adding "not maintained" notices to these repos and deprecated types starting 2023-04-03.

boxo's next milestone is to be setup for more example-driven-development with a consolidated set of repos that can be versioned and refactored collectively can be found here. More info and tracking about that effort can be fond here: https://github.com/ipfs/boxo/issues/196

Thanks and let us know if you have any questions, @kubo-maintianers

BigLep commented 1 year ago

I'm resolving this issue since

  1. https://github.com/ipfs/go-merkledag/pull/101 is merged

  2. bedrock (private FIL Slack channel) notified in https://filecoinproject.slack.com/archives/C03FK0JJ10E/p1680108077842429 (note that I adjusted the dates from what was originally in https://github.com/ipfs/boxo/issues/218#issuecomment-1486217454 (see edit history)

hannahhoward commented 1 year ago

Can I make sure I’m understanding #202 correctly go-car and go-merkledag will remain in boxo, but also exist as independent forks (maintained by individuals from Bedrock)?

That seems… like a worst of both worlds.

Re go-car: why would IPFS stewards maintain a fork of code from the ipld organization? Why would two separate teams, both actually part of the Protocol Labs organization, maintain two separate copies of a repository? It just seems terrible. What expertise do IPFS stewards bring to a CAR library? How do they plan to develop it? Will they incorporate go-car changes (go-car is in active development and will remain so). How will the boxo version diverge from go-car?

To me, the clear decision should be to simply have boxo use the external copies, rather than make two versions of the same software.

The current decision seems like a concession to individuals, all at Protocol Labs, who have personal relationships with the stewards team, that's unmaintainable over the long term. It doesn’t address the underlying concern, which is the entire Boxo effort is creating friction not just for individuals but the whole ecosystem. I would rather we think about: what is best for the ecosystem? Because most people in the ecosystem don't have personal relationships. The fact that the end result notification is "send a message to a private slack channel" to me feels like further evidence that we're not having discussions in the open or thinking holistically about the ecosystem value of the entire Boxo project.

If we're going with this, I sure hope it's temporary cause it seems untenable. Maybe stewards will show why go-car has to live inside of boxo and we decide we can remove go-car after all. If not, I hope we'll recognize that go-car is a seperate tool, maintained and developed most actively by other people, and there's no reason for it to live in boxo.

BigLep commented 1 year ago

I'm reopening as am already seeing some additional actions to take and will also work on responding to @hannahhoward

BigLep commented 1 year ago

Thanks for your message @hannah. Below are my thoughts and the actions I want to take as a result:

A few things I want to make sure are clear and understood:

  1. Consumers can still use ipld/go-car and boxo/ipld/car.
  2. We are not actively advocating for someone to use boxo/ipld/car over ipld/go-car. (I added an action item below on how we can make that more clear)
  3. Innovating and advancing on car libraries is not where Boxo maintainers are planning to apply their energies.
    1. It’s great that ipld/go-car is being innovated on! We have done this copy so that ipld/go-car maintainers aren’t slowed down by boxo work.
    2. CARs are obviously important to the ecosystem and boxo/ipld/car will conform to however the specs evolve.
    3. boxo has a limited of set car needs, and it will meet those needs directly. In terms of specifics that Boxo maintainer would take on if/as we’re freed up to adjust boxo/ipld/car…
      1. Boxo and Kubo uses cars as streamable block containers. We will trim out features like indexes and traversal related helpers (reduce maintenance overhead). If someone cares about a features we removed, they are free to use the more feature rich ipld/go-car.
      2. Create a new easier way to emit block streams with a new WritableBlockStream (instead of Ldwrite).
  4. Boxo maintainers will maintain any code that is copied into boxo. We’re not expecting other to do this (although we certainly welcome others to help maintain, even just certain modules.). Boxo maintainers have also participated in security fixes around cars and can navigate those waters if/as needed.

The fact that the end result notification is “send a message to a private slack channel” to me feels like further evidence that we’re not having discussions in the open or thinking holistically about the ecosystem value of the entire Boxo project.

I’m not sure where the flag about not having discussions in the open is coming from. The message in the private FIL Slack #bedrock channel was an additional measure taken beyond all the other public steps that have been taken out of awareness that there is a large stream of notifications coming through. If there are other suggestions for working more in the open here, I certainly welcome hearing them.

I think we have already hit “agree to disagree” on the value of the Boxo effort. At this piont, I think we need to get over the hump and we can evaluate after if the decision was right. Given the lack of shared cross-group certainty about this decision, our shifted emphasis has been to do it in a way that minimizes the blast-radius and impact on others in case it turns out to be a misstep. Better/consolidated justification for the Boxo work is in progress here.

For the record, alternatives were attempted in the past around ipld/go-car:

🎬 I think these actions should be taken in light of this conversation. I’ll add them to the done criteria before closing this issue:

  1. Update https://github.com/ipfs/boxo/tree/main/ipld/car to make clear that this is a copy of ipld/go-car that will be trimmed down for the specific boxo needs. One should definitely consider using the maintained ipld/go-car project.
  2. Update the migration tool default config to not convert ipld/go-car to boxo/ipld/car. (This was originally done to help with the Kubo update to Boxo, but was not intended as the default)
BigLep commented 1 year ago

PR to better clarify boxo/ipld/car's relationship with ipld/go-car: https://github.com/ipfs/boxo/pull/244

BigLep commented 1 year ago

There has been some more discussion about a future with boxo depending on ipld/go-car if there's some refactoring in: https://github.com/ipfs/boxo/pull/244#issuecomment-1492546786

BigLep commented 1 year ago

Per 2023-04-12 maintainer conversation, @Jorropo is going to respond here.

Jorropo commented 1 year ago

I have posted a feedback about go-car in https://github.com/ipld/go-car/issues/416, this include the changes I'll plan to make in boxo/car, if we can agree on a lighter package and simpler out streaming API and cleaner go.mod I don't see any issues dropping the one from go-car.

BigLep commented 1 year ago

Doh - we have comments in here and https://github.com/ipfs/boxo/pull/244

I meant to put https://github.com/ipfs/boxo/issues/218#issuecomment-1504258929 in https://github.com/ipfs/boxo/pull/244

willscott commented 1 year ago

if we can agree on a lighter package and simpler out streaming API and cleaner go.mod I don't see any issues dropping the one from go-car.

BigLep commented 1 year ago

Per IPFS Thing 2023 conversations, we are going to be able to remove Boxo's ipld/go-car copy from Boxo. When that happens, this issue becomes moot and can be closed. @Jorropo will do this work.

BigLep commented 1 year ago

@Jorropo : just checking in on how this going.

BigLep commented 1 year ago

I updated the issue description to make clear that the done criteria is now to to get boxo/car removed.

We can remove boxo/car once https://github.com/ipfs/boxo/issues/291 is handled.

BigLep commented 1 year ago

Given that the work for https://github.com/ipfs/boxo/issues/291 has made its way into Boxo and been validated in upstream consumers (Kubo, Boost, Lotus), we are unblocked to close this out. Completing this issue means "boxo/car is removed. boxo will use ipld/go-car." @Jorropo said on 2023-06-13 he will take this since it is quick.

BigLep commented 1 year ago

This was closed given the work to replace boxolo/ipld/car by ipld/go-car in https://github.com/ipfs/boxo/pull/400 .