Skip to content

Various CMake improvements #565

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

Merged
merged 4 commits into from
Dec 19, 2016
Merged

Various CMake improvements #565

merged 4 commits into from
Dec 19, 2016

Conversation

dean0x7d
Copy link
Member

Note: The changes are best viewed individually because the first commit has a bit of a messy diff.

1. add_subdirectory vs. find_package

The first commit makes sure that these two methods for including pybind11 are 100% interchangeable. This involves adding a BUILD_INTERFACE and a namespaced alias for the interface library.

Tests are added to ensure this. To avoid quadrupling some files, the directory structure was changed a bit. This yields a messy diff, but the changes are straightforward:

Before:

tests
|-- test_installed_module
|   |-- CMakeLists.txt
|   |-- main.cpp
|   \-- test.py
\-- test_installed_target
    |-- CMakeLists.txt
    |-- main.cpp
    \-- test.py

After:

tests
\-- test_cmake_build
    |-- installed_function/CMakeLists.txt
    |-- installed_target/CMakeLists.txt
    |-- subdirectory_function/CMakeLists.txt
    |-- subdirectory_target/CMakeLists.txt
    |-- main.cpp
    \-- test.py

Renamed test_install target to test_cmake_build because it's now also testing the non-installed subdirectory build.

2. pybind11_add_module and docs

See the documentation for this one. Everything about pybind11_add_module is pulled into one subsection and some duplicate function explanations are removed from other parts of the text. Same thing for configuration variables: consolidated to one subsection.

3. Rename pybind11::pybind11 -> pybind11::module

Embedding the interpreter could use a future target called pybind11::embedded, so it might be good to make room for it already. Hence, this proposed rename for the extension module target.

cc @loriab

4. make check

Runs all available tests with a common target name to help out packaging as suggested in #442 (comment). Could be expanded with embedded tests in the future. Individual targets remain for selective testing.

Add a BUILD_INTERFACE and a pybind11::pybind11 alias for the interface
library to match the installed target.

Add new cmake tests for add_subdirectory and consolidates the
.cpp and .py files needed for the cmake build tests:

Before:
tests
|-- test_installed_module
|   |-- CMakeLists.txt
|   |-- main.cpp
|   \-- test.py
\-- test_installed_target
    |-- CMakeLists.txt
    |-- main.cpp
    \-- test.py

After:
tests
\-- test_cmake_build
    |-- installed_module/CMakeLists.txt
    |-- installed_target/CMakeLists.txt
    |-- subdirectory_module/CMakeLists.txt
    |-- subdirectory_target/CMakeLists.txt
    |-- main.cpp
    \-- test.py
See the documentation for a description of the options.
Makes room for an eventual pybind11::embedded target.
@wjakob
Copy link
Member

wjakob commented Dec 19, 2016

Beautiful, thank you -- the build system looks much more consistent now, and the documentation you added reads very well. I'll rebase the individual commits so that the history is preserved.

@wjakob wjakob merged commit 06b9397 into pybind:master Dec 19, 2016
@dean0x7d dean0x7d deleted the cmake branch December 19, 2016 19:02
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