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

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

Open
mathomp4 opened this issue Mar 31, 2022 · 3 comments
Open

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

mathomp4 opened this issue Mar 31, 2022 · 3 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mathomp4
Copy link
Member

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?

@mathomp4 mathomp4 added enhancement New feature or request help wanted Extra attention is needed labels Mar 31, 2022
@tclune
Copy link
Collaborator

tclune commented Mar 31, 2022

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
Copy link
Member Author

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
Copy link
Collaborator

tclune commented Mar 31, 2022

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants