-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
remove linking to libpython on linux/OSX #703
Conversation
This makes regular builds fail to compile. Any proposed updates to dlib must not interfere with existing common usage. |
Ah, I am sorry, this indeed breaks the OS X build. So #701 would need to be merged first and this needs to be edited to only add the |
It breaks on linux too. I haven't tried it on windows, but I would bet money it breaks it there too. Please submit a PR that doesn't break anything. I also don't understand what this PR is accomplishing. Maybe because I'm not familiar with manylinux. |
This PR tries to accomplish fixing the building in a way that is compatible with python wheels (see PEP-427: https://www.python.org/dev/peps/pep-0427/). Wheels have become a very popular way to distribute complex python packages (see for example numpy/scipy/pandas/tensorflow/pytorch/scikit-learn who also distribute their package this way). It allows the user to install dlib from pypi in a matter of seconds instead of having to go through a compilation phase. In order to be able to build a wheel one needs to precompile all assets, which is done on manylinux to be as widely compatible as possible. The travis builds are passing and I am very curious to hear why it fails building for you. I propose we do #701 first, so I can update this PR to work with every platform. If you prefer I can also take the changes of that PR and integrate it into this one. |
The travis builds weren't building the pyhton extension. I just updated the travis config to include them though, so if you trigger a rebuild on travis you will see your build fail. But yes, binary linux packages sound great. But whatever changes are made, it still needs to be possible for people to download dlib and run cmake or setup.py to build it themselves. |
Ah, that's great, thanks! I'll update this PR tomorrow to include the OSX bit, which will then automatically trigger the rebuild. These changes are not meant to break running cmake/setup.py manually and I'm keen to fix them. |
Cool, sounds good :) |
I cleaned up all the building files and ran:
output:
|
It is not reasonable to expect users to put |
I think I have found a way to allow users to build without specifying Please review the changes and let me know what you think. |
Cool, looks good. Thanks for the PR :) |
Great - many thanks for all your hard work on this. |
* remove linking to libpython on linux * add OSX libpython free building * add automatic discovery of include python dir back in * make the libs non required for building on manylinux
This allows building of dlib on linux without libpython and is the second step in enabling #138.
Depending on the merge order of this and #701 I'll refactor one or the other.
Also relevant for @matthew-brett
In order to build a wheel on manylinux:
Output of auditwheel: