Re: pgsql: Replace our hacked version of ax_pthread.m4 with latest upstream

On 07/09/2015 06:17 AM, Tom Lane wrote:
> While my RHEL6 box seems happy now, this change has completely broken
> pademelon/gaur (which were not broken prior to the rearrangement),
> and I believe it also accounts for frogmouth being unhappy.
>
> The pademelon/gaur failure is because we are now failing to detect
> any of the major standard C headers:
>
> $ diff pg_config.h.broken pg_config.h.saved
> 266c266
> < /* #undef HAVE_INTTYPES_H */
> ---
>> #define HAVE_INTTYPES_H 1
> 351c351
> < /* #undef HAVE_MEMORY_H */
> ---
>> #define HAVE_MEMORY_H 1
> 466c466
> < /* #undef HAVE_STDLIB_H */
> ---
>> #define HAVE_STDLIB_H 1
> 475c475
> < /* #undef HAVE_STRINGS_H */
> ---
>> #define HAVE_STRINGS_H 1
> 478c478
> < /* #undef HAVE_STRING_H */
> ---
>> #define HAVE_STRING_H 1
> 568c568
> < /* #undef HAVE_SYS_STAT_H */
> ---
>> #define HAVE_SYS_STAT_H 1
> 577c577
> < /* #undef HAVE_SYS_TYPES_H */
> ---
>> #define HAVE_SYS_TYPES_H 1
> 614c614
> < /* #undef HAVE_UNISTD_H */
> ---
>> #define HAVE_UNISTD_H 1
> 780c780
> < /* #undef STDC_HEADERS */
> ---
>> #define STDC_HEADERS 1
>
> Now, this is odd, because those critters build with
> --disable-thread-safety and thus shouldn't be executing AX_PTHREAD
> at all.  However, I can reproduce something similar to frogmouth's
> symptoms on RHEL6 by configuring HEAD with --disable-thread-safety:
> configure fails with "zlib version is too old".

A-ha, I was also able to reproduce that on my x86 debian system.

> And investigation makes it look like something is completely broken
> in the major system headers, not zlib proper, which is what frogmouth
> is showing.

Yeah, googling for "mingw stdint.h error: two or more data types in
declaration specifiers" shows up discussions on similar issues. Not sure
if there's a fix in later versions.

> I suspect the explanation is that physically moving the AX_PTHREAD call
> before our regular header checks causes autoconf to move some of the
> header checks to be before the AX_PTHREAD expansion, *without regard to
> the fact that AX_PTHREAD might not get executed*.

Yep. The AC_CHECK_HEADER(pthread.h, ...) check right after AX_PTHREAD is
now the very first AC_CHECK_HEADER check in the script. Apparently the
first call to one of those triggers those additional standard-header
tests. Autoconf puts them inside the if-block too.

Found this thread that came to the same conclusion:
http://permalink.gmane.org/gmane.comp.sysutils.autoconf.general/15324.
It suggests using AS_IF, which knows about that problem, instead of
plain if. I guess we could do that, but I'm not too excited about
replacing every if-call in the script with AS_IF. We could do it just
for the pthread-check, I guess.

>>> This kind of points up that I've always thought the advice at the top of
>>> configure.in is a tad broken:
>>>
>>> dnl 0. Initialization and options processing
>>> dnl 1. Programs
>>> dnl 2. Libraries
>>> dnl 3. Header files
>>> dnl 4. Types
>>> dnl 5. Structures
>>> dnl 6. Compiler characteristics
>>> dnl 7. Functions, global variables
>>> dnl 8. System services
>>>
>>> Why isn't "Compiler characteristics" a lot earlier in the list, certainly
>>> before "Libraries"?
>
>> Hmm, yeah, although it's not clear what category the pthread check falls
>> into.
>
> At this point it's crystal clear that that advice is dead wrong, and that
> what we need to be doing is something along the lines of
>
> Initialization and options processing
> Compiler characteristics
> Header files
> Types
> Structures
> Functions, global variables
> Pthreads
> Other libraries
> Programs
> System services
>
> Before we start such major whacking, though, I am curious to know where
> Peter got the current ordering advice from.

 From the autoconf manual, see
https://www.gnu.org/software/autoconf/manual/autoconf.html#Autoconf-Input-Layout.
I'm not eager to change that, even though I agree it feels wrong.

We could move the pthread-related AC_CHECK_HEADER and AC_CHECK_FUNCS
tests away from the if-block around AX_PTHREAD, and sprinkle them into
the correct sections, "Header files" and "Functions, global variables"
sections. I'm not sure if that would be an improvement, though, it's
nice to have all the pthread-related stuff in one place.

I'll change the pthread-block to use AS_IF, I think that's the
least-ugly option.

> Also, I'm wondering why we are doing this at all in the 9.5 branch,
> because this is sure looking like development not stabilization.

Yeah, I didn't think it would be when I started :-(. I'll revert all
this in 9.5.

Once the dust settles, I plan to go through the configure logs from the
buildfarm to verify that all platforms choose the same pthread-related
flags as before, except for the spurious additional flags that really
are not needed.

- Heikki