fix: remove the cgf to run the build.rs only when compiling on unix #58
Conversation
…argets. This allows to cross compile from another host to these targets
LeoBorai
changed the title
|
Hi @LostAquilae! Thatk you so much for openning this PR. I have shortened the PR title as then it will go as commit message. |
LeoBorai
left a comment
LeoBorai
left a comment
There was a problem hiding this comment.
I think we should consider using target_family (https://doc.rust-lang.org/reference/conditional-compilation.html#target_family) so we dont introduce unused dependencies nor build steps on non-unix systems.
| @@ -1,12 +1,4 @@ | |||
| fn main() { | |||
| #[cfg(any( | |||
There was a problem hiding this comment.
By doing this the build script will also run for Windows.
Perhaps we could constraint this to run on OS Family Unix only?
There was a problem hiding this comment.
Yes you are right, target_family seems a good way to allow build.rs to be run on linux while preventing it to be run on Windows. I don't think the use case of cross compiling from Windows to Freebsd will happen any time soon (I don't even know if that's actually possible). I fixed it in 7d45bff
Cargo.toml
Outdated
| [build-dependencies] | ||
| cc = "1.0.73" | ||
|
|
There was a problem hiding this comment.
cc is not used on Windows, so perhaps its better to scope this dependency to Unix based OS?
There was a problem hiding this comment.
I used the same codegen option, 'target_family', to restrain cc dependency to unix targets only
…unix targets in general
|
Thank you for your feedback ! I implemented what you suggested, it works perfectly for my use case. Let me know if everything is ok for you. |
Glad to hear @LostAquilae! |
|
@LostAquilae I just updated workflow action versions here: #59 Could you please pull these changes into your branch so tests runs? Thank you so much! |
* chore: updates actions * fix: remove lifetimes that could be elided
I merge your last commit in the current branch, it should be ok. Thank you ! |
|
@LeoBorai I fixed clippy warnings with last commit |
2 participants
When I tried to use this lib in a project where I was cross compiling from Linux to Freebsd, I ended up getting linking error because the cfg present in the build.rs doesn't allow the file to be run when compiling on Linux, preventing the Unix ffi from being built. I suggest to remove this cfg, as it allows cross compiling without breaking any other use case, as you check the actual target at runtime inside the build.rs