NOAA-EMC / NCEPLIBS-bacio

This library performs binary I/O for the NCEP models.
Other
2 stars 6 forks source link

fixed CMake install directory from lib to ${CMAKE_INSTALL_LIBDIR} #113

Closed edwardhartnett closed 1 year ago

edwardhartnett commented 1 year ago

Fixes #112

aerorahul commented 1 year ago

Just a note here that hpc-stack modulefiles will need updates corresponding to these updates. And possibly in spack (I don't know how those module variables are defined). Please check.

edwardhartnett commented 1 year ago

@AlexanderRichert-NOAA can you answer the question of how these are defined in spack?

@aerorahul I believe hpc-stack will never be updated with this release. We are switching to spack and this release is not urgent. So I suggest we leave hpc-stack alone.

AlexanderRichert-NOAA commented 1 year ago

@AlexanderRichert-NOAA can you answer the question of how these are defined in spack?

I tested installed this PR through spack just now on my personal machine and it appears to work as expected. Rahul, if you're referring to CMAKE_INSTALL_LIBDIR, that appears to be automatically set to "lib" and so the cmake files get installed under bacioinstalldir/lib/cmake/bacio/, so no further configuration by spack is necessary. Is that what you're asking?

aerorahul commented 1 year ago

@AlexanderRichert-NOAA can you answer the question of how these are defined in spack?

I tested installed this PR through spack just now on my personal machine and it appears to work as expected. Rahul, if you're referring to CMAKE_INSTALL_LIBDIR, that appears to be automatically set to "lib" and so the cmake files get installed under bacioinstalldir/lib/cmake/bacio/, so no further configuration by spack is necessary. Is that what you're asking?

The location CMAKE_INSTALL_LIBDIR is lib64 on a RH for e.g. and lib on a macOS. I don't know if spack does the same for RH.

AlexanderRichert-NOAA commented 1 year ago

I'm noticing that in w3emc, src/CMakeLists.txt sets "ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}" and "LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}", which I think makes sense, that way everything is in the same directory. So if I'm understanding the goal here correctly, I believe we'll want to do likewise for this and g2tmpl.

edwardhartnett commented 1 year ago

@AlexanderRichert-NOAA good catch, I will fix...