Skip to content

CMake: Inconsistent feature control #850

@dg0yt

Description

@dg0yt

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.

libgd/CMakeLists.txt

Lines 123 to 125 in 7b0d419

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:

libgd/CMakeLists.txt

Lines 192 to 196 in 7b0d419

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:

libgd/CMakeLists.txt

Lines 127 to 130 in 7b0d419

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions