Use target-based cmake settings

[Andrew-Au]

• Update minimum required to CMake version 3.5 (versions older than 3.5 are deprecated as of 3.27)
• update add_definitions to target_compile_definitions
• use target_include_directories for public library includes
• add jansson::jansson alias

This addresses comments in https://github.com/akheron/jansson/pull/664 . It deprecates that request.

[Andrew-Au]

resolves https://github.com/akheron/jansson/issues/663

[WizzardMaker]

When can this be merged? New GitHub Action Pipelines are now running on newer CMake versions that do not accept the 3.1 version

[akheron]

Sorry, I had forgotten about this.

I still don't understand the jansson::jansson alias 🤔

[JosiahWI]

The jansson::jansson alias is for consistency between alternative ways to include a dependency in a project.

Consider how the target for jansson is currently installed:

  # Install exports for the install-tree.
  install(EXPORT janssonTargets
          NAMESPACE jansson::
          DESTINATION "${JANSSON_INSTALL_CMAKE_DIR}")

The usage will look like this:

find_package(jansson REQUIRED)
target_link_libraries(my_program PRIVATE jansson::jansson)

Now suppose that, in the case that jansson is not installed, I want to fall back to a bundled version of jansson that I ship with my source code. (I don't know how common that build setup is, but I have seen it with other libraries; e.g. the open-source game engine Luanti can use system or bundled jsoncpp.)

find_package(jansson QUIET)
if(NOT JANSSON_FOUND)
  message(STATUS "System Jansson not found, using bundled version.")
  add_subdirectory(vendor/jansson)
endif()

# This only works if jansson provides the alias target.
target_link_libraries(my_program PRIVATE jansson::jansson)

Without the alias, we have to use a different name for the target depending on how we included the dependency. That makes extra mess for downstream projects; we like it when we only have to know 1 name to refer to the dependency.

For completeness, here is the necessary logic to handle the above case without the alias:

set(USING_BUNDLED_JANSSON FALSE)
mark_as_advanced(USING_BUNDLED_JANSSON)
find_package(jansson QUIET)
if(NOT JANSSON_FOUND)
  message(STATUS "System Jansson not found, using bundled version.")
  add_subdirectory(vendor/jansson)
  set(USING_BUNDLED_JANSSON TRUE)
endif()

if(USING_BUNDLED_JANSSON)
  target_link_libraries(my_program PRIVATE jansson::jansson)
else()
  target_link_libraries(my_program PRIVATE jansson)
endif()

This comparison demonstrates the motivation behind the convention for upstream projects to provide an alias target. Do you have any questions?

[JosiahWI]

An alternative approach to achieve consistency is to not install the target under a namespace. The purpose of the namespace is to avoid name conflicts, I think, and I personally like the consistency of most projects having a namespace. But is the target name jansson going to accidentally conflict with a different project's target name?

[Andrew-Au]

In addition to being "how it's done these days" and the above reasons, using an alias has a couple of other benefits:

1. If an unqualified target is missing, it becomes -ltarget and fails with an obscure error during link. If an aliased target is missng, it fails during CMake preprocessing.
2. Some 3rd party tools (e.g. Conan) assume that targets will be aliased.

[Andrew-Au]

Also, consider raising minimum version to at least 3.10: https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html#policy-version

[akheron]

Merged, thanks all!