Re: [PATCH] Fix build with LDFLAGS="-Wl,--as-needed"

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 1, 2014, 15:58 UTC
Message-ID: <xmqqsigzqs3m.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1417426409-7328-1-git-send-email-polynomial-c@gentoo.org>

Lars Wendler <polynomial-c@gentoo.org> writes:

> Subject: Re: [PATCH] Fix build with LDFLAGS="-Wl,--as-needed"

Please identify what area you are touching, e.g.

	Subject: contrib/svn-fe: avoid early $(EXTLIBS) on linker invocation

or whatever.

"Fix build" does not tell us why this change is needed; it does not say what breaks, how and most importantly why it breaks. Please have explanation in the commit message body, e.g.

	When attempting to build svn-fe with LDFLAGS=-Wl,--as-needed,
	I noticed that ... breaks in such and such way.  This is
	because you must not have X before Y due to Z.

        Fix this by doing A, B and C, which makes sure X comes after
        Y.

or somesuch.

> Signed-off-by: Lars Wendler <polynomial-c@gentoo.org>
> ---

Show 13 quoted lines

>  contrib/svn-fe/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile
> index e8651aa..b90cf87 100644
> --- a/contrib/svn-fe/Makefile
> +++ b/contrib/svn-fe/Makefile
> @@ -74,7 +74,7 @@ endif
>  endif
>  
>  svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(XDIFF_LIB) $(GIT_LIB)
> -	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(EXTLIBS) -o $@ svn-fe.o $(LIBS)
> +	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ svn-fe.o $(LIBS) $(EXTLIBS)

I do not understand the original, which has

	EXTLIBS =

	GIT_LIB = ../../libgit.a
	VCSSVN_LIB = ../../vcs-svn/lib.a
	LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(EXTLIBS)

i.e. it shouldn't be necessary to explicitly list EXTLIBS, as it already has LIBS at the end, which in turn has EXTLIBS at the end.

Which in turn means I do not understand your updated version, either. If having EXTLIBS before svn-fe.o is bad (and it is---a linker invocation lists *.o files that need to be linked first, and then libraries to find the symbols requested by those *.o files), wouldn't a fix be to just drop $(EXTLIBS)? Why is it necessary to list it twice?