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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 30, 2007, 06:09 UTC
Message-ID: <7vhclciszz.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <Pine.LNX.4.64.0709290134000.28395@racer.site>

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

Show 18 quoted lines

> 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()?

Show 22 quoted lines

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