search

filecoin-project / filecoin-ffi

C and CGO bindings for Filecoin's Rust libraries
Other
97 stars 136 forks source link

[DX Streamline] Offer prebuilt filecoin-ffi Go modules natively #209

Open mvdan opened 3 years ago

mvdan commented 3 years ago

Detailed design: https://hackmd.io/@mvdan/Hy7iK0TEY

Note that the design just outlines the problem and the high level design of how it would be fixed. I've deliberately left the implementation details vague, e.g. what kind of HTTP server to use for the proxy redirects.

mvdan commented 3 years ago

One alternative discussed with @Kubuxu on Slack: make the "forwarding FFI module" be separate, such as a new filecoin-ffi-prebuilt, so all the noise related to code generation or continuous bumping of go.mod dependency versions doesn't affect the main repo that humans are working on.

BigLep commented 3 years ago

Pasting from Slack...

From https://hackmd.io/@mvdan/Hy7iK0TEY , I get there is manual work and that it spreads to transitive dependencies. How big is that sprawl? I assume the Lotus repos is affected? Which others?

The sprawl reaches any repository that somehow ends up depending on filecoin-ffi. It affects 24 repositories in the filecoin org itself: https://github.com/search?q=org%3Afilecoin-project+filecoin+ffi+replace+filename%3Ago.mod

Is it correct to say that these repos that have a transitive dependency on filecoin-ffi can't be onboarded to unified CI?

Yes. We could teach Unified CI to special case filecoin-ffi, such as detecting the presence of the go.mod replace directive and acting accordingly, but that kinda defeats the purpose of Unified CI being generic and encouraging good practices :)

Is Unified CI itself held back in any way from this (or is the blast radius contained to the packages that have a transitive dependency on filecoin-ffi?)

Blast radius (i.e. inability to use Unified CI) is restricted to the modules which have the replace .../filecoin-ffi line, like the search results above.

The thread mentions https://github.com/filecoin-project/filecoin-ffi/issues/209 being owned by Filecoin Stewards, but this is ultimately changes in filecoin-ffi right (which I believe Filecoin Proofs owns)?

You might be right - my understanding of the Filecoin org is rather limited.

This is the list of filecoin org packages impacted:

filecoin-project/boost filecoin-project/chain-validation filecoin-project/dealbot filecoin-project/eudico filecoin-project/filecoin-datasets filecoin-project/gfm-priv filecoin-project/go-commp-utils filecoin-project/go-fil-markets filecoin-project/go-padreader filecoin-project/go-sectorbuilder filecoin-project/go-shared-types filecoin-project/go-storage-miner filecoin-project/lily filecoin-project/lotus filecoin-project/lotus-testground filecoin-project/oni filecoin-project/sector-storage filecoin-project/sentinel-drone filecoin-project/slingshot-stats filecoin-project/statediff filecoin-project/storage-fsm filecoin-project/test-vectors filecoin-project/venus filecoin-project/venus-market filecoin-project/venus-miner filecoin-project/venus-sealer

BigLep commented 3 years ago

Additional notes from Slack...

Unified CI isn't held up by this across PL. We're just unable to use Unified CI on a significant chunk of lotus-related repositories until we solve the problem.

As for my proposed solution with Go modules: it's certainly non-trivial, but we think it's in Filecoin's best interest to solve it sooner rather than later as well - not just for Unified CI, because it would also simplify building and development without manual steps.

Kubuxu commented 10 months ago

Of note, the redirect/forwarding can be achieved with Cloudflare workers, so no server/persistent infra is required.

BigLep commented 5 months ago

Recent discussion about the pain in FIL Slack #fil-lotus-dev: https://filecoinproject.slack.com/archives/CP50PPW2X/p1717676403286179

aarshkshah1992 commented 5 months ago

POC is up at https://github.com/aarshkshah1992/ffi-app 🎉

Putting together a plan/executing on taking this to prod now.

BigLep commented 5 months ago

Good stuff @aarshkshah1992 ! I assume you'll post the plan/steps here for review? You don't need to block on review, but I want to make sure we're all aligned.

Also, I think a sequence diagram / visual will be worth a lot here. I know there is some description in https://hackmd.io/@mvdan/Hy7iK0TEY and https://app.excalidraw.com/l/AtvYo7ZDrTn/57rJelzHyLp , but a fully accurate picture of the end state including request parameters, redirects, etc. I think could be useful.

aarshkshah1992 commented 5 months ago

@BigLep Yes, will post a detailed (plan + sequence diagram) here tomorrow to get buy-in as this workstream touches a LOT of touch points.

aarshkshah1992 commented 5 months ago

@BigLep Have detailed the work that needs to be done at https://github.com/aarshkshah1992/ffi-app/pull/1/files.

BigLep commented 4 months ago

From @rvagg in https://filecoinproject.slack.com/archives/CP50PPW2X/p1721617496659179

More contributor pain that would be fixed by the filecoin-ffi Go prebuilds https://github.com/filecoin-project/lotus/pull/12241

BigLep commented 4 months ago

Another instance where not having this caused trip-ups: https://github.com/filecoin-project/lotus/pull/12306#issuecomment-2249919475

BigLep commented 1 week ago

On 2024-11-11, @rjan90 , @masih , @rvagg , and I met to discuss this project. We reviewed https://github.com/aarshkshah1992/ffi-app/pull/1/files again and identified the following task list:

Code

CI related

Infra

Documentation

Cleanup