-
Notifications
You must be signed in to change notification settings - Fork 3
Introduced CMake options for the Fortran-API and compiler warnings #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e05179e
2be5e7d
1b5411a
7db05ef
e541130
bfa24a6
bf66eb7
e60d611
ca7e64f
f15865d
04737e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,8 @@ | ||||||||||||||||
| # Specify the minimum version | ||||||||||||||||
| # 3.9 required for regex submatches | ||||||||||||||||
| # 3.12 required for policy CMP0074 (using *_ROOT variables) | ||||||||||||||||
| cmake_minimum_required ( VERSION 3.12 ) | ||||||||||||||||
| # 3.13 required for policy CMP0076 (target_sources() command converts relative paths to absolute) | ||||||||||||||||
| cmake_minimum_required ( VERSION 3.13 ) | ||||||||||||||||
|
|
||||||||||||||||
| # Reconfigure if Project.toml has changed | ||||||||||||||||
| set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${CMAKE_SOURCE_DIR}/LibTrixi.jl/Project.toml") | ||||||||||||||||
|
|
@@ -23,13 +24,26 @@ file(WRITE "${CMAKE_BINARY_DIR}/LIBTRIXI_VERSION" "${LIBTRIXI_VERSION}\n") | |||||||||||||||
| project ( trixi VERSION ${LIBTRIXI_VERSION_MAJOR}.${LIBTRIXI_VERSION_MINOR}.${LIBTRIXI_VERSION_PATCH} | ||||||||||||||||
| DESCRIPTION "Interface library for using Trixi.jl from C/C++/Fortran" ) | ||||||||||||||||
|
|
||||||||||||||||
| # Enable C and Fortran | ||||||||||||||||
| enable_language(C Fortran) | ||||||||||||||||
| # Additional configuration options | ||||||||||||||||
| option( LIBTRIXI_BUILD_FORTRAN_API "Enable the build of the Fortran interface" ON ) | ||||||||||||||||
| option( LIBTRIXI_ENABLE_WARNINGS "Use compilation flags -Wall -Wextra" OFF ) | ||||||||||||||||
| option( LIBTRIXI_ENABLE_WERROR "Use compilation flag -Werror" OFF ) | ||||||||||||||||
| option( LIBTRIXI_ENABLE_TESTING "Build tests using Google Test (C) and test-drive (Fortran)" OFF ) | ||||||||||||||||
|
|
||||||||||||||||
| # Enabling setting rpath for installation | ||||||||||||||||
| SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) | ||||||||||||||||
| # Define the warnings to use if enabled | ||||||||||||||||
| set (LIBTRIXI_WARNING_OPTIONS -Wall -Wextra) | ||||||||||||||||
| set (LIBTRIXI_WERROR_OPTION -Werror) | ||||||||||||||||
|
|
||||||||||||||||
| # Enable C always | ||||||||||||||||
| enable_language( C ) | ||||||||||||||||
|
|
||||||||||||||||
| # Optionally enable Fortran | ||||||||||||||||
| if( LIBTRIXI_BUILD_FORTRAN_API ) | ||||||||||||||||
| enable_language( Fortran ) | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| # Enabling setting rpath for installation | ||||||||||||||||
| SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) | ||||||||||||||||
|
|
||||||||||||||||
| # Additional cmake modules | ||||||||||||||||
| list ( APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/" ) | ||||||||||||||||
|
|
@@ -47,8 +61,7 @@ endif() | |||||||||||||||
| find_package( MPI REQUIRED ) | ||||||||||||||||
|
|
||||||||||||||||
| # Find Google Test and test-drive on demand | ||||||||||||||||
| option( ENABLE_TESTING "Build tests using Google Test (C) and test-drive (Fortran)" ) | ||||||||||||||||
| if( ENABLE_TESTING ) | ||||||||||||||||
| if( LIBTRIXI_ENABLE_TESTING ) | ||||||||||||||||
| if ( NOT DEFINED JULIA_PROJECT_PATH ) | ||||||||||||||||
| message( FATAL_ERROR "JULIA_PROJECT_PATH not set, tests will not work.") | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
@@ -57,22 +70,26 @@ if( ENABLE_TESTING ) | |||||||||||||||
|
|
||||||||||||||||
| find_package( GTest REQUIRED ) | ||||||||||||||||
|
|
||||||||||||||||
| set ( TEST_DRIVE_FIND_METHOD fetch ) | ||||||||||||||||
| # Option TEST_DRIVE_BUILD_TESTING is hard-coded to ON, could be spared | ||||||||||||||||
| find_package( test-drive REQUIRED ) | ||||||||||||||||
| if( LIBTRIXI_BUILD_FORTRAN_API ) | ||||||||||||||||
| set ( TEST_DRIVE_FIND_METHOD fetch ) | ||||||||||||||||
| # Option TEST_DRIVE_BUILD_TESTING is hard-coded to ON, could be spared | ||||||||||||||||
| find_package( test-drive REQUIRED ) | ||||||||||||||||
| endif() | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| # Optionally use PackageCompiler.jl to build standalone libtrixi.so | ||||||||||||||||
| option( USE_PACKAGE_COMPILER "Build standalone libtrixi.so using PackageCompiler.jl" ) | ||||||||||||||||
|
|
||||||||||||||||
| # Fortran mod file location | ||||||||||||||||
| set(CMAKE_Fortran_MODULE_DIRECTORY ${CMAKE_BINARY_DIR}) | ||||||||||||||||
| if( LIBTRIXI_BUILD_FORTRAN_API ) | ||||||||||||||||
| # Fortran mod file location | ||||||||||||||||
| set(CMAKE_Fortran_MODULE_DIRECTORY ${CMAKE_BINARY_DIR}) | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| if( USE_PACKAGE_COMPILER ) | ||||||||||||||||
| if ( NOT DEFINED JULIA_PROJECT_PATH ) | ||||||||||||||||
| message( FATAL_ERROR "JULIA_PROJECT_PATH needs to be set for PackageCompiler.jl.") | ||||||||||||||||
| endif() | ||||||||||||||||
| if ( ENABLE_TESTING ) | ||||||||||||||||
| if ( LIBTRIXI_ENABLE_TESTING ) | ||||||||||||||||
| message( NOTICE "Testing is not supported when PackageCompiler is used.") | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -89,10 +106,12 @@ if( USE_PACKAGE_COMPILER ) | |||||||||||||||
| COMMAND ${CMAKE_COMMAND} -E copy ${PC_INIT_SOURCE} ${PC_INIT_BUILD} | ||||||||||||||||
| DEPENDS ${PC_INIT_SOURCE} ) | ||||||||||||||||
|
|
||||||||||||||||
| # Add a library target (libtrixi), only for Fortran module | ||||||||||||||||
| add_library( ${PROJECT_NAME} OBJECT | ||||||||||||||||
| src/api.f90 | ||||||||||||||||
| ) | ||||||||||||||||
| if( LIBTRIXI_BUILD_FORTRAN_API ) | ||||||||||||||||
| # Add a library target (libtrixi), only for Fortran module | ||||||||||||||||
| add_library( ${PROJECT_NAME} OBJECT | ||||||||||||||||
| src/api.f90 | ||||||||||||||||
| ) | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| # Custom command to run PackageCompiler.jl to produce libtrixi.so | ||||||||||||||||
| add_custom_command( OUTPUT ${PC_LIBTRIXI_SO} | ||||||||||||||||
|
|
@@ -123,12 +142,18 @@ else() | |||||||||||||||
| # Library target | ||||||||||||||||
| add_library ( ${PROJECT_NAME} SHARED | ||||||||||||||||
| src/api.c | ||||||||||||||||
| src/api.f90 | ||||||||||||||||
| src/auxiliary.h | ||||||||||||||||
| src/auxiliary.c | ||||||||||||||||
| src/trixi.h | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # Build Fortran library target on demand | ||||||||||||||||
| if( LIBTRIXI_BUILD_FORTRAN_API ) | ||||||||||||||||
| target_sources (${PROJECT_NAME} PRIVATE | ||||||||||||||||
| src/api.f90 | ||||||||||||||||
| ) | ||||||||||||||||
|
benegee marked this conversation as resolved.
|
||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| # Include directories, private | ||||||||||||||||
| target_include_directories ( ${PROJECT_NAME} PRIVATE src ) | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -138,8 +163,6 @@ else() | |||||||||||||||
| # Version info for the shared object | ||||||||||||||||
| set_target_properties ( ${PROJECT_NAME} PROPERTIES SOVERSION ${PROJECT_VERSION_MAJOR} ) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| # Include directories | ||||||||||||||||
| target_include_directories( ${PROJECT_NAME} PRIVATE src ${JULIA_INCLUDE_DIRS} ) | ||||||||||||||||
|
Comment on lines
166
to
167
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Instead of disabling warnings/errors, I recommend to just mark the Julia include dirs as "SYSTEM" includes such that GCC/Clang suppress warnings on them (see also the CMake docs. If this works as expected, I'd also recommend reverting the option of disabling warnings/errors: In practice this leads too often to issues that remain unfixed, and if the "SYSTEM" switch suppresses the warning, we can remove a lot of code complexity from the build system again |
||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -148,42 +171,52 @@ else() | |||||||||||||||
|
|
||||||||||||||||
| # Set appropriate compile flags | ||||||||||||||||
| target_compile_options( ${PROJECT_NAME} PUBLIC "-fPIC" ) | ||||||||||||||||
| target_compile_options( ${PROJECT_NAME} PRIVATE -Wall -Wextra -Werror) | ||||||||||||||||
|
|
||||||||||||||||
| # Potentially, enable warnings | ||||||||||||||||
| if( LIBTRIXI_ENABLE_WARNINGS ) | ||||||||||||||||
| target_compile_options( ${PROJECT_NAME} PRIVATE ${LIBTRIXI_WARNING_OPTIONS}) | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| # Potentially, enable to interpret warnings as errors | ||||||||||||||||
| if( LIBTRIXI_ENABLE_WERROR ) | ||||||||||||||||
| target_compile_options( ${PROJECT_NAME} PRIVATE ${LIBTRIXI_WERROR_OPTION}) | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| # Require C11 standard with GNU extensions for C files | ||||||||||||||||
| target_compile_options( ${PROJECT_NAME} PRIVATE $<$<COMPILE_LANGUAGE:C>:-std=gnu11>) | ||||||||||||||||
| # Require Fortran 2018 standard for Fortran files | ||||||||||||||||
| target_compile_options( ${PROJECT_NAME} PRIVATE $<$<COMPILE_LANGUAGE:Fortran>:-std=f2018>) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| # Add auxiliary *object* library to support fast thread-local storage (TLS) | ||||||||||||||||
| add_library ( ${PROJECT_NAME}_tls OBJECT | ||||||||||||||||
| src/tls.c | ||||||||||||||||
| ) | ||||||||||||||||
| target_include_directories( ${PROJECT_NAME}_tls PRIVATE ${JULIA_INCLUDE_DIRS} ) | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
As above |
||||||||||||||||
|
|
||||||||||||||||
| # Add test on demand | ||||||||||||||||
| if( ENABLE_TESTING ) | ||||||||||||||||
| if( LIBTRIXI_ENABLE_TESTING ) | ||||||||||||||||
| enable_testing() | ||||||||||||||||
| add_subdirectory( test/c ) | ||||||||||||||||
| add_subdirectory( test/fortran ) | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| if( LIBTRIXI_BUILD_FORTRAN_API ) | ||||||||||||||||
| add_subdirectory( test/fortran ) | ||||||||||||||||
| endif() | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
| # Install configuration | ||||||||||||||||
| install( FILES $<TARGET_OBJECTS:${PROJECT_NAME}_tls> TYPE LIB RENAME lib${PROJECT_NAME}_tls.o ) | ||||||||||||||||
| install( FILES "${CMAKE_BINARY_DIR}/LIBTRIXI_VERSION" DESTINATION share/julia ) | ||||||||||||||||
| endif() | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| # Public header for libtrixi | ||||||||||||||||
| set_target_properties ( ${PROJECT_NAME} PROPERTIES PUBLIC_HEADER src/trixi.h ) | ||||||||||||||||
|
|
||||||||||||||||
| # Common install configuration | ||||||||||||||||
| install( TARGETS ${PROJECT_NAME} ) | ||||||||||||||||
| install( DIRECTORY LibTrixi.jl DESTINATION share/libtrixi PATTERN "lib" EXCLUDE ) | ||||||||||||||||
| install( FILES ${CMAKE_Fortran_MODULE_DIRECTORY}/libtrixi.mod TYPE INCLUDE) | ||||||||||||||||
| if( LIBTRIXI_BUILD_FORTRAN_API ) | ||||||||||||||||
| install( FILES ${CMAKE_Fortran_MODULE_DIRECTORY}/libtrixi.mod TYPE INCLUDE) | ||||||||||||||||
| endif() | ||||||||||||||||
| install( PROGRAMS utils/libtrixi-init-julia TYPE BIN ) | ||||||||||||||||
|
|
||||||||||||||||
| # Add examples | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -56,6 +56,10 @@ For building, `cmake` and its typical workflow is used. | |||||
| - Specifying the directory `install_directory` for later installation is optional. | ||||||
| - Optional specification of build type sets some default compiler options for optimized | ||||||
| or debug code. | ||||||
| - By default the Fortran-Interface is built; it may be excluded with `DLIBTRIXI_BUILD_FORTRAN_API=OFF`. | ||||||
|
benegee marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| - Optional compiler options `-Wall -Wextra` are specified with `-DLIBTRIXI_ENABLE_WARNINGS=ON`. | ||||||
| - Optional compiler option `-Werror` is specified with `-DLIBTRIXI_ENABLE_WERROR=ON`. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So far, we have been very strict with respect to warning, because we could. I would thus like to keep it this way, or at least have them on by default. Regarding the Julia 1.12 issue: #259 (comment)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any strong opinions on this @sloede ?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my reply above |
||||||
| - Building of the test is enabled with `-DLIBTRIXI_ENABLE_TESTING=ON`. | ||||||
| - Building with t8code support is optional. It requires to pass | ||||||
| `-DT8CODE_ROOT=<t8code_install_directory>`. | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is added as an option, you could think about adding also a test - or at least a smoke test - to ensure that builds still succeed with the Fortran interface turned off (I don't think running tests is necessary). It's not super high priority though, so you might also skip the tests 🤷♂️