#### Pre-Submission Checklist - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [ ] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [x] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: - [ ] PR should be backported to stable branches - [x] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description I've tested cmake-build in the FreeBSD environment, as requested by @miconda in #4099. And founded some troubles. First of all are: specific flags for FreeBSD and arch-name for FreeBSD cmake-environment. I've fixed it in the PR.
But there're some unresolved problems I want to discuss...
A `LOCK_METHOD` problem: There's a `LOCK_METHOD` variable in defs.cmake: https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004... And the explicit usage `LOCK_METHOD` as `USE_FUTEX` in a compile_definitions: https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004... I suppose it's incorrect while some OSs (FreeBSD, ex.) doesn't tested with futexlock.h earlier.
A resolv-link problem: There're an explicit links of the resolv library: https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004... https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004... I think it's incorrect while some OSs (FreeBSD, ex.) uses builtin resolv (libc, etc).
Initially сmake is built successful after the above fixes. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/4104
-- Commit Summary --
* cmake: add amd64 arch fallback * cmake: add FreeBSD support
-- File Changes --
M cmake/compiler-specific.cmake (2) M cmake/defs.cmake (4) M cmake/os-specific.cmake (3) A cmake/os-specific/freebsd.cmake (37)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/4104.patch https://github.com/kamailio/kamailio/pull/4104.diff
Thanks for testing and the PR. @xkaraman will review it and merge or commit in an adapted version.
A `LOCK_METHOD` problem: There's a `LOCK_METHOD` variable in defs.cmake:
https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004...
And the explicit usage `LOCK_METHOD` as `USE_FUTEX` in a compile_definitions: https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004...
I suppose it's incorrect while some OSs (FreeBSD, ex.) doesn't tested with futexlock.h earlier.
That is the default one used if none is provided. If FreeBSD requires some other we can overwrite the value in the `freebsd.cmake` to have a more reasonable one. Or use an empty string and maybe force the user to choose the appropriate one, which seems to break the common build case. There should also be a way to offer only supported ones per OS, but that might need some more changes but might be worth it in the end. @henningw @miconda Thoughts on this?
A resolv-link problem: There're an explicit links of the resolv library:
https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004...
https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004...
I think it's incorrect while some OSs (FreeBSD, ex.) uses builtin resolv (libc, etc). Initially сmake is built successful after the above fixes.
So, for you to build you have removed the link against `resolv`? Or you mean did it compile with unnessecary linkage
There is a section in Makefiles.defs regarding freebsd. Here we can see that the resolv library is not added to the LIBS. Regarding the locking method, I think freebsd just uses fastlock if executed on a standard architecture (e.g. x86, x86_64). Here the file section [link](https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004...)
That is the default one used if none is provided.
I think, the default value should be a `USE_SYSV_SEM` as in Makefiles.defs: https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004...
If FreeBSD requires some other we can overwrite the value in the `freebsd.cmake` to have a more reasonable one.
Do you think it's need to explicit set `LOCK_METHOD` to `USE_PTHREAD_MUTEX` (example) in the `freebsd.cmake` and also duplicate it as `target_compile_definitions` in os-specific, isn't it? https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004... For example for freebsd.cmake: ``` message(STATUS "USE_FAST_LOCK = ${USE_FAST_LOCK}") if(NOT ${USE_FAST_LOCK}) target_compile_definitions(common INTERFACE USE_PTHREAD_MUTEX) endif() set(LOCK_METHOD USE_PTHREAD_MUTEX) ``` I think it seems ugly. Also MacOS doesn't tested with futexlock.h earlier too.
I think freebsd just uses fastlock if executed on a standard architecture
Yes. But I said, the explicit `USE_FUTEX` as a default isn't correct, in my mind.
So, for you to build you have removed the link against `resolv`
Yep, I've built with removed link only.
The PR itself looks ok to be merged.
Indeed we may have to alter some of the link stuff of `kamailio` and `kamcmd` as noted by @drTr0jan for proper `FreeBSD` support.
Regarding the https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004..., it will probably go away and just define it per OS like it was in the makefiles.
@xkaraman approved this pull request.
@@ -8,7 +8,7 @@ option(PROFILE "Enable profiling" OFF)
add_library(common_compiler_flags INTERFACE)
# Define the flags for the C compiler -if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
is `amd64` reported by cmake due to `FreeBSD`?
@xkaraman approved this pull request.
@@ -8,7 +8,7 @@ option(PROFILE "Enable profiling" OFF)
add_library(common_compiler_flags INTERFACE)
# Define the flags for the C compiler -if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64") +if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64")
is amd64 reported by cmake due to FreeBSD?
Merged #4104 into master.
@drTr0jan commented on this pull request.
@@ -8,7 +8,7 @@ option(PROFILE "Enable profiling" OFF)
add_library(common_compiler_flags INTERFACE)
# Define the flags for the C compiler -if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64") +if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64")
Yes, of course. ```sh boris@boris:~/sources/kamailio% cmake --system-information | grep CMAKE_SYSTEM_PROCESSOR CMAKE_SYSTEM_PROCESSOR "amd64" ```
Indeed we may have to alter some of the link stuff of `kamailio` and `kamcmd` as noted by @drTr0jan for proper `FreeBSD` support.
Regarding the
https://github.com/kamailio/kamailio/blob/6b0b8cb84b7d0e965d50bdf0dfa5423004...
, it will probably go away and just define it per OS like it was in the makefiles.
Sound like a good idea. Should I create a new issue?
@drTr0jan no need, i will take care of it! thanks for the contribution!
-
Boris Korzun -
Henning Westerholt
-
Xenofon Karamanos