TLDNR: Replace <Pkg>_FOUND with ENABLE_<Feature>.
Describe the bug
In CMakeLists.txt, there are ON/OFF-options for feature control, e.g. ENABLE_GD_FORMATS:
|
OPTION(ENABLE_GD_FORMATS "Enable GD image formats" 0) |
These options are used to control lookup of required dependencies, e.g.
|
if (ENABLE_GD_FORMATS) |
|
FIND_PACKAGE(ZLIB REQUIRED) |
|
endif (ENABLE_GD_FORMATS) |
This is fine.
However, in further processing (for macro definitions, selection of programs, export of gdlib.pc), the options are not used directly, but the <Pkg>_FOUND values are used instead:
|
IF(ZLIB_FOUND) |
|
INCLUDE_DIRECTORIES(${ZLIB_INCLUDE_DIR}) |
|
SET(HAVE_LIBZ 1) |
|
LIST(APPEND PKG_REQUIRES_PRIVATES zlib) |
|
ENDIF(ZLIB_FOUND) |
IMO this is wrong and should use the original ENABLE_<Feature> variables. A CMake package might have been found even if the user disabled the corresponding libgd feature. In particular, this happens in vcpkg for static triplets when CMake configs or Find module wrappers chainload additional packages to resolve transitive usage requirements.
- FreeType may use ZLIB.
- TIFF may use ZLIB, JPEG and WebP.
This problem is unlikely to be present with libgd's Find modules in cmake/modules which are unaware of actual static usage requirements. Modifying/replacing these modules is mandatory for static linkage. I still think it is an inconsistency which should be resolved.
As a variant, feature PNG also looks for ZLIB:
|
if (ENABLE_PNG) |
|
FIND_PACKAGE(ZLIB REQUIRED) |
|
FIND_PACKAGE(PNG REQUIRED) |
|
endif (ENABLE_PNG) |
I can imagine that this helps with static linkage. But I don't think it should override the effect of
ENABLE_GD_FORMATS.
I could create a PR (based on packaging in vcpkg). I would also merge the second spots (if(<Pkg>_FOUND)) with the first spots (if(<Feature>_ENABLED). This should be possible without source changes which would need adjustments to the autotools build system. But there maybe a need for upfront discussion.
To Reproduce
Build for static triplet in vcpkg with different feature configurations and check build, or do the equivalent directly in CMake.
for I in core core,jpeg core,tools,jpeg ; do
for J in "" ,png ,tiff ,freetype ,fontconfig ,webp ; do
./vcpkg remove libgd; ./vcpkg install libgd[$I$J] --no-binarycaching
<Check manually>
done
done
Or maybe (untested):
cmake ... -DENABLE_GD_FORMATS=0 -DZLIB_FOUND=1
Expected behavior
If a feature is not enabed, it shall never be compiled into libraries, and related executables must not be built.
Actual results
A feature and related executables may be built even if explicit disabled by the user.
Environment (please complete the following information):
Additional context
microsoft/vcpkg#27551
which now uses a minimal patch to forward ENABLE_<Feature> to <Pkg>_FOUND to achieve the desired build control.
TLDNR: Replace
<Pkg>_FOUNDwithENABLE_<Feature>.Describe the bug
In
CMakeLists.txt, there are ON/OFF-options for feature control, e.g.ENABLE_GD_FORMATS:libgd/CMakeLists.txt
Line 12 in 7b0d419
These options are used to control lookup of required dependencies, e.g.
libgd/CMakeLists.txt
Lines 123 to 125 in 7b0d419
This is fine.
However, in further processing (for macro definitions, selection of programs, export of gdlib.pc), the options are not used directly, but the
<Pkg>_FOUNDvalues are used instead:libgd/CMakeLists.txt
Lines 192 to 196 in 7b0d419
IMO this is wrong and should use the original
ENABLE_<Feature>variables. A CMake package might have been found even if the user disabled the corresponding libgd feature. In particular, this happens in vcpkg for static triplets when CMake configs or Find module wrappers chainload additional packages to resolve transitive usage requirements.This problem is unlikely to be present with libgd's Find modules in
cmake/moduleswhich are unaware of actual static usage requirements. Modifying/replacing these modules is mandatory for static linkage. I still think it is an inconsistency which should be resolved.As a variant, feature PNG also looks for ZLIB:
libgd/CMakeLists.txt
Lines 127 to 130 in 7b0d419
I can imagine that this helps with static linkage. But I don't think it should override the effect of
ENABLE_GD_FORMATS.I could create a PR (based on packaging in vcpkg). I would also merge the second spots (
if(<Pkg>_FOUND)) with the first spots (if(<Feature>_ENABLED). This should be possible without source changes which would need adjustments to the autotools build system. But there maybe a need for upfront discussion.To Reproduce
Build for static triplet in vcpkg with different feature configurations and check build, or do the equivalent directly in CMake.
Or maybe (untested):
Expected behavior
If a feature is not enabed, it shall never be compiled into libraries, and related executables must not be built.
Actual results
A feature and related executables may be built even if explicit disabled by the user.
Environment (please complete the following information):
Additional context
microsoft/vcpkg#27551
which now uses a minimal patch to forward
ENABLE_<Feature>to<Pkg>_FOUNDto achieve the desired build control.