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.12.0 on Windows creates extensions that are not ABI compatible with extensions compiled with pybind11 2.11.0 and/or with a different version of MSVC #95

Open
1 task done
traversaro opened this issue Apr 3, 2024 · 16 comments
Labels

Comments

@traversaro
Copy link

traversaro commented Apr 3, 2024

Solution to issue cannot be found in the documentation.

  • I checked the documentation.

Issue

There is an extensive discussion on this in pybind/pybind11#4779 and pybind/pybind11#4953 . I open this issue as this change got released in conda-forge as part of #94, so this started hitting usage downstream of conda-forge (for example conda-forge/bipedal-locomotion-framework-feedstock#61 (comment))

Installed packages

pybind11 2.12.0

Environment info

Not relevant (I guess).
@traversaro traversaro added the bug label Apr 3, 2024
@henryiii
Copy link
Contributor

henryiii commented Apr 3, 2024

I think there's a mechanism for an ABI bump that we should have / should use. Everything needs to be rebuilt with 2.12, since it's required for NumPy 2.0 compat.

@traversaro
Copy link
Author

Sorry, I edited the issue after creating, now I added a few links for context.

@traversaro
Copy link
Author

I think there's a mechanism for an ABI bump that we should have / should use. Everything needs to be rebuilt with 2.12, since it's required for NumPy 2.0 compat.

The defaults PR contains some reference to the pybind11-abi version changes: AnacondaRecipes#15 (comment) .

@henryiii
Copy link
Contributor

henryiii commented Apr 3, 2024

Nice, that looks right. We should do the same.

@traversaro
Copy link
Author

To add some more info, it seems that the bump to the 5 abi for Python 3.12+ already occured in 2.11.0 : https://github.com/pybind/pybind11/blob/v2.11.0/include/pybind11/detail/internals.h#L39 .

@henryiii
Copy link
Contributor

henryiii commented Apr 3, 2024

No, that means it happened in the development between releases, not that it was in a 2.11 release. Ahh, you mean for Python 3.12, yes.

@henryiii
Copy link
Contributor

henryiii commented Apr 3, 2024

That's complicated, actually, since we've probably been considering extensions build with 3.12 to be pybind11-abi==4, which is fine (since they never talk to extensions built with the actual ABI 4), but now fixing it the new extensions will need to be recompiled when they are actually compatible. But I assume that's okay, since the rest of the versions need recompilation anyway.

@JeanChristopheMorinPerso

What a nice coincidence! We (anaconda) were planning on creating an issue to discuss getting our changes into conda-forge :)

@traversaro
Copy link
Author

traversaro commented Jul 2, 2024

Ok, some time has passed, and my reckless workaround of just pinning pybind11 to <2.12.0 in my feedstock now fails as we need to migrate to numpy 2.0, that indeed is supported by pybind11 >=2.12.0 . So I need to look into this.

The quickest fix is to do something similar to AnacondaRecipes#15 in this recipe. This will ensure that downstream packages that build against future builds of pybind11 will have the correct export of pybind-abi (i.e. 5 for Windows and Python>=3.12, 4 for the rest).

The downside of this is what @henryiii reported in #95 (comment), this will create a conda incompatibility between packages that actually are built with pybind-abi==5 but they run export with pybind-abi==4. This may mitigate with some kind of repodata patch, but to be honest I am not sure how to prepare a repodata patch that looks into the rendered_recipe to decide if it is necessary to change the pybind-abi dependency.

Another related downside is that since the 2.12.0 release pybind11 extensions built with different version of MSVC (even just minor, like if an extension is compiled with Visual Studio 2022 version 17.9 and another with Visual Studio 2022 version 17.10, see https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170) are incompatible, even if those versions are ABI compatible. On one hand, this is similar to what happens already for gcc and clang (see #77), however for MSVC this behavior is particularly problematic:

  • We can't choose the version of Visual Studio to use, as that is given by the Azure image, not something that we control
  • For GCC and Clang the incompatibility happens for major version updates, while this one for MSVC happens for minor releases.

However, the MSVC incompatibility is already there since #94, and I wonder why no one reported any issue related to that. Perhaps it is not so common in conda-forge to have two pybind11-based extensions that interact (I know at least two cases of these kind of interactions, but are all from recipe that I mantain), or the errors that emerge are so confusing that users do not track it back to this feedstock?

@traversaro
Copy link
Author

Ah, we need alto to change the global pin (see #53) to be 5 on Windows or in Python 3.12 . Does this mean that we need to add pybind11-abi to the python zip_keys ?

@traversaro
Copy link
Author

This issue probably will become more relevant as pybind >= 2.12.0 is required for numpy 2.0 compatibility.

@traversaro
Copy link
Author

Ok, at this point I think it is useful to split the issue in two, to facilitate progress on it:

For P1 a possible solution is just to port AnacondaRecipes@73bc183 and AnacondaRecipes@1222336 here on conda-forge. For P2 the issue is more tricky, the proper solution is probably pybind/pybind11#4953 but I am not sure which actions are possible to unblock that.

@rwgk
Copy link

rwgk commented Sep 30, 2024

@traversaro wrote on July 2, 2024:

Another related downside is that since the 2.12.0 release pybind11 extensions built with different version of MSVC (even just minor, like if an extension is compiled with Visual Studio 2022 version 17.9 and another with Visual Studio 2022 version 17.10, see https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170) are incompatible, even if those versions are ABI compatible.

Ouch ... is that really true? That's absolutely not what I was hoping for when I cast my vote for merging pybind/pybind11#4779. Regarding pybind/pybind11#4953, that PR went stale, unfortunately.

How can we determine the unshakable ground truth for: Which MSVC versions are ABI-compatible? — The sooner we get that into pybind11, the better.

If anyone here has ideas, including maybe ideas for how to get there in a minimally disruptive way for conda-forge users, please let us know.

@isuruf
Copy link
Member

isuruf commented Sep 30, 2024

See pybind/pybind11#4953

@isuruf
Copy link
Member

isuruf commented Sep 30, 2024

How can we determine the unshakable ground truth for: Which MSVC versions are ABI-compatible?

You check for MD/MT, and you check the major version in MSC_VER

@rwgk
Copy link

rwgk commented Sep 30, 2024

Could you (or someone who is familiar with the affected environments, and set up for testing) help coding this up for pybind11 (i.e. create a pybind11 PR)?

(It's years that I last logged into a Windows system myself.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants