build: define more typical HAVE_PTHREAD if pthreads library can be used #6479

marisn · 2025-10-10T08:53:09Z

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_PTHREAD that generally should be used instead of HAVE_PTHREAD_H in most of our cases.

Actual change is only in configure.ac, the rest is just a side effect of running autoreconf

echoix · 2025-10-10T10:42:38Z

include/grass/config.h.in

-
-/*
- * configuration information solely dependent on the above
- * nothing below this point should need changing
- */
-
-#if defined(HAVE_VALUES_H) && !defined(HAVE_LIMITS_H)
-#define INT_MIN -MAXINT
-#endif
-
-
-
-/*
- * Defines needed to get large file support - from cdrtools-2.01
- */
-
-/* MINGW32 LFS */
-
-/* define if we have LFS */
-#undef	HAVE_LARGEFILES
-
-#ifdef	HAVE_LARGEFILES		/* If we have working largefiles at all	   */
-				/* This is not defined with glibc-2.1.3	   */
-
-#if 0
-
-/* what to do with these four? configure comments these out */
-
-#undef _LARGEFILE_SOURCE	/* To make ftello() visible (HP-UX 10.20). */
-#undef _LARGE_FILES		/* Large file defined on AIX-style hosts.  */
-#undef _XOPEN_SOURCE		/* To make ftello() visible (glibc 2.1.3). */
-#undef _XOPEN_SOURCE_EXTENDED
-				/* XXX We don't use this because glibc2.1.3*/
-				/* XXX is bad anyway. If we define	   */
-				/* XXX _XOPEN_SOURCE we will loose caddr_t */
-
-#endif
-
-#if defined(__MINGW32__) && (!defined(_FILE_OFFSET_BITS) || (_FILE_OFFSET_BITS != 64))
-
-/* add/remove as needed */
-/* redefine off_t */
-#include <sys/types.h>
-#define off_t off64_t
-/* fseeko and ftello are safe because not defined by MINGW */
-#define HAVE_FSEEKO
-#define fseeko fseeko64
-#define ftello ftello64
-/* redefine lseek */
-#include <unistd.h>
-#define lseek lseek64
-/* redefine stat and fstat */
-/* use _stati64 compatible with MSVCRT < 6.1 */
-#include <sys/stat.h>
-#define stat _stati64
-#define fstat _fstati64
-
-#endif /* MINGW32 LFS */
-
-#endif	/* HAVE_LARGEFILES */
-
-
-/* define if langinfo.h exists */
-#undef HAVE_LANGINFO_H
-
-/* Use framebuffer objects for off-screen OpenGL rendering */
-#define OPENGL_FBO 1
-
-#endif /* _config_h */


Is any of this supposed to be kept? I see some mingw stuff

No idea. But having custom code in files that are generated/re-generated is not the correct way to go. Anyone running autoreconf would then nuke them. Thus – if it is necessary, then this is a wrong place to have it.

My initial periodic update script was running that each time, and got trimmed down to only keep the guess file and another because of this. It would have kept it in sync and prevented the deviation from arriving

Moved to configure.ac thus making autotools friendly (survive autoreconfigure).

echoix · 2025-10-10T10:44:28Z

include/grass/config.h.in

-#ifndef _config_h
-#define _config_h
-
-#define GDEBUG 1


Is this one of our overrides on the file?

Same here. If we have overrides, we should implement them in a robust way.

Moved to configure.ac thus making autotools friendly (survive autoreconfigure).

So now we see the results after autoreconf?

For original history, see config.h.in before this commit.

echoix · 2025-10-10T12:05:29Z

include/grass/config.h.in

+/* MINGW32 LFS */
+
+/* define if we have LFS */
+#undef HAVE_LARGEFILES
+
+#ifdef HAVE_LARGEFILES         /* If we have working largefiles at all    */
+                               /* This is not defined with glibc-2.1.3    */
+
+#if defined(__MINGW32__) && (!defined(_FILE_OFFSET_BITS) || (_FILE_OFFSET_BITS != 64))
+
+/* add/remove as needed */
+/* redefine off_t */
+#include <sys/types.h>
+#define off_t off64_t
+/* fseeko and ftello are safe because not defined by MINGW */
+#define HAVE_FSEEKO
+#define fseeko fseeko64
+#define ftello ftello64
+/* redefine lseek */
+#include <unistd.h>
+#define lseek lseek64
+/* redefine stat and fstat */
+/* use _stati64 compatible with MSVCRT < 6.1 */
+#include <sys/stat.h>
+#define stat _stati64
+#define fstat _fstati64
+
+#endif /* MINGW32 LFS */


Does it matter if it isn't guarded by the #ifndef _config_h anymore? And does the order matter too? (I couldn't determine easily on my phone if the variables/defines used there were already set at the time of use)

echoix · 2025-10-10T12:14:46Z

configure.ac

 AC_CANONICAL_HOST
 AC_PROG_CC

+# Start some config.h customizations


Like comment on other file, is it possible to have it at the end, where the HAVE defines are already defined?

wenzeslaus

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

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.

wenzeslaus · 2025-10-10T13:46:28Z

Correction of my comment: The macOS build fails on the follow-up PR #6480 (I suppose because the OpenGL fixes are only here).

marisn · 2025-10-13T07:08:03Z

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.

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.

build: define more typical HAVE_PTHREAD if pthreads library can be used

841000f

marisn requested review from nilason and wenzeslaus October 10, 2025 08:53

github-actions bot added the libraries label Oct 10, 2025

marisn mentioned this pull request Oct 10, 2025

lib: Make random number generation thread safe #6480

Open

build: add OPENGL_AGL check to ensure compilation on MacOS

ee8a249

echoix reviewed Oct 10, 2025

View reviewed changes

build: restore config.h.in customizations in an autotools friendly way

e98967d

For original history, see config.h.in before this commit.

marisn requested a review from echoix October 10, 2025 11:42

echoix reviewed Oct 10, 2025

View reviewed changes

wenzeslaus reviewed Oct 10, 2025

View reviewed changes

wenzeslaus approved these changes Oct 10, 2025

View reviewed changes

marisn marked this pull request as draft October 13, 2025 06:58

Uh oh!

build: define more typical HAVE_PTHREAD if pthreads library can be used #6479

Are you sure you want to change the base?

build: define more typical HAVE_PTHREAD if pthreads library can be used #6479

Uh oh!

Conversation

marisn commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wenzeslaus left a comment

Choose a reason for hiding this comment

Uh oh!

wenzeslaus left a comment

Choose a reason for hiding this comment

Uh oh!

wenzeslaus commented Oct 10, 2025

Uh oh!

marisn commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants