[CONC-521] Fix ucontext detection on macOS #152
Conversation
On macOS, the ucontext functions require _XOPEN_SOURCE to be defined before including ucontext.h. (Any value is fine, but it's supposed to be a POSIX-defined number, and 600 seems like the most common choice.) These functions are required for MYSQL_OPT_NONBLOCK to work on non-x86 platforms, e.g. Apple's M1 chip. This change sets _XOPEN_SOURCE to 600 on Apple platforms during CMake's feature detection and before including <ucontext.h> in ma_context.h. Fixes CONC-521.
cmake/CheckIncludeFiles.cmake
Outdated
| @@ -49,7 +50,12 @@ CHECK_INCLUDE_FILES (sys/un.h HAVE_SYS_UN_H) | |||
| CHECK_INCLUDE_FILES (unistd.h HAVE_UNISTD_H) | |||
| CHECK_INCLUDE_FILES (utime.h HAVE_UTIME_H) | |||
|
|
|||
| CMAKE_PUSH_CHECK_STATE(RESET) | |||
| IF(APPLE) | |||
| SET(CMAKE_REQUIRED_DEFINITIONS -D_XOPEN_SOURCE=600) | |||
There was a problem hiding this comment.
It is a bad idea, to support deprecated ucontext.h. For some strange reasons, there are two ucontext.h files, the correct one can be found in /include/sys while the deprecated can be found in include.
So correct fix for apple should be
+IF(APPLE OR NOT HAVE_FILE_UCONTEXT_H)
+ CHECK_INCLUDE_FILES (sys/ucontext.h HAVE_FILE_SYS_UCONTEXT_H)
ENDIF()
then in ma_context.h you need to check HAVE_FILE_SYS_UCONTEXT_H:
@@ -31,7 +31,7 @@
#define MY_CONTEXT_USE_X86_64_GCC_ASM
#elif defined(__GNUC__) && __GNUC__ >= 3 && defined(__i386__)
#define MY_CONTEXT_USE_I386_GCC_ASM
-#elif defined(HAVE_UCONTEXT_H)
+#elif defined(HAVE_UCONTEXT_H) || defined(HAVE_SYS_UCONTEXT_H)
#define MY_CONTEXT_USE_UCONTEXT
#else
#define MY_CONTEXT_DISABLE
@@ -52,7 +52,11 @@ struct my_context {
#ifdef MY_CONTEXT_USE_UCONTEXT
+#ifdef HAVE_SYS_UCONTEXT_H
+#include <sys/ucontext.h>
+#else
#include <ucontext.h>
+#endif
On Apple platforms, <ucontext.h> is deprecated and requires _XOPEN_SOURCE to be set, but <sys/ucontext.h> does not. Simplify the detection and inclusion logic accordingly. See #152
|
@9EOR9 With your insight I have cleaned things up a bit. |
|
After doing some more investigation, I think your first patch was ok - sorry, for extra. work. |
|
I came to prefer the second patch but I will defer to your judgment! Glad to see that the functionality will be included in the upcoming release. |
|
Is the proposed fix working for you @evanmiller ? I applied the patch and compiled the C connector and MariaDB server, but I still get |
|
@robertbindar Which patch did you apply? I no longer have a test machine but I would check the values of HAVE_FILE_UCONTEXT_H and HAVE_UCONTEXT_H in CMakeCache.txt to see if they're getting picked up. |
|
@evanmiller you're right, for some reason cmake can't find ucontext.h, I need to investigate more, thanks for the tips. |
On macOS, the ucontext functions require
_XOPEN_SOURCEto be defined before including ucontext.h. (Any value is fine, but it's supposed to be a POSIX-defined number, and 600 seems like the most common choice.) These functions are required forMYSQL_OPT_NONBLOCKto work on non-x86 platforms, e.g. Apple's M1 chip. This change sets_XOPEN_SOURCEto 600 on Apple platforms during CMake's feature detection and before including ucontext.h in ma_context.h.Fixes CONC-521.