Re: [PATCH] Allow cloning an empty repository

From: Sverre Rabbelier <srabbelier@gmail.com>
Date: Jan 23, 2009, 01:46 UTC
Message-ID: <bd6139dc0901221746h258f548etf857ab37399133da@mail.gmail.com>
In-Reply-To: <7vwscm4xx0.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 23, 2009 at 02:24, Junio C Hamano <gitster@pobox.com> wrote:

> I think the basic idea is Ok, but is it a reliable check at this point to
> see if (refs == NULL) to tell if the target repository is an empty one?

This is the question I asked Dscho, and he said/guessed that it was.

> I am mostly worried about a failure case (connected but couldn't get the
> refs, or perhaps connection failed to start).  If you get a NULL in such a
> case you may end up saying "oh you cloned a void" when you should say
> "nah, such a remote repository does not exist".

Yes, this was my concern as well.

> If transport_get_remote_refs() dies without returning NULL, that would be
> sufficient, but I didn't check.

It does, transport_get_remote_refs() calls fetch_pack, which dies when 'refs' (the fourth argument) is NULL:

static int fetch_refs_via_pack(...) {
// setup code snipped
	if (!data->conn) {
		connect_setup(transport);
		get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);
	}

	refs = fetch_pack(&args, data->fd, data->conn,
			  refs_tmp ? refs_tmp : transport->remote_refs,
			  dest, nr_heads, heads, &transport->pack_lockfile);

So unless get_remote_heads messes with it, it will remain NULL. Now I must admit that I'm not familiar enough with get_remote_heads to know if this could cause a false positive, Dscho?

-- 
Cheers,

Sverre Rabbelier