ralfbiedert / interoptopus

The polyglot bindings generator for your library (C#, C, Python, …) 🐙
302 stars 27 forks source link

Use provided `my_api_guard()` function to generate bindings instead of assuming just inventory hash #91

Open kevin-cgc opened 1 year ago

kevin-cgc commented 1 year ago

I wanted to bump my api version after some internal bugfixes (without changing the API surface), and was confused when changing the APIVersion returned by my api_guard function did not change the bindings generated.

Here is an example of what I expected would update the version embedded in the (C#) bindings.

pub extern "C" fn ffi_api_guard() -> interoptopus::patterns::api_guard::APIVersion {
    let impl_version = 1;
    let api_version = interoptopus::patterns::api_guard::inventory_hash(&ffi_inventory());
    interoptopus::patterns::api_guard::APIVersion::new(api_version + impl_version)
}

It seems that the assumption that the APIVersion is always the inventory hash is made in the writer. I think it would make more sense, given that the api_guard function is developer defined, to use it's return value when generating the bindings.

https://github.com/ralfbiedert/interoptopus/blob/a2cc26a54f5600ece6ecabfa4328057116e872e1/backends/csharp/src/writer.rs#L90

As a workaround, I am just including a doc comment with "impl version: 1" on the ffi_api_guard function, and bumping that as needed to regenerate the bindings.

ralfbiedert commented 1 year ago

Maybe I misunderstand, but the API guard is (meant to be) strictly for catching mismatches between bindings and binaries, and it's only about catching mismatches between the binary API surface, and the way that surface is expressed in a binding.

kevin-cgc commented 1 year ago

I see. I was a bit confused about it initially, but realized that must have been the initial design intent after looking at the writer.

I think, especially since the api guard function is already explicitly implemented by the developer, it would be nice if we could have more control over what constitutes a mismatch between the bindings and binary. For my case, bumping the API guard version has been a very useful sanity check to make sure the correct version of the DLL is actually being loaded whenever I recompile against the latest bindings.

Otherwise, if the API guard pattern should be reserved for just the API surface, it may be less confusing if the API guard function implementation is provided by interoptopus rather than by developers? Not sure about that though.