Improved library detection and binding generation #30
Conversation
Adds libiio-sys to the workspace defined by the industrial-io package.
The libiio-sys build script can now automatically detect libiio versions 0.19 through 0.25 and generate the appropriate bindings using bindgen. This eliminates the need for version-specific features.
|
Hey, thanks for the PR! I think there are a few things to keep in mind for this crate:
So I think some combination of what you propose and what's already here would be optimal. First, it would be cool to be able to create the bindings, but only as an option. Something like: similar to what I did in the Paho MQTT client crate: Running bindgen in the build is painfully slow, and produces the same output every time given the same header. Why re-run it every time you do a clean build? Especially if you're building natively on a small board because you don't have access to the other C libs for the target that your app needs to build. So I definitely like the pre-generated bindings for compile time. The lesson I've learned with pre-generated bindings, though, is that you at least need to match the word size (64-bit vs 32-bit) for the intended target to avoid segfaults. But you're right in that it would be nice, when you're building natively on Linux, for the default build to try and figure out what version of IIO is installed on the machine. But it would be cool for it to determine the version and then use the pre-generated bindings for that version. And then maybe just default to the latest supported version if it can't find the installed library header. When you are cross-compiling, the IIO version on the target may not be the same as on your build host. And you may not even have the cross-compiled library on your build host. So it's nice to be able to specify the version of IIO on the target board. I can't give that up! And finally, you may want to build this as a network client on a non-Linux system. Someone got it working for macOS (see #17 and #18), and theoretically it should work for a Windows client as well. It seems you removed that? Does your solution still work on a Mac then? |
|
My rationale for removing the Mac-specific code was that it didn't seem to be doing anything special other than setting the library search path. pkg-config handles that internally, so it should just work, but I would need to test it. Same deal with Windows - if you can get everything configured properly ahead of time (and I've checked; the pkg-config crate supports Windows), it should just work. Testing on Windows is significantly easier for me than testing on Mac, so I can take care of that.
That being said, your point about cross compilation is a good one. I'm just not sure if having premade bindings is really the right solution. Logically - at least to me - if you're compiling against a specific version of a library, you should have the library available on the build machine. But I also know that we don't need to overcomplicate things, so I'm happy to reintroduce the pre-made bindings if you think it's the best move.
The issue with bindgen being re-run every time is also fixable.
So, how about this: you either choose a specific version (via a feature) or allow the build script to generate bindings from whatever you have installed, for the appropriate architecture.
If that works, I can update my branch and we can work from there.
Thanks!
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Frank Pagliughi ***@***.***>
Sent: Tuesday, March 5, 2024 1:26:53 AM
To: fpagliughi/rust-industrial-io ***@***.***>
Cc: Kaufman, Kai ***@***.***>; Author ***@***.***>
Subject: [EXT] Re: [fpagliughi/rust-industrial-io] Improved library detection and binding generation (PR #30)
Hey, thanks for the PR!
I think there are a few things to keep in mind for this crate:
* You may be cross-compiling to a target that has a different IIO version than the build host
* You might not have the IIO dev package (headers, etc) for the intended target - which is something you also discovered with the CI build.
* You might not be targeting Linux - such as the network client target for Mac which you seem to have removed.
* You might be building on a vastly underpowered Embedded Linux board
So I think some combination of what you propose and what's already here would be optimal.
First, it would be cool to be able to create the bindings, but only as an option. Something like:
$ cargo build --features=build_bindgen
similar to what I did in the Paho MQTT client crate:
https://github.com/eclipse/paho.mqtt.rust/blob/a8618882938e813483ed04b621a90295f1974661/paho-mqtt-sys/build.rs#L128-L222
Running bindgen in the build is painfully slow, and produces the same output every time given the same header. Why re-run it every time you do a clean build? Especially if you're building natively on a small board because you don't have access to the other C libs for the target that your app needs to build. So I definitely like the pre-generated bindings for compile time.
The lesson I've learned with pre-generated bindings, though, is that you at least need to match the word size (64-bit vs 32-bit) for the intended target to avoid segfaults.
But you're right in that it would be nice, when you're building natively on Linux, for the default build to try and figure out what version of IIO is installed on the machine. But it would be cool for it to determine the version and then use the pre-generated bindings for that version. And then maybe just default to the latest supported version if it can't find the installed library header.
When you are cross-compiling, the IIO version on the target may not be the same as on your build host. And you may not even have the cross-compiled library on your build host. So it's nice to be able to specify the version of IIO on the target board. I can't give that up!
And finally, you may want to build this as a network client on a non-Linux system. Someone got it working for macOS (see #17<#17> and #18<#18>), and theoretically it should work for a Windows client as well. It seems you removed that? Does your solution still work on a Mac then?
—
Reply to this email directly, view it on GitHub<#30 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AWE3VFTGE7LKOFIPHMNTBRTYWVQS3AVCNFSM6AAAAABEGNAVGWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZYGA2DQNBYHE>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
I have a Mac, but I don't use it that much, so I'm not too experienced with it. Is it common to have Ditto for Windows. I know that I don't have That said, it could definitely be useful if it's already installed. But if it isn't installed or fails for any reason, the build should attempt to recover. For example, try to use it, then, if it isn't installed look at the target OS; if we're targeting a Mac, then try the directories we're already using. Something similar for Windows. Does that make sense? |
|
Sorry for the late response.
I don't think it's uncommon. It's been a while since I used a Mac as my daily driver, but when I did, I definitely had pkg-config installed. It's not exactly an obscure tool - a lot of things rely on it in one way or another.
I do, but it's only because I have w64devkit installed. This isn't totally unreasonable to expect in 2024, but it's still a potentially faulty assumption that could hurt... granted, this library (or at least, this version of the library) can't even be used on Windows at the moment, so maybe we can take a risk there.
The Mac-specific code was actually assuming (when running on ARM) that Homebrew was installed, which doesn't seem ideal. I don't know how many developers with ARM-based Macs are using this library, but I kind of doubt there are so many that it would be unreasonable to make any changes. I made a diagram showing what I think is a reasonable flow for the build script to follow (with some minor details like caching removed, and some areas for discussion left in):
Let me know what you think. |
This PR removes the version-specific features and bindings from
libiioandlibiio-sys, and provides a new build script that usespkg-configto locate the native libiio (versions 0.19 through 0.25) on the system.bindgenis then used to generate bindings from the system-wide header.I've marked this PR as a draft because it currently causes CI failures due to libiio not being installed in the runner. I'm open to suggestions for ways to handle this. My first thought was to simply install a specific version (or maybe multiple versions) of libiio as part of the CI process, but there may be a better approach.