Обсуждение: configure.in and setproctitle/optreset problem

We are regularly teaching PostgreSQL courses at linuxhotel.de.
Starting from the course in November, PG doesn't compile anymore on
their default Debian Squeeze install laptops they hand out to the
participants. After ./configure && make, the error looks like this:

postmaster/postmaster.o: In function `PostmasterMain':
postmaster.c:(.text+0x40e2): undefined reference to `optreset'
tcop/postgres.o: In function `process_postgres_switches':
postgres.c:(.text+0x168c): undefined reference to `optreset'
utils/misc/ps_status.o: In function `set_ps_display':
ps_status.c:(.text+0xba): undefined reference to `setproctitle'
collect2: error: ld returned 1 exit status
make[2]: *** [postgres] Fehler 1
make[2]: Leaving directory `/home/cbe/projects/postgresql/postgresql/src/backend'

This is 9.2.2, but I've seen 9.1.6 failing in the same way. The
package set installed is a desktop install, plus what Debian
considers to be build dependencies of postgresql-9.2, that is,
libedit-dev is installed, but libreadline-dev isn't.

$ egrep 'edit|readline|setproc|optreset' configure.out
checking for library containing setproctitle... no
checking for library containing readline... -ledit
checking editline/readline.h usability... yes
checking editline/readline.h presence... yes
checking for editline/readline.h... yes
checking editline/history.h usability... yes
checking editline/history.h presence... yes
checking for editline/history.h... yes
checking for setproctitle... yes
checking for optreset... yes

I have no clue why no one else has seen this bug before, but the
reason for the error seems to be that configure is invoking the
setproctitle test including -ledit. libedit.so is linked to libbsd.so,
which in turn contains setproctitle(), so HAVE_SETPROCTITLE is set
even this is Linux, not BSD.

configure.in already contains a workaround for -ledit containing a
strlcpy() version, so I'd propose the attached patch to extend that
workaround to setproctitle. (Patch for 9.2's configure.in, but the
same moving of the two code blocks should work for head as well.)

optreset seems to be a similar problem, but I'm not yet sure the patch
really fixes it. (I've kind of got lost in too many configure runs...)

An alternative fix would be to move the __linux__ test in front of the
HAVE_SETPROCTITLE test in src/backend/utils/misc/ps_status.c.

Why is -ledit (or -lreadline) being passed to the function tests
anyway? Removing that would fix this as well, I guess.

(I can provide more verbose configure output on request.)

Christoph
--
cb@df7cb.de | http://www.df7cb.de/

Christoph Berg <christoph.berg@credativ.de> writes:
> We are regularly teaching PostgreSQL courses at linuxhotel.de.
> Starting from the course in November, PG doesn't compile anymore on
> their default Debian Squeeze install laptops they hand out to the
> participants. After ./configure && make, the error looks like this:

> postmaster/postmaster.o: In function `PostmasterMain':
> postmaster.c:(.text+0x40e2): undefined reference to `optreset'
> tcop/postgres.o: In function `process_postgres_switches':
> postgres.c:(.text+0x168c): undefined reference to `optreset'
> utils/misc/ps_status.o: In function `set_ps_display':
> ps_status.c:(.text+0xba): undefined reference to `setproctitle'
> collect2: error: ld returned 1 exit status
> make[2]: *** [postgres] Fehler 1
> make[2]: Leaving directory `/home/cbe/projects/postgresql/postgresql/src/backend'

> This is 9.2.2, but I've seen 9.1.6 failing in the same way. The
> package set installed is a desktop install, plus what Debian
> considers to be build dependencies of postgresql-9.2, that is,
> libedit-dev is installed, but libreadline-dev isn't.

I installed Debian for the first time in a long time, and verified that
their version of libedit exposes bogus definitions of setproctitle,
optreset, and god knows what else.

> configure.in already contains a workaround for -ledit containing a
> strlcpy() version, so I'd propose the attached patch to extend that
> workaround to setproctitle. (Patch for 9.2's configure.in, but the
> same moving of the two code blocks should work for head as well.)

I think we should assume that the libedit developers are utterly
clueless about not trampling on application namespace, and just cut
that library out of *all* our link checks except for the symbols we
specifically expect to get from libedit/libreadline.  Hence, I propose
the attached slightly more extensive patch.  I've verified that this
produces a working build with Debian's libedit.

            regards, tom lane

diff --git a/configure.in b/configure.in
index 2dee4b392b18511f18c4e2f49cab482f2ec72dc8..d6968bb9c0a82ab1c509443905fc5f5c1f13deab 100644
*** a/configure.in
--- b/configure.in
*************** PGAC_VAR_INT_TIMEZONE
*** 1202,1207 ****
--- 1202,1214 ----
  AC_FUNC_ACCEPT_ARGTYPES
  PGAC_FUNC_GETTIMEOFDAY_1ARG

+ # Some versions of libedit contain strlcpy(), setproctitle(), and other
+ # symbols that that library has no business exposing to the world.  Pending
+ # acquisition of a clue by those developers, ignore libedit (including its
+ # possible alias of libreadline) while checking for everything else.
+ pgac_save_LIBS="$LIBS"
+ LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
+
  AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat readlink
setproctitlesetsid sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l]) 

  AC_REPLACE_FUNCS(fseeko)
*************** else
*** 1316,1326 ****
    AC_CHECK_FUNCS([fpclass fp_class fp_class_d class], [break])
  fi

- # Some versions of libedit contain strlcpy(); so disregard that library while
- # checking for these standard libc functions.
- pgac_save_LIBS="$LIBS"
- LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-
  AC_REPLACE_FUNCS([crypt fls getopt getrusage inet_aton random rint srandom strerror strlcat strlcpy])

  case $host_os in
--- 1323,1328 ----
*************** case $host_os in
*** 1338,1346 ****
          ;;
  esac

-
- LIBS="$pgac_save_LIBS"
-
  # System's version of getaddrinfo(), if any, may be used only if we found
  # a definition for struct addrinfo; see notes in src/include/getaddrinfo.h.
  # (Note: the AC_REPLACE_FUNCS probe fails on Windows, where the available
--- 1340,1345 ----
*************** else
*** 1394,1406 ****
    AC_SUBST(have_win32_dbghelp,no)
  fi

- if test "$with_readline" = yes; then
-   PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
-   AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function])
-   AC_CHECK_FUNCS([append_history history_truncate_file])
- fi
-
-
  dnl Cannot use AC_CHECK_FUNC because sigsetjmp may be a macro
  dnl (especially on GNU libc)
  dnl See also comments in c.h.
--- 1393,1398 ----
*************** if test x"$pgac_cv_gcc_int_atomics" = x"
*** 1451,1456 ****
--- 1443,1457 ----
    AC_DEFINE(HAVE_GCC_INT_ATOMICS, 1, [Define to 1 if you have __sync_lock_test_and_set(int *) and friends.])
  fi

+ # Lastly, restore full LIBS list and check for readline/libedit symbols
+ LIBS="$pgac_save_LIBS"
+
+ if test "$with_readline" = yes; then
+   PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
+   AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function])
+   AC_CHECK_FUNCS([append_history history_truncate_file])
+ fi
+

  #
  # Pthreads

On 12/17/2012 09:44 PM, Tom Lane wrote:
>
> I think we should assume that the libedit developers are utterly
> clueless about not trampling on application namespace, and just cut
> that library out of *all* our link checks except for the symbols we
> specifically expect to get from libedit/libreadline.  Hence, I propose
> the attached slightly more extensive patch.  I've verified that this
> produces a working build with Debian's libedit.
>
>

+1. This being a family channel I'll refrain from expressing my opinion 
too frankly of libedit.

cheers

andrew

On Mon, 2012-12-17 at 18:02 +0100, Christoph Berg wrote:
> I have no clue why no one else has seen this bug before, but the
> reason for the error seems to be that configure is invoking the
> setproctitle test including -ledit. libedit.so is linked to libbsd.so,
> which in turn contains setproctitle(), so HAVE_SETPROCTITLE is set
> even this is Linux, not BSD. 

I reported this here:
http://archives.postgresql.org/pgsql-bugs/2012-07/msg00127.php

The correct fix, IMO/IIRC, is to add LDFLAGS=-Wl,--as-needed before
running most of the configure checks, instead of after.

Re: Tom Lane 2012-12-18 <26465.1355798654@sss.pgh.pa.us>
> I think we should assume that the libedit developers are utterly
> clueless about not trampling on application namespace, and just cut
> that library out of *all* our link checks except for the symbols we
> specifically expect to get from libedit/libreadline.  Hence, I propose
> the attached slightly more extensive patch.  I've verified that this
> produces a working build with Debian's libedit.

I confirm that your patch fixes the problem on master and
REL9_2_STABLE (manually applied, one hunk failed). Thanks!

Christoph
--
Tel.: +49 (0)21 61 / 46 43-187
credativ GmbH, HRB Mönchengladbach 12080
Hohenzollernstr. 133, 41061 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz

Peter Eisentraut <peter_e@gmx.net> writes:
> On Mon, 2012-12-17 at 18:02 +0100, Christoph Berg wrote:
>> I have no clue why no one else has seen this bug before, but the
>> reason for the error seems to be that configure is invoking the
>> setproctitle test including -ledit. libedit.so is linked to libbsd.so,
>> which in turn contains setproctitle(), so HAVE_SETPROCTITLE is set
>> even this is Linux, not BSD. 

> I reported this here:
> http://archives.postgresql.org/pgsql-bugs/2012-07/msg00127.php

I thought the issue sounded familiar ...

> The correct fix, IMO/IIRC, is to add LDFLAGS=-Wl,--as-needed before
> running most of the configure checks, instead of after.

Meh.  It's not clear to me at all that that fixes the issue here,
or at least that it does so in any way that's reliable.  The proposal
to add --as-needed during configure was made to fix a different problem,
namely making the wrong decision about whether libintl needs to be
pulled in explicitly.  We don't seem to have done anything about that
--- personally I was afraid that it could break as many cases as it
fixed.  Reviewing that thread, I still think that swapping the order
of the probes for libintl and libgssapi is the safest fix for the
previous issue.
        regards, tom lane

Re: Tom Lane 2012-12-18 <8705.1355845633@sss.pgh.pa.us>
> > The correct fix, IMO/IIRC, is to add LDFLAGS=-Wl,--as-needed before
> > running most of the configure checks, instead of after.
>
> Meh.  It's not clear to me at all that that fixes the issue here,
> or at least that it does so in any way that's reliable.  The proposal
> to add --as-needed during configure was made to fix a different problem,
> namely making the wrong decision about whether libintl needs to be
> pulled in explicitly.  We don't seem to have done anything about that

To me, twiddling with --as-needed sounds like trading one set of
possible problems for a different one, configure checks should be as
deterministic as possible. It might still be that Peter's --as-needed
suggestion is a good fix, but I believe the issue I reported should
also be fixed by the patch Tom sent.

Reiterating a point from my original message, why is -l{readline,edit}
included in these configure checks at all? Most (if not all) of the
function checks in that block are not related to input editing anyway.

Mit freundlichen Grüßen,
Christoph Berg
--
Tel.: +49 (0)21 61 / 46 43-187
credativ GmbH, HRB Mönchengladbach 12080
Hohenzollernstr. 133, 41061 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz