Skip to content

Comments

feat(bindings/c)!: Make features configurable via CMakeLists#6143

Merged
Xuanwo merged 23 commits intoapache:mainfrom
asukaminato0721:c-configure
May 20, 2025
Merged

feat(bindings/c)!: Make features configurable via CMakeLists#6143
Xuanwo merged 23 commits intoapache:mainfrom
asukaminato0721:c-configure

Conversation

@asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented May 2, 2025

Which issue does this PR close?

part of #4318.
Closes #4313.

Rationale for this change

What changes are included in this PR?

Enable build feature by option.

Are there any user-facing changes?

Enable build feature by option.

@asukaminato0721 asukaminato0721 changed the title try split feature feat(bindings/c): try split feature May 2, 2025
@asukaminato0721 asukaminato0721 marked this pull request as ready for review May 2, 2025 12:53
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels May 2, 2025
@asukaminato0721 asukaminato0721 force-pushed the c-configure branch 3 times, most recently from f036d6a to 9f77da9 Compare May 14, 2025 09:33
@asukaminato0721 asukaminato0721 requested a review from xxchan May 14, 2025 09:40
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels May 14, 2025
@asukaminato0721 asukaminato0721 marked this pull request as draft May 14, 2025 20:07
@asukaminato0721 asukaminato0721 marked this pull request as ready for review May 14, 2025 20:27
@asukaminato0721 asukaminato0721 requested a review from Xuanwo May 14, 2025 20:57
@asukaminato0721
Copy link
Contributor Author

   Compiling opendal-c v0.0.0 (/home/runner/work/opendal/opendal/bindings/c)
error: the `Err`-variant returned from this function is very large
  --> src/operator.rs:89:6
   |
89 | ) -> core::Result<core::Operator> {
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 128 bytes
   |
   = help: try reducing the size of `opendal::Error`, for example by boxing large elements or replacing it with `Box<opendal::Error>`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
   = note: `-D clippy::result-large-err` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::result_large_err)]`

error: could not compile `opendal-c` (lib) due to 1 previous error

@Xuanwo
Copy link
Member

Xuanwo commented May 16, 2025

   Compiling opendal-c v0.0.0 (/home/runner/work/opendal/opendal/bindings/c)
error: the `Err`-variant returned from this function is very large
  --> src/operator.rs:89:6
   |
89 | ) -> core::Result<core::Operator> {
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 128 bytes
   |
   = help: try reducing the size of `opendal::Error`, for example by boxing large elements or replacing it with `Box<opendal::Error>`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
   = note: `-D clippy::result-large-err` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::result_large_err)]`

error: could not compile `opendal-c` (lib) due to 1 previous error

Fixed in #6193

@Xuanwo Xuanwo changed the title feat(bindings/c): try split feature feat(bindings/c)!: Make features configurable via CMakeLists May 20, 2025
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice changes, thank you a lot!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 20, 2025
@Xuanwo Xuanwo merged commit 822fe30 into apache:main May 20, 2025
68 checks passed
@asukaminato0721 asukaminato0721 deleted the c-configure branch June 2, 2025 13:25
p1skiii added a commit to p1skiii/opendal that referenced this pull request Aug 9, 2025
- Add core, stable, database, minimal feature groups
- Following C bindings pattern (PR apache#6143)
- Zero breaking changes: default unchanged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/feat The PR implements a new feature or has a title that begins with "feat" size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support configure features for C binding

2 participants