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

CMake: refactor a few items #47

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Conversation

mwtoews
Copy link
Contributor

@mwtoews mwtoews commented Jun 14, 2023

  • Use GNUInstallDirs
  • Use variables for targets
  • Use built-in project variables for version

* Use GNUInstallDirs
* Use variables for targets
* Use built-in project variables for version
@mdpiper
Copy link
Member

mdpiper commented Jun 14, 2023

@mwtoews This is really helpful, thank you!

@mwtoews
Copy link
Contributor Author

mwtoews commented Jun 15, 2023

Note that in a related repo, I see csdms/bmi-c@a61e7f9
Shouldn't system-specific dirs like lib64 be ok?

@mdpiper
Copy link
Member

mdpiper commented Jun 15, 2023

Note that in a related repo, I see csdms/bmi-c@a61e7f9 Shouldn't system-specific dirs like lib64 be ok?

I agree. I'm not sure why this caused a problem. Perhaps on Windows? I'll have to look at my notes. Definitely an argument for a better commit message.

@mwtoews
Copy link
Contributor Author

mwtoews commented Jun 15, 2023

On a related note, I've also been thinking about bunding cmake-packages, for example here. This effort would enable other BMI applications to use:

find_package(bmif CONFIG REQUIRED)
add_executable(my_bmi_app my_bmi_app.f90)
target_link_libraries(my_bmi_app bmif::bmif_static)

which is more standard in CMake projects (it's also way more logic to add compared to pkg-config, as is done with bmi-c). There is a bit of careful handing around the *.mod files, which are slightly outside CMake norms. What do you think?

@mdpiper
Copy link
Member

mdpiper commented Oct 9, 2023

@mwtoews I apologize for disappearing for awhile. I finally have some time to work on BMI stuff again.

I'll merge this PR because what's you've proposed here is the right thing to do. I'll also apply your changes to the other BMI specs (C, C++) built with CMake.

I'd genuinely appreciate your help with cmake-packages. Recently, I've run into a few places where this would've been handy. If you send a PR to this repo, I can learn from what you've done & apply it to the other BMI specs.

@mdpiper mdpiper merged commit cd2d192 into csdms:master Oct 9, 2023
2 checks passed
@mwtoews mwtoews deleted the gnuinstalldirs branch October 9, 2023 20:45
This pull request was closed.
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

Successfully merging this pull request may close these issues.

2 participants