jedbrown / cmake-modules

CMake modules for some scientific libraries
BSD 2-Clause "Simplified" License
87 stars 54 forks source link

Fortran headers have new location in PETSc 3.6 #14

Closed jkollasch closed 8 years ago

jkollasch commented 8 years ago

From PETSc 3.5 to 3.6 my Fortran code stopped compiling, even with the updated FindPETSc.cmake. I figured out it was because the Fortran headers moved from $PETSC_DIR/include/finclude to $PETSC_DIR/include/petsc/finclude. Therefore any F90 code with a statement like

#include <finclude/petscsys.h>

would need to be changed to

#include <petsc/finclude/petscsys.h>

in the source files which is a bit of work (hypothetically more for people with bigger codes than mine) and breaks the source from working with PETSc <=3.5. I attached a minimal example of a code that breaks from the 3.5 to 3.6 PETSc update. Note: I found a work around by changing the attached CMakeLists.txt by editing the line

include_directories(${PETSC_INCLUDES})

to become

include_directories("${PETSC_INCLUDES}/petsc" "${PETSC_INCLUDES}")

which works for both 3.5 and 3.6 without editing my F90 source files by simply exhaustively putting all the options of where finclude might be in the include path. While I am fine with this, I thought I'd post something to draw attention to the issue if you want to change it. For example, defining a PETSC_Fortran_INCLUDES variable in FindPETSc.cmake that automatically points to the fincludes directory regardless of the PETSc version would avoid the need for the above trick (though it does work).

Here is the example: main.F90

PROGRAM main #include <finclude/petscsys.h> PetscInt :: i END PROGRAM

and CMakeLists.txt


set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/Modules/") # Where I have my FindPETSc.cmake

cmake_minimum_required` (VERSION 2.8.8)
project (HELLO)
enable_language (Fortran)

find_package(PETSc REQUIRED)
include_directories(${PETSC_INCLUDES})

find_package(MPI REQUIRED)
add_definitions(${MPI_Fortran_COMPILE_FLAGS})
include_directories(${MPI_Fortran_INCLUDE_PATH})
link_directories(${MPI_Fortran_LIBRARIES})

add_executable(main.x main.F90)
target_link_libraries(main.x ${PETSC_LIBRARIES}  ${MPI_Fortran_LIBRARIES})
jedbrown commented 8 years ago

My opinion, and that of PETSc upstream, is that you should change your code to include Fortran files using the new names. Adding that include path with compiler flags is basically counteracting the value of namespacing. If you want to do it in your code, that's your business, but I can't recommend it for users in general. I know it's annoying.

jkollasch commented 8 years ago

Thanks. I'll keep the compiler flag thing since leaving "finclude" in the source files prevents namespacing issues (I believe), while letting me use older PETSc's.

jedbrown commented 8 years ago

Well, finclude is not a PETSc-specific namespace, but chances are you'll be fine. In any case, it's your problem if you do find a conflict.