git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 02/13] Use an external program to implement fetching with curl
@ 2009-08-05  5:01 Daniel Barkalow
  2009-08-05 10:07 ` Johannes Schindelin
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Barkalow @ 2009-08-05  5:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johan Herland, Johannes Schindelin

Use the transport native helper mechanism to fetch by http (and ftp, etc).

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile      |    5 ++
 remote-curl.c |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.c   |  136 +-------------------------------------------------------
 3 files changed, 145 insertions(+), 135 deletions(-)
 create mode 100644 remote-curl.c

diff --git a/Makefile b/Makefile
index 504646a..35117fc 100644
--- a/Makefile
+++ b/Makefile
@@ -981,6 +981,7 @@ else
 		CURL_LIBCURL = -lcurl
 	endif
 	BUILTIN_OBJS += builtin-http-fetch.o
+	PROGRAMS += git-remote-http$X git-remote-https$X git-remote-ftp$X git-http-fetch$X
 	EXTLIBS += $(CURL_LIBCURL)
 	LIB_OBJS += http.o http-walker.o
 	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
@@ -1491,6 +1492,10 @@ 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)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
+
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
 builtin-revert.o wt-status.o: wt-status.h
diff --git a/remote-curl.c b/remote-curl.c
new file mode 100644
index 0000000..ad6a163
--- /dev/null
+++ b/remote-curl.c
@@ -0,0 +1,139 @@
+#include "cache.h"
+#include "remote.h"
+#include "strbuf.h"
+#include "walker.h"
+#include "http.h"
+
+static struct ref *get_refs(struct walker *walker, const char *url)
+{
+	struct strbuf buffer = STRBUF_INIT;
+	char *data, *start, *mid;
+	char *ref_name;
+	char *refs_url;
+	int i = 0;
+	int http_ret;
+
+	struct ref *refs = NULL;
+	struct ref *ref = NULL;
+	struct ref *last_ref = NULL;
+
+	refs_url = xmalloc(strlen(url) + 11);
+	sprintf(refs_url, "%s/info/refs", url);
+
+	http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
+	switch (http_ret) {
+	case HTTP_OK:
+		break;
+	case HTTP_MISSING_TARGET:
+		die("%s not found: did you run git update-server-info on the"
+		    " server?", refs_url);
+	default:
+		http_error(refs_url, http_ret);
+		die("HTTP request failed");
+	}
+
+	data = buffer.buf;
+	start = NULL;
+	mid = data;
+	while (i < buffer.len) {
+		if (!start) {
+			start = &data[i];
+		}
+		if (data[i] == '\t')
+			mid = &data[i];
+		if (data[i] == '\n') {
+			data[i] = 0;
+			ref_name = mid + 1;
+			ref = xmalloc(sizeof(struct ref) +
+				      strlen(ref_name) + 1);
+			memset(ref, 0, sizeof(struct ref));
+			strcpy(ref->name, ref_name);
+			get_sha1_hex(start, ref->old_sha1);
+			if (!refs)
+				refs = ref;
+			if (last_ref)
+				last_ref->next = ref;
+			last_ref = ref;
+			start = NULL;
+		}
+		i++;
+	}
+
+	strbuf_release(&buffer);
+
+	ref = alloc_ref("HEAD");
+	if (!walker->fetch_ref(walker, ref) &&
+	    !resolve_remote_symref(ref, refs)) {
+		ref->next = refs;
+		refs = ref;
+	} else {
+		free(ref);
+	}
+
+	strbuf_release(&buffer);
+	free(refs_url);
+	return refs;
+}
+
+int main(int argc, const char **argv)
+{
+	struct remote *remote;
+	struct strbuf buf = STRBUF_INIT;
+	const char *url;
+	struct walker *walker = NULL;
+
+	setup_git_directory();
+	if (argc < 2) {
+		fprintf(stderr, "Remote needed\n");
+		return 1;
+	}
+
+	remote = remote_get(argv[1]);
+
+	if (argc > 2) {
+		url = argv[2];
+	} else {
+		url = remote->url[0];
+	}
+
+	do {
+		if (strbuf_getline(&buf, stdin, '\n') == EOF)
+			break;
+		if (!prefixcmp(buf.buf, "fetch ")) {
+			char *obj = buf.buf + strlen("fetch ");
+			if (!walker)
+				walker = get_http_walker(url, remote);
+			walker->get_all = 1;
+			walker->get_tree = 1;
+			walker->get_history = 1;
+			walker->get_verbosely = 0;
+			walker->get_recover = 0;
+			if (walker_fetch(walker, 1, &obj, NULL, NULL))
+				die("Fetch failed.");
+			printf("\n");
+			fflush(stdout);
+		} else if (!strcmp(buf.buf, "list")) {
+			struct ref *refs;
+			struct ref *posn;
+			if (!walker)
+				walker = get_http_walker(url, remote);
+			refs = get_refs(walker, url);
+			for (posn = refs; posn; posn = posn->next) {
+				if (posn->symref)
+					printf("@%s %s\n", posn->symref, posn->name);
+				else
+					printf("%s %s\n", sha1_to_hex(posn->old_sha1), posn->name);
+			}
+			printf("\n");
+			fflush(stdout);
+		} else if (!strcmp(buf.buf, "capabilities")) {
+			printf("fetch\n");
+			printf("\n");
+			fflush(stdout);
+		} else {
+			return 1;
+		}
+		strbuf_reset(&buf);
+	} while (1);
+	return 0;
+}
diff --git a/transport.c b/transport.c
index de0d587..9935c85 100644
--- a/transport.c
+++ b/transport.c
@@ -1,9 +1,6 @@
 #include "cache.h"
 #include "transport.h"
 #include "run-command.h"
-#ifndef NO_CURL
-#include "http.h"
-#endif
 #include "pkt-line.h"
 #include "fetch-pack.h"
 #include "send-pack.h"
@@ -352,45 +349,6 @@ static int rsync_transport_push(struct transport *transport,
 	return result;
 }
 
-/* Generic functions for using commit walkers */
-
-#ifndef NO_CURL /* http fetch is the only user */
-static int fetch_objs_via_walker(struct transport *transport,
-				 int nr_objs, const struct ref **to_fetch)
-{
-	char *dest = xstrdup(transport->url);
-	struct walker *walker = transport->data;
-	char **objs = xmalloc(nr_objs * sizeof(*objs));
-	int i;
-
-	walker->get_all = 1;
-	walker->get_tree = 1;
-	walker->get_history = 1;
-	walker->get_verbosely = transport->verbose >= 0;
-	walker->get_recover = 0;
-
-	for (i = 0; i < nr_objs; i++)
-		objs[i] = xstrdup(sha1_to_hex(to_fetch[i]->old_sha1));
-
-	if (walker_fetch(walker, nr_objs, objs, NULL, NULL))
-		die("Fetch failed.");
-
-	for (i = 0; i < nr_objs; i++)
-		free(objs[i]);
-	free(objs);
-	free(dest);
-	return 0;
-}
-#endif /* NO_CURL */
-
-static int disconnect_walker(struct transport *transport)
-{
-	struct walker *walker = transport->data;
-	if (walker)
-		walker_free(walker);
-	return 0;
-}
-
 #ifndef NO_CURL
 static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
 {
@@ -432,96 +390,6 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons
 	return !!err;
 }
 
-static struct ref *get_refs_via_curl(struct transport *transport, int for_push)
-{
-	struct strbuf buffer = STRBUF_INIT;
-	char *data, *start, *mid;
-	char *ref_name;
-	char *refs_url;
-	int i = 0;
-	int http_ret;
-
-	struct ref *refs = NULL;
-	struct ref *ref = NULL;
-	struct ref *last_ref = NULL;
-
-	struct walker *walker;
-
-	if (for_push)
-		return NULL;
-
-	if (!transport->data)
-		transport->data = get_http_walker(transport->url,
-						transport->remote);
-
-	walker = transport->data;
-
-	refs_url = xmalloc(strlen(transport->url) + 11);
-	sprintf(refs_url, "%s/info/refs", transport->url);
-
-	http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
-	switch (http_ret) {
-	case HTTP_OK:
-		break;
-	case HTTP_MISSING_TARGET:
-		die("%s not found: did you run git update-server-info on the"
-		    " server?", refs_url);
-	default:
-		http_error(refs_url, http_ret);
-		die("HTTP request failed");
-	}
-
-	data = buffer.buf;
-	start = NULL;
-	mid = data;
-	while (i < buffer.len) {
-		if (!start)
-			start = &data[i];
-		if (data[i] == '\t')
-			mid = &data[i];
-		if (data[i] == '\n') {
-			data[i] = 0;
-			ref_name = mid + 1;
-			ref = xmalloc(sizeof(struct ref) +
-				      strlen(ref_name) + 1);
-			memset(ref, 0, sizeof(struct ref));
-			strcpy(ref->name, ref_name);
-			get_sha1_hex(start, ref->old_sha1);
-			if (!refs)
-				refs = ref;
-			if (last_ref)
-				last_ref->next = ref;
-			last_ref = ref;
-			start = NULL;
-		}
-		i++;
-	}
-
-	strbuf_release(&buffer);
-
-	ref = alloc_ref("HEAD");
-	if (!walker->fetch_ref(walker, ref) &&
-	    !resolve_remote_symref(ref, refs)) {
-		ref->next = refs;
-		refs = ref;
-	} else {
-		free(ref);
-	}
-
-	strbuf_release(&buffer);
-	free(refs_url);
-	return refs;
-}
-
-static int fetch_objs_via_curl(struct transport *transport,
-				 int nr_objs, const struct ref **to_fetch)
-{
-	if (!transport->data)
-		transport->data = get_http_walker(transport->url,
-						transport->remote);
-	return fetch_objs_via_walker(transport, nr_objs, to_fetch);
-}
-
 #endif
 
 struct bundle_transport_data {
@@ -950,14 +818,12 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	} else if (!prefixcmp(url, "http://")
 	        || !prefixcmp(url, "https://")
 	        || !prefixcmp(url, "ftp://")) {
+		transport_helper_init(ret);
 #ifdef NO_CURL
 		error("git was compiled without libcurl support.");
 #else
-		ret->get_refs_list = get_refs_via_curl;
-		ret->fetch = fetch_objs_via_curl;
 		ret->push = curl_transport_push;
 #endif
-		ret->disconnect = disconnect_walker;
 
 	} else if (is_local(url) && is_file(url)) {
 		struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
-- 
1.6.4.rc3.27.g95032.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 02/13] Use an external program to implement fetching with curl
  2009-08-05  5:01 [PATCH 02/13] Use an external program to implement fetching with curl Daniel Barkalow
@ 2009-08-05 10:07 ` Johannes Schindelin
  2009-08-05 15:52   ` Daniel Barkalow
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Schindelin @ 2009-08-05 10:07 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git, Johan Herland

Hi,

On Wed, 5 Aug 2009, Daniel Barkalow wrote:

> diff --git a/Makefile b/Makefile
> index 504646a..35117fc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -981,6 +981,7 @@ else
>  		CURL_LIBCURL = -lcurl
>  	endif
>  	BUILTIN_OBJS += builtin-http-fetch.o
> +	PROGRAMS += git-remote-http$X git-remote-https$X git-remote-ftp$X git-http-fetch$X
>  	EXTLIBS += $(CURL_LIBCURL)
>  	LIB_OBJS += http.o http-walker.o
>  	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
> @@ -1491,6 +1492,10 @@ 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)
> +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> +		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
> +

Ooops, I missed this part.  How about making git-remote-https and 
git-remote-ftp hardlinks to git-remote-http?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 02/13] Use an external program to implement fetching with curl
  2009-08-05 10:07 ` Johannes Schindelin
@ 2009-08-05 15:52   ` Daniel Barkalow
  2009-08-05 19:54     ` Johannes Schindelin
  2009-08-07  5:08     ` Junio C Hamano
  0 siblings, 2 replies; 7+ messages in thread
From: Daniel Barkalow @ 2009-08-05 15:52 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, Johan Herland

On Wed, 5 Aug 2009, Johannes Schindelin wrote:

> Hi,
> 
> On Wed, 5 Aug 2009, Daniel Barkalow wrote:
> 
> > diff --git a/Makefile b/Makefile
> > index 504646a..35117fc 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -981,6 +981,7 @@ else
> >  		CURL_LIBCURL = -lcurl
> >  	endif
> >  	BUILTIN_OBJS += builtin-http-fetch.o
> > +	PROGRAMS += git-remote-http$X git-remote-https$X git-remote-ftp$X git-http-fetch$X
> >  	EXTLIBS += $(CURL_LIBCURL)
> >  	LIB_OBJS += http.o http-walker.o
> >  	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
> > @@ -1491,6 +1492,10 @@ 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)
> > +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> > +		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
> > +
> 
> 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?

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 02/13] Use an external program to implement fetching with curl
  2009-08-05 15:52   ` Daniel Barkalow
@ 2009-08-05 19:54     ` Johannes Schindelin
  2009-08-07  5:08     ` Junio C Hamano
  1 sibling, 0 replies; 7+ messages in thread
From: Johannes Schindelin @ 2009-08-05 19:54 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git, Johan Herland

Hi,

On Wed, 5 Aug 2009, Daniel Barkalow wrote:

> On Wed, 5 Aug 2009, Johannes Schindelin wrote:
> 
> > On Wed, 5 Aug 2009, Daniel Barkalow wrote:
> > 
> > > diff --git a/Makefile b/Makefile
> > > index 504646a..35117fc 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -981,6 +981,7 @@ else
> > >  		CURL_LIBCURL = -lcurl
> > >  	endif
> > >  	BUILTIN_OBJS += builtin-http-fetch.o
> > > +	PROGRAMS += git-remote-http$X git-remote-https$X git-remote-ftp$X git-http-fetch$X
> > >  	EXTLIBS += $(CURL_LIBCURL)
> > >  	LIB_OBJS += http.o http-walker.o
> > >  	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
> > > @@ -1491,6 +1492,10 @@ 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)
> > > +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> > > +		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
> > > +
> > 
> > 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?

You mean as in

	$(BUILT_INS): git$X
	        $(QUIET_BUILT_IN)$(RM) $@ && \
	        ln git$X $@ 2>/dev/null || \
	        ln -s git$X $@ 2>/dev/null || \
	        cp git$X $@

?  I guess so ;-)

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 02/13] Use an external program to implement fetching with curl
  2009-08-05 15:52   ` Daniel Barkalow
  2009-08-05 19:54     ` Johannes Schindelin
@ 2009-08-07  5:08     ` Junio C Hamano
  2009-08-07  5:34       ` Daniel Barkalow
  2009-08-07 19:19       ` [PATCH/RFC] Makefile: build/install git-remote-<scheme> as hardlinks when able Junio C Hamano
  1 sibling, 2 replies; 7+ messages in thread
From: Junio C Hamano @ 2009-08-07  5:08 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Johannes Schindelin, git, Johan Herland

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.

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 suspect that the "install" target may need a patch similar to this one,
though...

-- >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 $@
+
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
 builtin-revert.o wt-status.o: wt-status.h

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 02/13] Use an external program to implement fetching with curl
  2009-08-07  5:08     ` Junio C Hamano
@ 2009-08-07  5:34       ` Daniel Barkalow
  2009-08-07 19:19       ` [PATCH/RFC] Makefile: build/install git-remote-<scheme> as hardlinks when able Junio C Hamano
  1 sibling, 0 replies; 7+ messages in thread
From: Daniel Barkalow @ 2009-08-07  5:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, Johan Herland

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

> 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.

> 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.

> -- >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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH/RFC] Makefile: build/install git-remote-<scheme> as hardlinks when able
  2009-08-07  5:08     ` Junio C Hamano
  2009-08-07  5:34       ` Daniel Barkalow
@ 2009-08-07 19:19       ` Junio C Hamano
  1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2009-08-07 19:19 UTC (permalink / raw)
  To: Daniel Barkalow, Johannes Schindelin; +Cc: git

Three git-remote-<scheme> helper programs are the same executable that
uses libcurl to implement the named protocol transfer.  Instead of
running the linker three times to build different programs, build one and
hardlink (or copy if the underlying filesystem does not support it) the
other two.

Install one to $(gitexecdir) and make the other two hardnlinks, in a way
similar to how we install programs that implement built-in commands.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

I do not particularly like this one.  ALL_PROGRAMS does not include any of
the built-in programs, but because CURL_SYNONYMS are part of PROGRAMS, the
install step needs to filter them out, so that we can arrange to make
hardlinks to the synonyms in a separate step.

Putting git-remote-http$X on OTHER_PROGRAMS like we do for git$X does not
work well either, as that won't install it in gitexecdir.

 Makefile |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index e29b15f..438cffb 100644
--- a/Makefile
+++ b/Makefile
@@ -383,7 +383,8 @@ BUILT_INS += git-stage$X
 BUILT_INS += git-status$X
 BUILT_INS += git-whatchanged$X
 
-# what 'all' will build and 'install' will install, in gitexecdir
+# what 'all' will build and 'install' will install in gitexecdir,
+# excluding programs for built-in commands
 ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
 
 # what 'all' will build but not install in gitexecdir
@@ -980,7 +981,8 @@ else
 	else
 		CURL_LIBCURL = -lcurl
 	endif
-	PROGRAMS += git-remote-http$X git-remote-https$X git-remote-ftp$X git-http-fetch$X
+	CURL_SYNONYMS = git-remote-https$X git-remote-ftp$X
+	PROGRAMS += git-remote-http$X $(CURL_SYNONYMS) git-http-fetch$X
 	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
 	ifeq "$(curl_check)" "070908"
 		ifndef NO_EXPAT
@@ -1256,6 +1258,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 +1497,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)
 
+$(CURL_SYNONYMS): git-remote-http$X
+	$(QUIET_LNCP)$(RM) $@ && \
+	ln $< $@ 2>/dev/null || \
+	ln -s $< $@ 2>/dev/null || \
+	cp $< $@
+
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
 builtin-revert.o wt-status.o: wt-status.h
@@ -1653,7 +1662,7 @@ export gitexec_instdir
 install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) $(filter-out $(CURL_SYNONYMS), $(ALL_PROGRAMS)) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_PERL
@@ -1679,6 +1688,12 @@ endif
 		ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \
 		cp "$$execdir/git$X" "$$execdir/$$p" || exit; \
 	  done; } && \
+	{ for p in $(CURL_SYNONYMS); do \
+		$(RM) "$$execdir/$$p" && \
+		ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
+		ln -s "git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
+		cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; \
+	  done; } && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
 install-doc:

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-08-07 19:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-05  5:01 [PATCH 02/13] Use an external program to implement fetching with curl Daniel Barkalow
2009-08-05 10:07 ` Johannes Schindelin
2009-08-05 15:52   ` Daniel Barkalow
2009-08-05 19:54     ` Johannes Schindelin
2009-08-07  5:08     ` Junio C Hamano
2009-08-07  5:34       ` Daniel Barkalow
2009-08-07 19:19       ` [PATCH/RFC] Makefile: build/install git-remote-<scheme> as hardlinks when able Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).