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

Fix CMake Python configuration bug #245

Closed
wants to merge 2 commits into from

Conversation

ahundt
Copy link
Contributor

@ahundt ahundt commented May 21, 2017

If multiple versions of python are installed, the current CMake scripts may not find the correct version of Python and may have version mismatches between the interpreter and python library.

Python remains optional with these changes.
A specific python version can now be selected by setting PYTHON_VERSION_PANGOLIN.
Python versions can vary by default unless EXACT_PYTHON_VERSION=ON
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

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
@stevenlovegrove
Copy link
Owner

Thanks for this patch, though it is failing the CI on Linux. Looking at your changes, I can't say that I really understand why.

@ahundt
Copy link
Contributor Author

ahundt commented May 23, 2017

Maybe one of the variable names was accidentally changed so something required isn't being linked?

Linking CXX executable VideoViewer
../../src/libpangolin.so: undefined reference to `forkpty'
../../src/libpangolin.so: undefined reference to `openpty'
collect2: error: ld returned 1 exit status
make[2]: *** [examples/HelloPangolin/HelloPangolin] Error 1
make[1]: *** [examples/HelloPangolin/CMakeFiles/HelloPangolin.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
../../src/libpangolin.so: undefined reference to `forkpty'
../../src/libpangolin.so: undefined reference to `openpty'

It is funny that mac and windows are where things are ok. I was hoping this would just be a helpful fix not a big debugging thing... hmm maybe it is flaky? will close and reopen

@ahundt
Copy link
Contributor Author

ahundt commented Sep 22, 2017

should work now, second commit based on bulletphysics/bullet3#1331

@ahundt
Copy link
Contributor Author

ahundt commented Sep 22, 2017

Its green 👍

@ahundt
Copy link
Contributor Author

ahundt commented Nov 16, 2017

@stevenlovegrove bump

@stevenlovegrove
Copy link
Owner

I'm sorry I never got to merging this - I was put off by it's size and references to irrelevant things like numpy etc.

Much better python integration has recently been added to Pangolin via pybind11, including more robust CMake python handling - hopefully that solves your problem?

@ahundt
Copy link
Contributor Author

ahundt commented Mar 30, 2018

no worries, I ended up sticking to v-rep in the end except for a utility I use occasionally that depends on pangolin. It seems nice overall! Thanks, closing.

@ahundt ahundt closed this Mar 30, 2018
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