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

CMake: Fix PThread Flag #318

Merged
merged 2 commits into from
Jun 22, 2021
Merged

Conversation

ax3l
Copy link
Contributor

@ax3l ax3l commented Jun 16, 2021

Addressed Issue

On recent manylinux builds for ppc64le, I realized problems with

undefined reference to `pthread_atfork'

at linktime when propagating a static blosc library to downstream libraries (all via CMake).

Fix: CMake Control Variable

An outdated CMake flag that would request the right pthread flags is used:

Old:
CMAKE_THREAD_PREFER_PTHREAD (docs)

New:
THREADS_PREFER_PTHREAD_FLAG (docs)

Fix: CMake Thread Target

Prefer Threads::Threads for CMake 3.1+: This target does potentially contain CXX flags and linker flags and should thus be preferred.

It should also address the already work-arounded pthread linker issue in one of the tests that @jack-pappas added in #123

To Do

  • finish tests

On recent manylinux builds for ppc64le, I realized problems with
```
undefined reference to `pthread_atfork'
```

at linktime.

The reason for that is likely an outdated CMake flag that would
request the right pthread flags.

Old:
`CMAKE_THREAD_PREFER_PTHREAD` ([docs](https://cmake.org/cmake/help/v3.2/module/FindThreads.html))

New:
`THREADS_PREFER_PTHREAD_FLAG` ([docs](https://cmake.org/cmake/help/latest/module/FindThreads.html))
@ax3l
Copy link
Contributor Author

ax3l commented Jun 16, 2021

We should maybe also use the new Threads::Threads target (CMake 3.1) instead of the result variables?

@ax3l

This comment has been minimized.

This target does potentially contain CXX flags and linker flags
and shoulds thus be preferred.

It should also address the already work-arounded pthread linker
issue in one of the tests.
@FrancescAlted
Copy link
Member

That looks good to me. Thanks for the fix!

@FrancescAlted FrancescAlted merged commit f4ef2f3 into Blosc:master Jun 22, 2021
FrancescAlted added a commit to Blosc/c-blosc2 that referenced this pull request Jun 22, 2021
@ax3l ax3l deleted the fix-cmakePthreadFlag branch December 7, 2021 21:40
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.

3 participants