Re: Non-configure build of thread_test has been broken for awhile - Mailing list pgsql-hackers

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2020-Oct-18, Tom Lane wrote:
>> It doesn't really seem sane to me to support two different build
>> environments for thread_test, especially when one of them is so
>> little-used that it can be broken for years before we notice.
>> So I'd be inclined to rip out the Makefile and just consider
>> that thread_test.c is *only* meant to be used by configure.
>> If we wish to resurrect the standalone build method, we could
>> probably do so by adding LIBS to the Makefile's link command
>> ... but what's the point, and what will keep it from getting
>> broken again later?

> Standalone usage of that program is evidently non-existant, so +1 for
> removing the Makefile and just keep the configure compile path for it.

I concluded that if thread_test.c will only be used by configure,
then we should stick it under $(SRCDIR)/config/ and nuke the
src/test/thread/ subdirectory altogether.  See attached.

> BTW the only animal reporting without thread-safety in the buildfarm is
> gaur.

Yeah.  At some point maybe we should just drop support for non-thread-safe
platforms, but I'm not proposing to do that yet.

            regards, tom lane

diff --git a/src/test/thread/thread_test.c b/config/thread_test.c
similarity index 93%
rename from src/test/thread/thread_test.c
rename to config/thread_test.c
index 09603c95dd..ff2eace87d 100644
--- a/src/test/thread/thread_test.c
+++ b/config/thread_test.c
@@ -1,12 +1,12 @@
 /*-------------------------------------------------------------------------
  *
  * thread_test.c
- *        libc thread test program
+ *        libc threading test program
  *
  * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *    src/test/thread/thread_test.c
+ *    config/thread_test.c
  *
  *    This program tests to see if your standard libc functions use
  *    pthread_setspecific()/pthread_getspecific() to be thread-safe.
@@ -20,12 +20,7 @@
  *-------------------------------------------------------------------------
  */

-#if !defined(IN_CONFIGURE) && !defined(WIN32)
-#include "postgres.h"
-
-/* we want to know what the native strerror does, not pg_strerror */
-#undef strerror
-#endif
+/* We cannot use c.h, as port.h will not exist yet */

 #include <stdio.h>
 #include <stdlib.h>
@@ -36,6 +31,7 @@
 #include <string.h>
 #include <fcntl.h>
 #include <errno.h>
+#include <pthread.h>

 /* CYGWIN requires this for MAXHOSTNAMELEN */
 #ifdef __CYGWIN__
@@ -47,25 +43,11 @@
 #include <winsock2.h>
 #endif

-
 /* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */
 #include <signal.h>
 int            sigwait(const sigset_t *set, int *sig);


-#if !defined(ENABLE_THREAD_SAFETY) && !defined(IN_CONFIGURE) && !defined(WIN32)
-int
-main(int argc, char *argv[])
-{
-    fprintf(stderr, "This PostgreSQL build does not support threads.\n");
-    fprintf(stderr, "Perhaps rerun 'configure' using '--enable-thread-safety'.\n");
-    return 1;
-}
-#else
-
-/* This must be down here because this is the code that uses threads. */
-#include <pthread.h>
-
 #define        TEMP_FILENAME_1 "thread_test.1"
 #define        TEMP_FILENAME_2 "thread_test.2"

@@ -119,11 +101,9 @@ main(int argc, char *argv[])
         return 1;
     }

-#ifdef IN_CONFIGURE
     /* Send stdout to 'config.log' */
     close(1);
     dup(5);
-#endif

 #ifdef WIN32
     err = WSAStartup(MAKEWORD(2, 2), &wsaData);
@@ -455,5 +435,3 @@ func_call_2(void)
     pthread_mutex_lock(&init_mutex);    /* wait for parent to test */
     pthread_mutex_unlock(&init_mutex);
 }
-
-#endif                            /* !ENABLE_THREAD_SAFETY && !IN_CONFIGURE */
diff --git a/configure b/configure
index 071d050ef0..ace4ed5dec 100755
--- a/configure
+++ b/configure
@@ -18992,23 +18992,21 @@ $as_echo_n "checking thread safety of required library functions... " >&6; }

 _CFLAGS="$CFLAGS"
 _LIBS="$LIBS"
-CFLAGS="$CFLAGS $PTHREAD_CFLAGS -DIN_CONFIGURE"
+CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
 LIBS="$LIBS $PTHREAD_LIBS"
 if test "$cross_compiling" = yes; then :
   { $as_echo "$as_me:${as_lineno-$LINENO}: result: maybe" >&5
 $as_echo "maybe" >&6; }
   { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING:
 *** Skipping thread test program because of cross-compile build.
-*** Run the program in src/test/thread on the target machine.
 " >&5
 $as_echo "$as_me: WARNING:
 *** Skipping thread test program because of cross-compile build.
-*** Run the program in src/test/thread on the target machine.
 " >&2;}
 else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
-#include "$srcdir/src/test/thread/thread_test.c"
+#include "$srcdir/config/thread_test.c"
 _ACEOF
 if ac_fn_c_try_run "$LINENO"; then :
   { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
@@ -19017,9 +19015,8 @@ else
   { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
 $as_echo "no" >&6; }
   as_fn_error $? "thread test program failed
-This platform is not thread-safe.  Check the file 'config.log' or compile
-and run src/test/thread/thread_test for the exact reason.
-Use --disable-thread-safety to disable thread safety." "$LINENO" 5
+This platform is not thread-safe.  Check the file 'config.log' for the
+exact reason, or use --disable-thread-safety to disable thread safety." "$LINENO" 5
 fi
 rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
   conftest.$ac_objext conftest.beam conftest.$ac_ext
diff --git a/configure.ac b/configure.ac
index e9ce611d23..5b91c83fd0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2295,20 +2295,18 @@ AC_MSG_CHECKING([thread safety of required library functions])

 _CFLAGS="$CFLAGS"
 _LIBS="$LIBS"
-CFLAGS="$CFLAGS $PTHREAD_CFLAGS -DIN_CONFIGURE"
+CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
 LIBS="$LIBS $PTHREAD_LIBS"
 AC_RUN_IFELSE(
-  [AC_LANG_SOURCE([[#include "$srcdir/src/test/thread/thread_test.c"]])],
+  [AC_LANG_SOURCE([[#include "$srcdir/config/thread_test.c"]])],
   [AC_MSG_RESULT(yes)],
   [AC_MSG_RESULT(no)
   AC_MSG_ERROR([thread test program failed
-This platform is not thread-safe.  Check the file 'config.log' or compile
-and run src/test/thread/thread_test for the exact reason.
-Use --disable-thread-safety to disable thread safety.])],
+This platform is not thread-safe.  Check the file 'config.log' for the
+exact reason, or use --disable-thread-safety to disable thread safety.])],
   [AC_MSG_RESULT(maybe)
   AC_MSG_WARN([
 *** Skipping thread test program because of cross-compile build.
-*** Run the program in src/test/thread on the target machine.
 ])])
 CFLAGS="$_CFLAGS"
 LIBS="$_LIBS"
diff --git a/src/Makefile b/src/Makefile
index bcdbd9588a..79e274a476 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -66,13 +66,11 @@ clean:
     $(MAKE) -C test $@
     $(MAKE) -C tutorial NO_PGXS=1 $@
     $(MAKE) -C test/isolation $@
-    $(MAKE) -C test/thread $@

 distclean maintainer-clean:
     $(MAKE) -C test $@
     $(MAKE) -C tutorial NO_PGXS=1 $@
     $(MAKE) -C test/isolation $@
-    $(MAKE) -C test/thread $@
     rm -f Makefile.port Makefile.global


diff --git a/src/port/thread.c b/src/port/thread.c
index 171df0f9e3..0941cb6a88 100644
--- a/src/port/thread.c
+++ b/src/port/thread.c
@@ -47,9 +47,6 @@
  *        use non-*_r functions if they are thread-safe
  *
  *    One thread-safe solution for gethostbyname() might be to use getaddrinfo().
- *
- *    Run src/test/thread to test if your operating system has thread-safe
- *    non-*_r functions.
  */


diff --git a/src/test/Makefile b/src/test/Makefile
index efb206aa75..9774f534d9 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -37,7 +37,7 @@ endif
 # clean" etc to recurse into them.  (We must filter out those that we
 # have conditionally included into SUBDIRS above, else there will be
 # make confusion.)
-ALWAYS_SUBDIRS = $(filter-out $(SUBDIRS),examples kerberos ldap locale thread ssl)
+ALWAYS_SUBDIRS = $(filter-out $(SUBDIRS),examples kerberos ldap locale ssl)

 # We want to recurse to all subdirs for all standard targets, except that
 # installcheck and install should not recurse into the subdirectory "modules".
diff --git a/src/test/README b/src/test/README
index b5ccfc0cf6..afdc767651 100644
--- a/src/test/README
+++ b/src/test/README
@@ -9,7 +9,7 @@ Not all these tests get run by "make check". Check src/test/Makefile to see
 which tests get run automatically.

 authentication/
-  Tests for authentication
+  Tests for authentication (but see also below)

 examples/
   Demonstration programs for libpq that double as regression tests via
@@ -18,6 +18,12 @@ examples/
 isolation/
   Tests for concurrent behavior at the SQL level

+kerberos/
+  Tests for Kerberos/GSSAPI authentication and encryption
+
+ldap/
+  Tests for LDAP-based authentication
+
 locale/
   Sanity checks for locale data, encodings, etc

@@ -42,6 +48,3 @@ ssl/

 subscription/
   Tests for logical replication
-
-thread/
-  A thread-safety-testing utility used by configure
diff --git a/src/test/thread/.gitignore b/src/test/thread/.gitignore
deleted file mode 100644
index 1d54d546a8..0000000000
--- a/src/test/thread/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-/thread_test
diff --git a/src/test/thread/Makefile b/src/test/thread/Makefile
deleted file mode 100644
index a13c0c6cf5..0000000000
--- a/src/test/thread/Makefile
+++ /dev/null
@@ -1,24 +0,0 @@
-#-------------------------------------------------------------------------
-#
-# Makefile for tools/thread
-#
-# Copyright (c) 2003-2020, PostgreSQL Global Development Group
-#
-# src/test/thread/Makefile
-#
-#-------------------------------------------------------------------------
-
-subdir = src/tools/thread
-top_builddir = ../../..
-include $(top_builddir)/src/Makefile.global
-
-override CFLAGS += $(PTHREAD_CFLAGS)
-
-all: thread_test
-
-thread_test: thread_test.o
-# no need for $LIBS, might not be compiled yet
-    $(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(PTHREAD_LIBS) -o $@$(X)
-
-clean distclean maintainer-clean:
-    rm -f thread_test$(X) thread_test.o
diff --git a/src/test/thread/README b/src/test/thread/README
deleted file mode 100644
index 4da23440f6..0000000000
--- a/src/test/thread/README
+++ /dev/null
@@ -1,54 +0,0 @@
-src/test/thread/README
-
-Threading
-=========
-
-This program is run by configure to determine if threading is
-properly supported on the platform.
-
-You can run the program manually to see details, which shows if your
-native libc functions are thread-safe, or if we use *_r functions or
-thread locking.
-
-To use this program manually, you must:
-
-    o run "configure"
-    o compile the main source tree
-    o compile and run this program
-
-If your platform requires special thread flags that are not tested by
-/config/acx_pthread.m4, add PTHREAD_CFLAGS and PTHREAD_LIBS defines to
-your template/${port} file.
-
-Windows Systems
-===============
-
-Windows systems do not vary in their thread-safeness in the same way that
-other systems might, nor do they generally have pthreads installed, hence
-on Windows this test is skipped by the configure program (pthreads is
-required by the test program, but not PostgreSQL itself). If you do wish
-to test your system however, you can do so as follows:
-
-1) Install pthreads in you Mingw/Msys environment. You can download pthreads
-   from ftp://sources.redhat.com/pub/pthreads-win32/.
-
-2) Build the test program:
-
-   gcc -o thread_test.exe \
-    -D_REENTRANT \
-    -D_THREAD_SAFE \
-    -D_POSIX_PTHREAD_SEMANTICS \
-    -I../../../src/include/port/win32 \
-    thread_test.c \
-    -lws2_32 \
-    -lpthreadgc2
-
-3) Run thread_test.exe. You should see output like:
-
-    dpage@PC30:/cvs/pgsql/src/tools/thread$ ./thread_test
-    Your GetLastError() is thread-safe.
-    Your system uses strerror() which is thread-safe.
-    getpwuid_r()/getpwuid() are not applicable to Win32 platforms.
-    Your system uses gethostbyname which is thread-safe.
-
-    Your platform is thread-safe.