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] Do not support Mac for now #11075

Merged
merged 8 commits into from
Jun 19, 2022

Conversation

uilianries
Copy link
Member

@uilianries uilianries commented Jun 7, 2022

Specify library name and version: pybind11/2.9.1

Related to #10817

/cc @ericLemanissier
/cc @vectorsli


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
@uilianries uilianries requested a review from SSE4 June 7, 2022 13:40
@ghost
Copy link

ghost commented Jun 7, 2022

I detected other pull requests that are modifying pybind11/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot
Copy link
Collaborator

All green in build 6 (2927b51771e6d2cf91198a0f223dbebda0a01876):

  • pybind11/2.9.1@:
    All packages built successfully! (All logs)

  • pybind11/2.7.1@:
    All packages built successfully! (All logs)

  • pybind11/2.8.1@:
    All packages built successfully! (All logs)

@uilianries uilianries marked this pull request as ready for review June 9, 2022 16:29
@uilianries
Copy link
Member Author

@SSE4 could you please review? So far, it failed with 11.0, 12.0 and 13.0, but those are the version that we have, I don't have an older compiler version to validate, so I only included those 3

Copy link
Contributor

@stefansli stefansli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uilianries Thanks for looking into this and providing a fix. The changes look good to me 👍

@SSE4
Copy link
Contributor

SSE4 commented Jun 13, 2022

@uilianries I've checked locally, and the same error (Fatal Python error: PyMUTEX_LOCK(_PyRuntime.ceval.gil.mutex) failed) happens for me as well.
checking the comment pybind/pybind11#3081 (comment), they say we're linking against the whole set of components. but seems we aren't, we are just using:

find_package(pybind11 REQUIRED CONFIG)

pybind11_add_module(test_package MODULE test_package.cpp)

so I think the reason it fails is different 🤔

@stefansli
Copy link
Contributor

Let me jump in since we are using pybind11 and this recipe. There are already several issues in CCI regarding the bugged pybind11 recipe.

To sum it up:
Conan creates the target pybind11:pybind11 and links all "components" into this "main" target. However the Module from pybind11 also configures a pybind11:pybind11 target which is the base for all subsequent targets (such as pybind11::embed or pybind11::lto. For more details please see #6605. Now some of the target pybind11 defines and uses in their cmake macros conflict. For example bindings created with pybind11 are either used as a module in which case they are compiled to a *.pyd shared library, or they are embedded in a library/executable.

From what I understand, the recipe will never work properly with cmake_find_package and cmake_find_package_multi (unless one uses the CMake helper files generated by the build).
It might be possible to configure the CMakeDeps generator correctly but I haven't tried this yet because we are not using the CMakeDeps generator in production.

@SSE4
Copy link
Contributor

SSE4 commented Jun 13, 2022

@vectorsli thanks for the clarification. this is beyond my knowledge on how to set up these components and modules correctly, to closely resemble upstream behavior. maybe @czoido or @jgsogo can help here.

@stefansli
Copy link
Contributor

A nice overview of the problem can be found in this comment:

#6605 (comment)

@conan-center-bot conan-center-bot requested a review from danimtb June 14, 2022 12:00
@prince-chrismc
Copy link
Contributor

The changes look good to me
Let me jump in since we are using pybind11 and this recipe.

Let's get this merged and since I have no clue, @vectorsli i'll ping you for help in the future ❤️

@conan-center-bot conan-center-bot merged commit 61691b0 into conan-io:master Jun 19, 2022
foundrytom added a commit to foundrytom/OpenAssetIO that referenced this pull request Jun 21, 2022
pybind is no longer available for mac in conan center.

See:
  conan-io/conan-center-index#11075
  pybind/pybind11#3081
Signed-off-by: Tom Cowland <tom@foundry.com>
hoxnox pushed a commit to hoxnox/conan-center-index that referenced this pull request Jun 30, 2022
* cache cmake

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Skip Mac support

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Remove old versions

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Update not supported OS

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Skip apple-clang 11

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Skip apple-clang 11

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Skip apple-clang 12

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Skip apple-clang 13

Signed-off-by: Uilian Ries <uilianries@gmail.com>
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.

5 participants