Skip to content
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

pybind11 2.6.0 #63231

Closed
wants to merge 1 commit into from
Closed

Conversation

henryiii
Copy link
Contributor

Created with brew bump-formula-pr.

@BrewTestBot BrewTestBot added the python Python use is a significant feature of the PR or issue label Oct 21, 2020
@fxcoudert
Copy link
Member

Please hold for #62560

@henryiii
Copy link
Contributor Author

Actually, in its current form, this recipe does not need or touch a specific version of Python. It only installs headers and CMake files. We could change it so the Python module is also installed, or we could change it so that Python is not searched on the configure step and drop the Python requirement except for testing.

@fxcoudert
Copy link
Member

fxcoudert commented Oct 21, 2020

Why does it depend on python, then? If it's not necessary, by all means let's remove it! (make it test-only)
In any case, since it is currently part of the files migrated in the mega-PR, we'll hold this to avoid creating conflicts.

@henryiii
Copy link
Contributor Author

Why does it depend on python, then?

Historical, the CMake files were not flexible enough to not look for Python even if all you were doing was installing headers and configure files. Now there is a switch.

Keep in mind, if any other formulas use this one to build, there is a bug in CPython 3.9.0 that causes pybind11 < 2.6 extensions to segfault (UB on all platforms, but random segfaulta on macOS). It's worked around in pybind11 2.6, and patched in CPython 3.9 and dev branches. Conda-forge has already applied the patch to their Python 3.9.0 build, maybe homebrew should too?

@fxcoudert
Copy link
Member

fxcoudert commented Oct 21, 2020

Formulas using this one: libtorch ocrmypdf scipy. At least the issue is not triggered in our tests.
We can update to pybind11 2.6.0 fast once those formulas are migrated: I'll rebase this PR and merge it ASAP.

If pybind11 2.6 works around the issue, I don't see a reason to carry the patch at python/cpython#22674 (unless some other code is known to hit the bug?)

@henryiii
Copy link
Contributor Author

Many libraries bundle pybind11 via submodule or use the PyPI package (even PyTorch does, even if libTorch does not), so it can't be fully controlled here. So I think it still might be a good idea. PyTorch is currently working on upgrading past 2.3, due to a change in 2.4 that broke mixing compilers (which PyTorch could do when JIT compiling).

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants