Try standard system path when pkg-config fails #444
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We're going to rely not only on pkg-config for locating Themis, but we cannot construct pkg_config::Library instances (it has private fields). Use a custom type with the same API instead.
We're going to provide an alternative code path for when pkg-config fails, so instead of panicking with an error message just output it to stderr and panic later. This enables us to insert some actions in between and prevent the panic.
Currently Rust-Themis requires pkg-config to successfully find Themis.
If it doesn't (for whatever reason) then the crate won't compile.
However, the library might be correctly installed in the system under
standard paths. This allows us to easily link against it dynamically.
Let's support this use case to allow non-packaged installations.
If pkg-config successfully locates core Themis library then we just go
with it. Otherwise we effectively try to run
cc -shared src/dummy.c -lthemis
and see if that compiles. If it does then apparently Themis is installed
and we can use it by emitting appropriate instructions to Cargo.
Note that we support only dynamic linkage in this case. Static linkage
will not work because we will have to figure out transitive dependencies
like Soter and OpenSSL, which is the reason for preferring pkg-config.
vixentael
reviewed
Mar 31, 2019
vixentael
added
the
installation
Lagovas
reviewed
Apr 1, 2019
| Ok(_) => { | ||
| eprintln!( | ||
| "\ | ||
| `libthemis-sys` tried using standard system paths and it seems that Themis |
There was a problem hiding this comment.
imho this message should be printed from get_themis function, not here. function try_system_themis just check is themis exists in themis and doesn't know anything about high-level concerns like libthemis-sys existence. here just return success/error code and maybe print how it was checked and why it is failed
There was a problem hiding this comment.
Makes sense. I'll move these error/status messages up the call stack.
We're printing status messages only there so let's keep it like that. Make try_system_themis() simply a utility function. Though, it will still print Cargo instructions if successful, like pkg_config::probe().
Lagovas
approved these changes
Apr 1, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently Rust-Themis requires pkg-config to successfully find Themis. If it doesn't (for whatever reason) then the crate won't compile. However, the library might be correctly installed in the system under standard paths. This allows us to easily link against it dynamically. Let's support this use case to allow non-packaged installations.
If pkg-config successfully locates core Themis library then we just go with it. Otherwise we effectively try to run
and see if that compiles. If it does then apparently Themis is installed and we can use it by emitting appropriate instructions to Cargo.
Note that we support only dynamic linkage in this case. Static linkage will not work because we will have to figure out transitive dependencies like Soter and OpenSSL, which is the reason for preferring pkg-config.