-
-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jack-sys: search for jack library with pkg-config #154
Conversation
I'm surprised that this library doesn't use I think there is also a problem with how this would interfere with the following code segment: Lines 1077 to 1104 in 90ab9bd
(as jack_lib is hard-coded, but if we get another value from pkg-config , we might e.g. dlopen the wrong file; although that risk is probably really small, as most systems which want to mess with library paths probably set rpaths (sic) properly)
|
jack-sys/build.rs
Outdated
@@ -0,0 +1,3 @@ | |||
fn main() { | |||
pkg_config::probe_library("jack").unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a good idea to not always fail hard when the pkg-config call fails, e.g.:
pkg_config::probe_library("jack").unwrap(); | |
let _ = pkg_config::probe_library("jack"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? In what scenario would that be helpful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
basically every scenario in which pkg-config
or the jack.pc
file is unavailable, and/but the jack runtime libraries are available, as pkg-config
isn't really necessary for the library code to work, the code doesn't access need to reference any include files or such which would be found that way, it is only used to increase the accuracy of the linking (which may be incomplete, as noted above).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additional, it is really likely to just break completely on windows, as it is even more likely that jack.pc
files or pkg-config
are just unavailable there. pkg_config::probe_library
prints a warning in that case anyway, so if the final linking is failing or does something unexpected the user has enough information to fix it, but I don't think that pkg-config
should be always required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I changed this and added a comment. I have no idea how Cargo locates C libraries on Windows.
(btw I really hope I'm not too intimidating, this is a small change and it appears to work, but I would like to make it somewhat more error-proof) |
I don't think there's any problem there. Fedora's pipewire-jack-audio-connection-kit package installs a /etc/ld.so.conf.d/pipewire-jack-x86_64.conf file with the contents |
This fixes linking with PipeWire's JACK headers. Fixes RustAudio#142
Yes, please make this happen :) |
Thanks for merging. Could you publish a patch release so I don't have to hack the Cargo.toml of everything using this crate to get it to build? |
Done. Live in |
jack 0.8.3 is still failing with the same link error for me. Do you need to bump the version of jack-sys too? |
Sorry about that. Just bumped |
Now it works, thanks! |
This fixes the build with Pipewire's implementation of JACK: RustAudio/rust-jack#154
This fixes the build with Pipewire: RustAudio/rust-jack#154
Worked flawlessly, thanks a lot ! |
This fixes linking with PipeWire's JACK headers.
Fixes #142