-
Notifications
You must be signed in to change notification settings - Fork 136
Optional fvm for curio #550
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR makes FVM (Filecoin Virtual Machine) dependencies optional by introducing a feature flag system. The changes allow building the project without FVM support, reducing dependencies for users who don't require FVM functionality.
- Moved FVM module in Rust behind a
fvmfeature flag - Made all FVM-related Rust dependencies optional
- Added
fvmbuild tag to Go files to conditionally compile FVM code - Provided C header fallback definitions for FVM error constants when FVM is disabled
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/src/lib.rs | Wrapped FVM module declaration with cfg(feature = "fvm") conditional compilation |
| rust/Cargo.toml | Made FVM dependencies optional and created an fvm feature that includes them; added fvm to default features |
| fvm.go | Added fvm build tag to conditionally compile FVM functionality |
| cgo/types.go | Removed FVM type definitions and helpers, replaced with comments referencing types_fvm.go |
| cgo/fvm.go | Added fvm build tag to conditionally compile FVM CGO bindings |
| cgo/errors.go | Added fallback C macro definitions for FVM error constants when FVM is not compiled |
Comments suppressed due to low confidence (3)
rust/Cargo.toml:76
- The
cudafeature unconditionally referencesfvm2/cuda,fvm3/cuda, andfvm4/cuda, but these dependencies are now optional (only available when thefvmfeature is enabled). This will cause build failures when using--no-default-features --features cuda. The FVM sub-features should be made conditional on thefvmfeature, for example:dep:fvm2?/cudaor the feature should depend onfvm.
cuda = [
"filecoin-proofs-api/cuda",
"rust-gpu-tools/cuda",
"fvm2/cuda",
"fvm3/cuda",
"fvm4/cuda",
]
rust/Cargo.toml:82
- The
cuda-suprasealfeature unconditionally referencesfvm3/cuda-suprasealandfvm4/cuda-supraseal, but these dependencies are now optional. This will cause build failures when using--no-default-features --features cuda-supraseal. The FVM sub-features should be made conditional on thefvmfeature.
cuda-supraseal = [
"filecoin-proofs-api/cuda-supraseal",
"rust-gpu-tools/cuda",
"fvm3/cuda-supraseal",
"fvm4/cuda-supraseal",
]
rust/Cargo.toml:89
- The
openclfeature unconditionally referencesfvm2/opencl,fvm3/opencl, andfvm4/opencl, but these dependencies are now optional. This will cause build failures when using--no-default-features --features opencl. The FVM sub-features should be made conditional on thefvmfeature.
opencl = [
"filecoin-proofs-api/opencl",
"rust-gpu-tools/opencl",
"fvm2/opencl",
"fvm3/opencl",
"fvm4/opencl",
]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
This simple change reduces Curio build time & saves 24mb off Curio's final filesize.
fvm becomes optional=true instead of required.