python: install to a proper libdir prefix #599
Conversation
Currently, the libraries are installed to `$PREFIX/python`, which is a non-standard prefix. Standard prefix looks more like `$PREFIX/lib/python3.XX/site-packages`.
|
Linking key4hep/EDM4hep#220 just to have a consistent way of setting this for both. Maybe @andresailer has a preference of the exact technicalities of how we achieve effectively the same path? |
Co-authored-by: Andre Sailer <[email protected]>
|
I think we need to harmonise a few places where we install python things? |
|
I think most of this is OK? And since EDM4hep builds downstream with this at least what we install and offer via CMake to downstream is consistent. What would you place differently? |
|
I would put everything under site-packages/podio. Probably would mean some |
|
Ah yes, that would make sense I think. Or maybe put the
at the top level directory. |
|
sounds like a better convention to me |
|
Rename podio_gen to |
|
And then still install to Regardless, I would move the |
|
Yes, |
|
I think I just remembered why we have |
|
Damn. Indeed that would be a bit awful... |
|
Let's leave it as is then. |
|
Keeping Indeed we have many repos that use After this change the spack recipe needs to be updated (spack/spack#44291) and I think |
|
Got here through the Spack PR. This ${CMAKE_INSTALL_LIBDIR}/python${Python_VERSION_MAJOR}.${Python_VERSION_MINOR}/site-packageslooks sensible, but unclear if The source of truth is python3 -c "import sysconfig, sys; sys.stdout.write(sysconfig.get_path('platlib'))"which CMake provides through The issue is that this is an absolute path to the system site packages dir, idk how you get just the relative bits I would recommend letting users pass the Python install dir as a cmake variable, cause in Spack we expose |
|
@haampie See also the original version. It's probably not platlib, but purelib, and would not need a multilib prefix. Python_SITEARCH is probably a bad idea, it would try to install to a system-wide python location. |
|
Than after the comments from @haampie, I think the answer would be something like: find_package(Python3 REQUIRED)
set(PYTHON_LIB_DIR lib)
if("${Python3_SITEARCH}" MATCHES "/lib64/")
set(PYTHON_LIB_DIR lib64)
endif()
set(podio_PYTHON_INSTALLDIR "${CMAKE_INSTALL_PREFIX}/${PYTHON_LIB_DIR}/python${Python3_VERSION_MAJOR}.${Python3_VERSION_MINOR}/site-packages")
set(podio_PYTHON_INSTALLDIR ${podio_PYTHON_INSTALLDIR} PARENT_SCOPE)
For the rest of the comment I think our users don't care where the python packages are installed as long as it works. |
|
I have now implemented the following:
@veprbl unless you have any objections, I would rebase this onto the latest master/main before we merge it. |
|
Thanks for taking care of this, @tmadlener. It looks fine to me. |
|
I checked that this works as expected locally with spack and |
Changed in AIDASoft#599, but the change is also needed in these two places
Changed in #599, but the change is also needed in these two places Co-authored-by: jmcarcell <[email protected]>
Currently, the libraries are installed to
$PREFIX/python, which is a non-standard prefix. Standard prefix looks more like$PREFIX/lib/python3.XX/site-packages.BEGINRELEASENOTES
lib/python3.XX/site-packagesinstead ofpythonby default. This is a more standard prefix for python packagespodio_PYTHON_INSTALLDIRcmake variable to change this.podio_python_setup)ENDRELEASENOTES