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

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Replace our hacked version of ax_pthread.m4 with latest upstream
Date: 2015-08-02 13:13:19
Message-ID: 55BE176F.3070901@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

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

Attachment Content-Type Size
configlogs-trimmed text/plain 9.3 KB
configlogs-raw text/plain 23.3 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Terry Scheingeld 2015-08-02 16:41:19 Taint mode in PL/Perl
Previous Message Tom Lane 2015-08-02 00:58:04 pgsql: Fix some planner issues with degenerate outer join clauses.