Re: [PATCH 3/3 v2] git-http-fetch: not a builtin

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 28, 2009, 13:23 UTC
Message-ID: <alpine.DEB.1.00.0907281520150.8306@pacific.mpi-cbg.de>
In-Reply-To: <alpine.LNX.2.00.0907280207480.2147@iabervon.org>

Hi,

On Tue, 28 Jul 2009, Daniel Barkalow wrote:

Show 10 quoted lines

> From: Linus Torvalds <torvalds@linux-foundation.org>
> 
> This splits up git-http-fetch so that it isn't built-in.
> 
> It also removes the general dependency on curl, because it is no
> longer used by any built-in code.
> 
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---

I think there is something missing that was not needed in Linus' original patch:

Show 13 quoted lines

> diff --git a/Makefile b/Makefile
> index d3dd2ed..3425336 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -979,10 +979,7 @@ else
>  	else
>  		CURL_LIBCURL = -lcurl
>  	endif
> -	BUILTIN_OBJS += builtin-http-fetch.o
> -	PROGRAMS += git-shim-curl$X
> -	EXTLIBS += $(CURL_LIBCURL)
> -	LIB_OBJS += http.o http-walker.o
> +	PROGRAMS += git-shim-curl$X git-http-fetch$X

So now, http is all excluded from libgit.a.  Okay.

Show 7 quoted lines

> @@ -1487,6 +1484,9 @@ git-imap-send$X: imap-send.o $(GITLIBS)
>  
>  http.o http-walker.o http-push.o transport.o: http.h
>  
> +git-http-fetch$X: revision.o http.o http-push.o $(GITLIBS)
> +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> +		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)

And git-http-fetch is linked to libcurl. But why is git-shim-curl not linked to libcurl? I can only guess (as I am running out of time) that git-shim-curl (in addition to having a doubly horrible name) has _no idea_ about curl? So why does it exist in the first place? Should not rather "git http-fetch" be turned _into_ that helper (albeit with a much better name)?

Ciao, Dscho