Skip to content
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

autodetect installed macfuse version 4 in build.rs #104

Merged
merged 2 commits into from
Jan 9, 2021

Conversation

mike-kfed
Copy link
Contributor

I was redirected here in my issue report for zargony/fuse-rs#155 when building the crate against macFUSE 4.x. There were lots of renames going on, one of them being the pkg-config information. This PR autodetects an installed v4 and makes it build against v4 - the hello.rs example plus tests work fine for me (tested on macOS 11.1 with macFUSE 4.0.5 on a M1 MacBook Air).

Copy link
Owner

@cberner cberner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! I had one comment that I think will make the detection of macfuse more robust

build.rs Outdated
@@ -6,9 +8,14 @@ fn main() {
{
#[cfg(target_os = "macos")]
{
let probelib = if std::path::Path::new(MAC_FUSE_4).exists() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking for the existence of this path seems brittle to me. Is it documented anywhere that it's guaranteed to exist for MacFuse 4.0? I think it would be better to detect this via pkg_config. In other words, first try to link against "fuse" and then fallback to "osxfuse", just like the non-macos code path below: https://github.com/cberner/fuser/pull/104/files#diff-d0d98998092552a1d3259338c2c71e118a5b8343dd4703c0c7f552ada7f9cb42L23

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my reasoning was that some package management tools might do some symlink shenanigans with the pkg_config files, so I decided to test for a file that only v4 installs. But you are totally right, simply trying to use it is a more stable approach - I implemented your suggested changes.

Copy link
Owner

@cberner cberner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for the contribution!

@cberner cberner merged commit 5fb2992 into cberner:master Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants