Skip to content
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

Self-describing build properties in ESMA repos #30

Open
LiamBindle opened this issue Sep 18, 2019 · 0 comments
Open

Self-describing build properties in ESMA repos #30

LiamBindle opened this issue Sep 18, 2019 · 0 comments

Comments

@LiamBindle
Copy link

LiamBindle commented Sep 18, 2019

Hi everyone,

This is a feature request to have repos like MAPL, FVdycoreCubed_GridComp, and GMAO_Shared self-describe their recommended build properties—their compiler options and definitions, include directories, and link directories. This would be helpful to external projects, like GCHP, that nest forks of these repos.

Using GCHP as an example

GCHP depends on MAPL, GMAO_Shared, and FVdycoreCubed_GridComp, and we nest our forks in GCHP's source code. Currently, there are two options for setting ESMA's build properties for these directory-trees:

  1. We can set them in their parent-directories' CMakeLists (e.g. here)
    • Pros: This approach is simple. Also, we don't have to modify our MAPL, GMAO_Shared, and FVdycoreCubed_GridComp forks.
    • Cons: Using operations like add_definition() in parent-directories affect the entire parent-directory's scope. This should be avoided.
  2. We can modify our MAPL, GMAO_Shared, and FVdycoreCubed_GridComp forks to set ESMA build properties for their trees. For example, putting add_definitions() calls in geoschem/MAPL/CMakeLists.txt.
    • Pros: Doesn't affect the scope of non-ESMA-projects' directory-trees.
    • Cons: It is a necessary divergence from the upstreams. External projects have to touch build properties which is potentially dangerous if they get it wrong (e.g. a missing preprocessor definition).

A third option would be to have the upstream MAPL, GMAO_Shared, and FVdycoreCubed_GridComp self-describe their build properties, and then external projects don't have to worry about properly setting these tree's build properties. I think this could be accomplished pretty easily by propagating build properties from the bottom-up, through PUBLIC links and a properly configured base target.

Co-benefit: Simplifying some ESMA projects' CMakeLists

Besides helping external projects, I think this could also help simplify some ESMA projects' CMakeLists. Take MAPL/MAPL_Base/CMakeLists.txt for example. If the target GMAO_pFIO's properties fully qualified its dependencies and build properties, doing

#                ${this} is "MAPL_Base"
esma_add_library(${this} SRCS ${srcs} DEPENDENCIES GMAO_pFIO MAPL_cfio_r4)  

would also fully qualify MAPL_Base's dependencies and build properties. This would allow for the following simplifications in MAPL/MAPL_Base/CMakeLists.txt:

@@ -68,30 +68,13 @@ set (srcs
   )
 
 esma_add_library(${this} SRCS ${srcs} DEPENDENCIES GMAO_pFIO MAPL_cfio_r4)
-target_compile_options (${this} PRIVATE $<$<COMPILE_LANGUAGE:Fortran>:${OpenMP_Fortran_FLAGS}>)
-
-# Kludge for OSX security and DYLD_LIBRARY_PATH ...
-foreach(dir ${OSX_EXTRA_LIBRARY_PATH})
-  target_link_libraries(${this} PUBLIC "-Xlinker -rpath -Xlinker ${dir}")
-endforeach()
-
 
 ecbuild_add_executable (TARGET cub2latlon.x SOURCES cub2latlon_regridder.F90)
-target_link_libraries (cub2latlon.x ${this} GMAO_pFIO ${ESMF_LIBRARIES} ${MPI_Fortran_LIBRARIES} ${OpenMP_Fortran_LIBRARIES})
-set_target_properties (cub2latlon.x PROPERTIES Fortran_MODULE_DIRECTORY ${esma_include}/${this} LINK_FLAGS "${OpenMP_Fortran_FLAGS}")
+target_link_libraries (cub2latlon.x PUBLIC ${this})
 
 ecbuild_add_executable (TARGET Regrid_Util.x SOURCES Regrid_Util.F90)
-target_link_libraries (Regrid_Util.x ${this} GMAO_pFIO ${ESMF_LIBRARIES} ${MPI_Fortran_LIBRARIES} ${OpenMP_Fortran_LIBRARIES})
-set_target_properties (Regrid_Util.x PROPERTIES Fortran_MODULE_DIRECTORY ${esma_include}/${this} LINK_FLAGS "${OpenMP_Fortran_FLAGS}")
+target_link_libraries (Regrid_Util.x PUBLIC ${this})
 
-if (EXTENDED_SOURCE)
-#    target_compile_options (${this} PRIVATE $<$<COMPILE_LANGUAGE:Fortran>:${EXTENDED_SOURCE}>)
-esma_fortran_generator_list (${this} ${EXTENDED_SOURCE})
-endif()
-
-#--------------------
-target_include_directories (${this} PUBLIC ${INC_ESMF} ${INC_NETCDF} ${INC_FLAP})
-target_link_libraries (${this} PUBLIC ${ESMF_LIBRARIES} ${MPI_Fortran_LIBRARIES} ${LIB_FLAP})
 
 #--------------------
 # Copy include files that are used by other libraries.

These simplifications are possible because GMAO_pFIO's interface properties are propagated to dependents when dependent's call target_link_libraries() with the PUBLIC or INTERFACE argument.


See PR #31 for my proposed fix.

Let me know if you have any questions, comments, concerns, or if there is anything I could clarify.

Liam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant