[PATCH 0/2] rsync support

[PATCH 0/2] rsync support

[Johannes Schindelin]

Hi,

I worked too long on this... So here it is, a 2-patch series to readd the 
rsync support.

One thing I did not do: fetch alternates.  My rationale is this: just as 
with HTTP, the rsync protocol does allow for having a different root than 
the file system root.  So, just as with HTTP, the alternates are likely to 
break.

But hey, if somebody feels like it, there is a NEEDSWORK pointer from 
which to start.

Ciao,
Dscho

[PATCH 1/2] Introduce remove_dir_recursively()

[Johannes Schindelin]

There was a function called remove_empty_dir_recursive() buried
in refs.c.  Expose a slightly enhanced version in dir.h: it can now
optionally remove a non-empty directory.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 dir.c  |   45 +++++++++++++++++++++++++++++++++++++++++++++
 dir.h  |    2 ++
 refs.c |   36 ++----------------------------------
 3 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/dir.c b/dir.c
index eb6c3ab..0fae400 100644
--- a/dir.c
+++ b/dir.c
@@ -685,3 +685,48 @@ int is_inside_dir(const char *dir)
 	char buffer[PATH_MAX];
 	return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
 }
+
+int remove_dir_recursively(char *path, int len, int only_empty)
+{
+	DIR *dir = opendir(path);
+	struct dirent *e;
+	int ret = 0;
+
+	if (!dir)
+		return -1;
+	if (path[len-1] != '/')
+		path[len++] = '/';
+	while ((e = readdir(dir)) != NULL) {
+		struct stat st;
+		int namlen;
+		if ((e->d_name[0] == '.') &&
+		    ((e->d_name[1] == 0) ||
+		     ((e->d_name[1] == '.') && e->d_name[2] == 0)))
+			continue; /* "." and ".." */
+
+		namlen = strlen(e->d_name);
+		if (len + namlen > PATH_MAX ||
+				!memcpy(path + len, e->d_name, namlen) ||
+				(path[len + namlen] = '\0') ||
+				lstat(path, &st))
+			; /* fall thru */
+		else if (S_ISDIR(st.st_mode)) {
+			if (!remove_dir_recursively(path, len + namlen,
+						only_empty))
+				continue; /* happy */
+		} else if (!only_empty &&
+				len + namlen + 1 < PATH_MAX &&
+				!unlink(path))
+			continue; /* happy, too */
+
+		/* path too long, stat fails, or non-directory still exists */
+		ret = -1;
+		break;
+	}
+	closedir(dir);
+	if (!ret) {
+		path[len] = 0;
+		ret = rmdir(path);
+	}
+	return ret;
+}
diff --git a/dir.h b/dir.h
index f55a87b..f52eea8 100644
--- a/dir.h
+++ b/dir.h
@@ -64,4 +64,6 @@ extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna
 extern char *get_relative_cwd(char *buffer, int size, const char *dir);
 extern int is_inside_dir(const char *dir);
 
+extern int remove_dir_recursively(char *dir, int len, int only_empty);
+
 #endif
diff --git a/refs.c b/refs.c
index 07e260c..f0336ee 100644
--- a/refs.c
+++ b/refs.c
@@ -2,6 +2,7 @@
 #include "refs.h"
 #include "object.h"
 #include "tag.h"
+#include "dir.h"
 
 /* ISSYMREF=01 and ISPACKED=02 are public interfaces */
 #define REF_KNOWS_PEELED 04
@@ -673,40 +674,7 @@ static struct ref_lock *verify_lock(struct ref_lock *lock,
 
 static int remove_empty_dir_recursive(char *path, int len)
 {
-	DIR *dir = opendir(path);
-	struct dirent *e;
-	int ret = 0;
-
-	if (!dir)
-		return -1;
-	if (path[len-1] != '/')
-		path[len++] = '/';
-	while ((e = readdir(dir)) != NULL) {
-		struct stat st;
-		int namlen;
-		if ((e->d_name[0] == '.') &&
-		    ((e->d_name[1] == 0) ||
-		     ((e->d_name[1] == '.') && e->d_name[2] == 0)))
-			continue; /* "." and ".." */
-
-		namlen = strlen(e->d_name);
-		if ((len + namlen < PATH_MAX) &&
-		    strcpy(path + len, e->d_name) &&
-		    !lstat(path, &st) &&
-		    S_ISDIR(st.st_mode) &&
-		    !remove_empty_dir_recursive(path, len + namlen))
-			continue; /* happy */
-
-		/* path too long, stat fails, or non-directory still exists */
-		ret = -1;
-		break;
-	}
-	closedir(dir);
-	if (!ret) {
-		path[len] = 0;
-		ret = rmdir(path);
-	}
-	return ret;
+	return remove_dir_recursively(path, len, 1);
 }
 
 static int remove_empty_directories(char *file)
-- 
1.5.3.2.1102.g9487

Re: [PATCH 1/2] Introduce remove_dir_recursively()

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +int remove_dir_recursively(char *path, int len, int only_empty)
> +{
> ...
> +		namlen = strlen(e->d_name);
> +		if (len + namlen > PATH_MAX ||
> +				!memcpy(path + len, e->d_name, namlen) ||
> +				(path[len + namlen] = '\0') ||
> +				lstat(path, &st))
> +			; /* fall thru */
> +		else if (S_ISDIR(st.st_mode)) {
> +			if (!remove_dir_recursively(path, len + namlen,
> +						only_empty))
> +				continue; /* happy */
> +		} else if (!only_empty &&
> +				len + namlen + 1 < PATH_MAX &&
> +				!unlink(path))
> +			continue; /* happy, too */
> +
> +		/* path too long, stat fails, or non-directory still exists */
> +		ret = -1;
> +		break;

Is it only me who finds the first if () condition way too
convoluted and needs to read three times to convince oneself
that it is doing a sane thing?

Please, especially...

 * For $DEITY's sake, memcpy() returns pointer to dst which you
   know is not NULL. so !memcpy() is always false here, which
   might be _convenient_ for you and the compiler but not for
   a human reader of the code who needs to blink twice wondering
   if you meant !memcmp().

 * Same for (path[] = '\0'), wondering if it is misspelled
   (path[] == '\0').

Re: [PATCH 1/2] Introduce remove_dir_recursively()

[Johannes Schindelin]

Hi,

On Fri, 28 Sep 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +int remove_dir_recursively(char *path, int len, int only_empty)
> > +{
> > ...
> > +		namlen = strlen(e->d_name);
> > +		if (len + namlen > PATH_MAX ||
> > +				!memcpy(path + len, e->d_name, namlen) ||
> > +				(path[len + namlen] = '\0') ||
> > +				lstat(path, &st))
> > +			; /* fall thru */
> > +		else if (S_ISDIR(st.st_mode)) {
> > +			if (!remove_dir_recursively(path, len + namlen,
> > +						only_empty))
> > +				continue; /* happy */
> > +		} else if (!only_empty &&
> > +				len + namlen + 1 < PATH_MAX &&
> > +				!unlink(path))
> > +			continue; /* happy, too */
> > +
> > +		/* path too long, stat fails, or non-directory still exists */
> > +		ret = -1;
> > +		break;
> 
> Is it only me who finds the first if () condition way too
> convoluted and needs to read three times to convince oneself
> that it is doing a sane thing?
> 
> Please, especially...
> 
>  * For $DEITY's sake, memcpy() returns pointer to dst which you
>    know is not NULL. so !memcpy() is always false here, which
>    might be _convenient_ for you and the compiler but not for
>    a human reader of the code who needs to blink twice wondering
>    if you meant !memcmp().
> 
>  * Same for (path[] = '\0'), wondering if it is misspelled
>    (path[] == '\0').

Okay, will fix (with an evil goto).

BTW it just hit me that this magic reliance on a buffer of size PATH_MAX 
is not good at all.  It even hit _me_ while developing that series.

So I'll change that to a strbuf, too.  (Which will fix the convoluted 
logic quite some, incidentally.)

Ciao,
Dscho

Re: [PATCH 1/2] Introduce remove_dir_recursively()

[David Kastrup]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Fri, 28 Sep 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > +int remove_dir_recursively(char *path, int len, int only_empty)
>> > +{
>> > ...
>> > +		namlen = strlen(e->d_name);
>> > +		if (len + namlen > PATH_MAX ||
>> > +				!memcpy(path + len, e->d_name, namlen) ||
>> > +				(path[len + namlen] = '\0') ||
>> > +				lstat(path, &st))
>> > +			; /* fall thru */
>> > +		else if (S_ISDIR(st.st_mode)) {
>> > +			if (!remove_dir_recursively(path, len + namlen,
>> > +						only_empty))
>> > +				continue; /* happy */
>> > +		} else if (!only_empty &&
>> > +				len + namlen + 1 < PATH_MAX &&
>> > +				!unlink(path))
>> > +			continue; /* happy, too */
>> > +
>> > +		/* path too long, stat fails, or non-directory still exists */
>> > +		ret = -1;
>> > +		break;
>> 
>> Is it only me who finds the first if () condition way too
>> convoluted and needs to read three times to convince oneself
>> that it is doing a sane thing?
>> 
>> Please, especially...
>> 
>>  * For $DEITY's sake, memcpy() returns pointer to dst which you
>>    know is not NULL. so !memcpy() is always false here, which
>>    might be _convenient_ for you and the compiler but not for
>>    a human reader of the code who needs to blink twice wondering
>>    if you meant !memcmp().
>> 
>>  * Same for (path[] = '\0'), wondering if it is misspelled
>>    (path[] == '\0').
>
> Okay, will fix (with an evil goto).

Uh, C has a comma operator.  And inverting the first condition makes
for a nicer flow, no goto.

		namlen = strlen(e->d_name);
		if (len + namlen <= PATH_MAX
			&& (memcpy(path + len, e->d_name, namlen),
			    path[len + namlen] = '\0',
			    lstat(path, &st) == 0)
		if (S_ISDIR(st.st_mode)) {
			if (!remove_dir_recursively(path, len + namlen,
						only_empty))
			continue; /* happy */
		} else if (!only_empty &&
				len + namlen + 1 < PATH_MAX &&
				!unlink(path))
			continue; /* happy, too */

		/* path too long, stat fails, or non-directory still exists */
		ret = -1;
		break;


-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[PATCH 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

We lost rsync support when transitioning from shell to C.  Support it
again (even if the transport is technically deprecated, some people just
do not have any chance to use anything else).

Also, add a test to t5510.  Since rsync transport is not configured by
default on most machines, and especially not such that you can
write to rsync://127.0.0.1$(pwd)/, it is disabled by default; you can
enable it by setting the environment variable TEST_RSYNC.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Maybe this should be moved into transport-rsync.c...

 t/t5510-fetch.sh |   35 ++++++
 transport.c      |  333 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 367 insertions(+), 1 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 439430f..73a4e3c 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -153,4 +153,39 @@ test_expect_success 'bundle should be able to create a full history' '
 
 '
 
+test "$TEST_RSYNC" && {
+test_expect_success 'fetch via rsync' '
+	git pack-refs &&
+	mkdir rsynced &&
+	cd rsynced &&
+	git init &&
+	git fetch rsync://127.0.0.1$(pwd)/../.git master:refs/heads/master &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	mkdir ../rsynced2 &&
+	(cd ../rsynced2 &&
+	 git init) &&
+	git push rsync://127.0.0.1$(pwd)/../rsynced2/.git master &&
+	cd ../rsynced2 &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	cd .. &&
+	mkdir rsynced3 &&
+	(cd rsynced3 &&
+	 git init) &&
+	git push --all rsync://127.0.0.1$(pwd)/rsynced3/.git &&
+	cd rsynced3 &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+}
+
 test_done
diff --git a/transport.c b/transport.c
index 4f9cddc..d50a311 100644
--- a/transport.c
+++ b/transport.c
@@ -6,6 +6,334 @@
 #include "fetch-pack.h"
 #include "walker.h"
 #include "bundle.h"
+#include "dir.h"
+#include "refs.h"
+
+/* rsync support */
+
+/*
+ * We copy packed-refs and refs/ into a temporary file, then read the
+ * loose refs recursively (sorting whenever possible), and then inserting
+ * those packed refs that are not yet in the list (not validating, but
+ * assuming that the file is sorted).
+ *
+ * Appears refactoring this from refs.c is too cumbersome.
+ */
+
+static int direntry_cmp(const void *a, const void *b)
+{
+	const struct dirent *d1 = a;
+	const struct dirent *d2 = b;
+
+	return strcmp(d1->d_name, d2->d_name);
+}
+
+/*
+ * path is assumed to point to a buffer of PATH_MAX bytes, and
+ * path + name_offset is expected to point to "refs/".
+ */
+
+static int read_loose_refs(char *path, int name_offset, struct ref **tail)
+{
+	DIR *dir = opendir(path);
+	struct dirent *de;
+	struct {
+		struct dirent *entries;
+		int nr, alloc;
+	} list;
+	int i, pathlen;
+
+	if (!dir)
+		return -1;
+
+	memset (&list, 0, sizeof(list));
+
+	while ((de = readdir(dir))) {
+		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
+				(de->d_name[1] == '.' &&
+				 de->d_name[2] == '\0')))
+			continue;
+		if (list.nr >= list.alloc) {
+			list.alloc = alloc_nr(list.nr);
+			list.entries = xrealloc(list.entries,
+				list.alloc * sizeof(*de));
+		}
+		list.entries[list.nr++] = *de;
+	}
+	closedir(dir);
+
+	/* sort the list */
+
+	qsort(list.entries, list.nr, sizeof(*de), direntry_cmp);
+
+	pathlen = strlen(path);
+	path[pathlen] = '/';
+
+	for (i = 0; i < list.nr; i++) {
+		strlcpy(path + pathlen + 1, list.entries[i].d_name,
+				PATH_MAX - pathlen - 1);
+		if (read_loose_refs(path, name_offset, tail)) {
+			int fd = open(path, O_RDONLY);
+			char buffer[40];
+			struct ref *next;
+
+			if (fd < 0)
+				continue;
+			next = alloc_ref(strlen(path + name_offset));
+			if (read_in_full(fd, buffer, 40) != 40 ||
+					get_sha1_hex(buffer, next->old_sha1)) {
+				free(next);
+				continue;
+			}
+			close(fd);
+			strcpy(next->name, path + name_offset);
+			(*tail)->next = next;
+			*tail = next;
+		}
+	}
+
+	path[pathlen] = '\0';
+
+	free(list.entries);
+	return 0;
+}
+
+/* insert the packed refs for which no loose refs were found */
+
+static void insert_packed_refs(const char *packed_refs, struct ref **list)
+{
+	FILE *f = fopen(packed_refs, "r");
+	static char buffer[PATH_MAX];
+
+	if (!f)
+		return;
+
+	for (;;) {
+		int cmp, len;
+
+		if (!fgets(buffer, sizeof(buffer), f)) {
+			fclose(f);
+			return;
+		}
+
+		if (hexval(buffer[0]) & 0x10)
+			continue;
+		len = strlen(buffer);
+		if (buffer[len - 1] == '\n')
+			buffer[--len] = '\0';
+		if (len < 41)
+			continue;
+		while ((*list)->next &&
+				(cmp = strcmp(buffer + 41,
+				      (*list)->next->name)) > 0)
+			list = &(*list)->next;
+		if (!(*list)->next || cmp < 0) {
+			struct ref *next = alloc_ref(len - 40);
+			buffer[40] = '\0';
+			if (get_sha1_hex(buffer, next->old_sha1)) {
+				warning ("invalid SHA-1: %s", buffer);
+				free(next);
+				continue;
+			}
+			strcpy(next->name, buffer + 41);
+			next->next = (*list)->next;
+			(*list)->next = next;
+			list = &(*list)->next;
+		}
+	}
+}
+
+static struct ref *get_refs_via_rsync(const struct transport *transport)
+{
+	char buffer[PATH_MAX];
+	struct strbuf buf = STRBUF_INIT;
+	struct ref dummy, *result = &dummy;
+	struct child_process rsync;
+	const char *args[5];
+	char *temp_dir;
+
+	/* copy the refs to the temporary directory */
+
+	temp_dir = mkdtemp(git_path("rsync-refs-XXXXXX"));
+	if (temp_dir == NULL)
+		die ("Could not make temporary directory");
+	temp_dir = strcpy(buffer, temp_dir);
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/refs");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = buf.buf;
+	args[3] = temp_dir;
+	args[4] = NULL;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/packed-refs");
+
+	args[2] = buf.buf;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	/* read the copied refs */
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, temp_dir);
+	strbuf_addstr(&buf, "/refs");
+	read_loose_refs(buf.buf, strlen(buf.buf) - 4, &result);
+
+	result = &dummy;
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, temp_dir);
+	strbuf_addstr(&buf, "/packed-refs");
+	insert_packed_refs(buf.buf, &result);
+
+	if (remove_dir_recursively(temp_dir, strlen(temp_dir), 0))
+		warning ("Could not remove temporary directory %s.", temp_dir);
+
+	return dummy.next;
+}
+
+static int fetch_objs_via_rsync(struct transport *transport,
+				 int nr_objs, struct ref **to_fetch)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process rsync;
+	const char *args[8];
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/objects/");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = buf.buf;
+	args[6] = get_object_directory();
+	args[7] = NULL;
+
+	/* NEEDSWORK: handle one level of alternates */
+	return run_command(&rsync);
+}
+
+static int write_one_ref(const char *name, const unsigned char *sha1,
+		int flags, void *data)
+{
+	struct strbuf *buf = data;
+	int len = buf->len;
+	FILE *f;
+
+	if (flags && prefixcmp(name, "refs/heads/") &&
+			prefixcmp(name, "refs/tags/"))
+		return 0;
+
+	strbuf_addstr(buf, name);
+	if (safe_create_leading_directories(buf->buf) ||
+			!(f = fopen(buf->buf, "w")) ||
+			!fwrite(sha1_to_hex(sha1), 40, 1, f) ||
+			fputc('\n', f) == EOF ||
+			fclose(f))
+		return error("problems writing temporary file %s", buf->buf);
+	strbuf_setlen(buf, len);
+	return 0;
+}
+
+static int write_refs_to_temp_dir(struct strbuf *temp_dir,
+		int refspec_nr, const char **refspec)
+{
+	int i;
+
+	for (i = 0; i < refspec_nr; i++) {
+		unsigned char sha1[20];
+		char *ref;
+
+		if (dwim_ref(refspec[i], strlen(refspec[i]), sha1, &ref) != 1)
+			return error("Could not get ref %s", refspec[i]);
+
+		if (write_one_ref(ref, sha1, 0, temp_dir)) {
+			free(ref);
+			return -1;
+		}
+		free(ref);
+	}
+	return 0;
+}
+
+static int rsync_transport_push(struct transport *transport,
+		int refspec_nr, const char **refspec, int flags) {
+	char buffer[PATH_MAX];
+	struct strbuf buf = STRBUF_INIT;
+	int result = 0, i;
+	struct child_process rsync;
+	const char *args[8];
+	char *temp_dir;
+
+	/* first push the objects */
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addch(&buf, '/');
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-av" : "-a";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = get_object_directory();;
+	args[6] = buf.buf;
+	args[7] = NULL;
+
+	if (run_command(&rsync))
+		return error("Could not push objects to %s", transport->url);
+
+	/* copy the refs to the temporary directory; they could be packed. */
+
+	temp_dir = mkdtemp(git_path("rsync-refs-XXXXXX"));
+	if (temp_dir == NULL)
+		die ("Could not make temporary directory");
+	temp_dir = strcpy(buffer, temp_dir);
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, temp_dir);
+	strbuf_addch(&buf, '/');
+
+	if (flags & TRANSPORT_PUSH_ALL) {
+		if (for_each_ref(write_one_ref, &buf))
+			return -1;
+	} else if (write_refs_to_temp_dir(&buf, refspec_nr, refspec))
+		return -1;
+
+	i = (flags & TRANSPORT_PUSH_FORCE) ? 2 : 3;
+	args[i++] = buf.buf;
+	args[i++] = transport->url;
+	args[i++] = NULL;
+	if (run_command(&rsync))
+		result = error("Could not push to %s", transport->url);
+
+	if (remove_dir_recursively(temp_dir, strlen(temp_dir), 0))
+		warning ("Could not remove temporary directory %s.", temp_dir);
+
+	return result;
+}
+
+static int disconnect_rsync(struct transport *transport)
+{
+	return 0;
+}
 
 /* Generic functions for using commit walkers */
 
@@ -402,7 +730,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	ret->url = url;
 
 	if (!prefixcmp(url, "rsync://")) {
-		/* not supported; don't populate any ops */
+		ret->get_refs_list = get_refs_via_rsync;
+		ret->fetch = fetch_objs_via_rsync;
+		ret->push = rsync_transport_push;
+		ret->disconnect = disconnect_rsync;
 
 	} else if (!prefixcmp(url, "http://")
 	        || !prefixcmp(url, "https://")
-- 
1.5.3.2.1102.g9487

Re: [PATCH 2/2] fetch/push: readd rsync support

[Shawn O. Pearce]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> +static int disconnect_rsync(struct transport *transport)
> +{
> +	return 0;
> +}
>  
>  /* Generic functions for using commit walkers */
>  
> @@ -402,7 +730,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
>  	ret->url = url;
>  
>  	if (!prefixcmp(url, "rsync://")) {
> -		/* not supported; don't populate any ops */
> +		ret->get_refs_list = get_refs_via_rsync;
> +		ret->fetch = fetch_objs_via_rsync;
> +		ret->push = rsync_transport_push;
> +		ret->disconnect = disconnect_rsync;
>  
>  	} else if (!prefixcmp(url, "http://")
>  	        || !prefixcmp(url, "https://")

For what it's worth disconnect is an optional operation.  You did
not need to implement it if you don't allocate a data member in
the struct transport.  So removing disconnect_rsync() could save
you 6 lines or so.

I see push is now supported again.  Didn't we remove rsync push
support a long time ago?  Like say in:

  commit c485104741ccdf32dd0c96fcb886c38a0b5badbd
  Author: c.shoemaker@cox.net <c.shoemaker@cox.net>
  Date:   Sat Oct 29 00:16:33 2005 -0400

    Add usage help to git-push.sh
    
    Also clarify failure to push to read-only remote.  Especially,
    state why rsync:// is not used for pushing.
    
    [jc: ideally rsync should not be used for anything]
    
    Signed-off-by: Chris Shoemaker <c.shoemaker at cox.net>
    Signed-off-by: Junio C Hamano <junkio@cox.net>

I guess it is nice to see that you can't kill rsync.  Like Windows
it always finds it way back into your life.

-- 
Shawn.

Re: [PATCH 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

Hi,

On Fri, 28 Sep 2007, Shawn O. Pearce wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > +static int disconnect_rsync(struct transport *transport)
> > +{
> > +	return 0;
> > +}
> >  
> >  /* Generic functions for using commit walkers */
> >  
> > @@ -402,7 +730,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
> >  	ret->url = url;
> >  
> >  	if (!prefixcmp(url, "rsync://")) {
> > -		/* not supported; don't populate any ops */
> > +		ret->get_refs_list = get_refs_via_rsync;
> > +		ret->fetch = fetch_objs_via_rsync;
> > +		ret->push = rsync_transport_push;
> > +		ret->disconnect = disconnect_rsync;
> >  
> >  	} else if (!prefixcmp(url, "http://")
> >  	        || !prefixcmp(url, "https://")
> 
> For what it's worth disconnect is an optional operation.

Strange.  When not assigning disconnect, I had an error last night.  But 
it could well be something independent.

Reading the code again, I should not even have to assign NULL to 
ret->disconnect.

Will fix.

> I see push is now supported again.  Didn't we remove rsync push support 
> a long time ago?  Like say in:
> 
>   commit c485104741ccdf32dd0c96fcb886c38a0b5badbd
>   Author: c.shoemaker@cox.net <c.shoemaker@cox.net>
>   Date:   Sat Oct 29 00:16:33 2005 -0400
> 
>     Add usage help to git-push.sh
>     
>     Also clarify failure to push to read-only remote.  Especially,
>     state why rsync:// is not used for pushing.
>     
>     [jc: ideally rsync should not be used for anything]
>     
>     Signed-off-by: Chris Shoemaker <c.shoemaker at cox.net>
>     Signed-off-by: Junio C Hamano <junkio@cox.net>

I know.  I hesitated, but then: it was all too easy to have it in, so why 
should we relinquish it?

> I guess it is nice to see that you can't kill rsync.  Like Windows it 
> always finds it way back into your life.

Hehe.

Ciao,
Dscho

Re: [PATCH 2/2] fetch/push: readd rsync support

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +/*
> + * path is assumed to point to a buffer of PATH_MAX bytes, and
> + * path + name_offset is expected to point to "refs/".
> + */
> +
> +static int read_loose_refs(char *path, int name_offset, struct ref **tail)
> +{
> +	DIR *dir = opendir(path);
> +	struct dirent *de;
> +	struct {
> +		struct dirent *entries;
> +		int nr, alloc;
> +	} list;
> +	int i, pathlen;
> +
> +	if (!dir)
> +		return -1;
> +
> +	memset (&list, 0, sizeof(list));
> +
> +	while ((de = readdir(dir))) {
> +		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> +				(de->d_name[1] == '.' &&
> +				 de->d_name[2] == '\0')))
> +			continue;
> +		if (list.nr >= list.alloc) {
> +			list.alloc = alloc_nr(list.nr);
> +			list.entries = xrealloc(list.entries,
> +				list.alloc * sizeof(*de));
> +		}

ALLOC_GROW() not applicable here?

> +		list.entries[list.nr++] = *de;

Are you sure about this?

The last paragraph in Rationale section, in

    http://www.opengroup.org/onlinepubs/000095399/basedefs/dirent.h.html

suggests that the d_name[] member in struct dirent could be
declared at the very end of the structure as length of 1 (the
traditional trick to implement a flex-array); your assignment
from *de into entries[] would not work as expected on such an
implementation.

On Linux with glibc it appears bits/dirent.h defines dirent with
"char d_name[256]", so you may not see a breakage there, though.

You only use a list of strings (char **), don't you?

> ...
> +			if (fd < 0)
> +				continue;
> +			next = alloc_ref(strlen(path + name_offset));

And as we discussed earlier you would need one more byte here ;-).

Re: [PATCH 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

Hi,

On Fri, 28 Sep 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +/*
> > + * path is assumed to point to a buffer of PATH_MAX bytes, and
> > + * path + name_offset is expected to point to "refs/".
> > + */
> > +
> > +static int read_loose_refs(char *path, int name_offset, struct ref **tail)
> > +{
> > +	DIR *dir = opendir(path);
> > +	struct dirent *de;
> > +	struct {
> > +		struct dirent *entries;
> > +		int nr, alloc;
> > +	} list;
> > +	int i, pathlen;
> > +
> > +	if (!dir)
> > +		return -1;
> > +
> > +	memset (&list, 0, sizeof(list));
> > +
> > +	while ((de = readdir(dir))) {
> > +		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> > +				(de->d_name[1] == '.' &&
> > +				 de->d_name[2] == '\0')))
> > +			continue;
> > +		if (list.nr >= list.alloc) {
> > +			list.alloc = alloc_nr(list.nr);
> > +			list.entries = xrealloc(list.entries,
> > +				list.alloc * sizeof(*de));
> > +		}
> 
> ALLOC_GROW() not applicable here?
> 
> > +		list.entries[list.nr++] = *de;
> 
> Are you sure about this?
> 
> The last paragraph in Rationale section, in
> 
>     http://www.opengroup.org/onlinepubs/000095399/basedefs/dirent.h.html
> 
> suggests that the d_name[] member in struct dirent could be
> declared at the very end of the structure as length of 1 (the
> traditional trick to implement a flex-array); your assignment
> >from *de into entries[] would not work as expected on such an
> implementation.
> 
> On Linux with glibc it appears bits/dirent.h defines dirent with
> "char d_name[256]", so you may not see a breakage there, though.

D'oh!  You're completely right.

> You only use a list of strings (char **), don't you?

Originally, I wanted to use the d_type, too, but as I mentioned on IRC, it 
is not reliable enough (shows DT_UNKNOWN _all_ the time here).

But yes, I'll redo it, using ALLOC_GROW.

> > ...
> > +			if (fd < 0)
> > +				continue;
> > +			next = alloc_ref(strlen(path + name_offset));
> 
> And as we discussed earlier you would need one more byte here ;-).

Well, not after my patch, which I thought of as (1/3), but then decided it 
is good enough to be (1/1) until you shot it down...

Will fix.

Ciao,
Dscho

[PATCH v2 0/2] rsync support, was Re: [PATCH 0/2] rsync support

[Johannes Schindelin]

Okay,

here we go again.  The changes are

- do not implement a dummy disconnect

- use strbuf much more (makes the code cleaner and shorter, too)

- do not rely on the rejected alloc_ref() patch

Ciao,
Dscho

[PATCH v2 1/2] Introduce remove_dir_recursively()

[Johannes Schindelin]

There was a function called remove_empty_dir_recursive() buried
in refs.c.  Expose a slightly enhanced version in dir.h: it can now
optionally remove a non-empty directory.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 dir.c  |   41 +++++++++++++++++++++++++++++++++++++++++
 dir.h  |    2 ++
 refs.c |   57 ++++++++++++---------------------------------------------
 3 files changed, 55 insertions(+), 45 deletions(-)

diff --git a/dir.c b/dir.c
index eb6c3ab..b18257e 100644
--- a/dir.c
+++ b/dir.c
@@ -685,3 +685,44 @@ int is_inside_dir(const char *dir)
 	char buffer[PATH_MAX];
 	return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
 }
+
+int remove_dir_recursively(struct strbuf *path, int only_empty)
+{
+	DIR *dir = opendir(path->buf);
+	struct dirent *e;
+	int ret = 0, original_len = path->len, len;
+
+	if (!dir)
+		return -1;
+	if (path->buf[original_len - 1] != '/')
+		strbuf_addch(path, '/');
+
+	len = path->len;
+	while ((e = readdir(dir)) != NULL) {
+		struct stat st;
+		if ((e->d_name[0] == '.') &&
+		    ((e->d_name[1] == 0) ||
+		     ((e->d_name[1] == '.') && e->d_name[2] == 0)))
+			continue; /* "." and ".." */
+
+		strbuf_setlen(path, len);
+		strbuf_addstr(path, e->d_name);
+		if (lstat(path->buf, &st))
+			; /* fall thru */
+		else if (S_ISDIR(st.st_mode)) {
+			if (!remove_dir_recursively(path, only_empty))
+				continue; /* happy */
+		} else if (!only_empty && !unlink(path->buf))
+			continue; /* happy, too */
+
+		/* path too long, stat fails, or non-directory still exists */
+		ret = -1;
+		break;
+	}
+	closedir(dir);
+
+	strbuf_setlen(path, original_len);
+	if (!ret)
+		ret = rmdir(path->buf);
+	return ret;
+}
diff --git a/dir.h b/dir.h
index f55a87b..a248a23 100644
--- a/dir.h
+++ b/dir.h
@@ -64,4 +64,6 @@ extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna
 extern char *get_relative_cwd(char *buffer, int size, const char *dir);
 extern int is_inside_dir(const char *dir);
 
+extern int remove_dir_recursively(struct strbuf *path, int only_empty);
+
 #endif
diff --git a/refs.c b/refs.c
index 07e260c..aff02cd 100644
--- a/refs.c
+++ b/refs.c
@@ -2,6 +2,7 @@
 #include "refs.h"
 #include "object.h"
 #include "tag.h"
+#include "dir.h"
 
 /* ISSYMREF=01 and ISPACKED=02 are public interfaces */
 #define REF_KNOWS_PEELED 04
@@ -671,57 +672,23 @@ static struct ref_lock *verify_lock(struct ref_lock *lock,
 	return lock;
 }
 
-static int remove_empty_dir_recursive(char *path, int len)
-{
-	DIR *dir = opendir(path);
-	struct dirent *e;
-	int ret = 0;
-
-	if (!dir)
-		return -1;
-	if (path[len-1] != '/')
-		path[len++] = '/';
-	while ((e = readdir(dir)) != NULL) {
-		struct stat st;
-		int namlen;
-		if ((e->d_name[0] == '.') &&
-		    ((e->d_name[1] == 0) ||
-		     ((e->d_name[1] == '.') && e->d_name[2] == 0)))
-			continue; /* "." and ".." */
-
-		namlen = strlen(e->d_name);
-		if ((len + namlen < PATH_MAX) &&
-		    strcpy(path + len, e->d_name) &&
-		    !lstat(path, &st) &&
-		    S_ISDIR(st.st_mode) &&
-		    !remove_empty_dir_recursive(path, len + namlen))
-			continue; /* happy */
-
-		/* path too long, stat fails, or non-directory still exists */
-		ret = -1;
-		break;
-	}
-	closedir(dir);
-	if (!ret) {
-		path[len] = 0;
-		ret = rmdir(path);
-	}
-	return ret;
-}
-
-static int remove_empty_directories(char *file)
+static int remove_empty_directories(const char *file)
 {
 	/* we want to create a file but there is a directory there;
 	 * if that is an empty directory (or a directory that contains
 	 * only empty directories), remove them.
 	 */
-	char path[PATH_MAX];
-	int len = strlen(file);
+	struct strbuf path;
+	int result;
 
-	if (len >= PATH_MAX) /* path too long ;-) */
-		return -1;
-	strcpy(path, file);
-	return remove_empty_dir_recursive(path, len);
+	strbuf_init(&path, 20);
+	strbuf_addstr(&path, file);
+
+	result = remove_dir_recursively(&path, 1);
+
+	strbuf_release(&path);
+
+	return result;
 }
 
 static int is_refname_available(const char *ref, const char *oldref,
-- 
1.5.3.2.1102.g9487

[PATCH 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

We lost rsync support when transitioning from shell to C.  Support it
again (even if the transport is technically deprecated, some people just
do not have any chance to use anything else).

Also, add a test to t5510.  Since rsync transport is not configured by
default on most machines, and especially not such that you can
write to rsync://127.0.0.1$(pwd)/, it is disabled by default; you can
enable it by setting the environment variable TEST_RSYNC.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t5510-fetch.sh |   35 ++++++
 transport.c      |  328 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 362 insertions(+), 1 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 439430f..73a4e3c 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -153,4 +153,39 @@ test_expect_success 'bundle should be able to create a full history' '
 
 '
 
+test "$TEST_RSYNC" && {
+test_expect_success 'fetch via rsync' '
+	git pack-refs &&
+	mkdir rsynced &&
+	cd rsynced &&
+	git init &&
+	git fetch rsync://127.0.0.1$(pwd)/../.git master:refs/heads/master &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	mkdir ../rsynced2 &&
+	(cd ../rsynced2 &&
+	 git init) &&
+	git push rsync://127.0.0.1$(pwd)/../rsynced2/.git master &&
+	cd ../rsynced2 &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	cd .. &&
+	mkdir rsynced3 &&
+	(cd rsynced3 &&
+	 git init) &&
+	git push --all rsync://127.0.0.1$(pwd)/rsynced3/.git &&
+	cd rsynced3 &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+}
+
 test_done
diff --git a/transport.c b/transport.c
index 4f9cddc..c8eed95 100644
--- a/transport.c
+++ b/transport.c
@@ -6,6 +6,330 @@
 #include "fetch-pack.h"
 #include "walker.h"
 #include "bundle.h"
+#include "dir.h"
+#include "refs.h"
+
+/* rsync support */
+
+/*
+ * We copy packed-refs and refs/ into a temporary file, then read the
+ * loose refs recursively (sorting whenever possible), and then inserting
+ * those packed refs that are not yet in the list (not validating, but
+ * assuming that the file is sorted).
+ *
+ * Appears refactoring this from refs.c is too cumbersome.
+ */
+
+static int direntry_cmp(const void *a, const void *b)
+{
+	const struct dirent *d1 = a;
+	const struct dirent *d2 = b;
+
+	return strcmp(d1->d_name, d2->d_name);
+}
+
+/*
+ * path is assumed to point to a buffer of PATH_MAX bytes, and
+ * path + name_offset is expected to point to "refs/".
+ */
+
+static int read_loose_refs(struct strbuf *path, int name_offset,
+		struct ref **tail)
+{
+	DIR *dir = opendir(path->buf);
+	struct dirent *de;
+	struct {
+		char **entries;
+		int nr, alloc;
+	} list;
+	int i, pathlen;
+
+	if (!dir)
+		return -1;
+
+	memset (&list, 0, sizeof(list));
+
+	while ((de = readdir(dir))) {
+		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
+				(de->d_name[1] == '.' &&
+				 de->d_name[2] == '\0')))
+			continue;
+		ALLOC_GROW(list.entries, list.nr + 1, list.alloc);
+		list.entries[list.nr++] = xstrdup(de->d_name);
+	}
+	closedir(dir);
+
+	/* sort the list */
+
+	qsort(list.entries, list.nr, sizeof(*de), direntry_cmp);
+
+	pathlen = path->len;
+	strbuf_addch(path, '/');
+
+	for (i = 0; i < list.nr; i++, strbuf_setlen(path, pathlen + 1)) {
+		strbuf_addstr(path, list.entries[i]);
+		if (read_loose_refs(path, name_offset, tail)) {
+			int fd = open(path->buf, O_RDONLY);
+			char buffer[40];
+			struct ref *next;
+
+			if (fd < 0)
+				continue;
+			next = alloc_ref(path->len - name_offset + 1);
+			if (read_in_full(fd, buffer, 40) != 40 ||
+					get_sha1_hex(buffer, next->old_sha1)) {
+				free(next);
+				continue;
+			}
+			close(fd);
+			strcpy(next->name, path->buf + name_offset);
+			(*tail)->next = next;
+			*tail = next;
+		}
+	}
+
+	for (i = 0; i < list.nr; i++)
+		free(list.entries[i]);
+	free(list.entries);
+
+	return 0;
+}
+
+/* insert the packed refs for which no loose refs were found */
+
+static void insert_packed_refs(const char *packed_refs, struct ref **list)
+{
+	FILE *f = fopen(packed_refs, "r");
+	static char buffer[PATH_MAX];
+
+	if (!f)
+		return;
+
+	for (;;) {
+		int cmp, len;
+
+		if (!fgets(buffer, sizeof(buffer), f)) {
+			fclose(f);
+			return;
+		}
+
+		if (hexval(buffer[0]) & 0x10)
+			continue;
+		len = strlen(buffer);
+		if (buffer[len - 1] == '\n')
+			buffer[--len] = '\0';
+		if (len < 41)
+			continue;
+		while ((*list)->next &&
+				(cmp = strcmp(buffer + 41,
+				      (*list)->next->name)) > 0)
+			list = &(*list)->next;
+		if (!(*list)->next || cmp < 0) {
+			struct ref *next = alloc_ref(len - 40);
+			buffer[40] = '\0';
+			if (get_sha1_hex(buffer, next->old_sha1)) {
+				warning ("invalid SHA-1: %s", buffer);
+				free(next);
+				continue;
+			}
+			strcpy(next->name, buffer + 41);
+			next->next = (*list)->next;
+			(*list)->next = next;
+			list = &(*list)->next;
+		}
+	}
+}
+
+static struct ref *get_refs_via_rsync(const struct transport *transport)
+{
+	struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;
+	struct ref dummy, *result = &dummy;
+	struct child_process rsync;
+	const char *args[5];
+	int temp_dir_len;
+
+	/* copy the refs to the temporary directory */
+
+	strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
+	if (!mkdtemp(temp_dir.buf))
+		die ("Could not make temporary directory");
+	temp_dir_len = temp_dir.len;
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/refs");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = buf.buf;
+	args[3] = temp_dir.buf;
+	args[4] = NULL;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/packed-refs");
+
+	args[2] = buf.buf;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	/* read the copied refs */
+
+	strbuf_addstr(&temp_dir, "/refs");
+	read_loose_refs(&temp_dir, temp_dir_len + 1, &result);
+	strbuf_setlen(&temp_dir, temp_dir_len);
+
+	result = &dummy;
+	strbuf_addstr(&temp_dir, "/packed-refs");
+	insert_packed_refs(temp_dir.buf, &result);
+	strbuf_setlen(&temp_dir, temp_dir_len);
+
+	if (remove_dir_recursively(&temp_dir, 0))
+		warning ("Error removing temporary directory %s.",
+				temp_dir.buf);
+
+	strbuf_release(&buf);
+	strbuf_release(&temp_dir);
+
+	return dummy.next;
+}
+
+static int fetch_objs_via_rsync(struct transport *transport,
+				 int nr_objs, struct ref **to_fetch)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process rsync;
+	const char *args[8];
+	int result;
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/objects/");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = buf.buf;
+	args[6] = get_object_directory();
+	args[7] = NULL;
+
+	/* NEEDSWORK: handle one level of alternates */
+	result = run_command(&rsync);
+
+	strbuf_release(&buf);
+
+	return result;
+}
+
+static int write_one_ref(const char *name, const unsigned char *sha1,
+		int flags, void *data)
+{
+	struct strbuf *buf = data;
+	int len = buf->len;
+	FILE *f;
+
+	if (flags && prefixcmp(name, "refs/heads/") &&
+			prefixcmp(name, "refs/tags/"))
+		return 0;
+
+	strbuf_addstr(buf, name);
+	if (safe_create_leading_directories(buf->buf) ||
+			!(f = fopen(buf->buf, "w")) ||
+			!fwrite(sha1_to_hex(sha1), 40, 1, f) ||
+			fputc('\n', f) == EOF ||
+			fclose(f))
+		return error("problems writing temporary file %s", buf->buf);
+	strbuf_setlen(buf, len);
+	return 0;
+}
+
+static int write_refs_to_temp_dir(struct strbuf *temp_dir,
+		int refspec_nr, const char **refspec)
+{
+	int i;
+
+	for (i = 0; i < refspec_nr; i++) {
+		unsigned char sha1[20];
+		char *ref;
+
+		if (dwim_ref(refspec[i], strlen(refspec[i]), sha1, &ref) != 1)
+			return error("Could not get ref %s", refspec[i]);
+
+		if (write_one_ref(ref, sha1, 0, temp_dir)) {
+			free(ref);
+			return -1;
+		}
+		free(ref);
+	}
+	return 0;
+}
+
+static int rsync_transport_push(struct transport *transport,
+		int refspec_nr, const char **refspec, int flags) {
+	struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;
+	int result = 0, i;
+	struct child_process rsync;
+	const char *args[8];
+
+	/* first push the objects */
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addch(&buf, '/');
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-av" : "-a";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = get_object_directory();;
+	args[6] = buf.buf;
+	args[7] = NULL;
+
+	if (run_command(&rsync))
+		return error("Could not push objects to %s", transport->url);
+
+	/* copy the refs to the temporary directory; they could be packed. */
+
+	strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
+	if (!mkdtemp(temp_dir.buf))
+		die ("Could not make temporary directory");
+	strbuf_addch(&temp_dir, '/');
+
+	if (flags & TRANSPORT_PUSH_ALL) {
+		if (for_each_ref(write_one_ref, &temp_dir))
+			return -1;
+	} else if (write_refs_to_temp_dir(&temp_dir, refspec_nr, refspec))
+		return -1;
+
+	i = (flags & TRANSPORT_PUSH_FORCE) ? 2 : 3;
+	args[i++] = temp_dir.buf;
+	args[i++] = transport->url;
+	args[i++] = NULL;
+	if (run_command(&rsync))
+		result = error("Could not push to %s", transport->url);
+
+	if (remove_dir_recursively(&temp_dir, 0))
+		warning ("Could not remove temporary directory %s.",
+				temp_dir.buf);
+
+	strbuf_release(&buf);
+	strbuf_release(&temp_dir);
+
+	return result;
+}
 
 /* Generic functions for using commit walkers */
 
@@ -402,7 +726,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	ret->url = url;
 
 	if (!prefixcmp(url, "rsync://")) {
-		/* not supported; don't populate any ops */
+		ret->get_refs_list = get_refs_via_rsync;
+		ret->fetch = fetch_objs_via_rsync;
+		ret->push = rsync_transport_push;
 
 	} else if (!prefixcmp(url, "http://")
 	        || !prefixcmp(url, "https://")
-- 
1.5.3.2.1102.g9487

Re: [PATCH 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

Darn.  this should read [PATCH v2 2/2].

Re: [PATCH 2/2] fetch/push: readd rsync support

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> diff --git a/transport.c b/transport.c
> index 4f9cddc..c8eed95 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -6,6 +6,330 @@
> ...
> +static int direntry_cmp(const void *a, const void *b)
> +{
> +	const struct dirent *d1 = a;
> +	const struct dirent *d2 = b;
> +
> +	return strcmp(d1->d_name, d2->d_name);
> +}
> ...
> +/*
> + * path is assumed to point to a buffer of PATH_MAX bytes, and
> + * path + name_offset is expected to point to "refs/".
> + */
> +
> +static int read_loose_refs(struct strbuf *path, int name_offset,
> +		struct ref **tail)
> +{
> +	DIR *dir = opendir(path->buf);
> +	struct dirent *de;
> +	struct {
> +		char **entries;
> +		int nr, alloc;
> +	} list;
> ...
> +	while ((de = readdir(dir))) {
> +		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> +				(de->d_name[1] == '.' &&
> +				 de->d_name[2] == '\0')))
> +			continue;
> +		ALLOC_GROW(list.entries, list.nr + 1, list.alloc);
> +		list.entries[list.nr++] = xstrdup(de->d_name);
> +	}
> +	closedir(dir);
> +
> +	/* sort the list */
> +
> +	qsort(list.entries, list.nr, sizeof(*de), direntry_cmp);

Hmmmph...?

Re: [PATCH 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

Hi,

On Fri, 28 Sep 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > diff --git a/transport.c b/transport.c
> > index 4f9cddc..c8eed95 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -6,6 +6,330 @@
> > ...
> > +static int direntry_cmp(const void *a, const void *b)
> > +{
> > +	const struct dirent *d1 = a;
> > +	const struct dirent *d2 = b;
> > +
> > +	return strcmp(d1->d_name, d2->d_name);
> > +}
> > ...
> > +/*
> > + * path is assumed to point to a buffer of PATH_MAX bytes, and
> > + * path + name_offset is expected to point to "refs/".
> > + */
> > +
> > +static int read_loose_refs(struct strbuf *path, int name_offset,
> > +		struct ref **tail)
> > +{
> > +	DIR *dir = opendir(path->buf);
> > +	struct dirent *de;
> > +	struct {
> > +		char **entries;
> > +		int nr, alloc;
> > +	} list;
> > ...
> > +	while ((de = readdir(dir))) {
> > +		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> > +				(de->d_name[1] == '.' &&
> > +				 de->d_name[2] == '\0')))
> > +			continue;
> > +		ALLOC_GROW(list.entries, list.nr + 1, list.alloc);
> > +		list.entries[list.nr++] = xstrdup(de->d_name);
> > +	}
> > +	closedir(dir);
> > +
> > +	/* sort the list */
> > +
> > +	qsort(list.entries, list.nr, sizeof(*de), direntry_cmp);
> 
> Hmmmph...?

Ouch.  Sorry.

Will resend in a few minutes. (After reading the patch -- again)

Ciao,
Dscho

[PATCH v3 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

We lost rsync support when transitioning from shell to C.  Support it
again (even if the transport is technically deprecated, some people just
do not have any chance to use anything else).

Also, add a test to t5510.  Since rsync transport is not configured by
default on most machines, and especially not such that you can
write to rsync://127.0.0.1$(pwd)/, it is disabled by default; you can
enable it by setting the environment variable TEST_RSYNC.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Sorry for the borked v2 of this patch...

	I cleaned up some other parts, such as free()ing or close()ing
	also in case of an error.

 t/t5510-fetch.sh |   35 ++++++
 transport.c      |  328 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 362 insertions(+), 1 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 439430f..73a4e3c 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -153,4 +153,39 @@ test_expect_success 'bundle should be able to create a full history' '
 
 '
 
+test "$TEST_RSYNC" && {
+test_expect_success 'fetch via rsync' '
+	git pack-refs &&
+	mkdir rsynced &&
+	cd rsynced &&
+	git init &&
+	git fetch rsync://127.0.0.1$(pwd)/../.git master:refs/heads/master &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	mkdir ../rsynced2 &&
+	(cd ../rsynced2 &&
+	 git init) &&
+	git push rsync://127.0.0.1$(pwd)/../rsynced2/.git master &&
+	cd ../rsynced2 &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	cd .. &&
+	mkdir rsynced3 &&
+	(cd rsynced3 &&
+	 git init) &&
+	git push --all rsync://127.0.0.1$(pwd)/rsynced3/.git &&
+	cd rsynced3 &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+}
+
 test_done
diff --git a/transport.c b/transport.c
index 4f9cddc..a2ee8f3 100644
--- a/transport.c
+++ b/transport.c
@@ -6,6 +6,330 @@
 #include "fetch-pack.h"
 #include "walker.h"
 #include "bundle.h"
+#include "dir.h"
+#include "refs.h"
+
+/* rsync support */
+
+/*
+ * We copy packed-refs and refs/ into a temporary file, then read the
+ * loose refs recursively (sorting whenever possible), and then inserting
+ * those packed refs that are not yet in the list (not validating, but
+ * assuming that the file is sorted).
+ *
+ * Appears refactoring this from refs.c is too cumbersome.
+ */
+
+static int str_cmp(const void *a, const void *b)
+{
+	const char *s1 = a;
+	const char *s2 = b;
+
+	return strcmp(s1, s2);
+}
+
+/* path->buf + name_offset is expected to point to "refs/" */
+
+static int read_loose_refs(struct strbuf *path, int name_offset,
+		struct ref **tail)
+{
+	DIR *dir = opendir(path->buf);
+	struct dirent *de;
+	struct {
+		char **entries;
+		int nr, alloc;
+	} list;
+	int i, pathlen;
+
+	if (!dir)
+		return -1;
+
+	memset (&list, 0, sizeof(list));
+
+	while ((de = readdir(dir))) {
+		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
+				(de->d_name[1] == '.' &&
+				 de->d_name[2] == '\0')))
+			continue;
+		ALLOC_GROW(list.entries, list.nr + 1, list.alloc);
+		list.entries[list.nr++] = xstrdup(de->d_name);
+	}
+	closedir(dir);
+
+	/* sort the list */
+
+	qsort(list.entries, list.nr, sizeof(char *), str_cmp);
+
+	pathlen = path->len;
+	strbuf_addch(path, '/');
+
+	for (i = 0; i < list.nr; i++, strbuf_setlen(path, pathlen + 1)) {
+		strbuf_addstr(path, list.entries[i]);
+		if (read_loose_refs(path, name_offset, tail)) {
+			int fd = open(path->buf, O_RDONLY);
+			char buffer[40];
+			struct ref *next;
+
+			if (fd < 0)
+				continue;
+			next = alloc_ref(path->len - name_offset + 1);
+			if (read_in_full(fd, buffer, 40) != 40 ||
+					get_sha1_hex(buffer, next->old_sha1)) {
+				close(fd);
+				free(next);
+				continue;
+			}
+			close(fd);
+			strcpy(next->name, path->buf + name_offset);
+			(*tail)->next = next;
+			*tail = next;
+		}
+	}
+	strbuf_setlen(path, pathlen);
+
+	for (i = 0; i < list.nr; i++)
+		free(list.entries[i]);
+	free(list.entries);
+
+	return 0;
+}
+
+/* insert the packed refs for which no loose refs were found */
+
+static void insert_packed_refs(const char *packed_refs, struct ref **list)
+{
+	FILE *f = fopen(packed_refs, "r");
+	static char buffer[PATH_MAX];
+
+	if (!f)
+		return;
+
+	for (;;) {
+		int cmp, len;
+
+		if (!fgets(buffer, sizeof(buffer), f)) {
+			fclose(f);
+			return;
+		}
+
+		if (hexval(buffer[0]) > 0xf)
+			continue;
+		len = strlen(buffer);
+		if (buffer[len - 1] == '\n')
+			buffer[--len] = '\0';
+		if (len < 41)
+			continue;
+		while ((*list)->next &&
+				(cmp = strcmp(buffer + 41,
+				      (*list)->next->name)) > 0)
+			list = &(*list)->next;
+		if (!(*list)->next || cmp < 0) {
+			struct ref *next = alloc_ref(len - 40);
+			buffer[40] = '\0';
+			if (get_sha1_hex(buffer, next->old_sha1)) {
+				warning ("invalid SHA-1: %s", buffer);
+				free(next);
+				continue;
+			}
+			strcpy(next->name, buffer + 41);
+			next->next = (*list)->next;
+			(*list)->next = next;
+			list = &(*list)->next;
+		}
+	}
+}
+
+static struct ref *get_refs_via_rsync(const struct transport *transport)
+{
+	struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;
+	struct ref dummy, *tail = &dummy;
+	struct child_process rsync;
+	const char *args[5];
+	int temp_dir_len;
+
+	/* copy the refs to the temporary directory */
+
+	strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
+	if (!mkdtemp(temp_dir.buf))
+		die ("Could not make temporary directory");
+	temp_dir_len = temp_dir.len;
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/refs");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = buf.buf;
+	args[3] = temp_dir.buf;
+	args[4] = NULL;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/packed-refs");
+
+	args[2] = buf.buf;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	/* read the copied refs */
+
+	strbuf_addstr(&temp_dir, "/refs");
+	read_loose_refs(&temp_dir, temp_dir_len + 1, &tail);
+	strbuf_setlen(&temp_dir, temp_dir_len);
+
+	tail = &dummy;
+	strbuf_addstr(&temp_dir, "/packed-refs");
+	insert_packed_refs(temp_dir.buf, &tail);
+	strbuf_setlen(&temp_dir, temp_dir_len);
+
+	if (remove_dir_recursively(&temp_dir, 0))
+		warning ("Error removing temporary directory %s.",
+				temp_dir.buf);
+
+	strbuf_release(&buf);
+	strbuf_release(&temp_dir);
+
+	return dummy.next;
+}
+
+static int fetch_objs_via_rsync(struct transport *transport,
+				 int nr_objs, struct ref **to_fetch)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process rsync;
+	const char *args[8];
+	int result;
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/objects/");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = buf.buf;
+	args[6] = get_object_directory();
+	args[7] = NULL;
+
+	/* NEEDSWORK: handle one level of alternates */
+	result = run_command(&rsync);
+
+	strbuf_release(&buf);
+
+	return result;
+}
+
+static int write_one_ref(const char *name, const unsigned char *sha1,
+		int flags, void *data)
+{
+	struct strbuf *buf = data;
+	int len = buf->len;
+	FILE *f;
+
+	/* when called via for_each_ref(), flags is non-zero */
+	if (flags && prefixcmp(name, "refs/heads/") &&
+			prefixcmp(name, "refs/tags/"))
+		return 0;
+
+	strbuf_addstr(buf, name);
+	if (safe_create_leading_directories(buf->buf) ||
+			!(f = fopen(buf->buf, "w")) ||
+			fprintf(f, "%s\n", sha1_to_hex(sha1)) < 0 ||
+			fclose(f))
+		return error("problems writing temporary file %s", buf->buf);
+	strbuf_setlen(buf, len);
+	return 0;
+}
+
+static int write_refs_to_temp_dir(struct strbuf *temp_dir,
+		int refspec_nr, const char **refspec)
+{
+	int i;
+
+	for (i = 0; i < refspec_nr; i++) {
+		unsigned char sha1[20];
+		char *ref;
+
+		if (dwim_ref(refspec[i], strlen(refspec[i]), sha1, &ref) != 1)
+			return error("Could not get ref %s", refspec[i]);
+
+		if (write_one_ref(ref, sha1, 0, temp_dir)) {
+			free(ref);
+			return -1;
+		}
+		free(ref);
+	}
+	return 0;
+}
+
+static int rsync_transport_push(struct transport *transport,
+		int refspec_nr, const char **refspec, int flags)
+{
+	struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;
+	int result = 0, i;
+	struct child_process rsync;
+	const char *args[8];
+
+	/* first push the objects */
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addch(&buf, '/');
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-av" : "-a";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = get_object_directory();;
+	args[6] = buf.buf;
+	args[7] = NULL;
+
+	if (run_command(&rsync))
+		return error("Could not push objects to %s", transport->url);
+
+	/* copy the refs to the temporary directory; they could be packed. */
+
+	strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
+	if (!mkdtemp(temp_dir.buf))
+		die ("Could not make temporary directory");
+	strbuf_addch(&temp_dir, '/');
+
+	if (flags & TRANSPORT_PUSH_ALL) {
+		if (for_each_ref(write_one_ref, &temp_dir))
+			return -1;
+	} else if (write_refs_to_temp_dir(&temp_dir, refspec_nr, refspec))
+		return -1;
+
+	i = (flags & TRANSPORT_PUSH_FORCE) ? 2 : 3;
+	args[i++] = temp_dir.buf;
+	args[i++] = transport->url;
+	args[i++] = NULL;
+	if (run_command(&rsync))
+		result = error("Could not push to %s", transport->url);
+
+	if (remove_dir_recursively(&temp_dir, 0))
+		warning ("Could not remove temporary directory %s.",
+				temp_dir.buf);
+
+	strbuf_release(&buf);
+	strbuf_release(&temp_dir);
+
+	return result;
+}
 
 /* Generic functions for using commit walkers */
 
@@ -402,7 +726,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	ret->url = url;
 
 	if (!prefixcmp(url, "rsync://")) {
-		/* not supported; don't populate any ops */
+		ret->get_refs_list = get_refs_via_rsync;
+		ret->fetch = fetch_objs_via_rsync;
+		ret->push = rsync_transport_push;
 
 	} else if (!prefixcmp(url, "http://")
 	        || !prefixcmp(url, "https://")
-- 
1.5.3.2.1102.g9487

Re: [PATCH v3 2/2] fetch/push: readd rsync support

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> We lost rsync support when transitioning from shell to C.  Support it
> again (even if the transport is technically deprecated, some people just
> do not have any chance to use anything else).

s/chance/choice/?

> +test "$TEST_RSYNC" && {

Somehow this feels dirty ... perhaps leave early like:

	if test -z "$TEST_RSYNC"
        then
        	test_expect_success 'skipping rsync transport tests' :
		test_done
                exit
	fi

> diff --git a/transport.c b/transport.c
> index 4f9cddc..a2ee8f3 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -6,6 +6,330 @@
> ...
> +{
> +	struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;
> +	struct ref dummy, *tail = &dummy;
> +	struct child_process rsync;
> +	const char *args[5];
> +	int temp_dir_len;
> +
> +	/* copy the refs to the temporary directory */
> +
> +	strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
> +	if (!mkdtemp(temp_dir.buf))
> +		die ("Could not make temporary directory");

I wonder how portable mkdtemp() is (it does not seem to be POSIX);
would we need something in compat/ perhaps based on tempnam()?

> +static int fetch_objs_via_rsync(struct transport *transport,
> +				 int nr_objs, struct ref **to_fetch)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	struct child_process rsync;
> +	const char *args[8];
> +	int result;
> +
> +	strbuf_addstr(&buf, transport->url);
> +	strbuf_addstr(&buf, "/objects/");
> +
> +	memset(&rsync, 0, sizeof(rsync));
> +	rsync.argv = args;
> +	rsync.stdout_to_stderr = 1;
> +	args[0] = "rsync";
> +	args[1] = transport->verbose ? "-rv" : "-r";
> +	args[2] = "--ignore-existing";
> +	args[3] = "--exclude";
> +	args[4] = "info";
> +	args[5] = buf.buf;
> +	args[6] = get_object_directory();
> +	args[7] = NULL;

Hmm, we used to do "rsync $remote/objects/ $our/.git/objects/",
but this omits the trailing "/" from our side.  I suspect the
reason was to deal with the case where our .git/objects was a
symlink to elsewhere (which was how you did alternates before
alternates was invented), which may not matter anymore these
days.

Re: [PATCH v3 2/2] fetch/push: readd rsync support

[Johannes Schindelin]

Hi,

On Sat, 29 Sep 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > We lost rsync support when transitioning from shell to C.  Support it
> > again (even if the transport is technically deprecated, some people just
> > do not have any chance to use anything else).
> 
> s/chance/choice/?

Hehe. I did mean it like that, but feel free to edit!

> > +test "$TEST_RSYNC" && {
> 
> Somehow this feels dirty ... perhaps leave early like:
> 
> 	if test -z "$TEST_RSYNC"
>         then
>         	test_expect_success 'skipping rsync transport tests' :
> 		test_done
>                 exit
> 	fi

Okay.

> > diff --git a/transport.c b/transport.c
> > index 4f9cddc..a2ee8f3 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -6,6 +6,330 @@
> > ...
> > +{
> > +	struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;
> > +	struct ref dummy, *tail = &dummy;
> > +	struct child_process rsync;
> > +	const char *args[5];
> > +	int temp_dir_len;
> > +
> > +	/* copy the refs to the temporary directory */
> > +
> > +	strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
> > +	if (!mkdtemp(temp_dir.buf))
> > +		die ("Could not make temporary directory");
> 
> I wonder how portable mkdtemp() is (it does not seem to be POSIX);
> would we need something in compat/ perhaps based on tempnam()?

My man page said BSD, so I did not bother any more...

Besides, using "tmpnam()" makes gcc say that it is dangerous, and I should 
use mkstemp() instead...

> > +static int fetch_objs_via_rsync(struct transport *transport,
> > +				 int nr_objs, struct ref **to_fetch)
> > +{
> > +	struct strbuf buf = STRBUF_INIT;
> > +	struct child_process rsync;
> > +	const char *args[8];
> > +	int result;
> > +
> > +	strbuf_addstr(&buf, transport->url);
> > +	strbuf_addstr(&buf, "/objects/");
> > +
> > +	memset(&rsync, 0, sizeof(rsync));
> > +	rsync.argv = args;
> > +	rsync.stdout_to_stderr = 1;
> > +	args[0] = "rsync";
> > +	args[1] = transport->verbose ? "-rv" : "-r";
> > +	args[2] = "--ignore-existing";
> > +	args[3] = "--exclude";
> > +	args[4] = "info";
> > +	args[5] = buf.buf;
> > +	args[6] = get_object_directory();
> > +	args[7] = NULL;
> 
> Hmm, we used to do "rsync $remote/objects/ $our/.git/objects/", but this 
> omits the trailing "/" from our side.  I suspect the reason was to deal 
> with the case where our .git/objects was a symlink to elsewhere (which 
> was how you did alternates before alternates was invented), which may 
> not matter anymore these days.

Nevertheless, this was an oversight on my side. Will change to use a 
second strbuf for the local objects directory.