Sys crate idiom review

help
1

https://lib.rs/crates/fmod-core-sys

Disclaimer: bindings to non-free code. The sys bindings crate(s) are (MIT OR Apache-2.0) AND FMOD_EULA licensed. The Rust-idomatic bindings crate is MIT OR Apache-2.0 licensed.

I'm hoping to publish the -sys crates for my FMOD bindings relatively soon. As such, I'm looking for input/review on the idioms for publishing mostly mechanical sys crates like this. More specifically, I'd like input on:

  • I do not set the cargo package.links key, as they do not compile/provide the FMOD symbols, merely link to them.
  • I add lib/{arch} to the cargo:rustc-link-search path.
  • FMOD provides two binaries: fmodL, a logging binary for development use; and fmod, for production use, without instrumentation. (Both are optimized release builds.) I currently choose between these based on the value of $PROFILE.
  • The FMOD version number is PRODUCT.MAJOR.MINOR (BUILD). I map this to cargo as MAJOR.MINOR.PATCH-PRODUCT.MAJOR.MINOR-BUILD, where PATCH is a monotonically increasing version for patches of the bindings.
  • Bindings are pregenerated with bindgen. A script to regenerate the bindings is best-effort provided (but may not fully work as intended due to bindgen bugs cleaned up after by hand).
  • Bindgen's type_ is replaced with r#type.
  • Bindgen's __bindgen identifiers have been replaced with an appropriate payload identifier, as union usage in the bound headers is for tagged unions.
  • Headers are shipped as part of the package, but not used.
  • A static function is manually translated to Rust is marked #[inline]
  • fmod-studio-sys's C headers also #include fmod-core-sys's C headers; this is done by symlink for the C headers[1] and Cargo dependency for the Rust symbols.
  • LICENSE.txt is the FMOD_EULA and is included via symlink.

Additionally, I don't have access to a macOS or Linux box to test installation on those. I'd appreciate any testing to ensure other people can successfully build the crate; if you get FMOD Engine[2] you should be able to get the fmod-rs crate building with just the information in the root README. If not, that's a documentation bug I'd like to hear about. (I'll probably need to link the platform docs[3] as well.)

Alternative linking idea I just had but don't know if I like: provide link_fmod! and link_logging_fmod! macros which expand to #[link = "fmod"] extern {} or #[link = "fmodL"] extern {} to let/require the consumer to link/choose which/how to link FMOD, rather than trying to provide this, since we are dynamically linking anyway and can't automatically find/provide a statically linked version...

  1. no idea what symlinks will look like in the packaged version... ↩︎

  2. Read the licensing terms first; usage is free under $200k revenue with attribution. ↩︎

  3. And I just found a docs bug in them: they claim which lib is linked depends on cfg(debug_assertions), not whether the build profile inherits from release or debug. This'll get fixed once the sys crates finalize what protocol they use for choosing which lib to link. ↩︎

2

I'm going to preface this with the following - the #1 pain in my current job is making native dependencies build and link properly.

If a crate's build.rs script doesn't work out of the box (or with minimal configuration/installation) then I'm going to be cursing the crate author's name every time I need to use it.

This seems like an odd decision. Even if your fmod-core-sys crate isn't compiling the library, I would expect it to set the links and do the work of finding the right library and folder.

The lib/{arch} thing feels quite magical because it depends on the user's project structure and which directory cargo invokes the compiler from. My expectations would be to first check for input explicitly passed by the user, otherwise using some common, project-agnostic heuristics. Maybe something like

  1. Check some sort of $FMOD_LIB_DIR variable, then
  2. Fall back to something like pkg-config, then
  3. Check system-wide places the library might be installed (e.g. your package manager could always put it in /opt/fmod-core/{arch}), otherwise
  4. fail the build with a useful error message (e.g. "Unable to locate the fmod library, please set $FMOD_LIB_DIR or make sure it is installed").

Bonus marks if you print things as build.rs checks them so an end user has somewhere to start troubleshooting, because things printed to stdout in a build script will only be shown when the build script fails.

I'm not a massive fan of this.

It means anyone using the fmod-core-sys crate will need to explicitly link to fmod. That means anything which may depend on fmod-core-sys transitively or intermediate libraries that are compiled in test mode (but you can't add the attribute to your intermediate library's lib.rs because then it will conflict with what your downstream users want, so wrap it in a #[cfg(test)] and make sure each integration test uses link_fmod!()).

Feature flags sound like the best way to switch between the logging and non-logging versions. It's effectively what they did when compiling the original fmodL library after all - define some symbol so #ifdef can enable/disable logging.

3

Some further questions I had,

  • What's with all the manual modifications to the generated bindings? You're going to wrap this in a safe interface anyway, so the only person who'll need to write type_ is the fmod-core author (presumably you)
  • What's the cross-compilation story like? You've mentioned lib/{arch} so it sounds like you've given it a bit of thought, but how does this actually look/feel in practice?
  • Aren't static functions like Rust's version of pub(crate)? C actually has its own inline specifier, so I don't think it is related to #[inline].
  • I didn't see any size_t's or uintptr_t's in your generated binding.rs. Does that mean bindgen has inlined the size_t typedef for your particular architecture? Because that could be bad... If you want to pre-generate bindings[1] then you might want to generate them for each build target in case fmod has funky #if's that modify the API based on architecture/os/whatever

  1. Which I'm all for!
    Having bindgen as a build dependency is a pain in terms of compilation times... It gets doubly painful when you have another dependency which also uses bindgen at build time, except it uses a different version which links to a different version of LLVM, and therefore cargo rejects your build because both versions of llvm-sys are incompatible and set the links key. ↩︎

4

Some clarification not explicit in the OP:

The lib I'm linking to is only available as a dynamic library, and is not available to statically link[1]. The provided lib files are import libraries[2]. The official directions for loading the runtime DLLs is to place them in the same directory as the executable[3] or manually load them before calling any FMOD functions.

This restriction is the reason why the linking situation is interesting for this sys crate.

The package.links key is unfortunately named; any number of crates can link to a library, but the only-one enforcement provided by the key is relevant if you're providing unmangled symbols, rather than just linking you them. [source]

The one thing this loses is the ability to override the buildscript to declaratively replace the linking information, which is unfortunate.

No symbols are being provided by these sys libs, because they legally cannot provide a static library (see above).

The lib/{arch} is just added to the lib search path, so if the libs are found on the search path

On Windows, there actually is a standard place for the lib installer[4] to put the import lib files, so I can check to see if that exists, as well as respect an environment variable explicit specification. However, the Linux libs are just provided as a tarball and the macOS libs as a DMG image (where you just drag a folder to wherever you want it) so there aren't any standard locations to check there.

I would just include the import libraries in the crate package, but this is legally questionable at best[5].

As a proprietary library distributed by tarball, I don't think that this would show up in pkg-config or similar. Also as a Windows user I don't know how these tools are supposed to work :upside_down_face:

Well, due to the need to provide the dylib runtime dependencies as well, they already do need to do some work to provide FMOD (and comply with the license).

The (slight) difference is that Cargo feature flags are intended to be purely additive. While semantically adding logging is purely additive, the build/runtime requirements are not additive (notably switching which dylib is required).

Additionally, it'd be nice if the root package has some way of forcing that yes they do or no they don't want the logging build, based on which DLL they're actually shipping.

A touch of perfectionism, maybe. The generated bindings need a bit of cleanup anyway, and I'm not a bindgen purist (anymore), so sticking in these fixups to make the raw bindings a little bit nicer is low cost.

Additionally, there are valid but nearly impossible to make Rust-safe usages[6] of the library. As such, I'd like usage of the raw APIs not to feel significantly worse than using the C API in C.

I haven't tested it (again, because I lack a box to test the compiled unit on), but in theory it should be as simple as providing the right import libraries and runtime dylibs. (Actually getting them is annoying due to the distribution in platform native containers not extractable on other platforms, but... that's unfortunately not a problem I can address.) FMOD is a port-capable library intended to run on most mainstream targets (including consoles) with 32 bit c_int and 64 bit c_long_long, and my bindings shouldn't make this any worse.

I've used --size_t-is-usize, but the FMOD headers actually don't use size_t/uintptr_t. All "pointer sized pointer-or-integer"s cross the API as pointer-to-incomplete, and the only target assumption is that enums which don't fit in int16_t are int32_t (which is part of FMOD, not my bindings).

In a .c file, static is internal linkage, meaning the symbol is not visible to other compilation units; basically pub(crate), yes. In a header file, though, it's that, but in every compilation unit. In Rust, the way to say "compile this in every compilation unit (and thus make it available for inlining)" is #[inline].

C inline provides an (optional) inlining hint along with internal linkage, so is a closer match to #[inline]. In practice, though, static and inline on header functions are alternatives with subtly different footguns for the same functionality (function implementation in a header).

[cppreference on static] [cppreference on inline]

I am thankful for the notes, it's just... linking to a proprietary non system library dynamically is not a normal use case for Cargo, and comes with its own peculiarities.

  1. Well... it's available as part of the highest cost license for large studio use, which gives you source access and permission to make your own static build. ↩︎

  2. Relevant: the number of entry points to the library is in the single digits, so instrumenting those to do dylib loading if necessary is low cost. These functions are also those expected to be called once at program startup and not again. ↩︎

  3. Experimentally, the working directory works, and I've not actually tested alongside the binary when that is not also the working directory. I expect the answer is just "wherever the OS looks for relative path dylib loading." ↩︎

  4. Why they don't just give you a zip I don't know. There's no actual software being installed... ↩︎

  5. I also lack the disposable income to have a lawyer determine that distributing the import libraries is in fact a violation of the EULA. However, it seems fairly clear to me that this would not be allowed. ↩︎

  6. Namely, FMOD provides flags to disable their built-in async task queue and do everything synchronously, with the caveat (and benefit) that this makes everything thread-unsafe and have to happen in a single thread. If you already have your own async task queue and can set up a worker thread to do all of the FMOD calls, this is beneficial, as you can avoid the extra overhead of using FMOD's synchronization behind your own.

    However, exposing this to Rust either requires duplicating the entire API surface to a thread unsafe version or giving all the types a MaybeSync generic type (and neither even completely work since there are more than one entry point function). So, my midterm compromise is to allow unsafe usage of the raw API (that can still bridge to/from the safe API) if this use case is actually desired, rather than put a lot of effort into a low payoff feature (that I, potentially the only user of the bindings, won't even use). ↩︎

5

Yeah, I think the bit I didn't read carefully enough is how you are on Windows and using import libraries. That changes things quite a bit because the linker doesn't need the actual DLL at compile time in order to generate an executable.

I feel like having such a Windows-centric way of doing your build script might make it difficult for non-Windows users to depend (possibly transitively) on the fmod-core-sys crate.

Wow, that's crazy!

It feels like the API version of someone asking "what colour is your function?" and FMOD replying, "yes".

I can understand why they did things they way they have, it makes sense, but that doesn't stop it from being a strange API decision.

Yeah, It depends on how far you want to take things and whether this is more for a personal/work project or for general purpose open-source.

It might be worth wiring up GitHub Actions so the library is compiled/tested on a Linux machine every time you do a commit. That way you would know you pass the smoke test. But again, it really depends on the amount of time you want to invest in Linux/MacOS users because it's a massive pain trying to debug build problems when CI is the only machine you can use to test changes[1].

If you do choose to go down that path, there are ways to pass secrets to a GitHub Action. That should give you a secure means of providing product codes or download URLs or whatever without violating your license.

Yeah, that's fair enough. It just means the fmod-core-sys crate is going to be quite an infectious/obtrusive dependency, but that's the nature of the beast :man_shrugging:

  1. Think typical edit-compile-test cycles of 5-10 minutes. Mine were something like 50 minutes because we were compiling TensorFlow from source and the build artefacts for one build were big enough that it'd evict the others from the GitHub Actions cache, so every build on every OS was effectively a fresh build :grimacing: ↩︎