-
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_exists
will 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.h
tosys/sendfilex.h
and re-ran configure - Changed
sendfile
tosendfilex
and re-ran configure - Changed
sendfile64
tosendfile64x
and 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
Metadata
Metadata
Assignees
Labels
Type
Projects
Status