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
- Branch
- inotify
- Lint
No Linters Available - Unit
No Unit Test Coverage
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 | ||
|---|---|---|
| 24 ↗ | (On Diff #9401) | 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 | ||
|---|---|---|
| 48 ↗ | (On Diff #9401) | 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 | ||
|---|---|---|
| 51 ↗ | (On Diff #9401) | Assuming that means "comments indent 0" and not "comments indentation should match surrounding indent, which is 4". It's a bit ambiguous. |
| 54 ↗ | (On Diff #9401) | Yes, good point -- since the instructions will be to add ${Inotify_INCLUDE_DIRS} to the include path, they must be set to avoid warnings. |