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

Fixed python discovery using more modern CMake functions. #356

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KOLANICH
Copy link

@KOLANICH KOLANICH commented Dec 2, 2019

For old CMake they are bundled and backported. For newer CMake the ones shipped with CMake are used.

Copy link
Member

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want to ship Python 2 stuff at all anymore?

@KOLANICH
Copy link
Author

KOLANICH commented Dec 2, 2019

Why do we want to ship Python 2 stuff at all anymore?

I am happy to drop it. I have not been using python 2 since 2013. But since python 2 support is already in the repo, I have implemented it too.

@KOLANICH
Copy link
Author

@mlschroe, @ignatenkobrain

@mlschroe
Copy link
Member

Where's the "fixed discovery"? The commit is just doing things different, but I don't see what it tries to fix.

@KOLANICH
Copy link
Author

KOLANICH commented Jan 21, 2020

Where's the "fixed discovery"? The commit is just doing things different, but I don't see what it tries to fix.

Now I don't remember all the details what the issues were, but was is related to python 2 being used somewhere where python 3 must have been used (in CMakeLists.txt for python 3 bindings). Instead of fixing your discovery code I have just replaced it with CMake one and that has worked. Since the lib supports old versions of CMake without these modules, I have just backported and bundled them (for newer CMake the modules shipped with CMake are used). When support of old CMake versions will be dropped these backported modules can be removed.

…ke they are bundled and backported. For newer CMake the ones shipped with CMake are used.
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