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

libdeflate: relocatable shared lib on macOS #9089

Merged

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Jan 26, 2022

see conan-io/hooks#376


  • 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.

@conan-center-bot

This comment has been minimized.

avoid error "Attempting to use @rpath without CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG being set."
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jgsogo
Copy link
Contributor

jgsogo commented Jan 28, 2022

You've been bitten by something we haven't considered regarding the epochs 😞

If your read through my post of one of the latest library updates, now we are using epochs to decide the configurations that we can run for a given package, one library cannot jump to a new epoch (more configs) until all its requirements have already jumped to it.

...but we use only the requirements of the main conanfile, not the ones in test_package/conanfile.py. Here openssl comes from the cmake dependency in the test_package.

The problem is that there is no Conan command that will expand the graph of the test-package as well, so we don't know those dependencies until we actually run it. So far, I can think about these alternatives:

  • this situation shouldn't happen so often, we can force PRs for those requirements
  • modify the library and try to expand the graph of the test-pacakge: conan info .../path/to/test_package/conanfile.py (for all the test-package folders).

I'll talk to @danimtb about it and see if we can figure out something else.

# Attempting to use @rpath without CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG being
# set. This could be because you are using a Mac OS X version less than 10.5
# or because CMake's platform configuration is corrupt.
# FIXME: Remove once CMake on macOS/M1 CI runners is upgraded.
Copy link
Contributor

Choose a reason for hiding this comment

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

MacOS runners are using CMake 3.15 following the requirement here: https://github.com/conan-io/tribe/blob/main/design/004-tools-cmake.md. If it is not enough, it's not going to be upgraded because of that restriction.

We can have a look to the other reasons you mention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took the trick from another recipe actually. I guess it's a bug in CMake with relocatable shared libs on macos M1, fixed recently.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added this temporary workaround, it's more or less explained in #7146
it's a CMake bug that was fixed only in 3.20.1+

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed: #7146 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's amazing the number of details you manage to keep in your head 😊 .

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

All green in build 17 (c4c0f003dc04669d87ccf7b2c148dd3927ded28c):

  • libdeflate/1.9@:
    All packages built successfully! (All logs)

  • libdeflate/1.7@:
    All packages built successfully! (All logs)

  • libdeflate/1.8@:
    All packages built successfully! (All logs)

@jgsogo
Copy link
Contributor

jgsogo commented Feb 5, 2022

The underlying issue with the requirements from the test_package remains, but I moved those dependencies to the new epoch so they have the configurations available. The fix is more or less decided and in the backlog, but it will take a bit of time to release it, anyway, this won't happen again (for this scenario) until we add more configurations.

@SpaceIm SpaceIm requested review from SSE4 and jgsogo February 6, 2022 21:09
@conan-center-bot conan-center-bot merged commit 97d04e6 into conan-io:master Feb 8, 2022
@SpaceIm SpaceIm deleted the libdeflate-relocatable-macos branch February 8, 2022 08:27
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.

6 participants