search

SpartanJ / efsw

efsw is a C++ cross-platform file system watcher and notifier.
Other
662 stars 101 forks source link

Cross building on macOS may cause `EFSW_FSEVENTS_NOT_SUPPORTED` incorrectly defined #175

Closed solarispika closed 5 months ago

solarispika commented 5 months ago

I tried to cross build the project for macOS intel from macOS apple silicon using Conan, and encountered problem of missing events. After some in-depth investigations, it turned out to be kqueue instead of expected FSEvents being used causing the problem.

The root causes is that CMAKE_SYSTEM_VERSION was empty, so the following marks FSEvents not available.

https://github.com/SpartanJ/efsw/blob/341934765471e4074e90bb5205ff4a65c16499c6/CMakeLists.txt#L102-L104

CMake requires CMAKE_SYSTEM_VERSION to be set explicitly during cross building, but conan didn't do it right. I have reported this to Conan and they confirm it as a bug.

However, they also suggest being explicit to enable/disable a library feature instead of let build system deteremining it. I agree to it too. User not using Conan can also suffer from it by forgetting to set CMAKE_SYSTEM_VERSION correctly.

Here come questions/requests:

  1. Does the library work well on kqueue of macOS?
  2. Can user of the library have more control on which frameworks being used on macOS?
  3. premake5.lua has no such logic to define EFSW_FSEVENTS_NOT_SUPPORTED, does it mean macOS using premake5 can only use FSEvents? What if they build it on old OSX systems?
SpartanJ commented 5 months ago

Hi! Thanks for reporting it.

  1. Most probably not, kqueue has some severe limitations and has not been used actively for ears.
  2. Given that FSEvents has been available for many years, I would just simply remove that condition from the CMakeFile. macOS should always be built with FSEvents.
  3. No, it compiles with kqueue and fsevents available, but it will pick always FSEvents. FSEvents has been available since 2007, but the development of the library started in 2008, that's why there are some really old logic that today are not necessary.

But let me add a couple of things:

  1. CMake file is not officially supported by me, it's a users collaboration as the README states. I'm not very familiar with CMake, I always dislike it so I avoid it at least in my personal projects.
  2. The Conan package it's also a collaboration, I don't really know who maintains it. I'm also not a user of Conan, so IDK exactly how the packages work, I guess there's a repo with recipes and we will need to send a PR to update the recipe to the latest efsw commit.

Meanwhile I'll update the CMake file to keep the FSEvents backend active. Thanks again.

solarispika commented 5 months ago

Hi! It's great to see this problem solved. Thanks!

The conan package is not offically provided, it's just part of our work, so I can easily update it.

BTW, I think it is more appropritate to also remove the macro in premake4.lua and src/efsw/base.hpp if possible.

Anyway, thanks again!