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

Switch to using GNUInstallDirs as defaults #2

Merged
merged 1 commit into from
Mar 13, 2025

Conversation

LecrisUT
Copy link
Contributor

GNUInstallDirs has been a standard since at least CMake 3.0 and distros and other packages (including emebeded environments) use it to set their own standards. This PR migrates the INSTALL variables to use it as a default.

There was previously a choice of making all of those paths as absolute, but I don't think it is needed, please advice on where the absolute path is needed so that it can be better migrated.

@LecrisUT LecrisUT force-pushed the GNUInstallDirs branch 4 times, most recently from 586592c to 69dcfba Compare March 13, 2025 12:57
@da115115 da115115 merged commit 5ada9cb into airsim:master Mar 13, 2025
@da115115
Copy link
Member

da115115 commented Mar 13, 2025

Thanks for your contribution!
If I can spend a few hours on the project, I will try to create GitHub Actions in order to run the test suite, and check that it does not brake anything.
So far, to a full integration test, we can use metasim

@LecrisUT
Copy link
Contributor Author

Well that was rather brave to merge it without tests and as I was just force-pushing just 1 minute before. Luckily it was in a final state at that point. Anyway, the patch should be reconstructed in the other projects as well. One thing to note is that the rmol-config.in script is not correctly relocatable even in the original design (see the comment I added there). With some elbow-grease it is possible to make it properly account for relative and absolute paths.

@da115115
Copy link
Member

da115115 commented Mar 13, 2025

The risk is low, as the exact same project_config_embeddable.cmake CMake file is used everywhere else, and we are using Git after all. So, if there is an issue, it will be easy to revert the change.

For the tests, yes, I will use the metasim project, as mentioned above. Note that RMOL depends on both stdair and airrac.

So, an easier way to test that the new CMake configuration files work is to patch stdair, as that latter has no other dependency. Once we checked that stdair works well on several platforms, then we can copy the new CMake files in all the other components (I have done that regularly in the past); here again, metasim provides a relatively easy way to do it (don't worry, I can/will do it).

@LecrisUT
Copy link
Contributor Author

If I can offer a design suggestion. Could the common parts be separated to a different project and then imported via FetchContent/find_package? This would make it easier to synchronize it across the projects and make metasim easier to test outside of a docker environment. It would also make it easier to review and suggest some better practices.

But for this issue, I will try to port it to stdair and then I will leave it in your hands if it's all ok with you.

@da115115
Copy link
Member

Sure, that is a good idea indeed!

As a matter of fact, that common CMake configuration repository already exists: https://github.com/cpp-projects-showcase/config-cmake . It would need an upgrade, obviously. But feel free to adapt it to something more modern and maintainable.

Note that for Fedora, it is easier if we do not have to introduce a new package. Otherwise we will have to submit a package review request, and that may take months/years. Hence, a way to solve that issue is to have all the CMake config in stdair, and then all the other components can import their CMake configuration from stdair.

@LecrisUT
Copy link
Contributor Author

Note that for Fedora, it is easier if we do not have to introduce a new package. Otherwise we will have to submit a package review request, and that may take months/years.

Call me to do the review and I can get it through in a timely manner. For debundling the review process is much easier. But there are other ways to bundle including using submodules.

One thing that I would like you to consider is how much is the autotools compatibility necessary. It would be much easier to design a templatable pkgconfig file which can then be imported to autotools projects if necessary. But I am strongly opinionated against autotools. Generally though I advocate for simple CMake project designs without templating because it makes it more readable for the user, but I would need a bit of a rundown of the requirements of these projects and then we can figure out what design would fit best.

It would also be nice to merge some of the efforts with these 2 projects of mine:

Although I haven't had time to review it lately to pull in some of the new things I have learned so far, but it would help if there is someone poking me or discuss about it. In particular I want to document some opinionated modern CMake standards in the CMake-Template and more general stuff in https://cliutils.gitlab.io/modern-cmake

@da115115
Copy link
Member

da115115 commented Mar 13, 2025

As a matter of fact, the autotools part may not work anymore, I haven't used it for over 15 years or so. Hence, it may be a good opportunity to drop the compatibility with the autotools, indeed.
The rationale, at the time (around 2009) was to be able to build those components almost anywhere, and CMake was not as mature as it has now become.

I think it's a good idea to merge with/use your CMake utility. I let you a wild card to fully rework the build engine around CMake only. As long as it works on Fedora, Debian, Ubuntu, WSL and MacOS, and works with Python extensions (as in RMOL), anything reengineering will work for me.
As the user base is pretty much limited, we can afford to break temporarily a few things if needed.

@LecrisUT
Copy link
Contributor Author

Thanks will see what I can do about these.

Python extensions would be the tricky bit. Using scikit-build-core should align it better with python environments, e.g. providing python package metadata so that it can be pip uninstall. One design for this is in https://github.com/spglib/spglib.

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