-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
cmake: python: use system pybind11 if installed #5092
Merged
alexreinking
merged 1 commit into
halide:master
from
acolinisi:PR--cmake-system-pybind11
Jul 8, 2020
Merged
cmake: python: use system pybind11 if installed #5092
alexreinking
merged 1 commit into
halide:master
from
acolinisi:PR--cmake-system-pybind11
Jul 8, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LGTM but deferring to @alexreinking; also cloning for testing |
alexreinking
requested changes
Jul 7, 2020
Fixes builds on offline hosts (common on HPC clusters), which otherwise fail with failure to fetch pybind11 from the Internet, introduced in c13efff: CMake Error at pybind11-subbuild/pybind11-populate-prefix/tmp/pybind11-populate-gitclone.cmake:31 (message): Failed to clone repository: 'https://github.com/pybind/pybind11.git' Also, make build friendly to distribution packages. Distribution package recipe should be in control of (1) dependencies to build against (e.g. some distros are on pybind11 2.5, and it should be used unless it's incompatible), and (2) when fetching happens vs when building happens. So, there is a need for a packager-friendly build mode where the build system doesn't go out and fetch whatever dependencies it chooses whenever it chooses. This patch gives priority to the system pybind11 with fallback to fetching.
acolinisi
force-pushed
the
PR--cmake-system-pybind11
branch
from
July 7, 2020 19:16
3ab5015
to
8551fac
Compare
alexreinking
approved these changes
Jul 7, 2020
@steven-johnson - if you would, please clone again for testing. |
Clone looks good to land |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes builds on offline hosts (common on HPC clusters), which otherwise
fail with failure to fetch pybind11 from the Internet, introduced in
c13efff:
Also, make build friendly to distribution packages. Distribution
package recipe should be in control of (1) dependencies to build against
(e.g. some distros are on pybind11 2.5, and it should be used unless
it's incompatible), and (2) when fetching happens vs when building
happens. So, there is a need for a packager-friendly build mode where
the build system doesn't go out and fetch whatever dependencies it
chooses whenever it chooses. This patch gives priority to the system
pybind11 with fallback to fetching.
An alternative (if you prefer) would be to keep fetch as the default, and add a flag for distributions to use: -DWITH_SYSTEM_PYBIND11 (default OFF).