build: define more typical HAVE_PTHREAD if pthreads library can be used - #6479
Conversation
wenzeslaus
left a comment
wenzeslaus
left a comment
There was a problem hiding this comment.
Maybe that's because I'm viewing this on my phone, but does the PR description capture what is in the changes? I can't match one to the other.
wenzeslaus
left a comment
wenzeslaus
left a comment
There was a problem hiding this comment.
As far as the content goes, this is fine with me, except two three things:
- Description of the minGW and OpenGL changes should be included somewhere.
- Why this does not include CMake change? (Included in #6480)
- OpenGL needs fixes for macOS which now fails in
/Users/runner/work/grass/grass/lib/ogsf
Since I already tested the follow-up PR #6480, I see that this is valuable and generally functional.
|
Correction of my comment: The macOS build fails on the follow-up PR #6480 (I suppose because the OpenGL fixes are only here). |
Yes, because three lines of code revealed a problem in our build system. I'll create a separate PR with build system fixes to be merged first and then reference it here. Thus there will be a chain of PRs to be merged to get #6480 in. CMake experts can comment more on this, but as I understood, modern CMake practice is to not pollute global space with local defines. |
|
As most changes went away with #6509, this PR now is tight and tidy. |
echoix
left a comment
echoix
left a comment
There was a problem hiding this comment.
If changing the configure.ac, shouldn't the configure and config.h.in be regenerated? I was expecting changes in more than a file
You are right. Here you go. I was too focused on getting other changes out of this PR :D |
3 participants
At the moment we usually just check for presence of pthread headers and then assume pthread library is also fine. This PR introduces more common
HAVE_PTHREADthat generally should be used instead ofHAVE_PTHREAD_Hin most of our cases.Actual change is only in
configure.ac, the rest is just a side effect of runningautoreconf