Обсуждение: The argument for reinstating --as-needed

As some of you may remember, a patch for adding --as-needed to the GNU
linker command line was added [1] and subsequently removed [2] under
the mistaken assumption that it was a linker bug [3]. It isn't.

The bug is actually in readline, in that it doesn't declare its
dependancy on termcap/ncurses. Note: this bug was fixed back in 2002 in
Debian (so no Debian systems are affected by this problem) but is still
present in the latest Redhat release and probably many other places.

I propose we add a workaround for readline and add the flag back again.
The benefits are obvious, from 228 to 87 DT_NEEDED records across a
normal PostgreSQL installation, and the server will no longer depend on
readline (always an odd point). The patch is attached: it basically
adds a reference to termcap/ncurses directly so the psql binary links
to it. psql is the only binary that uses readline so this solves the
problem.

Just a quick note as to why it's not a bug in the linker. The
--as-needed flag includes DT_NEEDED records only for libraries
referenced by the objects. The issue is that ld removed termcap, needed
by readline but not by psql directly.

To see why this is not a bug, think about what the -l options are for.
They are for resolving symbols not found in the objects you are
linking. The linker *does not* look for the symbols needed by the
shared libraries given. Static libraries (being sets of objects) yes,
shared libraries no. Shared libraries have their own DT_NEEDED records
to say what they depend on, they don't need to be checked again.

On my Debian system where readline is configured correctly, ncurses is
not mentioned on the link line and the linker doesn't look for it
either. It is used at runtime though.

Other issues:

(a) won't this happen again with some other library? Well, unlikely.
Obviously this can only affect libraries we list on our link line.
Note, a library having this problem would fail autoconf tests also, so
we'd know about it. We already have a hack in the autoconf stuff for
readline already, I just don't think people expected it to apply to
Redhat.

(b) it wastes 4 bytes. Well yes, but I think the benefits outweigh the
costs. I added it to input.c but it could be a seperate file.

(c) an autoconf test to test for this condition. Well, I thought about
it but the cost and effort of maintaining such a test is far higher
than just forcing the reference.

Have a nice day,

[1] http://archives.postgresql.org/pgsql-committers/2005-05/msg00042.php
[2] http://archives.postgresql.org/pgsql-hackers/2005-05/msg00488.php
[3] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=157126
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

At this stage, I am thinking this is best left for 8.2.  It is
impossible for us to test it enough.

---------------------------------------------------------------------------

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> As some of you may remember, a patch for adding --as-needed to the GNU
> linker command line was added [1] and subsequently removed [2] under
> the mistaken assumption that it was a linker bug [3]. It isn't.
> 
> The bug is actually in readline, in that it doesn't declare its
> dependancy on termcap/ncurses. Note: this bug was fixed back in 2002 in
> Debian (so no Debian systems are affected by this problem) but is still
> present in the latest Redhat release and probably many other places.
> 
> I propose we add a workaround for readline and add the flag back again.
> The benefits are obvious, from 228 to 87 DT_NEEDED records across a
> normal PostgreSQL installation, and the server will no longer depend on
> readline (always an odd point). The patch is attached: it basically
> adds a reference to termcap/ncurses directly so the psql binary links
> to it. psql is the only binary that uses readline so this solves the
> problem.
> 
> Just a quick note as to why it's not a bug in the linker. The
> --as-needed flag includes DT_NEEDED records only for libraries
> referenced by the objects. The issue is that ld removed termcap, needed
> by readline but not by psql directly.
> 
> To see why this is not a bug, think about what the -l options are for.
> They are for resolving symbols not found in the objects you are
> linking. The linker *does not* look for the symbols needed by the
> shared libraries given. Static libraries (being sets of objects) yes,
> shared libraries no. Shared libraries have their own DT_NEEDED records
> to say what they depend on, they don't need to be checked again.
> 
> On my Debian system where readline is configured correctly, ncurses is
> not mentioned on the link line and the linker doesn't look for it
> either. It is used at runtime though.
> 
> Other issues:
> 
> (a) won't this happen again with some other library? Well, unlikely.
> Obviously this can only affect libraries we list on our link line.
> Note, a library having this problem would fail autoconf tests also, so
> we'd know about it. We already have a hack in the autoconf stuff for
> readline already, I just don't think people expected it to apply to
> Redhat.
> 
> (b) it wastes 4 bytes. Well yes, but I think the benefits outweigh the
> costs. I added it to input.c but it could be a seperate file.
> 
> (c) an autoconf test to test for this condition. Well, I thought about
> it but the cost and effort of maintaining such a test is far higher
> than just forcing the reference.
> 
> Have a nice day,
> 
> [1] http://archives.postgresql.org/pgsql-committers/2005-05/msg00042.php
> [2] http://archives.postgresql.org/pgsql-hackers/2005-05/msg00488.php
> [3] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=157126
> -- 
> Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> > tool for doing 5% of the work and then sitting around waiting for someone
> > else to do the other 95% so you can sue them.

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 

On Sat, Oct 29, 2005 at 02:47:17PM -0400, Bruce Momjian wrote:
>
> At this stage, I am thinking this is best left for 8.2.  It is
> impossible for us to test it enough.

I agree. I missed the messages when it went in and missed where it came
out. So I looked into it and have found a solution, but like you said,
way too late for 8.1. Still maybe a distributor might pick it up since
reducing dependancies is fairly important to them.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> At this stage, I am thinking this is best left for 8.2.  It is
> impossible for us to test it enough.

Agreed --- the portability risks are way too high for 8.1.
I see no reason not to try it during the 8.2 cycle though.
        regards, tom lane

This has been saved for the 8.2 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> As some of you may remember, a patch for adding --as-needed to the GNU
> linker command line was added [1] and subsequently removed [2] under
> the mistaken assumption that it was a linker bug [3]. It isn't.
> 
> The bug is actually in readline, in that it doesn't declare its
> dependancy on termcap/ncurses. Note: this bug was fixed back in 2002 in
> Debian (so no Debian systems are affected by this problem) but is still
> present in the latest Redhat release and probably many other places.
> 
> I propose we add a workaround for readline and add the flag back again.
> The benefits are obvious, from 228 to 87 DT_NEEDED records across a
> normal PostgreSQL installation, and the server will no longer depend on
> readline (always an odd point). The patch is attached: it basically
> adds a reference to termcap/ncurses directly so the psql binary links
> to it. psql is the only binary that uses readline so this solves the
> problem.
> 
> Just a quick note as to why it's not a bug in the linker. The
> --as-needed flag includes DT_NEEDED records only for libraries
> referenced by the objects. The issue is that ld removed termcap, needed
> by readline but not by psql directly.
> 
> To see why this is not a bug, think about what the -l options are for.
> They are for resolving symbols not found in the objects you are
> linking. The linker *does not* look for the symbols needed by the
> shared libraries given. Static libraries (being sets of objects) yes,
> shared libraries no. Shared libraries have their own DT_NEEDED records
> to say what they depend on, they don't need to be checked again.
> 
> On my Debian system where readline is configured correctly, ncurses is
> not mentioned on the link line and the linker doesn't look for it
> either. It is used at runtime though.
> 
> Other issues:
> 
> (a) won't this happen again with some other library? Well, unlikely.
> Obviously this can only affect libraries we list on our link line.
> Note, a library having this problem would fail autoconf tests also, so
> we'd know about it. We already have a hack in the autoconf stuff for
> readline already, I just don't think people expected it to apply to
> Redhat.
> 
> (b) it wastes 4 bytes. Well yes, but I think the benefits outweigh the
> costs. I added it to input.c but it could be a seperate file.
> 
> (c) an autoconf test to test for this condition. Well, I thought about
> it but the cost and effort of maintaining such a test is far higher
> than just forcing the reference.
> 
> Have a nice day,
> 
> [1] http://archives.postgresql.org/pgsql-committers/2005-05/msg00042.php
> [2] http://archives.postgresql.org/pgsql-hackers/2005-05/msg00488.php
> [3] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=157126
> -- 
> Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> > tool for doing 5% of the work and then sitting around waiting for someone
> > else to do the other 95% so you can sue them.

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 

This patch is too general.  It targets all platforms, not just those
that might be affected, and it tries to fix an operating system bug.

Also, we removed the readling linking into the backend in 8.2, so that
is a non-issue now.

---------------------------------------------------------------------------

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> As some of you may remember, a patch for adding --as-needed to the GNU
> linker command line was added [1] and subsequently removed [2] under
> the mistaken assumption that it was a linker bug [3]. It isn't.
>
> The bug is actually in readline, in that it doesn't declare its
> dependancy on termcap/ncurses. Note: this bug was fixed back in 2002 in
> Debian (so no Debian systems are affected by this problem) but is still
> present in the latest Redhat release and probably many other places.
>
> I propose we add a workaround for readline and add the flag back again.
> The benefits are obvious, from 228 to 87 DT_NEEDED records across a
> normal PostgreSQL installation, and the server will no longer depend on
> readline (always an odd point). The patch is attached: it basically
> adds a reference to termcap/ncurses directly so the psql binary links
> to it. psql is the only binary that uses readline so this solves the
> problem.
>
> Just a quick note as to why it's not a bug in the linker. The
> --as-needed flag includes DT_NEEDED records only for libraries
> referenced by the objects. The issue is that ld removed termcap, needed
> by readline but not by psql directly.
>
> To see why this is not a bug, think about what the -l options are for.
> They are for resolving symbols not found in the objects you are
> linking. The linker *does not* look for the symbols needed by the
> shared libraries given. Static libraries (being sets of objects) yes,
> shared libraries no. Shared libraries have their own DT_NEEDED records
> to say what they depend on, they don't need to be checked again.
>
> On my Debian system where readline is configured correctly, ncurses is
> not mentioned on the link line and the linker doesn't look for it
> either. It is used at runtime though.
>
> Other issues:
>
> (a) won't this happen again with some other library? Well, unlikely.
> Obviously this can only affect libraries we list on our link line.
> Note, a library having this problem would fail autoconf tests also, so
> we'd know about it. We already have a hack in the autoconf stuff for
> readline already, I just don't think people expected it to apply to
> Redhat.
>
> (b) it wastes 4 bytes. Well yes, but I think the benefits outweigh the
> costs. I added it to input.c but it could be a seperate file.
>
> (c) an autoconf test to test for this condition. Well, I thought about
> it but the cost and effort of maintaining such a test is far higher
> than just forcing the reference.
>
> Have a nice day,
>
> [1] http://archives.postgresql.org/pgsql-committers/2005-05/msg00042.php
> [2] http://archives.postgresql.org/pgsql-hackers/2005-05/msg00488.php
> [3] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=157126
> --
> Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> > tool for doing 5% of the work and then sitting around waiting for someone
> > else to do the other 95% so you can sue them.

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: input.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/psql/input.c,v
retrieving revision 1.46
diff -u -r1.46 input.c
--- input.c    15 Oct 2005 02:49:40 -0000    1.46
+++ input.c    29 Oct 2005 16:57:48 -0000
@@ -34,6 +34,14 @@
     hctl_ignoredups = 2,
     hctl_ignoreboth = hctl_ignorespace | hctl_ignoredups
 };
+
+/* Work around a bug in some releases of readline. The shared lib doesn't
+ * doesn't always declare its dependancy on termcap/ncurses/curses. This
+ * creates a reference to termcap so it gets pulled in, but this is never
+ * actually used... */
+
+extern int tputs();
+int (*__pg_never_used)() = tputs;
 #endif

 #ifdef HAVE_ATEXIT