bitpit icon indicating copy to clipboard operation
bitpit copied to clipboard

Possible ways to slightly improve CMake build system

Open pkestene opened this issue 5 years ago • 3 comments

I'd like to suggest here two minor ideas to slightly improve cmake in bitpit, related to modern cmake best practice use

  1. Use imported targets:

e.g. when calling find_package(MPI), cmake defines several imported targets like MPI::MPI_CXX. By linking to MPI::MPI_CXX, cmake will automatically transfer both compile and link flags; and thus you don't need to manipulate CMAKE_CXX_FLAGS, MPI_CXX_LINK_FLAGS or include_directories anymore, all you need is just to link your target to MPI::MPI_CXX

find_package(MPI)

# define an executable or library target, e.g.
add_executable(my_executable my_source.cpp)
target_link_libraries(my_executable PRIVATE MPI::MPI_CXX)

It is possible to use this feature with executables, as well as STATIC / SHARED libraries, and since cmake 3.12 with OBJECT libraries too (as OBJECT lib are used in bitpit for the different modules).

  1. Defining an ALIAS library BITPIT::BITPIT in BITPITConfig.cmake would help application code use bitpit, for example in my top-level CMakeLists.txt I do :
#####################################################################
# Find BitPit
#####################################################################
find_package(BITPIT REQUIRED)

# create imported target for bitpit library
if (BITPIT_FOUND)
  if(NOT TARGET BITPIT::BITPIT)
    get_filename_component(LIB_EXT "${BITPIT_LIBRARY}" EXT)
    if(LIB_EXT STREQUAL ".a" OR LIB_EXT STREQUAL ".lib")
      set(LIB_TYPE STATIC)
    else()
      set(LIB_TYPE SHARED)
    endif()
    add_library(BITPIT::BITPIT ${LIB_TYPE} IMPORTED GLOBAL)
    set(_tmp_dep_libs "${BITPIT_LIBRARIES}")
    list(REMOVE_DUPLICATES _tmp_dep_libs)
    set_target_properties(BITPIT::BITPIT
      PROPERTIES
      IMPORTED_LOCATION "${BITPIT_LIBRARY}"
      INTERFACE_INCLUDE_DIRECTORIES "${BITPIT_INCLUDE_DIRS}"
      INTERFACE_LINK_LIBRARIES "${_tmp_dep_libs}")
  endif()
endif(BITPIT_FOUND)

Then I can use bitpit simply by :

target_link_libraries(my_exe PRIVATE BITPIT::BITPIT)

My suggestion is to add the cmake code above defining BITPIT::BITPIT alias library directly in BITPITConfig.cmake.

pkestene avatar Jan 24 '20 21:01 pkestene

I wasn't aware of those cmake features. They seem interesting. I will work on them. Thanks.

andrea-iob avatar Jan 29 '20 10:01 andrea-iob

Dear Bitpit developers,

+1 for this issue.

We have some users at Inria Bordeaux in the memphis team and it would be great if bitpit could improve the cmake in a more modern way, using targets and exporting its targets.

A real world example for export targets:

  • https://gitlab.inria.fr/solverstack/spm/-/blob/master/src/CMakeLists.txt#L168
  • https://gitlab.inria.fr/solverstack/spm/-/blob/master/cmake_modules/SPMConfig.cmake.in
  • https://gitlab.inria.fr/solverstack/spm/-/blob/master/CMakeLists.txt#L148

It would be great also to define an "imported target" when finding petsc, something called for example petsc::petsc so that we can use this target in all dependents projects. Something like

    if(NOT TARGET petsc::petsc)
      add_library(petsc::petsc INTERFACE IMPORTED)
      set_target_properties(petsc::petsc PROPERTIES
        INTERFACE_INCLUDE_DIRECTORIES "${PETSC_INCLUDES}"
        INTERFACE_LINK_LIBRARIES "${PETSC_LIBRARIES}")
    endif()

Thanks a lot

fpruvost avatar Nov 22 '21 15:11 fpruvost

Thanks for the example. I will look at it.

andrea-iob avatar Nov 29 '21 08:11 andrea-iob