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

Fixup for finding pthread.h in CMake #2156

Closed
wants to merge 1 commit into from

Conversation

albinahlback
Copy link
Collaborator

@tobiasdiez please check if this works.

@tobiasdiez
Copy link
Contributor

Sadly not:

CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
| Please set them or make sure they are set and tested correctly in the CMake files:
| D:/Programming/sage/subprojects/flint/PThreads_INCLUDE_DIRS
| used as include directory in directory D:/Programming/sage/subprojects/flint
| PThreads_LIBRARIES (ADVANCED)


 CMake Error in CMakeLists.txt:
| Target "flint" INTERFACE_INCLUDE_DIRECTORIES property contains path:

| "D:/Programming/sage/subprojects/flint/PThreads_INCLUDE_DIRS-NOTFOUND"

btw, is pthreads completely optional?

@albinahlback
Copy link
Collaborator Author

Yes, pthreads is optional, but I would like to avoid too many branches in CMake, so I would like to keep it mandatory in CMake. How do you proceed for other projects with your devel-Windows-thing for pthreads?

@tobiasdiez
Copy link
Contributor

I don't know - it's my first time as well and I'm honestly a bit confused by all the different pthreads ;-) So I would welcome if it could be made completely optional also for cmake.

Comment on lines +143 to +144
unset(PThreads_LIBRARIES)
unset(PThreads_INCLUDE_DIRS)
Copy link
Contributor

Choose a reason for hiding this comment

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

What works for me is

find_path(PThreads_INCLUDE_DIRS NAMES pthread.h )
find_library(PThreads_LIBRARIES NAMES libwinpthread.dll pthreads libpthreads pthread)

instead of line 138 to 144 (i.e. without the HAVE_PTHREAD_H check, which I think is made obsolete by the find_path).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please make a code suggestion.

@isuruf
Copy link
Member

isuruf commented Jan 19, 2025

See #2171

@albinahlback
Copy link
Collaborator Author

Closing in favor of #2171.

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