Page MenuHomePhabricator
Create Task
Maniphest T28002

MITK cannot handle CMake target namespaces in module and plugin package dependencies
Closed, ResolvedPublic
Actions

Description

More and more libraries use namespace notation for their targets like tinyxml2::tinyxml2, boost::headers, Qt5::Core, cpprestsdk::cpprest, or OpenSSL::SSL. It is not clear how they should be written in MITK's package dependency notation based on package names and component names. For some reason, Qt seems to work with Qt5|Core+Widgets for example, while all other examples above don't. The expected mix of both notations would be something like Boost|boost::headers which does not work. Same for Boost|headers. We have some workarounds in our code base like:

mitk_create_module(...)

if(TARGET ${MODULE_TARGET})
  target_link_libraries(${MODULE_TARGET} PUBLIC cpprestsdk::cpprest OpenSSL::SSL)
endif()

Related Objects

Event Timeline

kislinsk triaged this task as Unbreak Now! priority.Nov 23 2020, 11:35 PM
kislinsk created this task.
kislinsk mentioned this in T27985: Migrate from TinyXML to TinyXML-2.Nov 24 2020, 9:55 PM
kislinsk added a comment.EditedNov 25 2020, 8:02 AM
How the PACKAGE_DEPENDS parameter of mitk_create_module() works
  • mitk_create_module() passes PACKAGE_DEPENDS to _mitk_parse_package_args()
    • The PACKAGE[|COMPONENT1[+COMPONENT2]...] syntax is processed and the following variables are set in parent scope:
      • <PUBLIC|PRIVATE|INTERFACE>_PACKAGE_NAMES: Lists of package names for different visibilities
      • PACKAGE_NAMES: Joined lists from above (may contain duplicates!)
      • <PUBLIC|PRIVATE|INTERFACE>_<PACKAGE>_REQUIRED_COMPONENTS: Lists of components for different visibilities and packages
  • mitk_create_module() passes PACKAGE_NAMES to mitk_check_module_dependencies()
    • mitk_check_module_dependencies() passes PACKAGE_NAMES to _mitk_parse_package_args()
      • Notice that PACKAGE_NAMES does not contain visibilities anymore, so everything is put into PRIVATE_PACKAGE_NAMES by default
    • Nevertheless, all visibility lists are joined into package_names and duplicates are removed (why not just use PACKAGE_NAMES in the first place and remove duplicates?)
    • For each package, check if MITK_USE_<package> is not set and add package to _MISSING_DEP in parent scope together with missing modules
  • Also: Duplicates in PACKAGE_NAMES were removed by mitk_check_module_dependencies()
  • The module is disabled if there are any missing dependencies
  • PACKAGE_DEPENDS is passed to mitk_use_modules() as PACKAGES parameter
How the PACKAGE_DEPENDS parameter of mitk_create_plugin() works
  • mitk_create_plugin() just passes PACKAGE_DEPENDS to mitk_check_module_dependencies() and mitk_use_modules()
How mitk_use_modules() works
  • mitk_use_modules() is basically an interface between a target (module or plugin) and the corresponding CMake/PackageDepends/<package>.cmake file
    • Inputs for these files are:
      • MODULE_NAME: The target
      • <package>_REQUIRED_COMPONENTS_BY_MODULE: Required components
    • Outputs (variables that are parsed after including these files) are:
      • ALL_INCLUDE_DIRECTORIES: Is passed to target_include_directories()
      • ALL_LIBRARIES: Is passed to target_link_libraries()
      • ALL_COMPILE_DEFINITIONS: Is passed to target_compile_definitions(PRIVATE)
      • ALL_COMPILE_OPTIONS: Is passed to target_compile_options()
    • Notice that these files are included once per visibility
kislinsk added a comment.Nov 25 2020, 8:22 PM

Holy moly, I just learned that the package depends files work completely different than I thought/was told the past 9 years. Instead of fixing the mitk_use_modules() function we just need to use the package depends files correctly.

kislinsk added a comment.Nov 25 2020, 8:46 PM

Pushed new branch bugfix/T28002-ImprovePackageDependsFiles.

kislinsk mentioned this in rMITK68f5331438a2: Merge branch 'bugfix/T28002-ImprovePackageDependsFiles' into develop.Nov 25 2020, 8:46 PM
kislinsk added a comment.Nov 25 2020, 8:49 PM

Fixed VTK and TinyXML-2 so far.

kislinsk added a project: Noteworthy.Nov 25 2020, 9:05 PM

Public API change: Depending on VTK does not add a transitive dependency to Qt5 Widgets anymore, except, when the required VTK components have a public dependency to Qt5 Widgets of course.

kislinsk added a comment.Nov 25 2020, 11:34 PM

Introduced TARGET_DEPENDS option.

TARGET_DEPENDS parameters are directly passed to target_link_libraries() and bypass the PACKAGE_DEPENDS mechanism.

The option was added to:

  • mitk_create_module()
  • mitk_create_plugin()
  • mitk_create_executable()
  • MITK_CREATE_MODULE_TESTS()
  • mitkFunctionCreateCommandLineApp()
  • mitkFunctionCreateMatchPointDeployedAlgorithm()
kislinsk mentioned this in rMITKc670e289d6e7: Merge branch 'bugfix/T28002-ImprovePackageDependsFiles' into develop.Nov 27 2020, 9:59 AM
kislinsk added a comment.EditedNov 27 2020, 10:29 AM

PACKAGE_DEPENDS now properly supports Boost, OpenMesh, OpenMP, OpenSSL, and Python3. For example:

  • PACKAGE_DEPENDS Boost is short for Boost|boost (header-only part of Boost)
  • PACKAGE_DEPENDS Boost|date_time+random maps to Boost::date_time Boost::random
  • PACKAGE_DEPENDS OpenMesh|Tools maps to OpenMeshTools
  • PACKAGE_DEPENDS OpenMP maps to OpenMP::OpenMP_CXX
  • PACKAGE_DEPENDS OpenSSL|SSL maps to OpenSSL::SSL
  • PACKAGE_DEPENDS Python3|NumPy maps to Python3::NumPy

ITK, OpenGL, Poco and Qt5 dependencies are now based on targets instead of directories as well.

kislinsk mentioned this in rMITK43330b6521cc: Merge branch 'bugfix/T28002-ImprovePackageDependsFiles' into develop.Nov 28 2020, 4:57 AM
kislinsk added a comment.Nov 28 2020, 4:58 AM

Deleted branch bugfix/T28002-ImprovePackageDependsFiles.

kislinsk closed this task as Resolved.Nov 28 2020, 4:59 AM
kislinsk mentioned this in T28126: 2020 Week 52 (Late December).Dec 21 2020, 5:16 AM
MITK · MIC · IMSY · DKFZ