-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Update build commands for Linux / OS X in the docs #907
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
Conversation
|
|
|
Makes sense, crossed my mind as well.
Yea,
Yea, I've removed
Ditto, |
|
With all this in mind, I wonder... should we just ship a (To think about it, I've actually done something very similar in |
|
Added |
|
It looks like |
|
Fair enough; added |
|
+1 for updating the manual build commands. The Python 2/3 and platform nuances can be frustrating to figure out, so this should help a lot. One thing I would strongly suggest is moving the instructions from "First steps" to a new section in "Build systems" and replacing the section in "First steps" with a link to the |
|
I've moved the build notes to "Build systems" section and updated it to use |
|
(My rst-fu is pretty bad, I hope that |
docs/compiling.rst
Outdated
| pybind11 is a header-only-library, hence it is not necessary to link against | ||
| any special libraries (other than Python itself). | ||
|
|
||
| On Linux, the above example can be compiled using the following command: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no "above" anymore. Perhaps:
On Linux, you can compile an example such as the ones given in
:ref:`simple_example` using the following command:
with an addition of:
.. _simple_example:
in basics.rst just before:
Creating bindings for a simple function
=======================================
docs/compiling.rst
Outdated
| Note that on Python 2.7.x ``python-config`` has to be used instead of | ||
| ``python3-config`` in the command above. Besides, ``--extension-suffix`` | ||
| option may or may not be available, depending on the distribution; in the latter | ||
| case, the module extension can be manually set to ``.so``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this whole paragraph is slightly obsolete by the #921 changes (since the only python3-config is now the extension suffix one). Perhaps just simplify it to:
Note that Python 2.7 modules don't use a special suffix, so you should simply use `example.so` instead of ``example`python3-config --extension-suffix```.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the link to the Python library in this command? I need something like:
$ c++ -O3 -Wall -shared -std=c++11 -fPIC `python3 -m pybind11 --includes` `python3-config --libs` example.cpp -o example`python3-config --extension-suffix`I'm also on macOS and didn't been the --undefined dynamic_lookup part, at least for the built-in Python 2 and the sample in #1041.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Linux, it's better to (intentionally) not link against libpython. The symbols will be resolved when the extension library is loaded into a Python binary. This is preferable because you might have several different installations of a given Python version (e.g. the system-provided Python, and one that ships with a piece of commercial software). In this way, the plugin will work with both versions, instead of possibly importing a second Python library into a process that already contains one (which will lead to a segfault).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, so I traded the undefined for the link. Nevermind, then! Thanks.
You can |
I'm not sure about that. I can easily remember the two lines of CMake needed in the simplest case, but for the manual build I always need to refer to the docs to copy/paste the exact flags (and still watch out for the special cases). In my opinion, the manual build only has the advantage if:
The "First Steps" section should really be about what's easiest to learn but also what's most useful for future expansion. Users will naturally want to connect the binding code with a larger Python or C++ project and that will require a proper build/distribution system. The manual build doesn't offer that, so that means users learn something only to drop it and go in a completely different direction. The example repos offer a dead simple way to get started: From there it's easy to build on and experiment without worrying about Python versions/distributions or operating systems. So my suggestions are:
|
|
I think it's nice to at least mention the 1-line compile command to make it crystal clear that there is no magic happening in the background (compiling extra libraries, etc :)). |
[skip ci]
|
Since this would be very useful to have for the v2.2 release, I've pushed some changes to complete the docs, incorporated suggestions by @jagerman and I copy/pasted @wjakob's comment about linking to How does it look to everyone? |
|
|
||
| .. code-block:: bash | ||
| $ c++ -O3 -Wall -shared -std=c++11 -fPIC `python3 -m pybind11 --includes` example.cpp -o example`python3-config --extension-suffix` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes that pybind11 has been installed using pip or conda, which may be worth mentioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it hasn't, this needs to be the Python include directory (python-config3 --includes) and PyBind's include directory, as that command incorporates both. (Not sure if that's worth mentioning, just added it for completeness)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be addressed by the latest commit.
docs/compiling.rst
Outdated
|
|
||
| Note that Python 2.7 modules don't use a special suffix, so you should simply | ||
| use ``example.so`` instead of ``example`python3-config --extension-suffix```. | ||
| Besides, ``--extension-suffix`` option may or may not be available, depending |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing 'the' before --extension-suffix
|
Looks great (I added two minor comments) |
[skip ci]
Please correct me if I'm wrong; I've double-checked this today on ubuntu's python3-dev and pyenv Python on OS X:
-fPICexplicitly specifiedpython-config --ldflagsis unnecessarypython3-config-undefined dynamic_lookupThe changes to the docs reflect all of the above points.