feat(ffi): added default-engine-rustls feature and extern "C" for .h file

[mightyshazam]

This PR adds a feature default-engine-rustls to allow users to use the feature in the kernel. Additionally, it adds cpp_compat = true to the cbindgen.toml file so that it will add a section to add the extern "C" wrapper.

#ifdef __cplusplus
extern "C" {
#endif // __cplusplus

What changes are proposed in this pull request?

1. This commit updates the generated delta_kernel_ffi.h to include c++ compatibility to that it will include extern "C" so that code generation will appropriately not try to use mangled method names.
2. Added the feature delta-engine-rustls to the ffi project in order to use the same feature from the kernel.

How was this change tested?

Tested against the dotnet implementation of delta lake.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.97%. Comparing base (d3b47ce) to head (da5eaa9).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1023   +/-   ##
=======================================
  Coverage   82.97%   82.97%           
=======================================
  Files          93       93           
  Lines       21979    21979           
  Branches    21979    21979           
=======================================
  Hits        18238    18238           
  Misses       2786     2786           
  Partials      955      955           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hntd187]

Isn't the entire name space surrounded in extern "C" I could be wrong last I checked it was C++ compatible.

[mightyshazam]

Isn't the entire name space surrounded in extern "C" I could be wrong last I checked it was C++ compatible.

It is with this change. It wasn't previously. The ffi crate also generates a .hpp file for C++.

[hntd187]

LGMT

[zachschuermann]

in the core kernel crate we get away with defining a 'default-engine-base' feature - I wonder if we could do something similar here to avoid having to check both features at each cfg?

[mightyshazam]

in the core kernel crate we get away with defining a 'default-engine-base' feature - I wonder if we could do something similar here to avoid having to check both features at each cfg?

I considered it after looking at the kernel config. The difference between the two is that the kernel base feature actually does something. In the ffi crate the features would look like

default-engine = ["delta_kernel/default-engine", "default-engine-base", "delta_kernel/arrow"]
default-engine-rustls = ["delta_kernel/default-engine-rustls", "default-engine-base", "delta_kernel/arrow"]
default-engine-base = []

It saves us verbosity, but it doesn't provide any real value, and, if someone were to enable it without one of the corresponding engine features, the code wouldn't compile.
A base feature is my preferred option. I just couldn't think of a good way to make it work. Maybe if it were like this

default-engine = ["delta_kernel/default-engine", "default-engine-base"]
default-engine-rustls = ["delta_kernel/default-engine-rustls", "default-engine-base"]
default-engine-base = ["delta_kernel/default-engine-base", "delta_kernel/arrow"]

It's redundant, but it doesn't cause surprise failure.
EDIT: I see that delta_kernel doesn't build with just default-engine-base. I'll make the switch.

[nicklan]

Awesome thanks! Just a small nit. I can merge once that's addressed

[nicklan]

nit: can we add a comment that the check in kernel/lib.rs that you've enabled one of default-engine or default-engine-rustls will ensure that we don't just enable default-engine-base here too.

[mightyshazam]

I assumed you meant adding a comment referencing the check in kernel/lib.rs to the Cargo.toml for ffi. That's what I did. Let me know if that was what you were getting at.

[nicklan]

Lgtm! Thanks for taking this on!