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

On 07/26/2015 10:49 AM, Heikki Linnakangas wrote:
> On 07/26/2015 12:19 AM, Tom Lane wrote:
>> I've located another problem in the new improved ax_pthread.m4 script:
>> prairiedog is now concluding that it should use -pthread, which leads
>> to a whole bunch of build warnings along the lines of
>>
>> powerpc-apple-darwin8-gcc-4.0.1: unrecognized option '-pthread'
>>
>> Previously it concluded that it didn't need any special switch.
>>
>> ...
>
> Googling around, I found a ticket in the autoconf bug tracker for this:
> https://savannah.gnu.org/patch/?8186. The discussion died out in 2013,
> but it seems that the latest idea there was to change the order the
> flags are tested on Darwin, so that "none" is tested before "-pthread".
> That should fix the issue, and hopefully not create other problems on
> other Darwin variants. So I suggest that we do:
>
> @@ -160,7 +165,7 @@ case ${host_os} in
>            ;;
>
>            darwin*)
> -        ax_pthread_flags="-pthread $ax_pthread_flags"
> +        ax_pthread_flags="none -pthread $ax_pthread_flags"
>            ;;
>    esac
>
> It's sad that we have to hack this script again, I hoped we could use
> the upstream version as is. I'll comment on the upstream ticket, let's
> see if we can get the upstream version fixed too.

I commented on that autoconf bug tracker ticket, and Daniel Richard G
posted a new draft version there that fixes this problem. I pushed that
to master a few days ago, and the buildfarm results are now in. Seems to
have fixed the problem.


Andrew kindly let me run queries on the buildfarm database, and I was
able to extract the PTHREAD_CFLAGS chosen by the configure script, from
each buildfarm animal, with the three versions of the script that we've
had in the repository:

0 = Old hacked acx_pthread.m4 script, same as in 9.5 and below
1 = latest stable ax_pthread.m4 from upstream
2 = draft ax_pthread.m4, to be pushed upstream

I went through the results, and I believe the script is working the way
we want now. Attached is a trimmed result set, where I have removed all
the animals where there was no change in the PTHREAD_CFLAGS value chosen
between the three versions (configlogs-trimmed). I've also attached the
raw results and the query I used, for the sake of the archives
(configlogs-raw)


On some platforms, however, we are now getting duplicate -D options,
e.g. on dromedary:

> PTHREAD_CFLAGS='-D_REENTRANT  -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS'

That's harmless, but it would be nice to not clutter the cc command
line. It's happening because we are doing this in configure.in:

> # Some platforms use these, so just define them.  They can't hurt if they
> # are not supported.  For example, on Solaris -D_POSIX_PTHREAD_SEMANTICS
> # enables 5-arg getpwuid_r, among other things.
> PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS"

I'm tempted to just remove -D_REENTRAND and -D_THREAD_SAFE, because
ax_pthread.m4 knows about those, and will add them on platforms where
they are needed. Any objections?

- Heikki