GEOS-ESM / ESMA_cmake

Custom CMake macros for the GEOS Earth System Model
Apache License 2.0
4 stars 9 forks source link

Add generic "copy files to etc" macro/function #259

Open mathomp4 opened 2 years ago

mathomp4 commented 2 years ago

As noticed by @bena-nasa we have code like this:

file (GLOB_RECURSE rc_files CONFIGURE_DEPENDS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} *.rc)
foreach ( file ${rc_files} )
   get_filename_component( dir ${file} DIRECTORY )
   install( FILES ${file} DESTINATION etc/${dir} )
endforeach()

in a lot of places. We use it to copy over all the *.rc files under a directory into a similar directory tree in install/etc.

And @bena-nasa in his ExtData2G work would need to add *.yaml to all of these.

This leads to a couple things. First, this code might be better served with a CMake macro or function. We could do:

esma_copy_config_files()

say and that would just copy over the files.

But should it be more..."general"? Like maybe:

esma_copy_config_files(*.rc *.yaml)

or even:

esma_copy_config_files(*.rc *.yaml DESTINATION etc)

so that we could do it to an arbitrary location with arbitrary files?

Then again, if our config files are all *.rc, *.yaml and maybe *.nml we could just hard code those and if a new file glob needs to be added, we do so in this repo in the macro and then boom, all works?

tclune commented 2 years ago

I like being more explicit. Definitely needs the DESTINATION, and specifying the file extensions is a big clue to the user why the copy is needed.

mathomp4 commented 2 years ago

It does look like there are CMake-y ways to maybe have "optional" arguments:

https://gist.github.com/dberzano/8194ddfc9b8fbd5529ae https://stackoverflow.com/questions/1346731/optional-argument-in-cmake-macro

So maybe we require GLOBS but not DESTINATION? But then again, as @tclune says, explicit is better:

esma_copy_config_files(GLOBS *.rc *.yaml DESTINATION etc)

is nice.

And maybe then the name should be something like esma_copy_files_to_install() or something truly generic as it doesn't have to be only for config files.

tclune commented 2 years ago

we already do optional in a number of places. Indeed, one should almost always use the "args macro" (forget what it is called at the moment).