-
Notifications
You must be signed in to change notification settings - Fork 35
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
Replace hardcoded lib with CMAKE_INSTALL_LIBDIR #132
Conversation
Hi @andriish. I am not sure if we really want to install LCIO libraries in a |
Hi @gaede , The case with /usr is just an example. binaries --> CMAKE_INSTALL_BINDIR Best regards, Andrii |
Hi @andriish thanks for this. As @gaede has mentioned there are a places in ilcsoft that implicitly assume that shared libraries are installed in However, in general I do think that this is the right thing to do, and this definitely goes into the right direction also for an upcoming overhaul of the cmake configuration for LCIO (and other ilcsoft packages). Just for completeness and to avoid potential misunderstandings using If we make these changes we also have to keep track of them in iLCInstall, since that checks the presence of certain binaries to check if an installation has been successful. In fact In summary, I think these changes should go in, but the environment for the tests should be fixed first. |
Line 150 in 8f9e86b
|
Hi @tmadlener ,
Thanks! Good to know. Otherwise I would try to fix them as well :). Best regards, Andrii P.S. Also, just noted two smaller issues with the tests:
|
I think that is partially related to the fact that the minimum cmake is too old to have "native" support for this. I am actually not sure we have a I will try to fix the CI issues and have a look into why the one test is failing on some systems/environments. |
Hi @tmadlener ,
FindPython is available only from cmake 3.12+. Unfortunately some important systems (CentOS8) still use cmake 3.11. However, if only the python executable is needed, one can use
Best regards, Andrii |
Hi @andriish, The python test should be fixed in #136 together with an updated CI configuration for the macOS workflows. As already outlined in #133 the whole cmake configuration is in need of an update and in the course of that we will also bump the required minimum version of cmake to something more recent. I will link your comment to the discussion there as well. |
@andriish could you rebase this onto the new master please. Otherwise github will not pick up the new CI workflow configurations. |
Uh, actually I completely forgot that before cmake 3.12 there was a package FindPythonInterp which would satisfy the needs completely. No need for cmake version bump. At all. |
In this PR the hardcoded installation paths of CMake modules and libraries are replaced with CMAKE_INSTALL_LIBDIR, which is expanded either to lib or to lib64. E.g. this results in a correct location path /usr/lib64/cmake for modules like
MacroCheckPackageLibs.cmake, which otherwise are always installed to /usr/lib/cmake (which does not even exist).
BEGINRELEASENOTES
ENDRELEASENOTES