search

tudo-astroparticlephysics / PROPOSAL

Monte Carlo Simulation propagating charged Leptons through Media as C++ Library
GNU Lesser General Public License v3.0
35 stars 21 forks source link

Update repository for conan2 #365

Closed Jean1995 closed 1 year ago

Jean1995 commented 1 year ago

This PR updates the conanfile, the installation instructions, setup.py, and the github actions scripts to be compatible with conan2.

The old installation approach, which relied on the cmake_paths generator of conan, has been deprecated. Instead, I have oriented myself on the procedure presented in the conan2 tutorial, which uses the approach where conan creates a CMake toolchain file which tells CMake where to find the packages installed by conan. This has the advantage that the conan installation process is completely separated from the rest of our CMake structure.

The installation procedure using conan and CMake now has a few more steps, but is more transparent for the user.

pip install proposal still works ous of the box. With the update to conan2, a lot of issues that required manual intervention by us (for example the libcxx settings) have been resolved, which also simplified the setup.py.

The description of the installation process as well as the setup.py are not backwards-compatible with conan 1.x.
Therefore I gave pip install "conan~=2.0" as an explicit instruction in the INSTALL.md for now.

Other changes

maxnoe commented 1 year ago

Might this be a good point to also try to get rid of the custom cmake code in setup.py and try if we can use scikit-build or even scikit-build-core?

Jean1995 commented 1 year ago

Might this be a good point to also try to get rid of the custom cmake code in setup.py and try if we can use scikit-build or even scikit-build-core?

Since we are touching the setup.py anyway, now would be the right time if we want to improve it. However, I've never worked with the scikit-build generator so far.

The-Ludwig commented 1 year ago

Might this be a good point to also try to get rid of the custom cmake code in setup.py and try if we can use scikit-build or even scikit-build-core?

Since we are touching the setup.py anyway, now would be the right time if we want to improve it. However, I've never worked with the scikit-build generator so far.

I never worked with compiled parts of a python package, but I wonder how conan integrates into scikit-build? Wouldn't PROPOSAL still need custom setup.py code for conan? Interesting thread I found on this: https://github.com/orgs/scikit-build/discussions/645

maxnoe commented 1 year ago

Maybe also open an issue about a possible conan integration in the new scikit-build-core project? They are rewriting from scratch and handling the C++ dependency is probably important for more people: https://github.com/scikit-build/scikit-build-core/issues

The-Ludwig commented 1 year ago

Maybe also open an issue about a possible conan integration in the new scikit-build-core project? They are rewriting from scratch and handling the C++ dependency is probably important for more people: https://github.com/scikit-build/scikit-build-core/issues

Maybe it's better if you open an issue, since I have no idea about scikit-build and C++ distribution through pip and only used conan in one project :D

Jean1995 commented 1 year ago

How to we want to proceed with the scikit-build(-core) suggestion? Open an issue in their repository asking about conan, and (for now) use the custom cmake solution?

The-Ludwig commented 1 year ago

How to we want to proceed with the scikit-build(-core) suggestion? Open an issue in their repository asking about conan, and (for now) use the custom cmake solution?

I would do so, unless you we want this to be a pilot-project for a custom conan+scikit-build workflow...

Jean1995 commented 1 year ago

How to we want to go forward with this MR? This is the last issue I would like to solve for the next release.

Does this concept work for you?