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

Migrate ipfs/go-unixfsnode #26

Open
7 tasks
hacdias opened this issue Jan 12, 2023 · 9 comments
Open
7 tasks

Migrate ipfs/go-unixfsnode #26

hacdias opened this issue Jan 12, 2023 · 9 comments

Comments

@hacdias
Copy link
Member

hacdias commented Jan 12, 2023

@hacdias hacdias added the need/triage Needs initial labeling and prioritization label Jan 12, 2023
@hacdias hacdias self-assigned this Jan 12, 2023
@hacdias hacdias removed the need/triage Needs initial labeling and prioritization label Jan 12, 2023
@willscott
Copy link
Contributor

Is unixfsnode used in kubo? I don't think we ever fully migrated kubo from go-unixfs to go-unixfsnode.
I worry having both variants in this lib monorepo without documentation could get a bit confusing?

@hacdias
Copy link
Member Author

hacdias commented Jan 13, 2023

@willscott yes, it is used in Kubo (https://github.com/ipfs/kubo/pull/9537/files). And yes, Kubo also uses go-unixfs. Also, the documentation can be found at https://github.com/ipfs/go-libipfs/tree/main/unixfs/node, as we're migrating the READMes too.

The goal, right now, is to move everything to this repository, and only later start refactoring: https://github.com/ipfs/go-libipfs#readme

@willscott
Copy link
Contributor

I guess the thing of note is that the go-unixfsnode repo is used throughout filecoin extensively, and is the primary implementation of unixfs there, versus the go-unixfs, which remains the implementation used everywhere in kubo.

  • lotus / filecoin requires as part of it's release process to have tagged/released versions of dependent libraries.
  • that's something we (bedrock, who have been maintaining this code) have been able to do in a lightweight way by ourselves.
  • by claiming this into go-libipfs, we stop being able to do that and end up having to coordinate with ipfs stewards when we need changes.

@hacdias
Copy link
Member Author

hacdias commented Jan 13, 2023

Then, we have to decide whether or not this repository belongs here. If not, we should revert #25. It would also be important to check if any other repositories from this list should not be moved into this library. Otherwise, we'll just be wasting time. cc @ipfs/w3dt-stewards

@willscott
Copy link
Contributor

cc @hannahhoward - what are your thoughts on if my concern above is valid?

@guseggert
Copy link
Contributor

versus the go-unixfs, which remains the implementation used everywhere in kubo.

I think this is a pretty critical dependency for Kubo, no? https://github.com/ipfs/kubo/blob/5d864faac71b877ae30bd7b2f01c9dfaba68d8eb/core/node/core.go#L102

by claiming this into go-libipfs, we stop being able to do that and end up having to coordinate with ipfs stewards when we need changes.

Not necessarily, we can setup codeowners here. But shouldn't we be coordinating anyway since Kubo is a critical consumer?

This may result in a better state long-term, since changes will run the entire test suite against most Kubo code, instead of none like the current setup. Then ensuring the changes work with Kubo should be largely self-service.

Is there a way we could try this out, but make it easy to undo if we don't like it?

Other reasonable options are to move this into the IPLD org, or kick the can and come back to it later. I don't have enough information to feel strongly about any of these options.

@willscott
Copy link
Contributor

  • only a limited set of ipfs apis use the fetcher directly. go-unixfs is used through all of MFS for actually creating unixfs semantics in kubo still.
  • Codeowners as i understand it is different from my ability to update version.json and cut a release, e.g. https://github.com/ipfs/go-unixfsnode/releases/tag/v1.5.1, which is where maintenance of this code has been for the last year.
  • while it protects kubo, it hinders lotus/filecoin.

The meta point in ipfs/kubo#8543 (comment) is that just looking at dependencies in the org isn't going to be a clean boundary, because repo org membership has not always been clean.

I think my comment here is that there needs to be a pass for 'disruptiveness to external teams/projects' pass against the generated list of repos. In particular, go-cid is used by everything in the ecosystem, and the migration into a sub-directory of this repo without updating downstream uses uses is leaving an enormous amount of work for others.

@guseggert
Copy link
Contributor

guseggert commented Jan 18, 2023

while it protects kubo, it hinders lotus/filecoin.

If there is an adversarial relationship here then we should fork this and keep our own codebases. I don't think we want that, so I wouldn't characterize it as "hindering". The other way around would work too (e.g. move this into filecoin org), the idea isn't to prefer one over the other, it's to test more and reduce the risk of changes breaking either party, and to keep versions in sync. There are other problems with moving this into filecoin org which is why I don't suggest that (e.g. we broadly view Filecoin as an IPFS impl, so we prefer deps from IPFS->Filecoin and not Filecoin->IPFS).

Given the contention though I'm inclined to kick the can here, and revisit this after we have some more experience consolidating other repos and can have a more informed opinion.

Agree with not including go-cid or any multiformats stuff. I'll take a pass through that list and clarify what we definitely want in here, what is unclear (like this repo), and what we definitely don't want in libipfs.

lidel added a commit that referenced this issue Jan 18, 2023
This reverts commit 4d71fa3, reversing
changes made to 6736733.

Context for this decision can be found in
#26
lidel added a commit that referenced this issue Jan 18, 2023
This reverts commit 4d71fa3, reversing
changes made to 6736733.

Context for this decision can be found in
#26
@lidel
Copy link
Member

lidel commented Jan 18, 2023

To unblock Kubo 0.18, I reverted #25 in #32 and released go-libipfs v0.2.0 without go-unixfsnode.

@hacdias hacdias removed their assignment Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 🛑 Blocked
Development

No branches or pull requests

4 participants