[Pacemaker] Proposing patch for ld --as-needed

Wed Jul 21 15:40:10 EDT 2010

On Wednesday 21 July 2010 03:49:42 Simon Horman wrote:
> On Tue, Jul 20, 2010 at 05:35:01PM +0200, Ultrabug wrote:
> > ----- Original Message -----
> > From: Simon Horman <horms at verge.net.au>
> > To: The Pacemaker cluster resource manager
> > <pacemaker at oss.clusterlabs.org> Sent: Tue, 20 Jul 2010 05:10:32 +0200
> > (CEST)
> > Subject: Re: [Pacemaker] Proposing patch for ld --as-needed
> > 
> > On Sat, Jul 17, 2010 at 01:12:20PM +0200, Ultrabug wrote:
> > >Dear list,
> > >
> > >I would like to ask you about a possible upstream modification regarding
> > >the -- as-needed ld flag for which we Gentoo users need to patch the
> > >pacemaker sources to get it compile.
> > >
> > >I'm attaching the patch which, as you can see, is relatively small and
> > >simple (looks to me at least). The question is whether or not you think
> > >this could be done upstream ?
> > >
> > >Thank you for your interest in this and all you work,
> > 
> > Out of interest, could you explain why this is needed?
> > Is it because gold is being used as the linker?
> 
> [ please don't top-post ]

[ noticed after sending, sorry ]

> 
> Thanks for the link.
> 
> I guess what is happening without --as-needed is that the curses
> library is being dragged in somewhere, somehow - without your proposed
> change CURSESLIBS is used exactly nowhere.
> 

Actually the two chunks of the patch have different purposes.

The first one is needed because the linking order has a meaning on a as-needed 
system and libpengine uses functions that are defined in libpe_status.

Here is an example of failing build : http://paste.pocoo.org/show/239905/
If you try to compile a program that uses libpengine that doesn't need 
libpe_status, e.g. gcc -Wl,--as-needed ptest.c -o ptest -lpe_status -lpengine 
(shortened version of actual linking from pacemaker build.log) linking will 
fail. Linker evaluates that statement sequentially starting from the inner 
most lib:
  1) do I need libpe_status? No, forget about it.
  2) do I need libpengine? Yes, please.
  3) is everything all right? Ups, I don't know what `was_processing_warning' 
is, die...

The second one explicitly adds CURSESLIBS dependency to libpe_status.
If pacemaker detects ncurses, you get HAVE_NCURSES_H and e.g.
status_print (used in  lib/pengine/native.c etc.) becomes wrapper
around "printw" (see configure.ac). You need to provide `printw' or any linking 
with libpe_status will fail.
Fail build example : http://paste.pocoo.org/show/239916/

> So I think that your change is a step in the right direction,
> though for completeness I think that you also need to give the same
> treatment to libpe_rule as common.c seems to make curses calls.
> 
> Could you considering updating your patch to include my proposed
> additions below? And could you please include a description that describes
> what the patch does? Perhaps something like this:
> 

Sure, if you agree with the explanations above, I'll summarize them and add 
them in the patch which I'll resubmit to you for integration.

> 	Explicitly link curses to libpengine
> 
> 	When the ld option --as-needed is used the build fails, complaining
> 	about missing curses symbols. This appears to be because without
> 	the option curses was implicitly linked, somehow. But with the
> 	option the implicit linking disapears, presumably because curses is
> 	trimmed from the libraries linked to another library that is
> 	subsequently linked to pengine, implicitly or otherwise.
> 
> 	This patch resolves the problem by implicitly linking curses.
> 
> 	It is of note that without this patch CURSESLIBS is not used anywhere.
> 
> > >--- pengine/Makefile.am
> > >+++ pengine/Makefile.am
> > >@@ -58,6 +58,7 @@
> > >
> > > # -L$(top_builddir)/lib/pils -lpils -export-dynamic -module
> > > -avoid-version libpengine_la_SOURCES	= pengine.c allocate.c utils.c
> > > constraints.c \
> > > 
> > > 			native.c group.c clone.c master.c graph.c
> > >
> > >+libpengine_la_LIBADD    = $(top_builddir)/lib/pengine/libpe_status.la
> 
>    +libpengine_la_LIBADD    = $(top_builddir)/lib/pengine/libpe_status.la \
>    +			     $(top_builddir)/lib/pengine/libpe_rules.la
> 
> > > pengine_SOURCES	= main.c
> > > pengine_LDADD	= $(COMMONLIBS)	$(top_builddir)/lib/cib/libcib.la
> > >
> > >--- lib/pengine/Makefile.am
> > >+++ lib/pengine/Makefile.am
> > >@@ -34,7 +34,7 @@
> 
>     libpe_rules_la_LDFLAGS  = -version-info 2:0:0
>     libpe_rules_la_SOURCES  = $(rule_files)
>    +libpe_status_la_LIBADD  = @CURSESLIBS@
> 
> > > libpe_status_la_LDFLAGS	= -version-info 2:0:0
> > > libpe_status_la_SOURCES	=  $(rule_files) $(status_files)
> > >
> > >-libpe_status_la_LIBADD	= -llrm
> > >+libpe_status_la_LIBADD	= -llrm @CURSESLIBS@
> > >
> > > clean-generic:
> > > 	rm -f *.log *.debug *~
> > 
> > Hi,
> > 
> > This link explains the ld --as-needed flag better :
> > http://www.gentoo.org/proj/en/qa/asneeded.xml
> > 
> > As far as I know, Gentoo use the 'normal' ld GNU linker and those
> > changes are required because some libs need symbols defined
> > elsewhere.
> > For more information and to show the details behind this, I'm
> > attaching the failing build log I get if I don't patch before using
> > the ld --as-needed flag.
> 
> _______________________________________________
> Pacemaker mailing list: Pacemaker at oss.clusterlabs.org
> http://oss.clusterlabs.org/mailman/listinfo/pacemaker
> 
> Project Home: http://www.clusterlabs.org
> Getting started: http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> Bugs:
> http://developerbugs.linux-foundation.org/enter_bug.cgi?product=Pacemaker