Re: [PATCH 02/13] Use an external program to implement fetching with curl

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Aug 7, 2009, 05:34 UTC
Message-ID: <alpine.LNX.2.00.0908070111410.2147@iabervon.org>
In-Reply-To: <7v63d06wjq.fsf@alter.siamese.dyndns.org>

On Thu, 6 Aug 2009, Junio C Hamano wrote:

Show 22 quoted lines

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Wed, 5 Aug 2009, Johannes Schindelin wrote:
> >
> >> Ooops, I missed this part.  How about making git-remote-https and 
> >> git-remote-ftp hardlinks to git-remote-http?
> >
> > Sure. Is "ln ... || ln -s ... || cp ..." the right way to do this 
> > cross-platform?
> 
> I've queued the first three from this series to 'next' (and the rest to
> 'pu'), as I wanted to give wider testing to the smaller footprint git with
> the libcurl-less-ness they bring in, with Linus's standalone SHA-1.
> 
> Since then two fix-ups (adding git-remote-* to .gitignore and the
> dependency fix to git-http-fetch) were posted to the list, which I also
> rebased in to the series, making the total number of patches merged to
> 'next' from the series 5.
>
> If there are major changes/rewrites/redesign in the remaining part of the
> series that are only in 'pu', please feel free to either send incrementals
> or replacements.

Great. Johannes had a bunch of suggestions for making it less error-prone to extend, which I hope to get to this weekend.

Show 5 quoted lines

> I do not think I've seen any issues raised but unresolved against the part
> from this series already in 'next', other than that this builds three
> copies of git-remote-* programs based on libcurl.  I'll rebase a patch
> like this in after the Makefile fixup ae209bd (http-fetch: Fix Makefile
> dependancies, 2009-08-06) and queue the result to 'next'.

I think there were remaining improvements (e.g., transport-helper.c could write commands with strbufs), but nothing that couldn't just as well be applied afterwards.

> I suspect that the "install" target may need a patch similar to this one,
> though...

Ah, yes, avoiding the duplication in the build directory isn't a big win if the installed location doesn't get links.

Show 38 quoted lines

> -- >8 --
> Subject: Makefile: do not link three copies of git-remote-* programs
> 
> Instead, link only one and make the rest hardlinks/copies, like we do for
> the built-ins. 
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  Makefile |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 2900057..38924f2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1256,6 +1256,7 @@ ifndef V
>  	QUIET_LINK     = @echo '   ' LINK $@;
>  	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
>  	QUIET_GEN      = @echo '   ' GEN $@;
> +	QUIET_LNCP     = @echo '   ' LN/CP $@;
>  	QUIET_SUBDIR0  = +@subdir=
>  	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
>  			 $(MAKE) $(PRINT_DIR) -C $$subdir
> @@ -1494,10 +1495,16 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
>  		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
>  
> -git-remote-http$X git-remote-https$X git-remote-ftp$X: remote-curl.o http.o http-walker.o $(GITLIBS)
> +git-remote-http$X: remote-curl.o http.o http-walker.o $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
>  		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
>  
> +git-remote-https$X git-remote-ftp$X: git-remote-http$X
> +	$(QUIET_LNCP)$(RM) $@ && \
> +	ln git-remote-http$X $@ 2>/dev/null || \
> +	ln -s  git-remote-http$X $@ 2>/dev/null || \
> +	cp git-remote-http$X $@

Maybe "$<" for "git-remote-http$X", since that would make the rule portion generic.