git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Sverre Rabbelier <srabbelier@gmail.com>
Cc: "Björn Steinbrink" <B.Steinbrink@gmx.de>,
	"Matthieu Moy" <Matthieu.Moy@imag.fr>,
	"Sitaram Chamarty" <sitaramc@gmail.com>,
	git@vger.kernel.org
Subject: Re: clong an empty repo over ssh causes (harmless) fatal
Date: Mon, 31 Aug 2009 21:08:33 -0400	[thread overview]
Message-ID: <20090901010833.GA4033@sigill.intra.peff.net> (raw)
In-Reply-To: <fabb9a1e0908311550r1b549eb2k2df65c188a0ea6a0@mail.gmail.com>

On Tue, Sep 01, 2009 at 12:50:25AM +0200, Sverre Rabbelier wrote:

> 2009/9/1 Jeff King <peff@peff.net>:
> > AFAICT, this problem goes back to v1.6.2, the first version which
> > handled empty clones. So I blame Sverre. ;)
> 
> Eep :(. Any idea what is going on?

Yeah. We call upload-pack on the remote side, realize there are no refs,
and then we just stop talking. Meanwhile upload-pack is waiting for a
packet to say "these are the refs that I want". So the client really
needs to send an extra packet saying "list of refs is finished".

The patch below seems to work for me, but I'm a little concerned how it
might impact other transports. It actually calls the transport's
fetch method when we have no refs that we want. So each transport must
recognize that we want zero refs and do the appropriate thing. In this
case, for the git protocol, we want to:

  - do a packet_flush to signal "no more refs" to the remote side

  - be aware that we might have zero refs and avoid establishing a new
    connection in that case

Other transports might need to be tweaked similarly, but I don't have
time to test at the moment.

diff --git a/builtin-clone.c b/builtin-clone.c
index 0d2b4a8..f198c01 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -515,8 +515,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					     option_upload_pack);
 
 		refs = transport_get_remote_refs(transport);
-		if(refs)
-			transport_fetch_refs(transport, refs);
+		transport_fetch_refs(transport, refs);
 	}
 
 	if (refs) {
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 629735f..04a3776 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -803,6 +803,8 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 		nr_heads = remove_duplicates(nr_heads, heads);
 	if (!ref) {
 		packet_flush(fd[1]);
+		if (!nr_heads)
+			return NULL;
 		die("no matching remote head");
 	}
 	ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
diff --git a/transport.c b/transport.c
index f2bd998..25e8946 100644
--- a/transport.c
+++ b/transport.c
@@ -512,6 +512,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 		origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
 
 	if (!data->conn) {
+		if (!nr_heads)
+			return 0;
 		connect_setup(transport, 0, 0);
 		get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);
 	}

  reply	other threads:[~2009-09-01  1:08 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-31 11:14 clong an empty repo over ssh causes (harmless) fatal Sitaram Chamarty
2009-08-31 11:22 ` Matthieu Moy
2009-08-31 14:30   ` Sitaram Chamarty
2009-08-31 14:47     ` Matthieu Moy
2009-08-31 16:41     ` Jeff King
2009-08-31 17:12       ` Sverre Rabbelier
2009-08-31 19:08         ` Jeff King
2009-08-31 19:09           ` Sverre Rabbelier
2009-08-31 17:25       ` Matthieu Moy
2009-08-31 19:10         ` Jeff King
2009-08-31 20:19           ` Björn Steinbrink
2009-08-31 22:47             ` Jeff King
2009-08-31 22:50               ` Sverre Rabbelier
2009-09-01  1:08                 ` Jeff King [this message]
2009-09-02  4:33                   ` Daniel Barkalow
2009-09-02  5:16                     ` Jeff King
2009-09-02  6:02                       ` Daniel Barkalow
2009-09-02  6:36                         ` [PATCH] clone: disconnect transport after fetching Jeff King
2009-09-02  7:09                           ` Sverre Rabbelier
2009-09-02  7:26                             ` Jeff King
2009-09-02  7:37                               ` Sverre Rabbelier
2009-09-02 16:38                           ` Daniel Barkalow
2009-09-02 17:55                             ` Junio C Hamano
2009-09-04  2:30                           ` Jeff King
2009-09-02  5:30               ` clong an empty repo over ssh causes (harmless) fatal Sitaram Chamarty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090901010833.GA4033@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=B.Steinbrink@gmx.de \
    --cc=Matthieu.Moy@imag.fr \
    --cc=git@vger.kernel.org \
    --cc=sitaramc@gmail.com \
    --cc=srabbelier@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).