Skip to content

Add pybind11 patches #325

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

Closed

Conversation

KyleFromKitware
Copy link
Contributor

This adds a set of patches to pybind11 that fix pybind/pybind11#1276 (the changes I've made can be seen in pybind/pybind11#1286)

@dstoup
Copy link
Collaborator

dstoup commented Feb 16, 2018

A comment about patch branches in general. I prefer 2 commits. One which pushes the original files and a second which applies the patch. That will allow reviewers to see what's actually being changed.

@KyleFromKitware
Copy link
Contributor Author

Like this?

@dstoup
Copy link
Collaborator

dstoup commented Feb 16, 2018

Yes, thank you :)

@dstoup
Copy link
Collaborator

dstoup commented Feb 16, 2018

Jenkins test this please

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@KyleFromKitware
Copy link
Contributor Author

The builds are failing:

CMake Error at CMake/External_Tensorflow.cmake:4 (message):
  Error: A build with Python is required for building Tensorflow

Is Jenkins not configured for Python? Does this usually happen?

@dstoup
Copy link
Collaborator

dstoup commented Feb 16, 2018

FYI, this is more for Dawkins than anyone, but if there are useful changes being made, I would prefer they go into master, not some frankenbranch which will not merge.

Are these changes useful for pybind11 in general?

@hughed2
Copy link
Contributor

hughed2 commented Feb 16, 2018

Kyle, do you want to push that into viame/master or plain master? Jenkins is configured for master, if viame/master needs anything different I wouldn't expect it to work.

@dstoup
Copy link
Collaborator

dstoup commented Feb 16, 2018

Jenkins is not configured with python and any packages which are added need to assume it can be turned on or off.

@dstoup
Copy link
Collaborator

dstoup commented Feb 16, 2018

@hughed2 this branch cannot go into master, it was taken from viame/master.

@KyleFromKitware KyleFromKitware deleted the viame/pybind11 branch February 16, 2018 21:10
@KyleFromKitware KyleFromKitware restored the viame/pybind11 branch February 16, 2018 21:10
@KyleFromKitware KyleFromKitware deleted the viame/pybind11 branch February 16, 2018 21:18
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.

4 participants