On Linux, inotify always exists; all you need is the header file.
On the BSDs, inotify is provided through a shim to kqueue, which
must be installed separately. Add a FindInotify to help sort
that out.
Based on RB 129316 and RB 129549.
Details
Details
- Reviewers
apol skelly tcberner kfunk ervin dfaure arrowd - Group Reviewers
Frameworks Build System - Commits
- R240:a02c4d0152a7: Detect inotify.
- On FreeBSD, reliably detects presence of libinotify in $LOCALBASE,
- Needs testing on Linux that it does find the header file.
Diff Detail
Diff Detail
- Repository
- R240 Extra CMake Modules
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Looks good to me, that is all the ports that depend on it, still find libinotify after modifying the patches to use this version.
| find-modules/FindInotify.cmake | ||
|---|---|---|
| 25 | You should add yourself here. | |
Comment Actions
Adding Kevin as reviewer -- I feel uncomfortable having so little feedback and I'm not sure who should be added if #buildsystem and Frameworks don't ACK.
Comment Actions
Not my forte, but I don't see anything blatantly wrong in here. Adding dfaure to see if he got something to say about it.
| find-modules/FindInotify.cmake | ||
|---|---|---|
| 49 | What's the difference with check_include_files as used by kcoreaddons/CMakeLists.txt? It seems to me that find_path won't know where to look, no, at least check_include_files seems much more appropriate for includes? | |
Comment Actions
@dfaure find_path is okay here, it's commonly used for finding includes. See: https://cmake.org/cmake/help/v3.0/command/find_path.html
check_include_files is used for just checking the existence of a include, not finding out where it is.
Example use: CHECK_INCLUDE_FILES (malloc.h HAVE_MALLOC_H)
Comment Actions
OK I see. On Linux it was enough to check that the header is present (-> in /usr/include) while on BSD it's part of a library that could in theory be installed anywhere. Makes sense to set an _INCLUDE_DIRS variable then.
| find-modules/FindInotify.cmake | ||
|---|---|---|
| 52 | Assuming that means "comments indent 0" and not "comments indentation should match surrounding indent, which is 4". It's a bit ambiguous. | |
| 55 | Yes, good point -- since the instructions will be to add ${Inotify_INCLUDE_DIRS} to the include path, they must be set to avoid warnings. | |