MDEV-40081: Remove outdated PIC explictness#5250
Conversation
There where too many cases that forced PIC to be on. CMake has a POSITION_INDEPENDENT_CODE attribute where platform requirements can be handled in a consistent manner. ADD_CONVIENCE_FUNCTION sets the cmake property POSITION_INDEPENDENT_CODE for all except DISABLED_SHARED. All other explict enables of PIC/pie have been removed. * ASAN didn't need the -fPIC so removed. * gprof hasn't need -fPIC or -no-pie in recent history (gcc-10+). * Remove the explictness around -fPIC everywhere using the PIC property. * Security hardened profiles can set -fPIC/pie directly as they are default options now. * WITH_PIC options. "Not of much use," - so its removed. In MERGE_LIBRARIES, we check the PIC property directly rather than make assumptions based on the CFLAGS. Bundled PCRE has explicit -DCMAKE_POSITION_INDEPENDENT_CODE=ON. Create tpool_embedded as convience library for embedded server. CSV, Sequence and userstat plugins as manditory plugins need to be recompile for embedded. Remove TOKUDB only MYSQL_PROJECT_NAME_DOCSTRING
There was a problem hiding this comment.
Code Review
This pull request refactors the CMake build configuration to replace custom WITH_PIC and _SKIP_PIC logic with CMake's native POSITION_INDEPENDENT_CODE target property. It also updates several plugins and the tpool library to support embedded server builds. The review feedback identifies two critical issues: first, removing the platform-skipping logic in cmake/libutils.cmake will cause build failures on Windows and macOS due to missing POSITION_INDEPENDENT_CODE properties; second, passing the OBJECT keyword to ADD_CONVENIENCE_LIBRARY in tpool/CMakeLists.txt is unsupported and will lead to a compilation error as CMake attempts to find a source file named 'OBJECT'.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| FOREACH(LIB ${LIBS}) | ||
| GET_TARGET_PROPERTY(LTYPE ${LIB} TYPE) | ||
| IF(LTYPE STREQUAL "STATIC_LIBRARY") | ||
| GET_TARGET_PROPERTY(LIB_PIC ${LIB} POSITION_INDEPENDENT_CODE) | ||
| IF(NOT LIB_PIC) | ||
| MESSAGE(FATAL_ERROR | ||
| "Attempted to link non-PIC static library ${LIB} to shared library ${TARGET}\n" | ||
| "Please use ADD_CONVENIENCE_LIBRARY, instead of ADD_LIBRARY for ${LIB}" | ||
| ) | ||
| ENDIF() | ||
| ENDFOREACH() | ||
| ENDIF() | ||
| ENDIF() | ||
| ENDFOREACH() |
There was a problem hiding this comment.
On platforms like Windows (where PIC is not used) or macOS (where PIC is always enabled by default), the POSITION_INDEPENDENT_CODE target property is typically not explicitly set.
By removing the platform-skipping logic (previously handled by _SKIP_PIC), this check will run on all platforms. On Windows and macOS, GET_TARGET_PROPERTY(LIB_PIC ...) will return NOTFOUND, causing IF(NOT LIB_PIC) to evaluate to true and trigger a FATAL_ERROR during the build.
To prevent build failures on these platforms, we should skip this check on Windows, Cygwin, macOS, or when shared libraries are disabled.
IF(NOT WIN32 AND NOT CYGWIN AND NOT APPLE AND NOT DISABLE_SHARED)
FOREACH(LIB ${LIBS})
GET_TARGET_PROPERTY(LTYPE ${LIB} TYPE)
IF(LTYPE STREQUAL "STATIC_LIBRARY")
GET_TARGET_PROPERTY(LIB_PIC ${LIB} POSITION_INDEPENDENT_CODE)
IF(NOT LIB_PIC)
MESSAGE(FATAL_ERROR
"Attempted to link non-PIC static library ${LIB} to shared library ${TARGET}\n"
"Please use ADD_CONVENIENCE_LIBRARY, instead of ADD_LIBRARY for ${LIB}"
)
ENDIF()
ENDIF()
ENDFOREACH()
ENDIF()
| ADD_LIBRARY(tpool_common OBJECT ${COMMON_SOURCES}) | ||
| IF(WITH_EMBEDDED_SERVER) | ||
| ADD_CONVENIENCE_LIBRARY(tpool_embedded aio_linux.cc aio_simulated.cc) | ||
| ADD_CONVENIENCE_LIBRARY(tpool_common_embedded OBJECT ${COMMON_SOURCES}) |
There was a problem hiding this comment.
The ADD_CONVENIENCE_LIBRARY macro hardcodes the creation of a STATIC library and does not support the OBJECT keyword.
Passing OBJECT as the second argument to ADD_CONVENIENCE_LIBRARY will cause CMake to treat "OBJECT" as a source file name rather than a library type. This will result in a build failure because CMake will look for a source file named OBJECT (e.g., OBJECT.cc or OBJECT.cpp) which does not exist.
The OBJECT keyword should be removed.
ADD_CONVENIENCE_LIBRARY(tpool_common_embedded ${COMMON_SOURCES})
There where too many cases that forced PIC to be on.
CMake has a POSITION_INDEPENDENT_CODE attribute where platform requirements can be handled in a consistent manner.
ADD_CONVIENCE_FUNCTION sets the cmake property POSITION_INDEPENDENT_CODE for all except DISABLED_SHARED.
All other explict enables of PIC/pie have been removed.
In MERGE_LIBRARIES, we check the PIC property directly rather than make assumptions based on the CFLAGS.
Bundled PCRE has explicit -DCMAKE_POSITION_INDEPENDENT_CODE=ON.
Create tpool_embedded as convience library for embedded server.
CSV, Sequence and userstat plugins as manditory plugins need to be recompile for embedded.
Remove TOKUDB only MYSQL_PROJECT_NAME_DOCSTRING