-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Description
Background
I was investigating an issue with the conan recipe for Poco 1.13.3 while using MinGW. I modified the recipe for Poco 1.14.0 (locally) and stumbled upon a possible issue with 1.14.0.
Generally there seem to be a problem with discovering the correct value of HAVE_SENDFILE.
Even though I noticed this while building with MinGW, it does not actually affect the output using MinGW, because of:
if (MINGW)
target_link_libraries(Net PUBLIC "mswsock.lib")
endif()
I will get back to that later.
Describe the bug
When building Poco 1.14.0 with MinGW 12.2.0 and CMake 3.16.3 I noticed the following oddity:
-- Looking for include file sys/sendfile.h
-- Looking for include file sys/sendfile.h - not found
-- Looking for sendfile
-- Looking for sendfile - not found
-- OS has native sendfile function
-- CMake 3.16.3 successfully configured Poco using MinGW Makefiles generator
Net/CMakeLists.txt does not find neither 'sys/sendfile.h' nor a 'sendfile' symbol, but still prints out a STATUS message that 'OS has native sendfile support'.
There seem to be a number of problems with the following block:
if (MSVC)
set(HAVE_SENDFILE ON)
else()
include(CheckIncludeFiles)
include(CheckSymbolExists)
check_include_files(sys/sendfile.h HAVE_SYS_SENDFILE_H)
if(HAVE_SYS_SENDFILE_H)
check_symbol_exists(sendfile sys/sendfile.h HAVE_SENDFILE)
if (NOT DEFINED HAVE_SENDFILE)
check_symbol_exists(sendfile64 sys/sendfile.h HAVE_SENDFILE)
endif()
else()
# BSD version
check_symbol_exists(sendfile "sys/types.h;sys/socket.h;sys/uio.h" HAVE_SENDFILE)
endif()
endif()
if (DEFINED HAVE_SENDFILE)
message(STATUS "OS has native sendfile function")
add_compile_definitions(POCO_HAVE_SENDFILE)
endif()
My initial issue stems from if (DEFINED HAVE_SENDFILE). check_symbol_existswill define the variable HAVE_SENDFILE but set it to false.
This means that OS has native send function will always be written to the screen and POCO_HAVE_SENDFILE will always be defined.
The same problem exists for the line if (NOT DEFINED HAVE_SENDFILE). I do not believe the above block will ever check for sendfile64 as HAVE_SENDFILE is always defined but possibly false.
Even with the two above issues fixed, there seems to be a problem with check_symbol_exists(sendfile64 sys/sendfile.h HAVE_SENDFILE).
Because HAVE_SENDFILE is already defined, the check does not occur at all. Not sure if this is a bug in CMake 3.16.3 or expected behaviour. But, either HAVE_SENDFILE needs to be undefined again, using unset(HAVE_SENDFILE CACHE), or we need to use a different variable name, eg HAVE_SENDFILE64.
From my testing, with both Linux gcc 12 and MinGW 12, I believe the block should be:
if (MSVC OR MINGW)
set(HAVE_SENDFILE ON)
else()
include(CheckIncludeFiles)
include(CheckSymbolExists)
check_include_files(sys/sendfile.h HAVE_SYS_SENDFILE_H)
if(HAVE_SYS_SENDFILE_H)
check_symbol_exists(sendfile sys/sendfile.h HAVE_SENDFILE)
if (NOT HAVE_SENDFILE)
check_symbol_exists(sendfile64 sys/sendfile.h HAVE_SENDFILE64)
endif()
else()
# BSD version
check_symbol_exists(sendfile "sys/types.h;sys/socket.h;sys/uio.h" HAVE_SENDFILE)
endif()
endif()
if (HAVE_SENDFILE OR HAVE_SENDFILE64)
message(STATUS "OS has native sendfile function")
add_compile_definitions(POCO_HAVE_SENDFILE)
endif()
The reason I am adding OR MINGW to the initial line is the addition of mswsock.lib later in the CMakeLists.txt as described in the beginning.
This causes SocketImpl::sendFile to link to TransmitFile which matches the ìfdef POCO_OS_FAMILY_WINDOWS` block. (This is the issue I am investigating in the conan recipe, as it does not propagate this dependency to the recipe consumer).
To Reproduce
Made different changes for test purposes:
- Changed
sys/sendfile.htosys/sendfilex.hand re-ran configure - Changed
sendfiletosendfilexand re-ran configure - Changed
sendfile64tosendfile64xand re-ran configure
Expected behavior
POCO_HAVE_SENDFILE should not be defined as a compile definition if sendfile is unavailable.
Please add relevant environment information:
- Linux GCC 12 and Windows MinGW (12.2.0)
- Poco version 1.14.0
- CMake version 3.16.3