Обсуждение: configure can't detect proper pthread flags
Hi, when PostgreSQL is cross-compiled in the Buildroot with uClibc toolchain it may not correctly detect compiler/linker flags for threading. [1] The reason is that config/acx_pthread.m4:146 uses compiler and linker stdout and stderr to make decision if acx_pthread_ok should be yes or no: if test "`(eval $ac_link 2>&1 1>&5)`" = "" && test "`(eval $ac_compile 2>&1 1>&5)`" = ""; then and the toolchain emits the following warning at linking step: libcrypto.so: warning: gethostbyname is obsolescent, use getnameinfo() instead. git log doesn't tell much why it is done that way. Does anybody know? Can that test be rewritten as if eval $ac_link 2>&1 1>&5 && eval $ac_compile 2>&1 1>&5 ; then ? [1] http://comments.gmane.org/gmane.comp.lib.uclibc.buildroot/110204 -- Thanks. -- Max
Max Filippov <jcmvbkbc@gmail.com> writes:
> when PostgreSQL is cross-compiled in the Buildroot with uClibc toolchain
> it may not correctly detect compiler/linker flags for threading. [1]
> The reason is that config/acx_pthread.m4:146 uses compiler and linker
> stdout and stderr to make decision if acx_pthread_ok should be yes or no:
> if test "`(eval $ac_link 2>&1 1>&5)`" = "" && test "`(eval
> $ac_compile 2>&1 1>&5)`" = ""; then
> and the toolchain emits the following warning at linking step:
> libcrypto.so: warning: gethostbyname is obsolescent, use
> getnameinfo() instead.
> git log doesn't tell much why it is done that way. Does anybody know?
The short answer is that the linker you're using is written by pedantic
idiots. Notice that the gethostbyname call it's complaining about is
somewhere inside libcrypto; it's *not* in Postgres, much less the test
program being built here. As such, we have no options whatever for
suppressing the complaint, which means that the complaint is being
delivered inappropriately, and it shouldn't be there. The linker is a
silly choice for a place to impose this sort of value judgment anyway;
it has absolutely no way to know whether the use of gethostbyname in
a particular program is unsafe.
regards, tom lane
>> if test "`(eval $ac_link 2>&1 1>&5)`" = "" && test "`(eval>> $ac_compile 2>&1 1>&5)`" = ""; then FWIW, I happened to run into this recently on IRC with someone having compile problems on FreeBSD (10.1); they were using some nonstandard compile flags, and configure's pthread test was breaking as a result (they did not report what the actual warning was). While investigating that, I also noticed that this code prevents any attempt at running configure with -x in effect from working properly, making it a bit hard to debug. -- Andrew (irc:RhodiumToad)
Hi Tom, On Fri, Mar 20, 2015 at 3:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Max Filippov <jcmvbkbc@gmail.com> writes: >> when PostgreSQL is cross-compiled in the Buildroot with uClibc toolchain >> it may not correctly detect compiler/linker flags for threading. [1] >> The reason is that config/acx_pthread.m4:146 uses compiler and linker >> stdout and stderr to make decision if acx_pthread_ok should be yes or no: > >> if test "`(eval $ac_link 2>&1 1>&5)`" = "" && test "`(eval >> $ac_compile 2>&1 1>&5)`" = ""; then > >> and the toolchain emits the following warning at linking step: > >> libcrypto.so: warning: gethostbyname is obsolescent, use >> getnameinfo() instead. > >> git log doesn't tell much why it is done that way. Does anybody know? > > The short answer is that the linker you're using is written by pedantic > idiots. Well... That doesn't answer my question. > Notice that the gethostbyname call it's complaining about is > somewhere inside libcrypto; it's *not* in Postgres, much less the test > program being built here. Actually it *is* in the program being built here, because it's being linked with libcrypto. The full command line produced by the first eval is this: xtensa-linux-gcc -o conftest -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mtext-section-literals -Os -pthread -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE conftest.c -lssl -lcrypto -lz -lreadline -lrt -lcrypt -ldl -lm and if I drop irrelevant libraries from that command its stdout+stderr will probably be empty. But I was curious why this test is written *that* way. -- Thanks. -- Max
On Fri, Mar 20, 2015 at 04:51:55AM +0300, Max Filippov wrote: > xtensa-linux-gcc -o conftest -Wall -Wmissing-prototypes > -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels > -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing > -fwrapv -fexcess-precision=standard -D_LARGEFILE_SOURCE > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls > -mtext-section-literals -Os -pthread -D_LARGEFILE_SOURCE > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE conftest.c > -lssl -lcrypto -lz -lreadline -lrt -lcrypt -ldl -lm > > and if I drop irrelevant libraries from that command its stdout+stderr > will probably be empty. > > But I was curious why this test is written *that* way. Threading compiles are different for every platform so the code just tries everything --- we didn't anticipate that adding a useless library would actually cause a failure. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On Fri, Mar 20, 2015 at 5:09 AM, Bruce Momjian <bruce@momjian.us> wrote: > On Fri, Mar 20, 2015 at 04:51:55AM +0300, Max Filippov wrote: >> xtensa-linux-gcc -o conftest -Wall -Wmissing-prototypes >> -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels >> -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing >> -fwrapv -fexcess-precision=standard -D_LARGEFILE_SOURCE >> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls >> -mtext-section-literals -Os -pthread -D_LARGEFILE_SOURCE >> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE conftest.c >> -lssl -lcrypto -lz -lreadline -lrt -lcrypt -ldl -lm >> >> and if I drop irrelevant libraries from that command its stdout+stderr >> will probably be empty. >> >> But I was curious why this test is written *that* way. > > Threading compiles are different for every platform so the code just > tries everything --- we didn't anticipate that adding a useless library > would actually cause a failure. Sorry, I must be not clear enough: why checking compiler/linker output instead of checking their exit code or presence of produced object/ executable files? -- Thanks. -- Max
>>>>> "Max" == Max Filippov <jcmvbkbc@gmail.com> writes: Max> Sorry, I must be not clear enough: why checking compiler/linkerMax> output instead of checking their exit code or presenceof producedMax> object/ executable files? Going by the comment some lines above, my guess would be "because some compilers accept some option like -pthreads and issue a warning message saying that it is ignored, and pg wants to not treat such options as valid" -- Andrew (irc:RhodiumToad)
On Fri, Mar 20, 2015 at 05:15:51AM +0300, Max Filippov wrote: > On Fri, Mar 20, 2015 at 5:09 AM, Bruce Momjian <bruce@momjian.us> wrote: > > On Fri, Mar 20, 2015 at 04:51:55AM +0300, Max Filippov wrote: > >> xtensa-linux-gcc -o conftest -Wall -Wmissing-prototypes > >> -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels > >> -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing > >> -fwrapv -fexcess-precision=standard -D_LARGEFILE_SOURCE > >> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls > >> -mtext-section-literals -Os -pthread -D_LARGEFILE_SOURCE > >> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE conftest.c > >> -lssl -lcrypto -lz -lreadline -lrt -lcrypt -ldl -lm > >> > >> and if I drop irrelevant libraries from that command its stdout+stderr > >> will probably be empty. > >> > >> But I was curious why this test is written *that* way. > > > > Threading compiles are different for every platform so the code just > > tries everything --- we didn't anticipate that adding a useless library > > would actually cause a failure. > > Sorry, I must be not clear enough: why checking compiler/linker output > instead of checking their exit code or presence of produced object/ > executable files? Oh, uh, I don't rember the answer to that one. I think the code is in config/acx_pthread.m4 and I assume we are just checking using the configure macros that are provided. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Max" == Max Filippov <jcmvbkbc@gmail.com> writes:
> Max> Sorry, I must be not clear enough: why checking compiler/linker
> Max> output instead of checking their exit code or presence of produced
> Max> object/ executable files?
> Going by the comment some lines above, my guess would be "because some
> compilers accept some option like -pthreads and issue a warning message
> saying that it is ignored, and pg wants to not treat such options as
> valid"
Precisely. We don't want every link step producing a useless warning.
Ideally, "make -s" would print nothing whatsoever; to the extent that
tools produce unsuppressable routine chatter, that's evil because it
makes it harder to notice actually-useful warnings.
(My current ambition in this area is to shut up clang from complaining
like so:
clang: warning: argument unused during compilation: '-pthread'
clang: warning: argument unused during compilation: '-pthread'
clang: warning: argument unused during compilation: '-pthread'
clang: warning: argument unused during compilation: '-pthread'
clang: warning: argument unused during compilation: '-pthread'
which is another bit of entirely useless pedantry, but rather hard to work
around because we assume that CFLAGS should be included when linking.)
It's tempting to consider avoiding Max's problem by doing the ACX_PTHREAD
test before picking up any other libraries. But I'm worried that that
would cause more problems than it solves. It's worth noting that the
Autoconf documentation specifically recommends testing for libraries
before testing for compiler characteristics.
regards, tom lane
On Fri, Mar 20, 2015 at 5:20 AM, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: >>>>>> "Max" == Max Filippov <jcmvbkbc@gmail.com> writes: > > Max> Sorry, I must be not clear enough: why checking compiler/linker > Max> output instead of checking their exit code or presence of produced > Max> object/ executable files? > > Going by the comment some lines above, my guess would be "because some > compilers accept some option like -pthreads and issue a warning message > saying that it is ignored, and pg wants to not treat such options as > valid" I've somehow missed that comment, thank you Andrew. -- Max
On Fri, Mar 20, 2015 at 6:08 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > We don't want every link step producing a useless warning. > Ideally, "make -s" would print nothing whatsoever; to the extent that > tools produce unsuppressable routine chatter, that's evil because it > makes it harder to notice actually-useful warnings. Then maybe stderr tests should grep output for a specific option, the one we're currently testing, not just any noise? -- Thanks. -- Max
On Fri, Mar 20, 2015 at 7:01 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: > On Fri, Mar 20, 2015 at 6:08 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> We don't want every link step producing a useless warning. >> Ideally, "make -s" would print nothing whatsoever; to the extent that >> tools produce unsuppressable routine chatter, that's evil because it >> makes it harder to notice actually-useful warnings. > > Then maybe stderr tests should grep output for a specific option, the > one we're currently testing, not just any noise? That sounds awfully fragile to me. It can't really be safe to assume we know precisely what the warning messages will look like. But it seems to me that compiling every test program with every library we might need is not a great plan. (I don't know enough about autoconf to know whether changing that is realistic.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Mar 20, 2015 at 08:05:48AM -0400, Robert Haas wrote: > On Fri, Mar 20, 2015 at 7:01 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: > > On Fri, Mar 20, 2015 at 6:08 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> We don't want every link step producing a useless warning. > >> Ideally, "make -s" would print nothing whatsoever; to the extent that > >> tools produce unsuppressable routine chatter, that's evil because it > >> makes it harder to notice actually-useful warnings. > > > > Then maybe stderr tests should grep output for a specific option, the > > one we're currently testing, not just any noise? > > That sounds awfully fragile to me. It can't really be safe to assume > we know precisely what the warning messages will look like. But it > seems to me that compiling every test program with every library we > might need is not a great plan. > > (I don't know enough about autoconf to know whether changing that is realistic.) It was our only plan, and it has worked fine in the past. Someone is going to have to do a lot of portability research to improve it. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On Fri, Mar 20, 2015 at 3:05 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Mar 20, 2015 at 7:01 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: >> On Fri, Mar 20, 2015 at 6:08 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> We don't want every link step producing a useless warning. >>> Ideally, "make -s" would print nothing whatsoever; to the extent that >>> tools produce unsuppressable routine chatter, that's evil because it >>> makes it harder to notice actually-useful warnings. >> >> Then maybe stderr tests should grep output for a specific option, the >> one we're currently testing, not just any noise? > > That sounds awfully fragile to me. It can't really be safe to assume > we know precisely what the warning messages will look like. Yes, I agree, not very good. Ok, one more attempt: maybe instead of checking that stderr is empty we could check that stderr has changed in the presence of the option that we test? -- Thanks. -- Max
Hi, On 2015-03-20 03:14:48 +0300, Max Filippov wrote: > and the toolchain emits the following warning at linking step: > > libcrypto.so: warning: gethostbyname is obsolescent, use > getnameinfo() instead. FWIW, I think emitting such errors at link time is utterly pointless and rather annoying. I can see a point of emitting them them when compiling code that uses deprecated functions. But we quite obviously can't do much about openssl using gethostbyname. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund wrote: > Hi, > > On 2015-03-20 03:14:48 +0300, Max Filippov wrote: > > and the toolchain emits the following warning at linking step: > > > > libcrypto.so: warning: gethostbyname is obsolescent, use > > getnameinfo() instead. > > FWIW, I think emitting such errors at link time is utterly pointless and > rather annoying. I can see a point of emitting them them when compiling > code that uses deprecated functions. But we quite obviously can't do > much about openssl using gethostbyname. We don't seem have much leverage with the guys producing the linker. If we do, then surely our best bet is to get them to be quiet, or at least provide an --yes-i-know-your-crap-is-noisy-please-shut-it-up option. If we don't have leverage, and we really care enough about that platform to want to work around this problem, it seems that the latest suggestion of comparing the output of the linker with and without the option we're testing (rather than just assuming that the output without the option must surely be empty) is the safest bet ... It seems bad (fragile hack), but let's see a patch and then we can judge. Another option is to say "uclibc is broken beyond belief" and consider it unsupported. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2015-03-20 10:23:51 -0300, Alvaro Herrera wrote: > Andres Freund wrote: > > FWIW, I think emitting such errors at link time is utterly pointless and > > rather annoying. I can see a point of emitting them them when compiling > > code that uses deprecated functions. But we quite obviously can't do > > much about openssl using gethostbyname. > > We don't seem have much leverage with the guys producing the linker. If > we do, then surely our best bet is to get them to be quiet, or at least > provide an --yes-i-know-your-crap-is-noisy-please-shut-it-up option. It's not the linker, it's uclibc that adds the warning. http://git.uclibc.org/uClibc/commit/?id=fdc6f045fa8b71a91a0c55b6390f8d0741e9f374 Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund wrote: > On 2015-03-20 10:23:51 -0300, Alvaro Herrera wrote: > > Andres Freund wrote: > > > FWIW, I think emitting such errors at link time is utterly pointless and > > > rather annoying. I can see a point of emitting them them when compiling > > > code that uses deprecated functions. But we quite obviously can't do > > > much about openssl using gethostbyname. > > > > We don't seem have much leverage with the guys producing the linker. If > > we do, then surely our best bet is to get them to be quiet, or at least > > provide an --yes-i-know-your-crap-is-noisy-please-shut-it-up option. > > It's not the linker, it's uclibc that adds the warning. > > http://git.uclibc.org/uClibc/commit/?id=fdc6f045fa8b71a91a0c55b6390f8d0741e9f374 Wow, that stuff has been there since 2009. So there's no way to shut it up at all, is there. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
By the way, acx-pthread.m4 has an outdated link to upstream acx_pthread.m4. The correct link is http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=history;f=m4/ax_pthread.m4 -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>>>>> "Robert" == Robert Haas <robertmhaas@gmail.com> writes: >> Then maybe stderr tests should grep output for a specific option,>> the one we're currently testing, not just any noise? Robert> That sounds awfully fragile to me. It can't really be safe toRobert> assume we know precisely what the warning messageswill lookRobert> like. But how safe is it to assume that a warning message about option '-foo' will contain the string '-foo' in it somewhere? (though the trace output from -x still should be dealt with separately) -- Andrew (irc:RhodiumToad)
On Fri, Mar 20, 2015 at 2:51 PM, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: >>>>>> "Robert" == Robert Haas <robertmhaas@gmail.com> writes: > >> Then maybe stderr tests should grep output for a specific option, > >> the one we're currently testing, not just any noise? > > Robert> That sounds awfully fragile to me. It can't really be safe to > Robert> assume we know precisely what the warning messages will look > Robert> like. > > But how safe is it to assume that a warning message about option '-foo' > will contain the string '-foo' in it somewhere? > > (though the trace output from -x still should be dealt with separately) I wouldn't like to bet on it. I mean, if the world weren't full of weird platforms that behave in crazy, messed-up ways, autoconf output wouldn't look like like noise. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: > Ok, one more attempt: maybe instead of checking that stderr is empty > we could check that stderr has changed in the presence of the option > that we test? The patch: http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvbkbc@gmail.com -- Thanks. -- Max
On Sat, Mar 21, 2015 at 2:06 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: > On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: >> Ok, one more attempt: maybe instead of checking that stderr is empty >> we could check that stderr has changed in the presence of the option >> that we test? > > The patch: > http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvbkbc@gmail.com Ping? Are there any issues with that approach and the patch? -- Thanks. -- Max
On Wed, Apr 8, 2015 at 2:31 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: > On Sat, Mar 21, 2015 at 2:06 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: >> On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: >>> Ok, one more attempt: maybe instead of checking that stderr is empty >>> we could check that stderr has changed in the presence of the option >>> that we test? >> >> The patch: >> http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvbkbc@gmail.com > > Ping? > Are there any issues with that approach and the patch? I think the thing to do is add your patch here so that it doesn't get forgotten about: https://commitfest.postgresql.org/action/commitfest_view/open -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Apr 30, 2015 at 3:51 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Wed, Apr 8, 2015 at 2:31 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: >> On Sat, Mar 21, 2015 at 2:06 AM, Max Filippov <jcmvbkbc@gmail.com> wrote: >>> On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: >>>> Ok, one more attempt: maybe instead of checking that stderr is empty >>>> we could check that stderr has changed in the presence of the option >>>> that we test? >>> >>> The patch: >>> http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvbkbc@gmail.com >> >> Ping? >> Are there any issues with that approach and the patch? > > I think the thing to do is add your patch here so that it doesn't get > forgotten about: > > https://commitfest.postgresql.org/action/commitfest_view/open Done: https://commitfest.postgresql.org/5/232/ -- Thanks. -- Max
On 03/21/2015 01:06 AM, Max Filippov wrote: > On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: >> Ok, one more attempt: maybe instead of checking that stderr is empty >> we could check that stderr has changed in the presence of the option >> that we test? > > The patch: > http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvbkbc@gmail.com Let's step back a bit. If I understand correctly, we have that "does -foo compiler option produce a warning?" check because we're trying all the different pthread-related flags, and use everything that works. We don't want to use flags that add warnings, however, so we need that check. But why do we try to use all the flags, and not just the first one that works? The original script stopped at the first one that works, but we changed that in this commit: commit e48322a6d6cfce1ec52ab303441df329ddbc04d1 Author: Bruce Momjian <bruce@momjian.us> Date: Thu Aug 12 16:39:50 2004 +0000 Be more aggressive about adding flags to thread compiles. The configure test only tests for building a binary, not building a shared library. On Linux, you can build a binary with -pthread, but you can't build a binary that uses a threaded shared library unless you also use -pthread when building the binary, or adding -lpthread to the shared library build. This patch has the effect of doing the later by adding both -pthread and -lpthread when building libpq. The discussion that lead to that commit is here: http://www.postgresql.org/message-id/flat/200408101453.36209.xzilla@users.sourceforge.net#200408101453.36209.xzilla@users.sourceforge.net. I tried reverting that commit, and lo-and-behold everything still works. Turns out that using -lpthread is not in fact necessary when building libpq on Linux. It seems that it was in fact a GCC bug all along, that it didn't link the shared library with libpthread, when you passed just the -pthread flag; see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=8888. I suggest that we revert that work-around for that GCC bug, and stop testing the pthread flags as soon as we find one that works. Then we can also remove the test for whether the compiler produces any warnings. AFAICS, after that our ax_thread.m4 file is not materially different from the upstream autoconf-archive version. Let's just replace our ax_pthread.m4 file with the latest upstream version. Of course, that breaks this again for anyone compiling on Linux with GCC version 3.2 or older. I don't have much sympathy for that old systems, and there is a work-around available for them anyway: specify PTHREAD_CFLAGS="-pthread -lpthread" on the configure command line. Then the configure script will just verify that that works, and not run the auto-detection code. You can also use that work-around to build older branches with uClibc, which produces the annoying gethostbyname() warnings that started this thread. To err on the side of caution, I'm thinking this should be committed to master and REL9_5_STABLE only. Thoughts? - Heikki
Вложения
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> I suggest that we revert that work-around for that GCC bug, and stop
> testing the pthread flags as soon as we find one that works.
OK ...
> Then we can
> also remove the test for whether the compiler produces any warnings.
Don't see how that follows?
> AFAICS, after that our ax_thread.m4 file is not materially different
> from the upstream autoconf-archive version. Let's just replace our
> ax_pthread.m4 file with the latest upstream version.
It definitely *would* be nice to get back in sync with the upstream
version of that file. But I'm unconvinced about the warnings angle.
regards, tom lane
On 07/08/2015 04:38 PM, Tom Lane wrote: > Heikki Linnakangas <hlinnaka@iki.fi> writes: >> I suggest that we revert that work-around for that GCC bug, and stop >> testing the pthread flags as soon as we find one that works. > > OK ... > >> Then we can >> also remove the test for whether the compiler produces any warnings. > > Don't see how that follows? That test was added after the GCC-bug workaround, because that workaround caused warnings. The upstream philosophy is to have a list of flags, and try them in order until you find one that works. With the workaround that we added, after it finds one flag that makes the pthread test program to compile, it adds every flag in the list to the command line as long as they donn't make the test program to fail again. For example, after it found out that "-pthread" makes the compilation to work, it appended "-pthreads -mthreads -mt -lpthread --thread-safe" to PTHREAD_CFLAGS, as long as none of those caused a compiler error. They could cause warnings though. That's why we had to add the test to check for warnings. The only scenario where you might now get warnings if we switch to upstream version, and didn't before, is if one of the flags makes pthreads to work, but also creates compiler warnings, while another flag later in the list would make it work without warnings. That's not totally inconceivable, but I doubt it happens. In any case, there's nothing PostgreSQL-specific about that, so if that happens, I'd like to find out so that we can complain to the upstream. I'll commit the upstream version, and we'll see what the buildfarm thinks. - Heikki
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> The only scenario where you might now get warnings if we switch to
> upstream version, and didn't before, is if one of the flags makes
> pthreads to work, but also creates compiler warnings, while another flag
> later in the list would make it work without warnings. That's not
> totally inconceivable, but I doubt it happens. In any case, there's
> nothing PostgreSQL-specific about that, so if that happens, I'd like to
> find out so that we can complain to the upstream.
Actually, it looks like the modern version of ax_pthread.m4 adds -Werror
while testing, so that this should not be an issue anyway (at least on
compilers that accept -Werror).
regards, tom lane
On 07/08/2015 08:50 PM, Tom Lane wrote: > Heikki Linnakangas <hlinnaka@iki.fi> writes: >> The only scenario where you might now get warnings if we switch to >> upstream version, and didn't before, is if one of the flags makes >> pthreads to work, but also creates compiler warnings, while another flag >> later in the list would make it work without warnings. That's not >> totally inconceivable, but I doubt it happens. In any case, there's >> nothing PostgreSQL-specific about that, so if that happens, I'd like to >> find out so that we can complain to the upstream. > > Actually, it looks like the modern version of ax_pthread.m4 adds -Werror > while testing, so that this should not be an issue anyway (at least on > compilers that accept -Werror). To conclude this thread: I replaced our custom pthread-checking autoconf macro with the latest upstream version, in PostgreSQL master. That should fix your original problem with the uClibc, OpenSSL, and pthreads combination, in PostgreSQL master (i.e. the future 9.6 version). As a workaround for current versions, you can give PTHREAD_CFLAGS=-pthread (or whatever the right flag for that platform is) explicitly on the configure command line. That way the autoconf script will only test if that option works, skipping the autodetection logic and the warnings-test, so it will pass. - Heikki