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

cmake python config bugs fixed #1131

Merged
merged 3 commits into from
May 23, 2017
Merged

Conversation

ahundt
Copy link

@ahundt ahundt commented May 21, 2017

  • Python and pybullet remain optional with these changes.
  • A specific python version can now be selected by setting PYTHON_VERSION_PYBULLET.
  • The python library version must now match the interpreter version exactly.
  • If all required python dependencies are found, pybullet is now enabled by default.

Changes incorporate the following BSD licensed cmake code:

https://github.com/BVLC/caffe/blob/32bf5c7ad804ad683aa5ea9382209e9284451e5f/CMakeScripts/FindNumPy.cmake
https://github.com/NikolausDemmel/CMake/pull/2/files
https://github.com/Kitware/CMake/blob/86578eccf2e82286248796bad1032cd0e3a5e1e2/Modules/SelectLibraryConfigurations.cmake

This addresses many of the cmake issues blocking #1107

ahundt added 3 commits May 20, 2017 20:47
Python and pybullet remain optional with these changes.
A specific python version can now be selected by setting PYTHON_VERSION_PYBULLET.
The python library version must now match the interpreter version exactly.
If all required python dependencies are found, pybullet is now enabled by default.
Changes incorporate the following BSD licensed cmake code:
https://github.com/BVLC/caffe/blob/32bf5c7ad804ad683aa5ea9382209e9284451e5f/CMakeScripts/FindNumPy.cmake
https://github.com/NikolausDemmel/CMake/pull/2/files
https://github.com/Kitware/CMake/blob/86578eccf2e82286248796bad1032cd0e3a5e1e2/Modules/SelectLibraryConfigurations.cmake
Still ensures library version matches interpreter version exactly.
ahundt added a commit to ahundt/Pangolin that referenced this pull request May 21, 2017
A specific python version can now be selected by setting PYTHON_VERSION_PANGOLIN.
The python library version must now match the interpreter version exactly.
If all required python dependencies are found, pangolin python utilities are now enabled by default.
Changes incorporate the following BSD licensed cmake code:

https://github.com/BVLC/caffe/blob/32bf5c7ad804ad683aa5ea9382209e9284451e5f/CMakeScripts/FindNumPy.cmake
https://github.com/NikolausDemmel/CMake/pull/2/files
https://github.com/Kitware/CMake/blob/86578eccf2e82286248796bad1032cd0e3a5e1e2/Modules/SelectLibraryConfigurations.cmake

Based on my pull request for the same python cmake issues in pybullet:
bulletphysics/bullet3#1131
bulletphysics/bullet3#1107
@erwincoumans
Copy link
Member

erwincoumans commented May 21, 2017

Thanks for the work on this. I will test it on a few platforms before merging.
I think it is better to disable the BUILD_PYBULLET by default, unless the user manually enables it, since I really want Bullet to build out-of-the-box, undisturbed by broken python installations. We can leave it switched on if Python versions match for now. If people report broken builds, we switch it off.

@ahundt
Copy link
Author

ahundt commented May 21, 2017

Okay, I tried to be aware of that concern. It should try and ensure if any problems are detected python automatically switches off, standard warnings are printed, and the cmake configuration + build continues & succeeds without python.

@erwincoumans
Copy link
Member

Thanks for the contribution, looks good to me!

@erwincoumans erwincoumans merged commit 2f3844e into bulletphysics:master May 23, 2017
@erwincoumans
Copy link
Member

People report issues with cmake and python, and the continuous integration also fails.
#1322
We need to have a more reliable way for cmake to locate python

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