git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Shawn Pearce <spearce@spearce.org>,
	Johan Herland <johan@herland.net>,
	git@vger.kernel.org
Subject: [PATCHv4 02/10] send-pack: Attempt to retrieve remote status even if pack-objects fails
Date: Mon, 23 May 2011 02:51:55 +0200	[thread overview]
Message-ID: <1306111923-16859-3-git-send-email-johan@herland.net> (raw)
In-Reply-To: <1306111923-16859-1-git-send-email-johan@herland.net>

When pushing, send-pack uses pack-objects to write the pack data to the
receive-pack process running on the remote end. The scenarios where
pack-objects dies unexpectedly, can be roughly divided based on whether
the reason for the failure is _local_ (i.e. something in pack-objects
caused it to fail of its own accord), or _remote_ (i.e. something in
the remote receive-pack process caused it to fail, leaving the local
pack-objects process with a broken pipe)

If the reason for the failure is local, we expect pack-objects to report
an appropriate error message to the user.

However, if the reason for the failure is remote, pack-objects will merely
abort because of the broken pipe, and the user is left with no clue as to
the reason why the remote receive-pack process died.

In certain cases, though, the receive-pack process on the other end may have
produced an error message immediately before exiting. This error message may
be currently waiting to be read by the local send-pack process.

Therefore, we should try to read from the remote end, even when pack-objects
dies unexepectedly. We accomplish this by _always_ calling receive_status()
after pack_objects(). If the remote end managed to produce a well-formed
status report before exiting, then receive_status() simply presents that to
the user. Even if the data from the remote end cannot be understood by
receive_status(), it will print that data as part of its error message. In
any case, we give the user as much information about the failure as possible.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin/send-pack.c |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index c1f6ddd..5ba5262 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -251,7 +251,7 @@ int send_pack(struct send_pack_args *args,
 	int status_report = 0;
 	int use_sideband = 0;
 	unsigned cmds_sent = 0;
-	int ret;
+	int ret = 0;
 	struct async demux;
 
 	/* Does the other end support the reporting? */
@@ -339,25 +339,18 @@ int send_pack(struct send_pack_args *args,
 	}
 
 	if (new_refs && cmds_sent) {
-		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
-			for (ref = remote_refs; ref; ref = ref->next)
-				ref->status = REF_STATUS_NONE;
+		if ((ret = pack_objects(out, remote_refs, extra_have, args))) {
 			if (args->stateless_rpc)
 				close(out);
 			if (git_connection_is_socket(conn))
 				shutdown(fd[0], SHUT_WR);
-			if (use_sideband)
-				finish_async(&demux);
-			return -1;
 		}
 	}
 	if (args->stateless_rpc && cmds_sent)
 		packet_flush(out);
 
 	if (status_report && cmds_sent)
-		ret = receive_status(in, remote_refs);
-	else
-		ret = 0;
+		ret |= receive_status(in, remote_refs);
 	if (args->stateless_rpc)
 		packet_flush(out);
 
-- 
1.7.5.rc1.3.g4d7b

  parent reply	other threads:[~2011-05-23  0:52 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-23  0:51 [PATCHv4 00/10] Push limits Johan Herland
2011-05-23  0:51 ` [PATCHv4 01/10] Update technical docs to reflect side-band-64k capability in receive-pack Johan Herland
2011-05-23  0:51 ` Johan Herland [this message]
2011-05-23 20:06   ` [PATCHv4 02/10] send-pack: Attempt to retrieve remote status even if pack-objects fails Junio C Hamano
2011-05-23 22:58     ` Johan Herland
2011-05-23  0:51 ` [PATCHv4 03/10] Tighten rules for matching server capabilities in server_supports() Johan Herland
2011-05-23  0:51 ` [PATCHv4 04/10] receive-pack: Prepare for addition of the new 'limit-*' family of capabilities Johan Herland
2011-05-23 20:21   ` Junio C Hamano
2011-05-24  0:16     ` Johan Herland
2011-05-23  0:51 ` [PATCHv4 05/10] pack-objects: Teach new option --max-commit-count, limiting #commits in pack Johan Herland
2011-05-23 23:17   ` Junio C Hamano
2011-05-24  0:18     ` Johan Herland
2011-05-23  0:51 ` [PATCHv4 06/10] send-pack/receive-pack: Allow server to refuse pushes with too many commits Johan Herland
2011-05-23 23:39   ` Junio C Hamano
2011-05-24  1:11     ` Johan Herland
2011-05-23  0:52 ` [PATCHv4 07/10] pack-objects: Allow --max-pack-size to be used together with --stdout Johan Herland
2011-05-24  0:09   ` Junio C Hamano
2011-05-24  1:15     ` Johan Herland
2011-05-23  0:52 ` [PATCHv4 08/10] send-pack/receive-pack: Allow server to refuse pushing too large packs Johan Herland
2011-05-24  0:12   ` Junio C Hamano
2011-05-23  0:52 ` [PATCHv4 09/10] pack-objects: Estimate pack size; abort early if pack size limit is exceeded Johan Herland
2011-05-23 16:11   ` Shawn Pearce
2011-05-23 17:07     ` Johan Herland
2011-05-24  0:18   ` Junio C Hamano
2011-05-24  1:17     ` Johan Herland
2011-05-23  0:52 ` [PATCHv4 10/10] receive-pack: Allow server to refuse pushes with too many objects Johan Herland

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=1306111923-16859-3-git-send-email-johan@herland.net \
    --to=johan@herland.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=spearce@spearce.org \
    /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).