-
Notifications
You must be signed in to change notification settings - Fork 137
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
Generate a pkgconf pc file from cmake #1565
Conversation
95703c6
to
bdf5ec6
Compare
@harshula Sorry for the delay, didn't notice that question in there initially. The required flags would be for netcdf and mpi. libyaml is also used optionally. I think the relevant CMake variables would be here: Lines 34 to 36 in 53c55b4
Lines 3 to 4 in 53c55b4
I think @climbfuji Do you know why there would be the difference in the install directory locations? Since we mainly utilize the autotools build I figure you may have more insight than me. |
Are you referring to the comment above that libfms goes into |
@climbfuji Yeah that's what I was referring to, thanks for the info. It seems like this line should be updated to use whichever lib directory is set by Line 438 in d13c129
|
Is that line needed at all? Shouldn't cmake place those cmake files in the correct place w/o it? |
I think it might be needed as a required argument for |
Hi @rem1776 & @climbfuji , I've created a separate PR, #1589 , to decide on |
a3ff020
to
903379d
Compare
Hi @rem1776 & @climbfuji , I did an FMS 2024.03.tar.gz autotools build while using Spack built dependencies. I built netcdf as static libraries, using Spack ( The FMS.pc file looks like:
The above appears to confirm the Guide to pkg-config:
|
3a7bc8f
to
f07294f
Compare
Hi @rem1776 & @climbfuji , I'm not confident if the third commit (f07294f) is the right way to set LIBS. What do you think? Here's the output:
|
One option is to merge the first two commits and drop the third for the time being. |
Output of
|
NOTES https://docs.open-mpi.org/en/v5.0.x/building-apps/extracting-wrapper-flags.html
|
f07294f
to
cd681aa
Compare
cd681aa
to
a3fdc7f
Compare
Hi @rem1776 & @climbfuji , Ready for Review This PR will focus on generating a pkgconf pc file via CMake and adding NetCDF ldflags to Testing [root@2b59a8c74ebf opt]# cat /opt/release/linux-rocky8-x86_64/intel-2021.2.0/fms-git.generate-pc-file-from-cmake_0-git.3310-wuhdgq5lshh22567xitavf2rvcybkyxo/lib64/pkgconfig/FMS.pc
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New output file looks good to me, thanks for these updates.
Is the library name really capitalized, as suggested in
? |
Yes FMS is capitalized in the linker flag, at least for the autotools build. I would think it should be the same for CMake. |
While adding pkgconf support to an old fork of FMS, built via Spack, I noticed that the upstream FMS' CMake was not generating the pkgconf pc file.
Testing output:
What are the relevant CMake variables that should go in
Libs.private
? Also, ${CMAKE_INSTALL_LIBDIR} islib64
whereas the FMS library is installed inlib
.