Best practice for sys bindings to a non OSS library

code review
1

(CYA disclaimer: I am not a lawyer and nothing in this post constitutes legal advice. TL;DR at the bottom.)

My bindings to the FMOD Engine are nearing a usefully usable state. Unfortunately, due to FMOD being proprietary licensed software, there are some complications involved in packaging the bindings for distribution via crates-io. Specifically, while the EULA permits use of the SDK (that being, roughly, the headers and documentation) for this purpose[1], the actual engine binaries MUST NOT be redistributed as part of a game engine or tool set.

So, okay, I can publish my bindings for working with the FMOD SDK, but must leave acquiring of the FMOD Engine to be done by the downstream user of my bindings. How should this be done to best integrate into Cargo's build system, and in a way that's at least somewhat portable?

My current solution is that the sys crate sets the package.links manifest key, despite this being questionably a misuse[2], specifically such that downstream can use a link override to link the library if the behavior I provide isn't sufficient. (Although, there is a license available which grants source access and static linking, and I might eventually add support for that mode to FMOD.rs if I purchase that license for other reasons, in which case the links key becomes a lot more appropriate.)

Then, by default, the buildscript just emits the cargo:rustc-link-lib=LIB directive, expecting the library to be available in the default search path. (If it's not, use the link override.) I also offer an optional link-search Cargo feature which attempts to determine what the conventional install location for the SDK on the host would be in order to emit a cargo:rustc-link-search=PATH directive, but notably I'm as of current choosing to panic the build if link-search is set but I can't guess a conventional install path[3]. (I currently allow the build to continue if I can guess a conventional path even if the path doesn't exist on disk; I wonder if that should also be a fail-the-build condition, notifying the developer that they need to acquire the engine separately instead of getting an ugly link error...)

Finally, since feature = "link-search" is searching outside of OUT_DIR, it matters for build-time linking, but doesn't impact the dynamic library search path, meaning that even with feature = "link-search" the developer still needs to copy the runtime dylib into the directory where they're doing cargo run to allow the built binary to actually load the dylib and work.

Finally, FMOD ships two versions of the library: fmod.dll (release binary for production code) and fmodL.dll (release binary with logging enabled for development). Which one gets asked for is determined by the PROFILE environment variable, which reports whether the used profile inherits from dev or release (roughly whether --release is being used). No way to change this other than link overrides is provided.

I'm agonizing over immediately getting this "right" for the sys crate perhaps a bit more than I should because in order to usefully match the sys crate version to the library version, they're starting out at a semver-stable 2.YY.ZZ version, making fixing mistakes harder. Whatever interface I'm committing to for the buildscript should stay backwards compatible for approximately forever.

Concrete questions:

  • Is this use of package.links (enabling link overrides but not defining symbols) appropriate?
  • Is the default behavior of just emitting cargo:link-lib and hoping for the best reasonable?
  • Is deciding which binary to link based on PROFILE reasonable?
  • Should feature = "link-search"...
    • Panic the build if there isn't a known conventional SDK path?
    • Panic the build if the conventional SDK path is known but doesn't exist and contain the binary?
    • Copy the searched out binary into the output directory somehow?
  • Am I overly stressing about this?

  1. I believe the EULA permits this use, anyway, but IANAL and this does not constitute legal advice. I have reached out to Firelight Technologies to ask for clarification before publishing. ↩︎

  2. Specifically, a primary benefit of the links key is to ensure only one crate links a native library and provides its symbols, as multiple crates providing a native lib could lead to duplicated symbols. Since linking to the dynamic library doesn't define any symbols, and it's perfectly fine to link the same library multiple times, I don't necessarily need this first behavior, and am using the links key primarily for the ability for downstream to override the link handling. (There is global state that needs a global lock to be exposed safely, but for better or worse, that lives in the wrapper crate without a links key. Maybe I should pull the global lock into a separate crate shared between breaking revisions to the wrapper so duplication isn't a soundness hole...) ↩︎

  3. There certainly exists a conventional directory for Windows (the download comes as an installer), and there might be on macOS (I need to pull out my Mac to check how the .dmg behaves there), but there certainly isn't for Linux (the download is just a .tar.gz). I have no idea how the pkg-config/vcpkg/etc tools are supposed to work, but this very much isn't a "system library" anyway, it's a dylib (Windows .lib and .dll; macOS .dylib; Linux .so) that you're expected to ship with your application. (I wonder if I can/should be using raw-dylib on Windows now rather than the .lib import library...) ↩︎

4 Likes
2

Since I labeled this as code-review, here's the buildscript for fmod-core-sys in question as it currently exists, so there's less need to jump to and look around my GitHub project:

use std::env;

fn main() {
    build::rerun_if_changed("build.rs");

    link_lib();
    if cfg!(feature = "link-search") {
        link_search();
    }

    println!("cargo:version=2.02.16");
}

fn link_lib() {
    let dev = build::profile() == "debug";
    let windows = build::cargo_cfg_target_os() == "windows";
    let lib = match (dev, windows) {
        (true, true) => "fmodL_vc",
        (true, false) => "fmodL",
        (false, true) => "fmod_vc",
        (false, false) => "fmod",
    };
 
    build::rustc_link_lib(lib);
}
  
fn link_search() {
    if cfg!(windows) {
        link_search_windows();
    } else {
        panic!("failed to guess conventional FMOD Studio API path for this host");
    }
}

fn link_search_windows() {
    let arch = build::cargo_cfg_target_arch();
    let os = build::cargo_cfg_target_os();

    let program_files = env::var("ProgramFiles(x86)").expect("failed to get ProgramFiles(x86)");
    let (fmod_os, lib_dir) = match (&*os, &*arch) {
        ("windows", "x86_64") => ("Windows", "x64"),
        ("windows", "x86") => ("Windows", "x86"),
        ("linux", "arm") => ("Linux", "arm"),
        ("linux", "aarch64") => ("Linux", "arm64"),
        ("linux", "x86") => ("Linux", "x86"),
        ("linux", "x86_64") => ("Linux", "x86_64"),
        ("macos", _) => ("Mac", ""),
        _ => {
            panic!("failed to guess conventional FMOD Studio API path for this target");
        },
    };

    let link_dir = format!(
        "{program_files}\\FMOD SoundSystem\\FMOD Studio API {fmod_os}\\api\\core\\lib\\{lib_dir}"
    );
    build::rerun_if_changed(&*link_dir);
    build::rustc_link_search(&*link_dir);
}

This is using build-rs (transparency: one of my crates) as a convenience typed wrapper[1] around the cargo envs and printlns.

  1. The primary benefit being that it should have every potentially multi-valued input returning Vec, so e.g. you can't be surprised by target_family reporting as wasm,unix instead of just wasm. Which is actually the change (to wasm32-wasi, IIRC) that prompted to publish the thin wrapper. ↩︎

3

I have no opinion on the specific question, but imho rushing into 2.x.y version immediately is a bad idea. Rust versioning works differently from fmod versioning, you'd just be inviting trouble. And certainly there will be some big & small issues which will uncover after the release, and require a semver bump anyway.

I'd suggest to start with 0.1.0 and use the usual pre-release versioning until you're sure you're happy. If you want, you can embed fmod version into the patch, e.g. 0.1.2xy, or a similar scheme. In fact, does fmod even obey semver? If not, setting the crate version to be tautologically equal to fmod version would cause nasty surpises for downstream crates when fmod version bumps.

Perhaps you should consider a version-agnostic (best effort) binding crate, with the exact library version selected via some configuration option, e.g. like inkwell does, or embedding fmod version into the major version of your crate, like llvm-sys.

2 Likes
4

For an example of a sys crate that has separate versioning from the library it's binding to, there's at least zstd-sys, which notes the zstd version after the Rust library version, e.g. 2.0.8+zstd.1.5.5. IIRC Cargo won't check the metadata that comes after a version and will warn you if you try to include it when specifying it as a dependency which is a little unfortunate I think, but otherwise this scheme seems like a good option.

1 Like
5

To be clear, my high-level adapter is definitely starting at 0.1.0, it's just the raw generated -sys crates that are (potentially) jumping straight to 2.x.y.

So the scope of potential problems due to specifically my code is almost entirely contained to just the buildscript, and the rest is just an autogenerated translation of the strict-ABI-safe C headers.

...but I will probably start with a -preview release version, at least.

This is exactly what the +meta part of the version is for, and I'm using it for that.

Quoting from my own docs here since I address this:

The bindings crate has its version derived directly from the FMOD library version, for easier pinning of a specific header version. To be specific, for a given FMOD version aaaa.bb.cc (aaaa = product version, bb = major version, cc = minor version), the bindings crate is released as version bb.cc.dd+aaaa.bb.cc-BUILD, where BUILD is the specific FMOD build version, and dd is an FMOD.rs-specific patch number, to allow for bindings updates if necessary, though these are expected to be quite rare in practice.

In application use, it is recommended to pin a specific version of this crate. FMOD checks that the header version matches the dynamic library version, so if a version mismatch occurs, FMOD will fail to initialize.

The currently vendored headers are for FMOD Engine 2.02.16 (build 135072) and the crate version is 2.16.0+2.02.16-135072.

Based on the detailed change notes scanned going back to at least 2.02.00 (released 2021), FMOD does maintain ABI compatibility (stricter than API compatibility) within the same major version (the second component of the FMOD version). I would need to reverify, but I'm pretty sure I successfully ran a test with a temporary header version mismatch during an update, indicating that the version match validation permits a compatible mismatch in minor version.

Of course, C API compatibility doesn't directly translate to Rust API compatibility (e.g. C always implicitly has ..default() in struct initialization), but that's an issue shared by every -sys crate in existence. Plus, the FMOD API is carefully designed in the true ABI-safe C subset. (E.g. all function parameters are scalars, user defined types only cross the DLL boundary by pointer, and the big argument bundle types are prefixed with a struct size field.)

I've got the initial plumbing set up for the high-level adapter crate to do so, but it'd be a bit more work for the raw bindings. Not insurmountable, but I want to avoid needing to run bindgen or extract API information from the dylib in the buildscript, so being version agnostic would make the generated bindings a lot less generated. Plus, the version number is part of the header and needs to be passed in to initialize the engine, so that makes it more difficult as well. (Especially since it's encoded as binary coded decimal (e.g. 0x020216) instead of something reasonable.) I'd be a bit more likely to not couple the versions so directly if the library version weren't hardcoded into the header.

6

Should this be using a target-checking env var instead? e.g.,

build::cargo_cfg_target_family().iter().any(|s| s == "windows")
1 Like
7

That level of the dispatch actually does want to know the host, since it's about finding where the SDK is located to link against it at build time. It doesn't impact the target runtime dylib path at all.

Inside link_search_windows() the target OS is dispatched over.

1 Like
8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.