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

Prepare for conan package #234

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

Prepare for conan package #234

wants to merge 47 commits into from

Conversation

phbasler
Copy link
Contributor

@phbasler phbasler commented Apr 8, 2022

In preparation for the conan package I added the options to get the necessary dependencies through conan.

If you have installed conan you can use -DLIBCMAES_USE_CONAN=On and cmake will download the matching conan packages of eigen, boost( if LIBCMAES_BUILD_PYTHON = On) and gflags (if LIBCMAES_BUILD_TESTS=On).

While configuring this I saw that the PythonInterp find_package call was deprecated and so I updated it to find_package(Python3) with the modules. This also allowed me to make the python and t_* tests run with ctest.
You can now just call ctest in your build directory and the 6 tests run.

Furthermore, we can get rid of the custom FindNumpy and FindEigen module. cmake has it's own find modules for eigen and python.

@phbasler
Copy link
Contributor Author

I just saw that it still does not compile on windows with mvsc. I convert this PR to a draft until I fixed it as well.

@phbasler phbasler marked this pull request as draft April 10, 2022 14:24
@phbasler
Copy link
Contributor Author

phbasler commented Apr 10, 2022

@beniz It would not compile on windows until I defined an explicit copy ctr for scalar_normal_dist_op. Can you check if this one is ok?

ATM the python tests are failing because the module is not found for the ctest call, but everything compiles. (Only with MVSC, with gcc ctest runs the python test sucessfully)

@phbasler
Copy link
Contributor Author

phbasler commented Apr 13, 2022

@beniz @nikohansen

I found the problem and maybe one of you has an idea.
On my fork https://github.com/phbasler/libcmaes/tree/conan-windows I set up some CI to run the unit tests, including the python tests, on windows, mac and ubuntu.
Mac and ubuntu work without a problem, but on windows the lcames python module is seperated in two files. One build/python/Release/lcames.pyd and the dll build/src/Release/cmaes.dll. It works there if I copy the cmaes.dll to the directory which contains the .pyd. If I don't copy it, then I get an Import Error because the dll is not found.

What should we do here? Do you have an idea on how to fix it or should we copy it during the cmake call?

Edit: A possible cmake solution to fix this with MVSC using cmake is given in phbasler@bfe0eb4
But it is just copying/installing the cmaes.dll to the python location, so the dll would be in two places.

@nikohansen
Copy link
Collaborator

Sorry, I have no idea regarding the windows test setup.

phbasler and others added 2 commits April 28, 2022 06:30
* added CI

* Added gflags

* Includ numpy and boost-python

* Changed pip to pip3

* Extended PYTHONPATH

* Extending pythonpath

* Adjusted  Pythonpath to use Config if available

* Update windows_unit_tests.yml

* Update windows_unit_tests.yml

* Update windows_unit_tests.yml

* Update windows_unit_tests.yml

* Modified python path

* Update CMakeLists.txt

* Moved pythonpath to target generator expression

* Add post_build/install of cmaes.dll to python location on MVSC

* Adjusted workflows to only trigger on master

Co-authored-by: Phil <[email protected]>
@phbasler phbasler marked this pull request as ready for review April 28, 2022 04:35
@beniz
Copy link
Collaborator

beniz commented May 4, 2022

Hi @phbasler I would go for the copy of the DLL is that helps. Thanks for all the work on this!

@phbasler
Copy link
Contributor Author

phbasler commented May 4, 2022

@beniz That's how I implemented now. I also fixed a small mistake I introduced with another PR:
If you include libcmaes as a subdirectory then the doc target was also created, even if you have one of your one (like I have in https://github.com/phbasler/BSMPT) and then I got a cmake error there. I fixed that with the last commit that the doc target is only created if libcmaes is build as the top project

Philipp Basler and others added 3 commits May 4, 2022 21:14
Fixed wrong doxygen link to match the move of the project
Add conanfile to generate conan package
@phbasler
Copy link
Contributor Author

@nikohansen @beniz I finished the conan package. You can build it with `conan create conanfile.py --profile=SomeProfile" where SomeProfile is a conan profile with your compiler and settings how you want to build. You need to fill in the author and topics in the conanfile.

@beniz
Copy link
Collaborator

beniz commented Mar 22, 2023

@phbasler hello, apologies for missing this, you should put your name in the conan file!
I am not familiar with conan, if it works for you, then maybe squash and get ready for a merge!

@phbasler
Copy link
Contributor Author

@beniz In the meantime conan released v2 with some breaking changes, so we would have to make sure everything still works.

@phbasler
Copy link
Contributor Author

Seems that there are some changes necessary. I will have a look at it if I have some time to spare, but this may take a while.

@phbasler
Copy link
Contributor Author

phbasler commented Jun 2, 2024

@beniz @nikohansen
So after some time I got around to update this PR and now it works with conan 2
If we can go through this PR and you can release a new version, then I can make the official conan recipe

@phbasler phbasler changed the title Enable to get dependencies through conan and cmake Updates Prepare for conan package Jun 2, 2024
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.

3 participants